| Message ID | 20140728075514.GB3797@grmbl.mre (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 28, 2014 at 01:25:14PM +0530, Amit Shah wrote: > On (Mon) 28 Jul 2014 [15:32:42], Amos Kong wrote: > > QEMU commandline: > > > > ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -m 2000 -drive file=/images/nolvm.qcow2 --kernel /home/devel/linux/arch/x86/boot/bzImage -append "ro root=/dev/sda1 console=ttyS0,115200" -monitor unix:/tmp/m,nowait,server -device virtio-net-pci,netdev=h0,vectors=17,mq=on,id=n0 -netdev tap,id=h0,queues=8 -device virtio-net-pci,netdev=h1,vectors=0,mq=on,id=n1 -netdev tap,id=h1,queues=8 -vnc :0 -mon chardev=qmp,mode=control,pretty=on -chardev socket,id=qmp,host=localhost,port=1234,server,nowait -serial stdio -object rng-random,filename=/dev/urandom,id=rng0 -device virtio-rng-pci,rng=rng0,id=h0 -object rng-random,filename=/dev/urandom,id=rng1 -device virtio-rng-pci,rng=rng1,id=h1 > > > > It works when I only add one virtio-rng device. Did you touch this > > problem? > > > > I'm using latest net-next/master (ac3d2e5a9ef2f4d8f57c50070c4883ecb7cec29f) Hi Amit, > <snip> > > [ 0.223503] Non-volatile memory driver v1.3 > > [ 1.172293] tsc: Refined TSC clocksource calibration: 2893.436 MHz > > qemu: terminating on signal 2 <---------- (I have to cancel QEMU process by Ctrl + C) > > This looks similar to what I saw when driver asks for randomness from the host > before probe is completed. > > Does the following patch help? This patch was already inclued in latest net-next/master patch commit: e052dbf554610e2104c5a7518c4d8374bed701bb > While the driver is setup (DRIVER_OK is set), the vqs aren't marked > usable before the probe for the other device finishes as well. That's > not a scenario I considered. We can try to put this patch in 3.16, > but it won't be applicable for 3.17, where this is handled the proper > way. Also, 3.15 isn't affected, since that doesn't have multi-device > support. > > Also attaching a patch that enables traces in qemu so it's easier to > see what's going on. > > diff --git a/drivers/char/hw_random/virtio-rng.c > b/drivers/char/hw_random/virtio-rng.c > index e9b15bc..124cac5 100644 > --- a/drivers/char/hw_random/virtio-rng.c > +++ b/drivers/char/hw_random/virtio-rng.c > @@ -34,12 +34,11 @@ struct virtrng_info { > unsigned int data_avail; > struct completion have_data; > bool busy; > + bool probe_done; > char name[25]; > int index; > }; > > -static bool probe_done; > - > static void random_recv_done(struct virtqueue *vq) > { > struct virtrng_info *vi = vq->vdev->priv; > @@ -73,7 +72,7 @@ static int virtio_read(struct hwrng *rng, void *buf, > -size_t size, bool wait) > * Don't ask host for data till we're setup. This call can > * happen during hwrng_register(), after commit d9e7972619. > */ > - if (unlikely(!probe_done)) > + if (unlikely(!vi->probe_done)) > return 0; > > if (!vi->busy) { > @@ -146,7 +145,7 @@ static int probe_common(struct virtio_device > *vdev) > return err; > } > > - probe_done = true; > + vi->probe_done = true; > return 0; > } > > > Amit > >From ec1aa555b67628beefa0ac6902baa2cc2e156f58 Mon Sep 17 00:00:00 2001 > Message-Id: <ec1aa555b67628beefa0ac6902baa2cc2e156f58.1406533638.git.amit.shah@redhat.com> > From: Amit Shah <amit.shah@redhat.com> > Date: Mon, 21 Jul 2014 14:46:28 +0530 > Subject: [PATCH 1/1] virtio-rng: add some trace events > > Add some trace events to virtio-rng for easier debugging > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > --- > hw/virtio/virtio-rng.c | 7 +++++++ > trace-events | 5 +++++ > 2 files changed, 12 insertions(+) > > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c > index 7c5a675..4a6472b 100644 > --- a/hw/virtio/virtio-rng.c > +++ b/hw/virtio/virtio-rng.c > @@ -16,6 +16,7 @@ > #include "hw/virtio/virtio-rng.h" > #include "sysemu/rng.h" > #include "qom/object_interfaces.h" > +#include "trace.h" > > static bool is_guest_ready(VirtIORNG *vrng) > { > @@ -24,6 +25,7 @@ static bool is_guest_ready(VirtIORNG *vrng) > && (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) { > return true; > } > + trace_virtio_rng_guest_not_ready(vrng); > return false; > } > > @@ -62,6 +64,7 @@ static void chr_read(void *opaque, const void *buf, size_t size) > offset += len; > > virtqueue_push(vrng->vq, &elem, len); > + trace_virtio_rng_pushed(vrng, len); > } > virtio_notify(vdev, vrng->vq); > } > @@ -81,7 +84,11 @@ static void virtio_rng_process(VirtIORNG *vrng) > quota = MIN((uint64_t)vrng->quota_remaining, (uint64_t)UINT32_MAX); > } > size = get_request_size(vrng->vq, quota); > + > + trace_virtio_rng_request(vrng, size, quota); > + > size = MIN(vrng->quota_remaining, size); > + > if (size) { > rng_backend_request_entropy(vrng->rng, size, chr_read, vrng); > } > diff --git a/trace-events b/trace-events > index 11a17a8..99f39ac 100644 > --- a/trace-events > +++ b/trace-events > @@ -41,6 +41,11 @@ virtio_irq(void *vq) "vq %p" > virtio_notify(void *vdev, void *vq) "vdev %p vq %p" > virtio_set_status(void *vdev, uint8_t val) "vdev %p val %u" > > +# hw/virtio/virtio-rng.c > +virtio_rng_guest_not_ready(void *rng) "rng %p: guest not ready" > +virtio_rng_pushed(void *rng, size_t len) "rng %p: %zd bytes pushed" > +virtio_rng_request(void *rng, size_t size, unsigned quota) "rng %p: %zd bytes requested, %u bytes quota left" > + > # hw/char/virtio-serial-bus.c > virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t value) "port %u, event %u, value %u" > virtio_serial_throttle_port(unsigned int port, bool throttle) "port %u, throttle %d" > -- > 1.9.3 >
On (Mon) 28 Jul 2014 [16:49:20], Amos Kong wrote: > On Mon, Jul 28, 2014 at 01:25:14PM +0530, Amit Shah wrote: > > On (Mon) 28 Jul 2014 [15:32:42], Amos Kong wrote: > > > QEMU commandline: > > > > > > ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -m 2000 -drive file=/images/nolvm.qcow2 --kernel /home/devel/linux/arch/x86/boot/bzImage -append "ro root=/dev/sda1 console=ttyS0,115200" -monitor unix:/tmp/m,nowait,server -device virtio-net-pci,netdev=h0,vectors=17,mq=on,id=n0 -netdev tap,id=h0,queues=8 -device virtio-net-pci,netdev=h1,vectors=0,mq=on,id=n1 -netdev tap,id=h1,queues=8 -vnc :0 -mon chardev=qmp,mode=control,pretty=on -chardev socket,id=qmp,host=localhost,port=1234,server,nowait -serial stdio -object rng-random,filename=/dev/urandom,id=rng0 -device virtio-rng-pci,rng=rng0,id=h0 -object rng-random,filename=/dev/urandom,id=rng1 -device virtio-rng-pci,rng=rng1,id=h1 > > > > > > It works when I only add one virtio-rng device. Did you touch this > > > problem? > > > > > > I'm using latest net-next/master (ac3d2e5a9ef2f4d8f57c50070c4883ecb7cec29f) > > Hi Amit, > > > <snip> > > > [ 0.223503] Non-volatile memory driver v1.3 > > > [ 1.172293] tsc: Refined TSC clocksource calibration: 2893.436 MHz > > > qemu: terminating on signal 2 <---------- (I have to cancel QEMU process by Ctrl + C) > > > > This looks similar to what I saw when driver asks for randomness from the host > > before probe is completed. > > > > Does the following patch help? > > This patch was already inclued in latest net-next/master > patch commit: e052dbf554610e2104c5a7518c4d8374bed701bb No, it's a different one, goes on top of the commit you referenced. Amit -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 28, 2014 at 02:42:13PM +0530, Amit Shah wrote: > On (Mon) 28 Jul 2014 [16:49:20], Amos Kong wrote: > > On Mon, Jul 28, 2014 at 01:25:14PM +0530, Amit Shah wrote: > > > On (Mon) 28 Jul 2014 [15:32:42], Amos Kong wrote: > > > > QEMU commandline: > > > > > > > > ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -m 2000 -drive file=/images/nolvm.qcow2 --kernel /home/devel/linux/arch/x86/boot/bzImage -append "ro root=/dev/sda1 console=ttyS0,115200" -monitor unix:/tmp/m,nowait,server -device virtio-net-pci,netdev=h0,vectors=17,mq=on,id=n0 -netdev tap,id=h0,queues=8 -device virtio-net-pci,netdev=h1,vectors=0,mq=on,id=n1 -netdev tap,id=h1,queues=8 -vnc :0 -mon chardev=qmp,mode=control,pretty=on -chardev socket,id=qmp,host=localhost,port=1234,server,nowait -serial stdio -object rng-random,filename=/dev/urandom,id=rng0 -device virtio-rng-pci,rng=rng0,id=h0 -object rng-random,filename=/dev/urandom,id=rng1 -device virtio-rng-pci,rng=rng1,id=h1 > > > > > > > > It works when I only add one virtio-rng device. Did you touch this > > > > problem? > > > > > > > > I'm using latest net-next/master (ac3d2e5a9ef2f4d8f57c50070c4883ecb7cec29f) > > > > Hi Amit, > > > > > <snip> > > > > [ 0.223503] Non-volatile memory driver v1.3 > > > > [ 1.172293] tsc: Refined TSC clocksource calibration: 2893.436 MHz > > > > qemu: terminating on signal 2 <---------- (I have to cancel QEMU process by Ctrl + C) > > > > > > This looks similar to what I saw when driver asks for randomness from the host > > > before probe is completed. > > > > > > Does the following patch help? > > > > This patch was already inclued in latest net-next/master > > patch commit: e052dbf554610e2104c5a7518c4d8374bed701bb > > No, it's a different one, goes on top of the commit you referenced. Thanks. This patch fixed the problem.
3.16 (guest hangs with two rng devices) 3.16 + quick fix (can startup with two rng devices) (hotplug issue 1 + hotplug issue 2 exist) lates torvalds/linux.git + amit 4 patches (can startup with two rng devices) (only hotplug issue 2 exists) However, the 4 patches also fixed the hang issue, the hotplug issue was fixed a little. The hotplug issue is effected by the backend, or maybe it's not a real issue, because the rng device can be hot-removed after dd process is killed. Hotplug issue 1: 1. boot up guest with two rng device (rng0 uses /dev/urandom, rng1 uses /dev/random) 2. read data by dd in guest 3 (option 1). hot-remove rng0, then hot-remove rng1 -> result: _only rng1_ can't be removed until dd process is killed 3 (option 2). hot-remove rng1, then hot-remove rng0 -> result: two devices can be removed successfully, dd process will exit automatically. If we use /dev/urandom for rng0 and rng1, _rng0 & rng1_ can be removed, dd process will exit automatically. Hotplug issue 2: If we use /dev/random for rng0 and rng1, _rng0 & rng1_ can't be removed until dd process is killed. Hotplug issue 3: If we use /dev/random for rng0 and rng1, _only rng1_ can't be removed until dd process is killed. (The difference between /dev/random and /dev/urandom is the speed.) Thanks, Amos -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 05, 2014 at 06:28:54PM +0800, Amos Kong wrote: > 3.16 (guest hangs with two rng devices) > 3.16 + quick fix (can startup with two rng devices) (hotplug issue 1 + hotplug issue 2 exist) > lates torvalds/linux.git + amit 4 patches (can startup with two rng devices) (only hotplug issue 2 exists) > > However, the 4 patches also fixed the hang issue, the hotplug issue was fixed a little. > The hotplug issue is effected by the backend, or maybe it's not a real issue, because > the rng device can be hot-removed after dd process is killed. > > > Hotplug issue 1: > 1. boot up guest with two rng device (rng0 uses /dev/urandom, rng1 uses /dev/random) > 2. read data by dd in guest > 3 (option 1). hot-remove rng0, then hot-remove rng1 -> result: _only rng1_ can't be removed until dd process is killed > 3 (option 2). hot-remove rng1, then hot-remove rng0 -> result: two devices can be removed successfully, dd process will exit automatically. > > If we use /dev/urandom for rng0 and rng1, _rng0 & rng1_ can be removed, dd process will exit automatically. > > Hotplug issue 2: > If we use /dev/random for rng0 and rng1, _rng0 & rng1_ can't be removed until dd process is killed. > > Hotplug issue 3: > If we use /dev/random for rng0 and rng1, _only rng1_ can't be removed until dd process is killed. Hi Amit, I finally found the root problem and posted a fix to upstream: http://lists.linuxfoundation.org/pipermail/virtualization/2014-August/027049.html It can help to fix the hotplug issues on 3.16 & latest kernel, so stable kernel is CCed. > (The difference between /dev/random and /dev/urandom is the speed.) > > Thanks, Amos
diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c index e9b15bc..124cac5 100644 --- a/drivers/char/hw_random/virtio-rng.c +++ b/drivers/char/hw_random/virtio-rng.c @@ -34,12 +34,11 @@ struct virtrng_info { unsigned int data_avail; struct completion have_data; bool busy; + bool probe_done; char name[25]; int index; }; -static bool probe_done; - static void random_recv_done(struct virtqueue *vq) { struct virtrng_info *vi = vq->vdev->priv; @@ -73,7 +72,7 @@ static int virtio_read(struct hwrng *rng, void *buf, -size_t size, bool wait) * Don't ask host for data till we're setup. This call can * happen during hwrng_register(), after commit d9e7972619. */ - if (unlikely(!probe_done)) + if (unlikely(!vi->probe_done)) return 0; if (!vi->busy) { @@ -146,7 +145,7 @@ static int probe_common(struct virtio_device *vdev) return err; } - probe_done = true; + vi->probe_done = true; return 0; }