@@ -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);
}
@@ -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;
}
@@ -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;
}
@@ -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"
@@ -218,6 +220,7 @@ struct BusClass {
};
typedef struct BusChild {
+ struct rcu_head rcu;
DeviceState *child;
int index;
QTAILQ_ENTRY(BusChild) sibling;
@@ -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.
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 | 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(-)