@@ -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_init(BusState *bus, DeviceState *parent, const char *name)
@@ -194,9 +200,11 @@ 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;
- qdev_unrealize(dev);
+ WITH_RCU_READ_LOCK_GUARD() {
+ QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+ DeviceState *dev = kid->child;
+ qdev_unrealize(dev);
+ }
}
if (bc->unrealize) {
bc->unrealize(bus);
@@ -51,6 +51,12 @@ 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;
@@ -60,15 +66,16 @@ static void bus_remove_child(BusState *bus, DeviceState *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);
- 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;
}
}
}
@@ -83,7 +90,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
kid->child = child;
object_ref(OBJECT(kid->child));
- QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
+ QTAILQ_INSERT_HEAD_RCU(&bus->children, kid, sibling);
/* This transfers ownership of kid->child to the property. */
snprintf(name, sizeof(name), "child[%d]", kid->index);
@@ -689,17 +696,19 @@ 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;
+ }
}
}
}
@@ -400,7 +400,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);
@@ -421,7 +424,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);
@@ -430,6 +433,9 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
i += 8;
}
}
+
+ rcu_read_unlock();
+
assert(i == n + 8);
r->len = len;
return true;
@@ -1572,12 +1578,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;
}
@@ -1591,6 +1600,8 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
}
}
}
+
+ rcu_read_unlock();
return target_dev;
}
@@ -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;
@@ -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"
@@ -230,6 +232,7 @@ struct BusClass {
};
typedef struct BusChild {
+ struct rcu_head rcu;
DeviceState *child;
int index;
QTAILQ_ENTRY(BusChild) sibling;
@@ -250,6 +253,12 @@ struct BusState {
int max_index;
bool realized;
int num_children;
+
+ /*
+ * children is a RCU QTAILQ, thus readers must use RCU to access it,
+ * and writers must hold the big qemu lock
+ */
+
QTAILQ_HEAD(, BusChild) children;
QLIST_ENTRY(BusState) sibling;
ResettableState reset;
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 <pbonzini@redhat.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- hw/core/bus.c | 28 ++++++++++++++++++---------- hw/core/qdev.c | 37 +++++++++++++++++++++++-------------- hw/scsi/scsi-bus.c | 17 ++++++++++++++--- hw/scsi/virtio-scsi.c | 6 +++++- include/hw/qdev-core.h | 9 +++++++++ 5 files changed, 69 insertions(+), 28 deletions(-)