Re: [PATCH v2] virtio-rng: return available data with O_NONBLOCK

On Tue, Aug 11, 2020 at 02:07:23PM +0200, Martin Wilck wrote:

On Tue, 2020-08-11 at 07:26 -0400, Michael S. Tsirkin wrote:

quoted

On Wed, Jul 15, 2020 at 03:32:55PM +0200, mwilck@suse.com wrote:

quoted

 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;

I wonder what purpose does this line serve: busy is false
which basically means data_avail is invalid, right?
A following non blocking call will not enter here.

Well, I thought this is just how reading data normally works. But
you're right, the remainder will not be used. I can remove the line, or
reset data_avail to 0 at this point. What do you prefer?

Regards,
Martin

Removing seems cleaner.

But looking at it, it is using the API in a very strange way:
a buffer is placed in the ring by one call, and *assumed*
to still be there in the next call.

which it might not be if one call is from userspace and the
next one is from fill kthread.

I guess this is why it's returning 0: yes it knows there's
data, but it does not know where it is.

-- 
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization