On Tue, Nov 04, 2014 at 02:59:17AM +1100, Bruce Evans wrote: > On Mon, 3 Nov 2014, Konstantin Belousov wrote: > > > On Mon, Nov 03, 2014 at 11:53:26AM +1100, Bruce Evans wrote: > >> On Sun, 2 Nov 2014, Ian Lepore wrote: > >> > >>> On Sun, 2014-11-02 at 12:27 -0800, Xin Li wrote: > >>>> -----BEGIN PGP SIGNED MESSAGE----- > >>>> Hash: SHA512 > >>>> > >>>> Hi, Mark, > >>>> > >>>> I'd like to propose the attached patch for review. It replaces > >>>> tsleep's with sx_sleep's, then checks the return value and quit the loop. > >>> > >>> It still doesn't handle the partial read/write case Kostik mentioned, > >>> but there are plenty of other drivers that don't get that right. > >> > >> Returning an error for a partial read is good enough for random devices, > >> since there is no problem with discarding the input. Upper layers are > >> still broken, so this (discarding the input is what happens automatically > >> except for ERESTART, EINTR and EWOULDBLOCK. > > But usermode buffer is already partially accessed and modified. > > Yes, I am picky about it after vn_io_fault() work. > > Urk. For some reason I was thinking that the uio modifications were > in a kernel buffer, so they could be backed out of easily. > > But surely the user buffer is indeterminate after a read error? Even > after success, the region beyond the part read should be specified as > indeterminate. I do not agree with this statement.
> Consider an implementation that asks the hardware to > DMA into part of the read buffer beyond a (possibly null) region already > successfully read. If the DMA fails, you might not know how far it > got. The best you can do to recover is to back out of the failed part > of the read only. Possibly zero the part that failed for security. > For further complications, consider a similar implementations but > with multiple DMA channels writing in indeterminate order. This is theoretical. I am not aware of such driver. > > If (part of) the buffer is not allowed to be indeterminate after a > success or failure, then both the success and the failure cases are > broken in the FreeBSD implementation (apart from not handling short > I/O's correctly). In the failure case, not backing out of partial > I/O's leaves a trashed buffer. In the success case, if it is > implemented by a partial backout, the part not backed out of is still > trashed. I am not sure I follow. There are two aspects of the buffer validity issue. One is the correctness of the file pointer (in other words, the report of number of bytes read or written). This includes the correct return of the short i/o count instead of error. Another is the guarantee of the state of the buffer part after the file pointer. I do not believe there is any statement about this in POSIX, nor most applications depend on the state, but some are. E.g., cyclic buffer used by sound or video recording functioning might depend on such guarantees. Overall, both issues determine the quality of implementation, and I prefer to implement them if not too costly. For the /dev/random, which does byte-streaming using uiomove(9), both are trivially implementable, so there is no reason not to do everything correctly. _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
