Laurent Vivier
2020-Aug-11 12:39 UTC
[PATCH v2] virtio-rng: return available data with O_NONBLOCK
On 11/08/2020 14:22, Martin Wilck wrote:> On Tue, 2020-08-11 at 14:02 +0200, Laurent Vivier wrote: >> >>>> drivers/char/hw_random/virtio-rng.c | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/drivers/char/hw_random/virtio-rng.c >>>> b/drivers/char/hw_random/virtio-rng.c >>>> index 79a6e47b5fbc..984713b35892 100644 >>>> --- a/drivers/char/hw_random/virtio-rng.c >>>> +++ b/drivers/char/hw_random/virtio-rng.c >>>> @@ -59,6 +59,20 @@ static int virtio_read(struct hwrng *rng, void >>>> *buf, size_t size, bool wait) >>>> if (vi->hwrng_removed) >>>> return -ENODEV; >>>> >>>> + /* >>>> + * If the previous call was non-blocking, we may have got some >>>> + * randomness already. >>>> + */ >>>> + if (vi->busy && completion_done(&vi->have_data)) { >>>> + unsigned int len; >>>> + >>>> + vi->busy = false; >>>> + len = vi->data_avail > size ? size : vi->data_avail; >>>> + vi->data_avail -= len; >> >> You don't need to modify data_avail. As busy is set to false, the >> buffer >> will be reused. and it is always overwritten by virtqueue_get_buf(). >> And moreover, if it was reused it would be always the beginning. > > Ok. > >> >>>> + if (len) >>>> + return len; >>>> + } >>>> + >>>> if (!vi->busy) { >>>> vi->busy = true; >>>> reinit_completion(&vi->have_data); >>>> >> >> Why don't you modify only the wait case? >> >> Something like: >> >> if (!wait && !completion_done(&vi->have_data)) { >> return 0; >> } >> >> then at the end you can do "return min(size, vi->data_avail);". > > Sorry, I don't understand what you mean. Where would you insert the > above "if" clause? Are you saying I should call > wait_for_completion_killable() also in the (!wait) case?Yes, but only if a the completion is done, so it will not wait.> > I must call check completion_done() before calling reinit_completion().Normally, the busy flag is here for that. If busy is true, a buffer is already registered. reinit_completion() must not be called if busy is true. busy becomes false when the before is ready to be reused.> OTOH, if completion_done() returns false, I can't simply return 0, I > must at least start fetching new random data, so that a subsequent > virtio_read() call has a chance to return something.if you modify "if (!wait)" to becomes "if (!wait && !completion_done(&vi->have_data))", either we have already a registered buffer from a previous call or the one we have registered if busy is false. So you can return 0 as nothing is ready but we have a registered buffer for the next time. Thanks, Laurent
Reasonably Related Threads
- [PATCH v2] virtio-rng: return available data with O_NONBLOCK
- [PATCH v2] virtio-rng: return available data with O_NONBLOCK
- [PATCH v2] virtio-rng: return available data with O_NONBLOCK
- [PATCH v2] virtio-rng: return available data with O_NONBLOCK
- [PATCH v2] virtio-rng: return available data with O_NONBLOCK