Date: Mon, 3 Nov 2014 11:53:26 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ian Lepore <ian@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, d@delphij.net, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Mark R V Murray <mark@grondar.org>, Konstantin Belousov <kostikbel@gmail.com> Subject: Re: svn commit: r273958 - head/sys/dev/random Message-ID: <20141103113629.I3149@besplex.bde.org> In-Reply-To: <1414961583.1200.27.camel@revolution.hippie.lan> References: <201411020201.sA221unt091493@svn.freebsd.org> <720EB74E-094A-43F3-8B1C-47BC7F6FECC3@grondar.org> <1414934579.17308.248.camel@revolution.hippie.lan> <6FB65828-6A79-4BDE-A9F7-BC472BA538CE@grondar.org> <CAJ-VmomeOwE3LOpehhJ__G=FCoBDRXrrn%2BSfjwPFODts6YYHNQ@mail.gmail.com> <20141102192057.GB53947@kib.kiev.ua> <29A795E1-19E2-49E4-9653-143D3F6F3F12@grondar.org> <20141102194625.GC53947@kib.kiev.ua> <751CD860-95B9-4F68-AE69-976B42823AD0@grondar.org> <54568E41.8030305@delphij.net> <20141102201331.GE53947@kib.kiev.ua> <545693B4.8030602@delphij.net> <1414961583.1200.27.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
> Given
> that the ra_read/ra_write functions can't return error, it would only be
> errors from uiomove() in play. I guess it would be something like this:
>
> nbytes = uio->uio_resid;
> while (uio->uio_resid && !error) {
> c = MIN(uio->uio_resid, PAGE_SIZE);
> (random_adaptor->ra_read)(random_buf, c);
> error = uiomove(random_buf, c, uio);
> }
> if (uio->uio_resid != nbytes)
> error = 0; /* Return partial read, not error. */
This is unnecessary except for upper layer bugs. Upper layers already
convert error to 0 under the condition (uio->uio_resid != original_resid),
except they only do this if 'error' was EINTR, ERESTART and EWOULDBLOCK --
other cases are broken. Thus all drivers have the burden of doing the
conversion if they want to be correct. Some file systems generally back
out of failing writes so that returning an error is correct.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141103113629.I3149>