Sorry, I think existing code should already handle retry for blcoking case. My
idea to retry anyway @line328 is just intention to make the code looks better
and handle the return of -EAGAIN in one place which is handled by retry logic.
Anyway, this should be a minor suggestion, you can ignore this :)
----------------------------------------
> From: unicorn_w...@outlook.com
> To: oneu...@suse.de
> CC: linux-usb@vger.kernel.org
> Subject: RE: question on skel_read func of usb_skeleton.c
> Date: Wed, 17 Jul 2013 03:02:49 +0000
>
> Oliver,
> For the func skel_read(), line 328~330, for the case rv ==0, can we just do
> retry, similar as the code line 301. I hope this can help read() get data in
> one time for blocking IO if the data to be read is short and can be received
> in quick time. The existing retry logic upon should be able to handle all the
> other stuff such as nonblockking IO in one place.
> ----------------------------------------
>> From: unicorn_w...@outlook.com
>> To: oneu...@suse.de
>> CC: linux-usb@vger.kernel.org
>> Subject: RE: question on skel_read func of usb_skeleton.c
>> Date: Sun, 14 Jul 2013 08:48:52 +0000
>>
>> Well, so my called strictly blocking IO (do and wait till get result before
>> returning from the syscall) only applies on read(). For write(), I agree,
>> kernel may perfer defer the actual write to the physical device so our
>> driver better supports this.
>> ----------------------------------------
>>> From: oneu...@suse.de
>>> To: unicorn_w...@outlook.com
>>> CC: linux-usb@vger.kernel.org
>>> Subject: Re: question on skel_read func of usb_skeleton.c
>>> Date: Fri, 12 Jul 2013 15:38:04 +0200
>>>
>>> On Friday 12 July 2013 13:09:23 WangChen wrote:
>>>> Oliver, my understanding is the limit_sem will only cause write() to sleep
>>>> and wait for other IOs to finish when there are alreay WRITES_IN_FLIGHT
>>>> URBs are on going. I see code line 509: sema_init(&dev->limit_sem,
>>>> WRITES_IN_FLIGHT);
>>>> My understanding of Blocking IO should block every write() when
>>>> corresponding URBs is not finished, am I right?
>>>
>>> Quoting from the man page of close():
>>>
>>> NOTES
>>> Not checking the return value of close() is a common but nevertheless
>>> serious programming error. It is quite possible that errors on a previous
>>> write(2) operation are first reported at the final close(). Not checking
>>> the return value when closing the file
>>> may lead to silent loss of data. This can especially be observed with NFS
>>> and with disk quota.
>>>
>>> A successful close does not guarantee that the data has been successfully
>>> saved to disk, as the kernel defers writes. It is not common for a file
>>> system to flush the buffers when the stream is closed. If you need to be
>>> sure that the data is physically
>>> stored use fsync(2). (It will depend on the disk hardware at this point.)
>>>
>>> It is probably unwise to close file descriptors while they may be in use by
>>> system calls in other threads in the same process. Since a file descriptor
>>> may be reused, there are some obscure race conditions that may cause
>>> unintended side effects.
>>>
>>>
>>> The logic for doing this is implemented in:
>>>
>>> static int skel_flush(struct file *file, fl_owner_t id)
>>> {
>>> struct usb_skel *dev;
>>> int res;
>>>
>>> dev = file->private_data;
>>> if (dev == NULL)
>>> return -ENODEV;
>>>
>>> /* wait for io to stop */
>>> mutex_lock(&dev->io_mutex);
>>> skel_draw_down(dev);
>>>
>>> /* read out errors, leave subsequent opens a clean slate */
>>> spin_lock_irq(&dev->err_lock);
>>> res = dev->errors ? (dev->errors == -EPIPE ? -EPIPE : -EIO) : 0;
>>> dev->errors = 0;
>>> spin_unlock_irq(&dev->err_lock);
>>>
>>> mutex_unlock(&dev->io_mutex);
>>>
>>> return res;
>>> }
>>>
>>> Regards
>>> Oliver
>>>
