| Message ID | 20170314132654.6751-1-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 14/03/2017 14:26, Fam Zheng wrote: > After the AioContext lock push down, there is a race between > virtio_scsi_dataplane_start and those "assert(s->ctx && > s->dataplane_started)", because the latter doesn't isn't wrapped in > aio_context_acquire. > > Reproducer is simply booting a Fedora guest with an empty > virtio-scsi-dataplane controller: > > qemu-system-x86_64 \ > -drive if=none,id=root,format=raw,file=Fedora-Cloud-Base-25-1.3.x86_64.raw \ > -device virtio-scsi \ > -device scsi-disk,drive=root,bootindex=1 \ > -object iothread,id=io \ > -device virtio-scsi-pci,iothread=io \ > -net user,hostfwd=tcp::10022-:22 -net nic,model=virtio -m 2048 \ > --enable-kvm > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > hw/scsi/virtio-scsi-dataplane.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c > index 74c95e0..12b44b7 100644 > --- a/hw/scsi/virtio-scsi-dataplane.c > +++ b/hw/scsi/virtio-scsi-dataplane.c > @@ -52,28 +52,40 @@ void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp) > static bool virtio_scsi_data_plane_handle_cmd(VirtIODevice *vdev, > VirtQueue *vq) > { > - VirtIOSCSI *s = (VirtIOSCSI *)vdev; > + bool progress; > + VirtIOSCSI *s = VIRTIO_SCSI(vdev); > > + aio_context_acquire(s->ctx); > assert(s->ctx && s->dataplane_started); > - return virtio_scsi_handle_cmd_vq(s, vq); > + progress = virtio_scsi_handle_cmd_vq(s, vq); > + aio_context_release(s->ctx); Can you use virtio_scsi_acquire/release, and remove it from virtio_scsi_handle_*_vq? Paolo > + return progress; > } > > static bool virtio_scsi_data_plane_handle_ctrl(VirtIODevice *vdev, > VirtQueue *vq) > { > + bool progress; > VirtIOSCSI *s = VIRTIO_SCSI(vdev); > > + aio_context_acquire(s->ctx); > assert(s->ctx && s->dataplane_started); > - return virtio_scsi_handle_ctrl_vq(s, vq); > + progress = virtio_scsi_handle_ctrl_vq(s, vq); > + aio_context_release(s->ctx); > + return progress; > } > > static bool virtio_scsi_data_plane_handle_event(VirtIODevice *vdev, > VirtQueue *vq) > { > + bool progress; > VirtIOSCSI *s = VIRTIO_SCSI(vdev); > > + aio_context_acquire(s->ctx); > assert(s->ctx && s->dataplane_started); > - return virtio_scsi_handle_event_vq(s, vq); > + progress = virtio_scsi_handle_event_vq(s, vq); > + aio_context_release(s->ctx); > + return progress; > } > > static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n, >
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index 74c95e0..12b44b7 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -52,28 +52,40 @@ void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp) static bool virtio_scsi_data_plane_handle_cmd(VirtIODevice *vdev, VirtQueue *vq) { - VirtIOSCSI *s = (VirtIOSCSI *)vdev; + bool progress; + VirtIOSCSI *s = VIRTIO_SCSI(vdev); + aio_context_acquire(s->ctx); assert(s->ctx && s->dataplane_started); - return virtio_scsi_handle_cmd_vq(s, vq); + progress = virtio_scsi_handle_cmd_vq(s, vq); + aio_context_release(s->ctx); + return progress; } static bool virtio_scsi_data_plane_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) { + bool progress; VirtIOSCSI *s = VIRTIO_SCSI(vdev); + aio_context_acquire(s->ctx); assert(s->ctx && s->dataplane_started); - return virtio_scsi_handle_ctrl_vq(s, vq); + progress = virtio_scsi_handle_ctrl_vq(s, vq); + aio_context_release(s->ctx); + return progress; } static bool virtio_scsi_data_plane_handle_event(VirtIODevice *vdev, VirtQueue *vq) { + bool progress; VirtIOSCSI *s = VIRTIO_SCSI(vdev); + aio_context_acquire(s->ctx); assert(s->ctx && s->dataplane_started); - return virtio_scsi_handle_event_vq(s, vq); + progress = virtio_scsi_handle_event_vq(s, vq); + aio_context_release(s->ctx); + return progress; } static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n,
After the AioContext lock push down, there is a race between virtio_scsi_dataplane_start and those "assert(s->ctx && s->dataplane_started)", because the latter doesn't isn't wrapped in aio_context_acquire. Reproducer is simply booting a Fedora guest with an empty virtio-scsi-dataplane controller: qemu-system-x86_64 \ -drive if=none,id=root,format=raw,file=Fedora-Cloud-Base-25-1.3.x86_64.raw \ -device virtio-scsi \ -device scsi-disk,drive=root,bootindex=1 \ -object iothread,id=io \ -device virtio-scsi-pci,iothread=io \ -net user,hostfwd=tcp::10022-:22 -net nic,model=virtio -m 2048 \ --enable-kvm Signed-off-by: Fam Zheng <famz@redhat.com> --- hw/scsi/virtio-scsi-dataplane.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)