search for: data_avail

...leting have_data completion before > > unregistering a virtio-rng device. > > Hi Amit, > > Before applying this patch, it's blocking insider wait_for_completion_killable() > Applied this patch, wait_for_completion_killable() returns 0, > and vi->data_avail becomes 0, then rng_get_date() will return 0. Thanks for checking this. > Is it expected result? I think what will happen is vi->data_avail will be set to whatever it was set last. In case of a previous successful read request, the data_avail will be set to whatever number of bytes the ho...

...leting have_data completion before > > unregistering a virtio-rng device. > > Hi Amit, > > Before applying this patch, it's blocking insider wait_for_completion_killable() > Applied this patch, wait_for_completion_killable() returns 0, > and vi->data_avail becomes 0, then rng_get_date() will return 0. Thanks for checking this. > Is it expected result? I think what will happen is vi->data_avail will be set to whatever it was set last. In case of a previous successful read request, the data_avail will be set to whatever number of bytes the ho...

...- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -53,7 +53,10 @@ static struct hwrng *current_rng; static struct task_struct *hwrng_fill; static LIST_HEAD(rng_list); +/* Protects rng_list and current_rng */ static DEFINE_MUTEX(rng_mutex); +/* Protects rng read functions, data_avail, rng_buffer and rng_fillbuf */ +static DEFINE_MUTEX(reading_mutex); static int data_avail; static u8 *rng_buffer, *rng_fillbuf; static unsigned short current_quality; @@ -81,7 +84,9 @@ static void add_early_randomness(struct hwrng *rng) unsigned char bytes[16]; int bytes_read; + mutex_lock...

...g.c b/drivers/char/hw_random/virtio-rng.c index 2ce0e22..12e242b 100644 --- a/drivers/char/hw_random/virtio-rng.c +++ b/drivers/char/hw_random/virtio-rng.c @@ -25,88 +25,108 @@ #include <linux/virtio_rng.h> #include <linux/module.h> -static struct virtqueue *vq; -static unsigned int data_avail; -static DECLARE_COMPLETION(have_data); -static bool busy; + +struct virtrng_info { + struct virtio_device *vdev; + struct hwrng hwrng; + struct virtqueue *vq; + unsigned int data_avail; + struct completion have_data; + bool busy; +}; static void random_recv_done(struct virtqueue *vq) { + struc...

...g.c b/drivers/char/hw_random/virtio-rng.c index 2ce0e22..12e242b 100644 --- a/drivers/char/hw_random/virtio-rng.c +++ b/drivers/char/hw_random/virtio-rng.c @@ -25,88 +25,108 @@ #include <linux/virtio_rng.h> #include <linux/module.h> -static struct virtqueue *vq; -static unsigned int data_avail; -static DECLARE_COMPLETION(have_data); -static bool busy; + +struct virtrng_info { + struct virtio_device *vdev; + struct hwrng hwrng; + struct virtqueue *vq; + unsigned int data_avail; + struct completion have_data; + bool busy; +}; static void random_recv_done(struct virtqueue *vq) { + struc...

When we try to hot-remove a busy virtio-rng device from QEMU monitor, the device can't be hot-removed. Because virtio-rng driver hangs at wait_for_completion_killable(). This patch exits the waiting by completing have_data completion before unregistering, resets data_avail to avoid the hwrng core use wrong buffer bytes. Before real unregistering we should return -ENODEV for reading. When we hot-unplug the device, dd process in guest will exit. $ dd if=/dev/hwrng of=/dev/null dd: error reading ?/dev/hwrng?: No such device Signed-off-by: Amos Kong <akong at re...

When we try to hot-remove a busy virtio-rng device from QEMU monitor, the device can't be hot-removed. Because virtio-rng driver hangs at wait_for_completion_killable(). This patch exits the waiting by completing have_data completion before unregistering, resets data_avail to avoid the hwrng core use wrong buffer bytes. Before real unregistering we should return -ENODEV for reading. When we hot-unplug the device, dd process in guest will exit. $ dd if=/dev/hwrng of=/dev/null dd: error reading ?/dev/hwrng?: No such device Signed-off-by: Amos Kong <akong at re...

