device-core: use RCU for list of children of a bus
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> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200913160259.32145-5-mlevitsk@redhat.com> [Use RCU_READ_LOCK_GUARD in more places, adjust testcase now that the delay in DEVICE_DELETED due to RCU is more consistent. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20201006123904.610658-9-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
parent
7bed89958b
commit
2d24a64661
|
@ -49,12 +49,14 @@ int qbus_walk_children(BusState *bus,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
QTAILQ_FOREACH(kid, &bus->children, sibling) {
|
WITH_RCU_READ_LOCK_GUARD() {
|
||||||
err = qdev_walk_children(kid->child,
|
QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
|
||||||
pre_devfn, pre_busfn,
|
err = qdev_walk_children(kid->child,
|
||||||
post_devfn, post_busfn, opaque);
|
pre_devfn, pre_busfn,
|
||||||
if (err < 0) {
|
post_devfn, post_busfn, opaque);
|
||||||
return err;
|
if (err < 0) {
|
||||||
|
return err;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -90,8 +92,10 @@ static void bus_reset_child_foreach(Object *obj, ResettableChildCallback cb,
|
||||||
BusState *bus = BUS(obj);
|
BusState *bus = BUS(obj);
|
||||||
BusChild *kid;
|
BusChild *kid;
|
||||||
|
|
||||||
QTAILQ_FOREACH(kid, &bus->children, sibling) {
|
WITH_RCU_READ_LOCK_GUARD() {
|
||||||
cb(OBJECT(kid->child), opaque, type);
|
QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
|
||||||
|
cb(OBJECT(kid->child), opaque, type);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -194,9 +198,11 @@ static void bus_set_realized(Object *obj, bool value, Error **errp)
|
||||||
|
|
||||||
/* TODO: recursive realization */
|
/* TODO: recursive realization */
|
||||||
} else if (!value && bus->realized) {
|
} else if (!value && bus->realized) {
|
||||||
QTAILQ_FOREACH(kid, &bus->children, sibling) {
|
WITH_RCU_READ_LOCK_GUARD() {
|
||||||
DeviceState *dev = kid->child;
|
QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
|
||||||
qdev_unrealize(dev);
|
DeviceState *dev = kid->child;
|
||||||
|
qdev_unrealize(dev);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if (bc->unrealize) {
|
if (bc->unrealize) {
|
||||||
bc->unrealize(bus);
|
bc->unrealize(bus);
|
||||||
|
|
|
@ -51,6 +51,12 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
|
||||||
return dc->vmsd;
|
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)
|
static void bus_remove_child(BusState *bus, DeviceState *child)
|
||||||
{
|
{
|
||||||
BusChild *kid;
|
BusChild *kid;
|
||||||
|
@ -60,15 +66,16 @@ static void bus_remove_child(BusState *bus, DeviceState *child)
|
||||||
char name[32];
|
char name[32];
|
||||||
|
|
||||||
snprintf(name, sizeof(name), "child[%d]", kid->index);
|
snprintf(name, sizeof(name), "child[%d]", kid->index);
|
||||||
QTAILQ_REMOVE(&bus->children, kid, sibling);
|
QTAILQ_REMOVE_RCU(&bus->children, kid, sibling);
|
||||||
|
|
||||||
bus->num_children--;
|
bus->num_children--;
|
||||||
|
|
||||||
/* This gives back ownership of kid->child back to us. */
|
/* This gives back ownership of kid->child back to us. */
|
||||||
object_property_del(OBJECT(bus), name);
|
object_property_del(OBJECT(bus), name);
|
||||||
object_unref(OBJECT(kid->child));
|
|
||||||
g_free(kid);
|
/* free the bus kid, when it is safe to do so*/
|
||||||
return;
|
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;
|
kid->child = child;
|
||||||
object_ref(OBJECT(kid->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. */
|
/* This transfers ownership of kid->child to the property. */
|
||||||
snprintf(name, sizeof(name), "child[%d]", kid->index);
|
snprintf(name, sizeof(name), "child[%d]", kid->index);
|
||||||
|
@ -672,17 +679,19 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id)
|
||||||
DeviceState *ret;
|
DeviceState *ret;
|
||||||
BusState *child;
|
BusState *child;
|
||||||
|
|
||||||
QTAILQ_FOREACH(kid, &bus->children, sibling) {
|
WITH_RCU_READ_LOCK_GUARD() {
|
||||||
DeviceState *dev = kid->child;
|
QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
|
||||||
|
DeviceState *dev = kid->child;
|
||||||
|
|
||||||
if (dev->id && strcmp(dev->id, id) == 0) {
|
if (dev->id && strcmp(dev->id, id) == 0) {
|
||||||
return dev;
|
return dev;
|
||||||
}
|
}
|
||||||
|
|
||||||
QLIST_FOREACH(child, &dev->child_bus, sibling) {
|
QLIST_FOREACH(child, &dev->child_bus, sibling) {
|
||||||
ret = qdev_find_recursive(child, id);
|
ret = qdev_find_recursive(child, id);
|
||||||
if (ret) {
|
if (ret) {
|
||||||
return ret;
|
return ret;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -400,7 +400,10 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
|
||||||
id = r->req.dev->id;
|
id = r->req.dev->id;
|
||||||
found_lun0 = false;
|
found_lun0 = false;
|
||||||
n = 0;
|
n = 0;
|
||||||
QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
|
|
||||||
|
RCU_READ_LOCK_GUARD();
|
||||||
|
|
||||||
|
QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
|
||||||
DeviceState *qdev = kid->child;
|
DeviceState *qdev = kid->child;
|
||||||
SCSIDevice *dev = SCSI_DEVICE(qdev);
|
SCSIDevice *dev = SCSI_DEVICE(qdev);
|
||||||
|
|
||||||
|
@ -421,7 +424,7 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
|
||||||
memset(r->buf, 0, len);
|
memset(r->buf, 0, len);
|
||||||
stl_be_p(&r->buf[0], n);
|
stl_be_p(&r->buf[0], n);
|
||||||
i = found_lun0 ? 8 : 16;
|
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;
|
DeviceState *qdev = kid->child;
|
||||||
SCSIDevice *dev = SCSI_DEVICE(qdev);
|
SCSIDevice *dev = SCSI_DEVICE(qdev);
|
||||||
|
|
||||||
|
@ -430,6 +433,7 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
|
||||||
i += 8;
|
i += 8;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
assert(i == n + 8);
|
assert(i == n + 8);
|
||||||
r->len = len;
|
r->len = len;
|
||||||
return true;
|
return true;
|
||||||
|
@ -1572,7 +1576,8 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
|
||||||
BusChild *kid;
|
BusChild *kid;
|
||||||
SCSIDevice *target_dev = NULL;
|
SCSIDevice *target_dev = NULL;
|
||||||
|
|
||||||
QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
|
RCU_READ_LOCK_GUARD();
|
||||||
|
QTAILQ_FOREACH_RCU(kid, &bus->qbus.children, sibling) {
|
||||||
DeviceState *qdev = kid->child;
|
DeviceState *qdev = kid->child;
|
||||||
SCSIDevice *dev = SCSI_DEVICE(qdev);
|
SCSIDevice *dev = SCSI_DEVICE(qdev);
|
||||||
|
|
||||||
|
@ -1591,6 +1596,7 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return target_dev;
|
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:
|
case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
|
||||||
target = req->req.tmf.lun[1];
|
target = req->req.tmf.lun[1];
|
||||||
s->resetting++;
|
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);
|
d = SCSI_DEVICE(kid->child);
|
||||||
if (d->channel == 0 && d->id == target) {
|
if (d->channel == 0 && d->id == target) {
|
||||||
qdev_reset_all(&d->qdev);
|
qdev_reset_all(&d->qdev);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
rcu_read_unlock();
|
||||||
|
|
||||||
s->resetting--;
|
s->resetting--;
|
||||||
break;
|
break;
|
||||||
|
|
||||||
|
|
|
@ -3,6 +3,8 @@
|
||||||
|
|
||||||
#include "qemu/queue.h"
|
#include "qemu/queue.h"
|
||||||
#include "qemu/bitmap.h"
|
#include "qemu/bitmap.h"
|
||||||
|
#include "qemu/rcu.h"
|
||||||
|
#include "qemu/rcu_queue.h"
|
||||||
#include "qom/object.h"
|
#include "qom/object.h"
|
||||||
#include "hw/hotplug.h"
|
#include "hw/hotplug.h"
|
||||||
#include "hw/resettable.h"
|
#include "hw/resettable.h"
|
||||||
|
@ -238,6 +240,7 @@ struct BusClass {
|
||||||
};
|
};
|
||||||
|
|
||||||
typedef struct BusChild {
|
typedef struct BusChild {
|
||||||
|
struct rcu_head rcu;
|
||||||
DeviceState *child;
|
DeviceState *child;
|
||||||
int index;
|
int index;
|
||||||
QTAILQ_ENTRY(BusChild) sibling;
|
QTAILQ_ENTRY(BusChild) sibling;
|
||||||
|
@ -258,6 +261,12 @@ struct BusState {
|
||||||
int max_index;
|
int max_index;
|
||||||
bool realized;
|
bool realized;
|
||||||
int num_children;
|
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;
|
QTAILQ_HEAD(, BusChild) children;
|
||||||
QLIST_ENTRY(BusState) sibling;
|
QLIST_ENTRY(BusState) sibling;
|
||||||
ResettableState reset;
|
ResettableState reset;
|
||||||
|
|
Loading…
Reference in a new issue