[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active |
Date: |
Mon, 14 Nov 2016 19:10:54 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 14/11/2016 18:09, Alex Williamson wrote:
> Hmm, fixed yet not fixed. I get a nice shutdown and it even eliminates
> a cpu spike shown in virt-manager at the end of shutdown that was
> typical previously, but then I noticed dmesg showing me segfaults, so I
> hooked up gdb and:
Well, that I don't get the segfault already says something... Though
it's not even clear from the backtrace what is causing the segfault.
Let's try the same printf without the change.
Paolo
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index f2ea29d..ec0f750 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -108,11 +108,13 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s)
VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
int i;
+ printf("before clear\n");
virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, NULL);
virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, NULL);
for (i = 0; i < vs->conf.num_queues; i++) {
virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx,
NULL);
}
+ printf("after clear\n");
}
/* Context: QEMU global mutex held */
@@ -202,15 +204,18 @@ void virtio_scsi_dataplane_stop(VirtIODevice *vdev)
aio_context_acquire(s->ctx);
virtio_scsi_clear_aio(s);
aio_context_release(s->ctx);
+ printf("before drain\n");
blk_drain_all(); /* ensure there are no in-flight requests */
+ printf("after drain\n");
for (i = 0; i < vs->conf.num_queues + 2; i++) {
virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
}
/* Clean up guest notifier (irq) */
+ printf("end of virtio_scsi_dataplane_stop\n");
k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
s->dataplane_stopping = false;
s->dataplane_started = false;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3e5ae6a..dabcb54 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -75,6 +75,7 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
}
if (req->sreq) {
+ printf("finish %x\n", req->sreq->tag);
req->sreq->hba_private = NULL;
scsi_req_unref(req->sreq);
}
@@ -549,6 +550,7 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI
*s, VirtIOSCSIReq *req)
return -ENOENT;
}
virtio_scsi_ctx_check(s, d);
+ printf("prepare %lx %x\n", req->req.cmd.tag, req->req.cmd.cdb[0]);
req->sreq = scsi_req_new(d, req->req.cmd.tag,
virtio_scsi_get_lun(req->req.cmd.lun),
req->req.cmd.cdb, req);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 62001b4..c75dec3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -336,11 +336,13 @@ static int virtio_pci_ioeventfd_assign(DeviceState *d,
EventNotifier *notifier,
static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
{
+ printf("start ioeventfd %s\n",
object_class_get_name(object_get_class(OBJECT(proxy))));
virtio_bus_start_ioeventfd(&proxy->bus);
}
static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
{
+ printf("stop ioeventfd %s\n",
object_class_get_name(object_get_class(OBJECT(proxy))));
virtio_bus_stop_ioeventfd(&proxy->bus);
}
@@ -376,6 +378,7 @@ static void virtio_ioport_write(void *opaque, uint32_t
addr, uint32_t val)
}
break;
case VIRTIO_PCI_STATUS:
+ printf("set status %s %x\n",
object_class_get_name(object_get_class(OBJECT(proxy))), val & 0xFF);
if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
virtio_pci_stop_ioeventfd(proxy);
}
@@ -1274,6 +1277,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr
addr,
vdev->config_vector = val;
break;
case VIRTIO_PCI_COMMON_STATUS:
+ printf("set status %s %x\n",
object_class_get_name(object_get_class(OBJECT(proxy))), (uint8_t)val);
if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
virtio_pci_stop_ioeventfd(proxy);
}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 89b0b80..9c894d7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2018,7 +2018,7 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue
*vq)
return &vq->guest_notifier;
}
-static void virtio_queue_host_notifier_aio_read(EventNotifier *n)
+void virtio_queue_host_notifier_aio_read(EventNotifier *n)
{
VirtQueue *vq = container_of(n, VirtQueue, host_notifier);
if (event_notifier_test_and_clear(n)) {
@@ -2046,6 +2046,9 @@ void virtio_queue_host_notifier_read(EventNotifier *n)
{
VirtQueue *vq = container_of(n, VirtQueue, host_notifier);
if (event_notifier_test_and_clear(n)) {
+ VirtIODevice *vdev = vq->vdev;
+
+ printf("virtio_queue_host_notifier_read %ld\n", vq - vdev->vq);
virtio_queue_notify_vq(vq);
}
}
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 35ede30..d3dfc69 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -274,6 +274,7 @@ int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
void virtio_device_release_ioeventfd(VirtIODevice *vdev);
bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
+void virtio_queue_host_notifier_aio_read(EventNotifier *n);
void virtio_queue_host_notifier_read(EventNotifier *n);
void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
void (*fn)(VirtIODevice *,
> Thread 3 "CPU 0/KVM" received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7fb4f73ba700 (LWP 2713)]
> 0x00005593dacc2800 in virtio_queue_notify_aio_vq (vq=0x5593dd4a7378) at
> /net/gimli/home/alwillia/Work/qemu.git/hw/virtio/virtio.c:1242
> 1242 trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
> (gdb) bt
> #0 0x00005593dacc2800 in virtio_queue_notify_aio_vq (vq=0x5593dd4a7378) at
> /net/gimli/home/alwillia/Work/qemu.git/hw/virtio/virtio.c:1242
> #1 0x00005593dacc4a4e in virtio_queue_host_notifier_aio_read
> (n=0x5593dd4a73d8) at
> /net/gimli/home/alwillia/Work/qemu.git/hw/virtio/virtio.c:2025
> #2 0x00005593daca4997 in virtio_scsi_dataplane_stop (vdev=0x5593dc0cc0f0) at
> /net/gimli/home/alwillia/Work/qemu.git/hw/scsi/virtio-scsi-dataplane.c:209
> #3 0x00005593daf6a4b7 in virtio_bus_stop_ioeventfd (bus=0x5593dc0cc078) at
> hw/virtio/virtio-bus.c:219
> #4 0x00005593daf64279 in virtio_pci_stop_ioeventfd (proxy=0x5593dc0c3ce0) at
> hw/virtio/virtio-pci.c:344
> #5 0x00005593daf643d5 in virtio_ioport_write (opaque=0x5593dc0c3ce0,
> addr=18, val=0) at hw/virtio/virtio-pci.c:380
> #6 0x00005593daf6484d in virtio_pci_config_write (opaque=0x5593dc0c3ce0,
> addr=18, val=0, size=1) at hw/virtio/virtio-pci.c:508
> #7 0x00005593dac592fd in memory_region_write_accessor (mr=0x5593dc0c45d0,
> addr=18, value=0x7fb4f73b74b8, size=1, shift=0, mask=255, attrs=...)
> at /net/gimli/home/alwillia/Work/qemu.git/memory.c:526
> #8 0x00005593dac59515 in access_with_adjusted_size (addr=18,
> value=0x7fb4f73b74b8, size=1, access_size_min=1, access_size_max=4, access=
> 0x5593dac59213 <memory_region_write_accessor>, mr=0x5593dc0c45d0,
> attrs=...) at /net/gimli/home/alwillia/Work/qemu.git/memory.c:592
> #9 0x00005593dac5bc55 in memory_region_dispatch_write (mr=0x5593dc0c45d0,
> addr=18, data=0, size=1, attrs=...) at
> /net/gimli/home/alwillia/Work/qemu.git/memory.c:1323
> #10 0x00005593dac07583 in address_space_write_continue (as=0x5593db727de0
> <address_space_io>, addr=49298, attrs=..., buf=0x7fb520354000 "", len=1,
> addr1=18, l=1, mr=0x5593dc0c45d0)
> at /net/gimli/home/alwillia/Work/qemu.git/exec.c:2621
> #11 0x00005593dac076cb in address_space_write (as=0x5593db727de0
> <address_space_io>, addr=49298, attrs=..., buf=0x7fb520354000 "", len=1)
> at /net/gimli/home/alwillia/Work/qemu.git/exec.c:2666
> #12 0x00005593dac07a57 in address_space_rw (as=0x5593db727de0
> <address_space_io>, addr=49298, attrs=..., buf=0x7fb520354000 "", len=1,
> is_write=true)
> at /net/gimli/home/alwillia/Work/qemu.git/exec.c:2768
> #13 0x00005593dac558d7 in kvm_handle_io (port=49298, attrs=...,
> data=0x7fb520354000, direction=1, size=1, count=1) at
> /net/gimli/home/alwillia/Work/qemu.git/kvm-all.c:1800
> #14 0x00005593dac55ddd in kvm_cpu_exec (cpu=0x5593dc0a6490) at
> /net/gimli/home/alwillia/Work/qemu.git/kvm-all.c:1958
> #15 0x00005593dac3cc58 in qemu_kvm_cpu_thread_fn (arg=0x5593dc0a6490) at
> /net/gimli/home/alwillia/Work/qemu.git/cpus.c:998
> #16 0x00007fb5054715ca in start_thread (arg=0x7fb4f73ba700) at
> pthread_create.c:333
> #17 0x00007fb5051ab0ed in clone () at
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
>
>> And if it doesn't work here is some printf debugging. It's pretty verbose
>> but
>> the interesting part starts pretty much where you issue the virsh shutdown or
>> system_powerdown command:
>>
>> diff --git a/hw/scsi/virtio-scsi-dataplane.c
>> b/hw/scsi/virtio-scsi-dataplane.c
>> index f2ea29d..ec0f750 100644
>> --- a/hw/scsi/virtio-scsi-dataplane.c
>> +++ b/hw/scsi/virtio-scsi-dataplane.c
>> @@ -108,11 +108,13 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s)
>> VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
>> int i;
>>
>> + printf("before clear\n");
>> virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, NULL);
>> virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, NULL);
>> for (i = 0; i < vs->conf.num_queues; i++) {
>> virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx,
>> NULL);
>> }
>> + printf("after clear\n");
>> }
>>
>> /* Context: QEMU global mutex held */
>> @@ -202,15 +204,20 @@ void virtio_scsi_dataplane_stop(VirtIODevice *vdev)
>>
>> aio_context_acquire(s->ctx);
>> virtio_scsi_clear_aio(s);
>> - aio_context_release(s->ctx);
>> -
>> - blk_drain_all(); /* ensure there are no in-flight requests */
>>
>> for (i = 0; i < vs->conf.num_queues + 2; i++) {
>> + VirtQueue *vq = virtio_get_queue(vdev, i);
>> virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
>> +
>> virtio_queue_host_notifier_aio_read(virtio_queue_get_guest_notifier(vq));
>> }
>> + aio_context_release(s->ctx);
>> +
>> + printf("before drain\n");
>> + blk_drain_all(); /* ensure there are no in-flight requests */
>> + printf("after drain\n");
>>
>> /* Clean up guest notifier (irq) */
>> + printf("end of virtio_scsi_dataplane_stop\n");
>> k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
>> s->dataplane_stopping = false;
>> s->dataplane_started = false;
>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>> index 3e5ae6a..e8b83d4 100644
>> --- a/hw/scsi/virtio-scsi.c
>> +++ b/hw/scsi/virtio-scsi.c
>> @@ -75,6 +75,7 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
>> }
>>
>> if (req->sreq) {
>> + printf("finish %x\n", req->sreq->tag);
>> req->sreq->hba_private = NULL;
>> scsi_req_unref(req->sreq);
>> }
>> @@ -549,6 +549,7 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI
>> *s, VirtIOSCSIReq *req)
>> return -ENOENT;
>> }
>> virtio_scsi_ctx_check(s, d);
>> + printf("prepare %lx %x\n", req->req.cmd.tag, req->req.cmd.cdb[0]);
>> req->sreq = scsi_req_new(d, req->req.cmd.tag,
>> virtio_scsi_get_lun(req->req.cmd.lun),
>> req->req.cmd.cdb, req);
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index 62001b4..c75dec3 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -336,11 +336,13 @@ static int virtio_pci_ioeventfd_assign(DeviceState *d,
>> EventNotifier *notifier,
>>
>> static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
>> {
>> + printf("start ioeventfd %s\n",
>> object_class_get_name(object_get_class(OBJECT(proxy))));
>> virtio_bus_start_ioeventfd(&proxy->bus);
>> }
>>
>> static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
>> {
>> + printf("stop ioeventfd %s\n",
>> object_class_get_name(object_get_class(OBJECT(proxy))));
>> virtio_bus_stop_ioeventfd(&proxy->bus);
>> }
>>
>> @@ -376,6 +378,7 @@ static void virtio_ioport_write(void *opaque, uint32_t
>> addr, uint32_t val)
>> }
>> break;
>> case VIRTIO_PCI_STATUS:
>> + printf("set status %s %x\n",
>> object_class_get_name(object_get_class(OBJECT(proxy))), val & 0xFF);
>> if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
>> virtio_pci_stop_ioeventfd(proxy);
>> }
>> @@ -1274,6 +1277,7 @@ static void virtio_pci_common_write(void *opaque,
>> hwaddr addr,
>> vdev->config_vector = val;
>> break;
>> case VIRTIO_PCI_COMMON_STATUS:
>> + printf("set status %s %x\n",
>> object_class_get_name(object_get_class(OBJECT(proxy))), (uint8_t)val);
>> if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
>> virtio_pci_stop_ioeventfd(proxy);
>> }
>>
>
> This required making virtio_queue_host_notifier_aio_read() non-static
> and adding the forward declaration, stolen from the first patch. The
> attached log starts at the point where there guest is idle and I issue
> a virsh shutdown. This also results in a segfault nearly identical to
> above:
>
> Thread 4 "CPU 1/KVM" received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7f7194901700 (LWP 3804)]
> 0x000056358070788a in virtio_queue_notify_aio_vq (vq=0x56358310d378) at
> /net/gimli/home/alwillia/Work/qemu.git/hw/virtio/virtio.c:1242
> 1242 trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
> (gdb) bt
> #0 0x000056358070788a in virtio_queue_notify_aio_vq (vq=0x56358310d378) at
> /net/gimli/home/alwillia/Work/qemu.git/hw/virtio/virtio.c:1242
> #1 0x0000563580709ad8 in virtio_queue_host_notifier_aio_read
> (n=0x56358310d3d8) at
> /net/gimli/home/alwillia/Work/qemu.git/hw/virtio/virtio.c:2025
> #2 0x00005635806e99fd in virtio_scsi_dataplane_stop (vdev=0x563581d320f0) at
> /net/gimli/home/alwillia/Work/qemu.git/hw/scsi/virtio-scsi-dataplane.c:211
> #3 0x00005635809af5fe in virtio_bus_stop_ioeventfd (bus=0x563581d32078) at
> hw/virtio/virtio-bus.c:219
> #4 0x00005635809a9353 in virtio_pci_stop_ioeventfd (proxy=0x563581d29ce0) at
> hw/virtio/virtio-pci.c:346
> #5 0x00005635809a94e0 in virtio_ioport_write (opaque=0x563581d29ce0,
> addr=18, val=0) at hw/virtio/virtio-pci.c:383
> #6 0x00005635809a995d in virtio_pci_config_write (opaque=0x563581d29ce0,
> addr=18, val=0, size=1) at hw/virtio/virtio-pci.c:511
> #7 0x000056358069e2fd in memory_region_write_accessor (mr=0x563581d2a5d0,
> addr=18, value=0x7f71948fe4b8, size=1, shift=0, mask=255, attrs=...)
> at /net/gimli/home/alwillia/Work/qemu.git/memory.c:526
> #8 0x000056358069e515 in access_with_adjusted_size (addr=18,
> value=0x7f71948fe4b8, size=1, access_size_min=1, access_size_max=4, access=
> 0x56358069e213 <memory_region_write_accessor>, mr=0x563581d2a5d0,
> attrs=...) at /net/gimli/home/alwillia/Work/qemu.git/memory.c:592
> #9 0x00005635806a0c55 in memory_region_dispatch_write (mr=0x563581d2a5d0,
> addr=18, data=0, size=1, attrs=...) at
> /net/gimli/home/alwillia/Work/qemu.git/memory.c:1323
> #10 0x000056358064c583 in address_space_write_continue (as=0x56358116cde0
> <address_space_io>, addr=49298, attrs=..., buf=0x7f71be099000 "", len=1,
> addr1=18, l=1, mr=0x563581d2a5d0)
> at /net/gimli/home/alwillia/Work/qemu.git/exec.c:2621
> #11 0x000056358064c6cb in address_space_write (as=0x56358116cde0
> <address_space_io>, addr=49298, attrs=..., buf=0x7f71be099000 "", len=1)
> at /net/gimli/home/alwillia/Work/qemu.git/exec.c:2666
> #12 0x000056358064ca57 in address_space_rw (as=0x56358116cde0
> <address_space_io>, addr=49298, attrs=..., buf=0x7f71be099000 "", len=1,
> is_write=true)
> at /net/gimli/home/alwillia/Work/qemu.git/exec.c:2768
> #13 0x000056358069a8d7 in kvm_handle_io (port=49298, attrs=...,
> data=0x7f71be099000, direction=1, size=1, count=1) at
> /net/gimli/home/alwillia/Work/qemu.git/kvm-all.c:1800
> #14 0x000056358069addd in kvm_cpu_exec (cpu=0x563581d6d030) at
> /net/gimli/home/alwillia/Work/qemu.git/kvm-all.c:1958
> #15 0x0000563580681c58 in qemu_kvm_cpu_thread_fn (arg=0x563581d6d030) at
> /net/gimli/home/alwillia/Work/qemu.git/cpus.c:998
> #16 0x00007f71a31b95ca in start_thread (arg=0x7f7194901700) at
> pthread_create.c:333
> #17 0x00007f71a2ef30ed in clone () at
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
>
> If you care to match line numbers, my tree is based on
> 6bbcb76301a72dc80c8d29af13d40bb9a759c9c6, it includes you patch:
>
> virtio: introduce grab/release_ioeventfd to fix vhost
>
> Plus your first fix removing the assert and return 0 case from
> virtio_bus_set_host_notifier(). Thanks,
>
> Alex
>
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active, Alex Williamson, 2016/11/10
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active, Paolo Bonzini, 2016/11/11
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active, Alex Williamson, 2016/11/11
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active, Paolo Bonzini, 2016/11/14
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active, Alex Williamson, 2016/11/14
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active, Alex Williamson, 2016/11/14
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active, Paolo Bonzini, 2016/11/14
- Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active, Alex Williamson, 2016/11/14