...> > > unregistering a virtio-rng device. > > > > Hi Amit, > > > > Before applying this patch, it's blocking insider wait_for_completion_killable() > > Applied this patch, wait_for_completion_killable() returns 0, > > and vi->data_avail becomes 0, then rng_get_date() will return 0. > > Thanks for checking this. > > > Is it expected result? > > I think what will happen is vi->data_avail will be set to whatever it > was set last. In case of a previous successful read request, the > data_avail will b...

When we try to hot-remove a busy virtio-rng device from QEMU monitor, the device can't be hot-removed. Because virtio-rng driver hangs at wait_for_completion_killable(). This patch fixed the hang by completing have_data completion before unregistering a virtio-rng device. Signed-off-by: Amos Kong <akong at redhat.com> Cc: stable at vger.kernel.org ---

When we try to hot-remove a busy virtio-rng device from QEMU monitor, the device can't be hot-removed. Because virtio-rng driver hangs at wait_for_completion_killable(). This patch fixed the hang by completing have_data completion before unregistering a virtio-rng device. Signed-off-by: Amos Kong <akong at redhat.com> Cc: stable at vger.kernel.org ---

...y 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...

...locking, 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...

...- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -53,7 +53,10 @@ static struct hwrng *current_rng; static struct task_struct *hwrng_fill; static LIST_HEAD(rng_list); +/* Protects rng_list and current_rng */ static DEFINE_MUTEX(rng_mutex); +/* Protects rng read functions, data_avail, rng_buffer and rng_fillbuf */ +static DEFINE_MUTEX(reading_mutex); static int data_avail; static u8 *rng_buffer, *rng_fillbuf; static unsigned short current_quality; @@ -81,7 +84,9 @@ static void add_early_randomness(struct hwrng *rng) unsigned char bytes[16]; int bytes_read; + mutex_lock...

...ry to hot-unplugging a busy virtio-rng device, the device can't be removed. And the reading process in guest gets stuck. Those two patches fixed this issue by completing have_data completion and preventing invalid reading. Thanks for the help of Amit. Cc: stable at vger.kernel.org V2: reset data_avail (Amit) adjust unregister order V3: split patch, update commitlog Amos Kong (2): virtio-rng: fix stuck of hot-unplugging busy device virtio-rng: skip reading when we start to remove the device drivers/char/hw_random/virtio-rng.c | 7 +++++++ 1 file changed, 7 insertions(+) -- 1.9.3

...ry to hot-unplugging a busy virtio-rng device, the device can't be removed. And the reading process in guest gets stuck. Those two patches fixed this issue by completing have_data completion and preventing invalid reading. Thanks for the help of Amit. Cc: stable at vger.kernel.org V2: reset data_avail (Amit) adjust unregister order V3: split patch, update commitlog Amos Kong (2): virtio-rng: fix stuck of hot-unplugging busy device virtio-rng: skip reading when we start to remove the device drivers/char/hw_random/virtio-rng.c | 7 +++++++ 1 file changed, 7 insertions(+) -- 1.9.3

When I hotunplug a busy virtio-rng device or try to access hwrng attributes in non-smp guest, it gets stuck. My hotplug tests: | test 0: | hotunplug rng device from qemu monitor | | test 1: | guest) # dd if=/dev/hwrng of=/dev/null & | hotunplug rng device from qemu monitor | | test 2: | guest) # dd if=/dev/random of=/dev/null & | hotunplug rng device from qemu monitor | | test

When I hotunplug a busy virtio-rng device or try to access hwrng attributes in non-smp guest, it gets stuck. My hotplug tests: | test 0: | hotunplug rng device from qemu monitor | | test 1: | guest) # dd if=/dev/hwrng of=/dev/null & | hotunplug rng device from qemu monitor | | test 2: | guest) # dd if=/dev/random of=/dev/null & | hotunplug rng device from qemu monitor | | test

...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 ove...

...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 ove...

...t;> + * 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. >> +...