From nobody Tue May 7 02:18:57 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1489505860338255.51737660471815; Tue, 14 Mar 2017 08:37:40 -0700 (PDT) Received: from localhost ([::1]:60168 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cnoW1-0000c2-16 for importer@patchew.org; Tue, 14 Mar 2017 11:37:37 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45310) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cnoV1-0000ZO-MS for qemu-devel@nongnu.org; Tue, 14 Mar 2017 11:36:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cnoUz-0001SQ-Mk for qemu-devel@nongnu.org; Tue, 14 Mar 2017 11:36:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54764) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cnoUz-0001S2-HG for qemu-devel@nongnu.org; Tue, 14 Mar 2017 11:36:33 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A41BBC04B945 for ; Tue, 14 Mar 2017 15:36:33 +0000 (UTC) Received: from lemon.redhat.com (ovpn-8-40.pek2.redhat.com [10.72.8.40]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v2EFaSvH005802; Tue, 14 Mar 2017 11:36:32 -0400 From: Fam Zheng To: qemu-devel@nongnu.org Date: Tue, 14 Mar 2017 23:36:25 +0800 Message-Id: <20170314153626.16989-2-famz@redhat.com> In-Reply-To: <20170314153626.16989-1-famz@redhat.com> References: <20170314153626.16989-1-famz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 14 Mar 2017 15:36:33 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 1/2] virtio-scsi: Make virtio_scsi_acquire/release public X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" They will be used in virtio-scsi-dataplane.c as well, so move them to header. Signed-off-by: Fam Zheng --- hw/scsi/virtio-scsi.c | 14 -------------- include/hw/virtio/virtio-scsi.h | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 1dbc4bc..e7466d3 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -422,20 +422,6 @@ static void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s,= VirtIOSCSIReq *req) } } =20 -static inline void virtio_scsi_acquire(VirtIOSCSI *s) -{ - if (s->ctx) { - aio_context_acquire(s->ctx); - } -} - -static inline void virtio_scsi_release(VirtIOSCSI *s) -{ - if (s->ctx) { - aio_context_release(s->ctx); - } -} - bool virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq) { VirtIOSCSIReq *req; diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scs= i.h index f536f77..8ae0aca 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -121,6 +121,20 @@ typedef struct VirtIOSCSIReq { } req; } VirtIOSCSIReq; =20 +static inline void virtio_scsi_acquire(VirtIOSCSI *s) +{ + if (s->ctx) { + aio_context_acquire(s->ctx); + } +} + +static inline void virtio_scsi_release(VirtIOSCSI *s) +{ + if (s->ctx) { + aio_context_release(s->ctx); + } +} + void virtio_scsi_common_realize(DeviceState *dev, Error **errp, VirtIOHandleOutput ctrl, VirtIOHandleOutpu= t evt, VirtIOHandleOutput cmd); --=20 2.9.3 From nobody Tue May 7 02:18:57 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1489505860732136.1136368706351; Tue, 14 Mar 2017 08:37:40 -0700 (PDT) Received: from localhost ([::1]:60169 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cnoW2-0000dn-Fo for importer@patchew.org; Tue, 14 Mar 2017 11:37:38 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45383) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cnoV5-0000bq-P6 for qemu-devel@nongnu.org; Tue, 14 Mar 2017 11:36:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cnoV1-0001TD-Fv for qemu-devel@nongnu.org; Tue, 14 Mar 2017 11:36:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40000) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cnoV1-0001Sl-7U for qemu-devel@nongnu.org; Tue, 14 Mar 2017 11:36:35 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 58040128D for ; Tue, 14 Mar 2017 15:36:35 +0000 (UTC) Received: from lemon.redhat.com (ovpn-8-40.pek2.redhat.com [10.72.8.40]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v2EFaSvI005802; Tue, 14 Mar 2017 11:36:34 -0400 From: Fam Zheng To: qemu-devel@nongnu.org Date: Tue, 14 Mar 2017 23:36:26 +0800 Message-Id: <20170314153626.16989-3-famz@redhat.com> In-Reply-To: <20170314153626.16989-1-famz@redhat.com> References: <20170314153626.16989-1-famz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 14 Mar 2017 15:36:35 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 2/2] virtio-scsi: Fix acquire/release in dataplane handlers X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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=3Dnone,id=3Droot,format=3Draw,file=3DFedora-Cloud-Base-25-1= .3.x86_64.raw \ -device virtio-scsi \ -device scsi-disk,drive=3Droot,bootindex=3D1 \ -object iothread,id=3Dio \ -device virtio-scsi-pci,iothread=3Dio \ -net user,hostfwd=3Dtcp::10022-:22 -net nic,model=3Dvirtio -m 2048 \ --enable-kvm Fix this by moving acquire/release pairs from virtio_scsi_handle_*_vq to their callers - and wrap the broken assertions in. Signed-off-by: Fam Zheng Tested-by: Ed Swierk --- hw/scsi/virtio-scsi-dataplane.c | 20 ++++++++++++++++---- hw/scsi/virtio-scsi.c | 26 ++++++++++++++------------ 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplan= e.c index 74c95e0..944ea4e 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 =3D (VirtIOSCSI *)vdev; + bool progress; + VirtIOSCSI *s =3D VIRTIO_SCSI(vdev); =20 + virtio_scsi_acquire(s); assert(s->ctx && s->dataplane_started); - return virtio_scsi_handle_cmd_vq(s, vq); + progress =3D virtio_scsi_handle_cmd_vq(s, vq); + virtio_scsi_release(s); + return progress; } =20 static bool virtio_scsi_data_plane_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) { + bool progress; VirtIOSCSI *s =3D VIRTIO_SCSI(vdev); =20 + virtio_scsi_acquire(s); assert(s->ctx && s->dataplane_started); - return virtio_scsi_handle_ctrl_vq(s, vq); + progress =3D virtio_scsi_handle_ctrl_vq(s, vq); + virtio_scsi_release(s); + return progress; } =20 static bool virtio_scsi_data_plane_handle_event(VirtIODevice *vdev, VirtQueue *vq) { + bool progress; VirtIOSCSI *s =3D VIRTIO_SCSI(vdev); =20 + virtio_scsi_acquire(s); assert(s->ctx && s->dataplane_started); - return virtio_scsi_handle_event_vq(s, vq); + progress =3D virtio_scsi_handle_event_vq(s, vq); + virtio_scsi_release(s); + return progress; } =20 static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n, diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index e7466d3..4939f1f 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -427,12 +427,10 @@ bool virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQu= eue *vq) VirtIOSCSIReq *req; bool progress =3D false; =20 - virtio_scsi_acquire(s); while ((req =3D virtio_scsi_pop_req(s, vq))) { progress =3D true; virtio_scsi_handle_ctrl_req(s, req); } - virtio_scsi_release(s); return progress; } =20 @@ -446,7 +444,9 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev,= VirtQueue *vq) return; } } + virtio_scsi_acquire(s); virtio_scsi_handle_ctrl_vq(s, vq); + virtio_scsi_release(s); } =20 static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req) @@ -590,7 +590,6 @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue= *vq) =20 QTAILQ_HEAD(, VirtIOSCSIReq) reqs =3D QTAILQ_HEAD_INITIALIZER(reqs); =20 - virtio_scsi_acquire(s); do { virtio_queue_set_notification(vq, 0); =20 @@ -618,7 +617,6 @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue= *vq) QTAILQ_FOREACH_SAFE(req, &reqs, next, next) { virtio_scsi_handle_cmd_req_submit(s, req); } - virtio_scsi_release(s); return progress; } =20 @@ -633,7 +631,9 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, = VirtQueue *vq) return; } } + virtio_scsi_acquire(s); virtio_scsi_handle_cmd_vq(s, vq); + virtio_scsi_release(s); } =20 static void virtio_scsi_get_config(VirtIODevice *vdev, @@ -709,12 +709,10 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice= *dev, return; } =20 - virtio_scsi_acquire(s); - req =3D virtio_scsi_pop_req(s, vs->event_vq); if (!req) { s->events_dropped =3D true; - goto out; + return; } =20 if (s->events_dropped) { @@ -724,7 +722,7 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *= dev, =20 if (virtio_scsi_parse_req(req, 0, sizeof(VirtIOSCSIEvent))) { virtio_scsi_bad_req(req); - goto out; + return; } =20 evt =3D &req->resp.event; @@ -744,19 +742,15 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice= *dev, evt->lun[3] =3D dev->lun & 0xFF; } virtio_scsi_complete_req(req); -out: - virtio_scsi_release(s); } =20 bool virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq) { - virtio_scsi_acquire(s); if (s->events_dropped) { virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0); virtio_scsi_release(s); return true; } - virtio_scsi_release(s); return false; } =20 @@ -770,7 +764,9 @@ static void virtio_scsi_handle_event(VirtIODevice *vdev= , VirtQueue *vq) return; } } + virtio_scsi_acquire(s); virtio_scsi_handle_event_vq(s, vq); + virtio_scsi_release(s); } =20 static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense se= nse) @@ -780,8 +776,10 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevic= e *dev, SCSISense sense) =20 if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_CHANGE) && dev->type !=3D TYPE_ROM) { + virtio_scsi_acquire(s); virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE, sense.asc | (sense.ascq << 8)); + virtio_scsi_release(s); } } =20 @@ -803,9 +801,11 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplu= g_dev, DeviceState *dev, } =20 if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { + virtio_scsi_acquire(s); virtio_scsi_push_event(s, sd, VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_RESCAN); + virtio_scsi_release(s); } } =20 @@ -817,9 +817,11 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotp= lug_dev, DeviceState *dev, SCSIDevice *sd =3D SCSI_DEVICE(dev); =20 if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { + virtio_scsi_acquire(s); virtio_scsi_push_event(s, sd, VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_REMOVED); + virtio_scsi_release(s); } =20 qdev_simple_device_unplug_cb(hotplug_dev, dev, errp); --=20 2.9.3