[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v6 06/10] device-core: use atomic_set on .realized property
From: |
Paolo Bonzini |
Subject: |
[PATCH v6 06/10] device-core: use atomic_set on .realized property |
Date: |
Fri, 2 Oct 2020 19:35:54 +0200 |
From: Maxim Levitsky <mlevitsk@redhat.com>
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 <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-6-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/qdev.c | 19 ++++++++++++++++++-
include/hw/qdev-core.h | 2 ++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 59e5e710b7..fc4daa36fa 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -946,7 +946,25 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
}
}
+ qatomic_store_release(&dev->realized, value);
+
} else if (!value && dev->realized) {
+
+ /*
+ * Change the value so that any concurrent users are aware
+ * that the device is going to be unrealized
+ *
+ * TODO: change .realized property to enum that states
+ * each phase of the device realization/unrealization
+ */
+
+ qatomic_set(&dev->realized, value);
+ /*
+ * Ensure that concurrent users see this update prior to
+ * any other changes done by unrealize.
+ */
+ smp_wmb();
+
QLIST_FOREACH(bus, &dev->child_bus, sibling) {
qbus_unrealize(bus);
}
@@ -961,7 +979,6 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
}
assert(local_err == NULL);
- dev->realized = value;
return;
child_realize_fail:
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 2c6307e3ed..868973319e 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -163,6 +163,8 @@ struct NamedClockList {
/**
* DeviceState:
* @realized: Indicates whether the device has been fully constructed.
+ * When accessed outsize big qemu lock, must be accessed with
+ * atomic_load_acquire()
* @reset: ResettableState for the device; handled by Resettable interface.
*
* This structure should not be accessed directly. We declare it here
--
2.26.2
- [PATCH v6 00/10] Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread, Paolo Bonzini, 2020/10/02
- [PATCH v6 01/10] qdev: add "check if address free" callback for buses, Paolo Bonzini, 2020/10/02
- [PATCH v6 02/10] scsi: switch to bus->check_address, Paolo Bonzini, 2020/10/02
- [PATCH v6 04/10] device_core: use drain_call_rcu in in qmp_device_add, Paolo Bonzini, 2020/10/02
- [PATCH v6 03/10] scsi/scsi_bus: switch search direction in scsi_device_find, Paolo Bonzini, 2020/10/02
- [PATCH v6 05/10] device-core: use RCU for list of children of a bus, Paolo Bonzini, 2020/10/02
- [PATCH v6 06/10] device-core: use atomic_set on .realized property,
Paolo Bonzini <=
- [PATCH v6 07/10] scsi/scsi-bus: scsi_device_find: don't return unrealized devices, Paolo Bonzini, 2020/10/02
- [PATCH v6 10/10] scsi/scsi_bus: fix races in REPORT LUNS, Paolo Bonzini, 2020/10/02
- [PATCH v6 08/10] scsi/scsi_bus: Add scsi_device_get, Paolo Bonzini, 2020/10/02
- [PATCH v6 09/10] virtio-scsi: use scsi_device_get, Paolo Bonzini, 2020/10/02
- Re: [PATCH v6 00/10] Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread, no-reply, 2020/10/03