From patchwork Thu Apr 16 20:36:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxim Levitsky X-Patchwork-Id: 11493741 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E16606CA for ; Thu, 16 Apr 2020 20:38:33 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B4B53206A2 for ; Thu, 16 Apr 2020 20:38:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="S9eJX6KF" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B4B53206A2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Received: from localhost ([::1]:39094 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jPBHM-00024H-RT for patchwork-qemu-devel@patchwork.kernel.org; Thu, 16 Apr 2020 16:38:32 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60490) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jPBFX-00080d-N0 for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jPBFW-00042d-PU for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:39 -0400 Received: from us-smtp-1.mimecast.com ([207.211.31.81]:37706 helo=us-smtp-delivery-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jPBFW-00042N-MH for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587069398; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ap+Pt4fSPMW5Snij4O6Cep7cm0g2WkDrl1yiJWCRlzk=; b=S9eJX6KF2jmkb95LUUTgsN9tgIS+Io4dhsHttQEEwrDZ7sJLZ+CLRvTZH3h+Ooq+zIOdB7 YsBCorlP7FVjMrbqz2H+WOwOitAzyzttct89OO8DWqY3Cs+TrzXIAPb+3Y/guk2uHmy0Y7 4KNei1cG1SB5fDnwH1oym+jxfrCUss8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-51-77ki8K5WNXmGOUm74Y_f4A-1; Thu, 16 Apr 2020 16:36:34 -0400 X-MC-Unique: 77ki8K5WNXmGOUm74Y_f4A-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CC57018C43C0; Thu, 16 Apr 2020 20:36:33 +0000 (UTC) Received: from maximlenovopc.usersys.redhat.com (unknown [10.35.206.56]) by smtp.corp.redhat.com (Postfix) with ESMTP id C165CA099C; Thu, 16 Apr 2020 20:36:31 +0000 (UTC) From: Maxim Levitsky To: qemu-devel@nongnu.org Subject: [PATCH 1/4] scsi/scsi_bus: switch search direction in scsi_device_find Date: Thu, 16 Apr 2020 23:36:21 +0300 Message-Id: <20200416203624.32366-2-mlevitsk@redhat.com> In-Reply-To: <20200416203624.32366-1-mlevitsk@redhat.com> References: <20200416203624.32366-1-mlevitsk@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 207.211.31.81 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Fam Zheng , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , Eduardo Habkost , "Michael S. Tsirkin" , Maxim Levitsky , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" This change will allow us to convert the bus children list to RCU, while not changing the logic of this function Signed-off-by: Maxim Levitsky --- hw/scsi/scsi-bus.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 1c980cab38..7bbc37acec 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1584,7 +1584,7 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun) BusChild *kid; SCSIDevice *target_dev = NULL; - QTAILQ_FOREACH_REVERSE(kid, &bus->qbus.children, sibling) { + QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) { DeviceState *qdev = kid->child; SCSIDevice *dev = SCSI_DEVICE(qdev); @@ -1592,7 +1592,15 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun) if (dev->lun == lun) { return dev; } - target_dev = dev; + + /* + * If we don't find exact match (channel/bus/lun), + * we will return the first device which matches channel/bus + */ + + if (!target_dev) { + target_dev = dev; + } } } return target_dev; From patchwork Thu Apr 16 20:36:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxim Levitsky X-Patchwork-Id: 11493739 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E5BCC6CA for ; Thu, 16 Apr 2020 20:37:16 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AB11A206A2 for ; Thu, 16 Apr 2020 20:37:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="OgjBV05Y" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AB11A206A2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Received: from localhost ([::1]:39084 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jPBG7-0000VA-Qi for patchwork-qemu-devel@patchwork.kernel.org; Thu, 16 Apr 2020 16:37:15 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60501) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jPBFZ-000838-J9 for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jPBFX-00043K-T0 for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:41 -0400 Received: from us-smtp-2.mimecast.com ([205.139.110.61]:55970 helo=us-smtp-delivery-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jPBFX-00042v-PY for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587069399; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oMa/5d3tGunme1aUOe1TbXlozhP9DjIscaSAWG74tqw=; b=OgjBV05YxsxohNOb/IgI4Q/OlZNwz12mnnPQJqPxChcRa7rYuCWKNmqiB7coAfuCAIiXPI h0nF21PTpuM4PzhOWRuyHqEHoCH//tLMbgXLWNA8ftJ8hnx9B5/Q2NGW5C2BThIrvezXcr ZJ2wNN5AIXKFEYjFWierL8lYvgyzONM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-220-Ho9U53NXNYqTsfC2X5ZzFA-1; Thu, 16 Apr 2020 16:36:37 -0400 X-MC-Unique: Ho9U53NXNYqTsfC2X5ZzFA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 32CEC8010F1; Thu, 16 Apr 2020 20:36:36 +0000 (UTC) Received: from maximlenovopc.usersys.redhat.com (unknown [10.35.206.56]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4EFB5A099C; Thu, 16 Apr 2020 20:36:34 +0000 (UTC) From: Maxim Levitsky To: qemu-devel@nongnu.org Subject: [PATCH 2/4] device-core: use RCU for list of childs of a bus Date: Thu, 16 Apr 2020 23:36:22 +0300 Message-Id: <20200416203624.32366-3-mlevitsk@redhat.com> In-Reply-To: <20200416203624.32366-1-mlevitsk@redhat.com> References: <20200416203624.32366-1-mlevitsk@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 205.139.110.61 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Fam Zheng , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , Eduardo Habkost , "Michael S. Tsirkin" , Maxim Levitsky , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" This fixes the race between device emulation code that tries to find a child device to dispatch the request to (e.g a scsi disk), and hotplug of a new device to that bus. Note that this doesn't convert all the readers of the list but only these that might go over that list without BQL held. This is a very small first step to make this code thread safe. Suggested-by: Paolo Bonzini Signed-off-by: Maxim Levitsky --- hw/core/bus.c | 43 ++++++++++++++++++++----------- hw/core/qdev.c | 46 +++++++++++++++++++++++----------- hw/scsi/scsi-bus.c | 17 ++++++++++--- hw/scsi/virtio-scsi.c | 6 ++++- include/hw/qdev-core.h | 3 +++ include/hw/virtio/virtio-bus.h | 7 ++++-- 6 files changed, 87 insertions(+), 35 deletions(-) diff --git a/hw/core/bus.c b/hw/core/bus.c index 3dc0a825f0..cb7756ded1 100644 --- a/hw/core/bus.c +++ b/hw/core/bus.c @@ -49,12 +49,14 @@ int qbus_walk_children(BusState *bus, } } - QTAILQ_FOREACH(kid, &bus->children, sibling) { - err = qdev_walk_children(kid->child, - pre_devfn, pre_busfn, - post_devfn, post_busfn, opaque); - if (err < 0) { - return err; + WITH_RCU_READ_LOCK_GUARD(){ + QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) { + err = qdev_walk_children(kid->child, + pre_devfn, pre_busfn, + post_devfn, post_busfn, opaque); + if (err < 0) { + return err; + } } } @@ -90,9 +92,13 @@ static void bus_reset_child_foreach(Object *obj, ResettableChildCallback cb, BusState *bus = BUS(obj); BusChild *kid; - QTAILQ_FOREACH(kid, &bus->children, sibling) { + rcu_read_lock(); + + QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) { cb(OBJECT(kid->child), opaque, type); } + + rcu_read_unlock(); } static void qbus_realize(BusState *bus, DeviceState *parent, const char *name) @@ -138,10 +144,15 @@ static void bus_unparent(Object *obj) /* Only the main system bus has no parent, and that bus is never freed */ assert(bus->parent); - while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) { + rcu_read_lock(); + + while ((kid = QTAILQ_FIRST_RCU(&bus->children)) != NULL) { DeviceState *dev = kid->child; object_unparent(OBJECT(dev)); } + + rcu_read_unlock(); + QLIST_REMOVE(bus, sibling); bus->parent->num_child_bus--; bus->parent = NULL; @@ -185,14 +196,18 @@ static void bus_set_realized(Object *obj, bool value, Error **errp) /* TODO: recursive realization */ } else if (!value && bus->realized) { - QTAILQ_FOREACH(kid, &bus->children, sibling) { - DeviceState *dev = kid->child; - object_property_set_bool(OBJECT(dev), false, "realized", - &local_err); - if (local_err != NULL) { - break; + + WITH_RCU_READ_LOCK_GUARD(){ + QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) { + DeviceState *dev = kid->child; + object_property_set_bool(OBJECT(dev), false, "realized", + &local_err); + if (local_err != NULL) { + break; + } } } + if (bc->unrealize && local_err == NULL) { bc->unrealize(bus, &local_err); } diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 85f062def7..f0c87e582e 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -50,26 +50,37 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev) return dc->vmsd; } +static void bus_free_bus_child(BusChild *kid) +{ + object_unref(OBJECT(kid->child)); + g_free(kid); +} + static void bus_remove_child(BusState *bus, DeviceState *child) { BusChild *kid; - QTAILQ_FOREACH(kid, &bus->children, sibling) { + rcu_read_lock(); + + QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) { if (kid->child == child) { char name[32]; snprintf(name, sizeof(name), "child[%d]", kid->index); - QTAILQ_REMOVE(&bus->children, kid, sibling); + QTAILQ_REMOVE_RCU(&bus->children, kid, sibling); bus->num_children--; /* This gives back ownership of kid->child back to us. */ object_property_del(OBJECT(bus), name, NULL); - object_unref(OBJECT(kid->child)); - g_free(kid); - return; + + /* free the bus kid, when it is safe to do so*/ + call_rcu(kid, bus_free_bus_child, rcu); + break; } } + + rcu_read_unlock(); } static void bus_add_child(BusState *bus, DeviceState *child) @@ -82,7 +93,9 @@ static void bus_add_child(BusState *bus, DeviceState *child) kid->child = child; object_ref(OBJECT(kid->child)); - QTAILQ_INSERT_HEAD(&bus->children, kid, sibling); + rcu_read_lock(); + QTAILQ_INSERT_HEAD_RCU(&bus->children, kid, sibling); + rcu_read_unlock(); /* This transfers ownership of kid->child to the property. */ snprintf(name, sizeof(name), "child[%d]", kid->index); @@ -681,20 +694,23 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id) DeviceState *ret; BusState *child; - QTAILQ_FOREACH(kid, &bus->children, sibling) { - DeviceState *dev = kid->child; + WITH_RCU_READ_LOCK_GUARD() { + QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) { + DeviceState *dev = kid->child; - if (dev->id && strcmp(dev->id, id) == 0) { - return dev; - } + if (dev->id && strcmp(dev->id, id) == 0) { + return dev; + } - QLIST_FOREACH(child, &dev->child_bus, sibling) { - ret = qdev_find_recursive(child, id); - if (ret) { - return ret; + QLIST_FOREACH(child, &dev->child_bus, sibling) { + ret = qdev_find_recursive(child, id); + if (ret) { + return ret; + } } } } + return NULL; } diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 7bbc37acec..2bf6d005f3 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -412,7 +412,10 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r) id = r->req.dev->id; found_lun0 = false; n = 0; - QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) { + + rcu_read_lock(); + + QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) { DeviceState *qdev = kid->child; SCSIDevice *dev = SCSI_DEVICE(qdev); @@ -433,7 +436,7 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r) memset(r->buf, 0, len); stl_be_p(&r->buf[0], n); i = found_lun0 ? 8 : 16; - QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) { + QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) { DeviceState *qdev = kid->child; SCSIDevice *dev = SCSI_DEVICE(qdev); @@ -442,6 +445,9 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r) i += 8; } } + + rcu_read_unlock(); + assert(i == n + 8); r->len = len; return true; @@ -1584,12 +1590,15 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun) BusChild *kid; SCSIDevice *target_dev = NULL; - QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) { + rcu_read_lock(); + + QTAILQ_FOREACH_RCU(kid, &bus->qbus.children, sibling) { DeviceState *qdev = kid->child; SCSIDevice *dev = SCSI_DEVICE(qdev); if (dev->channel == channel && dev->id == id) { if (dev->lun == lun) { + rcu_read_unlock(); return dev; } @@ -1603,6 +1612,8 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun) } } } + + rcu_read_unlock(); return target_dev; } diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 472bbd233b..b0f4a35f81 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -367,12 +367,16 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET: target = req->req.tmf.lun[1]; s->resetting++; - QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { + + rcu_read_lock(); + QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) { d = SCSI_DEVICE(kid->child); if (d->channel == 0 && d->id == target) { qdev_reset_all(&d->qdev); } } + rcu_read_unlock(); + s->resetting--; break; diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 1405b8a990..196253978b 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -3,6 +3,8 @@ #include "qemu/queue.h" #include "qemu/bitmap.h" +#include "qemu/rcu.h" +#include "qemu/rcu_queue.h" #include "qom/object.h" #include "hw/hotplug.h" #include "hw/resettable.h" @@ -218,6 +220,7 @@ struct BusClass { }; typedef struct BusChild { + struct rcu_head rcu; DeviceState *child; int index; QTAILQ_ENTRY(BusChild) sibling; diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index 38c9399cd4..58733f28e2 100644 --- a/include/hw/virtio/virtio-bus.h +++ b/include/hw/virtio/virtio-bus.h @@ -128,8 +128,11 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config); static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus) { BusState *qbus = &bus->parent_obj; - BusChild *kid = QTAILQ_FIRST(&qbus->children); - DeviceState *qdev = kid ? kid->child : NULL; + BusChild *kid; + DeviceState *qdev; + + kid = QTAILQ_FIRST(&qbus->children); + qdev = kid ? kid->child : NULL; /* This is used on the data path, the cast is guaranteed * to succeed by the qdev machinery. From patchwork Thu Apr 16 20:36:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxim Levitsky X-Patchwork-Id: 11493745 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 760636CA for ; Thu, 16 Apr 2020 20:40:58 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4B61E221F4 for ; Thu, 16 Apr 2020 20:40:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Z20H8sUG" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4B61E221F4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Received: from localhost ([::1]:39128 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jPBJh-0004wf-FB for patchwork-qemu-devel@patchwork.kernel.org; Thu, 16 Apr 2020 16:40:57 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60524) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jPBFh-0008FD-NU for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jPBFg-000487-Ps for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:49 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:44903 helo=us-smtp-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jPBFg-00047l-Mp for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587069408; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2RHcLfzv/AHZUi5OBodeG6jtQjhpZVEt+eRcfP9lsEw=; b=Z20H8sUGQBZuPRqNv/tKe6PR5Qj5rCRxxyxN8Q9HzS4CHsjPaQTgSE0CtBXi2Myvy00niJ Ih/4Krfaw4+JeDhoPE/rZ5KESigzZLr/Atq9VNhF6XxiSvU3py1fPB1b+Jb2vpHoVeFN67 yicGyMcHsmhIibijH37d/c0pvc3nncw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-141-d8kh3UcoPWaZFPyePOHkqg-1; Thu, 16 Apr 2020 16:36:39 -0400 X-MC-Unique: d8kh3UcoPWaZFPyePOHkqg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7124B18C43C1; Thu, 16 Apr 2020 20:36:38 +0000 (UTC) Received: from maximlenovopc.usersys.redhat.com (unknown [10.35.206.56]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8F8DAA099C; Thu, 16 Apr 2020 20:36:36 +0000 (UTC) From: Maxim Levitsky To: qemu-devel@nongnu.org Subject: [PATCH 3/4] device-core: use atomic_set on .realized property Date: Thu, 16 Apr 2020 23:36:23 +0300 Message-Id: <20200416203624.32366-4-mlevitsk@redhat.com> In-Reply-To: <20200416203624.32366-1-mlevitsk@redhat.com> References: <20200416203624.32366-1-mlevitsk@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 207.211.31.120 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Fam Zheng , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , Eduardo Habkost , "Michael S. Tsirkin" , Maxim Levitsky , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" Some code might race with placement of new devices on a bus. We currently first place a (unrealized) device on the bus and then realize it. As a workaround, users that scan the child device list, can check the realized property to see if it is safe to access such a device. Use an atomic write here too to aid with this. A separate discussion is what to do with devices that are unrealized: It looks like for this case we only call the hotplug handler's unplug callback and its up to it to unrealize the device. An atomic operation doesn't cause harm for this code path though. Signed-off-by: Maxim Levitsky --- hw/core/qdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index f0c87e582e..bbb1ae3eb3 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -983,7 +983,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp) } assert(local_err == NULL); - dev->realized = value; + atomic_set(&dev->realized, value); return; child_realize_fail: From patchwork Thu Apr 16 20:36:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxim Levitsky X-Patchwork-Id: 11493743 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2B13C6CA for ; Thu, 16 Apr 2020 20:39:38 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 00D4D221F4 for ; Thu, 16 Apr 2020 20:39:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="LeXzGWYY" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 00D4D221F4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Received: from localhost ([::1]:39102 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jPBIP-0003ju-4p for patchwork-qemu-devel@patchwork.kernel.org; Thu, 16 Apr 2020 16:39:37 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60512) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jPBFd-000883-5k for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jPBFb-00045V-V2 for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:45 -0400 Received: from us-smtp-2.mimecast.com ([205.139.110.61]:36626 helo=us-smtp-delivery-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jPBFb-00045E-Ra for qemu-devel@nongnu.org; Thu, 16 Apr 2020 16:36:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587069403; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YCfvYIFWj9x4jvO82y2dyfecH7qtqDb9QbAc2gJHHU0=; b=LeXzGWYY+ggVmvmMyuna4ZK440KfWVN1myzVVyiKhHDhvTwAhRD9lK0EPvsx8v5XTn+htv IqLo9OvORvnba9rh7mtJVcZvX3SmNuEETR5RZRd3cL2XVe0ysQB+ozdtNh7GsL+pwapLIb +UB2uviTpdW2t7zNf5stpKZj0o6MVOk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-246-ekIrcDidM3eogEqhEeWr7g-1; Thu, 16 Apr 2020 16:36:41 -0400 X-MC-Unique: ekIrcDidM3eogEqhEeWr7g-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ACD30107ACC9; Thu, 16 Apr 2020 20:36:40 +0000 (UTC) Received: from maximlenovopc.usersys.redhat.com (unknown [10.35.206.56]) by smtp.corp.redhat.com (Postfix) with ESMTP id CDD296092D; Thu, 16 Apr 2020 20:36:38 +0000 (UTC) From: Maxim Levitsky To: qemu-devel@nongnu.org Subject: [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized Date: Thu, 16 Apr 2020 23:36:24 +0300 Message-Id: <20200416203624.32366-5-mlevitsk@redhat.com> In-Reply-To: <20200416203624.32366-1-mlevitsk@redhat.com> References: <20200416203624.32366-1-mlevitsk@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 205.139.110.61 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Fam Zheng , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , Eduardo Habkost , "Michael S. Tsirkin" , Maxim Levitsky , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399 Suggested-by: Paolo Bonzini Signed-off-by: Maxim Levitsky --- hw/scsi/virtio-scsi.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index b0f4a35f81..e360b4e03e 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -35,13 +35,29 @@ static inline int virtio_scsi_get_lun(uint8_t *lun) static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) { + SCSIDevice *device = NULL; + if (lun[0] != 1) { return NULL; } if (lun[2] != 0 && !(lun[2] >= 0x40 && lun[2] < 0x80)) { return NULL; } - return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); + + device = scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); + + /* This function might run on the IO thread and we might race against + * main thread hot-plugging the device. + * + * We assume that as soon as .realized is set to true we can let + * the user access the device. + */ + + if (!device || !atomic_read(&device->qdev.realized)) { + return NULL; + } + + return device; } void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)