On Mon, May 06, 2013 at 09:54:03AM +0200, Paolo Bonzini wrote: > Il 03/05/2013 18:03, Michael Roth ha scritto: > > virtio-blk dataplane currently creates/manages it's own thread to > > offload work to a separate event loop. > > > > This patch insteads allows us to specify a QContext-based event loop by > > adding a "context" property for virtio-blk we can use like so: > > > > qemu ... \ > > -object glib-qcontext,id=ctx0,threaded=yes > > -drive file=file.raw,id=drive0,aio=native,cache=none \ > > -device virtio-blk,drive=drive0,scsi=off,x-data-plane=on,context=ctx0 > > > > virtio-blk dataplane then simply attachs/detaches it's AioContext to the > > ctx0 event loop on start/stop. > > > > This also makes available the option to drive a virtio-blk dataplane via > > the default main loop: > > > > qemu ... \ > > -drive file=file.raw,id=drive0,aio=native,cache=none \ > > -device virtio-blk,drive=drive0,scsi=off,x-data-plane=on,context=main > > > > This doesn't do much in and of itself, but helps to demonstrate how we > > might model a general mechanism to offload device workloads to separate > > threads. > > > > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com> > > --- > > hw/block/dataplane/virtio-blk.c | 46 > > ++++++++++++--------------------------- > > include/hw/virtio/virtio-blk.h | 7 ++++-- > > 2 files changed, 19 insertions(+), 34 deletions(-) > > > > diff --git a/hw/block/dataplane/virtio-blk.c > > b/hw/block/dataplane/virtio-blk.c > > index 0356665..08ea10f 100644 > > --- a/hw/block/dataplane/virtio-blk.c > > +++ b/hw/block/dataplane/virtio-blk.c > > @@ -24,6 +24,8 @@ > > #include "virtio-blk.h" > > #include "block/aio.h" > > #include "hw/virtio/virtio-bus.h" > > +#include "qcontext/qcontext.h" > > +#include "qcontext/glib-qcontext.h" > > > > enum { > > SEG_MAX = 126, /* maximum number of I/O segments */ > > @@ -60,6 +62,7 @@ struct VirtIOBlockDataPlane { > > * use it). > > */ > > AioContext *ctx; > > + QContext *qctx; > > EventNotifier io_notifier; /* Linux AIO completion */ > > EventNotifier host_notifier; /* doorbell */ > > > > @@ -375,26 +378,6 @@ static void handle_io(EventNotifier *e) > > } > > } > > > > -static void *data_plane_thread(void *opaque) > > -{ > > - VirtIOBlockDataPlane *s = opaque; > > - > > - do { > > - aio_poll(s->ctx, true); > > - } while (!s->stopping || s->num_reqs > 0); > > - return NULL; > > -} > > - > > -static void start_data_plane_bh(void *opaque) > > -{ > > - VirtIOBlockDataPlane *s = opaque; > > - > > - qemu_bh_delete(s->start_bh); > > - s->start_bh = NULL; > > - qemu_thread_create(&s->thread, data_plane_thread, > > - s, QEMU_THREAD_JOINABLE); > > -} > > - > > bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, > > VirtIOBlockDataPlane **dataplane) > > { > > @@ -460,6 +443,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane > > *s) > > VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > > VirtQueue *vq; > > int i; > > + Error *err = NULL; > > > > if (s->started) { > > return; > > @@ -502,9 +486,16 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane > > *s) > > /* Kick right away to begin processing requests already in vring */ > > event_notifier_set(virtio_queue_get_host_notifier(vq)); > > > > - /* Spawn thread in BH so it inherits iothread cpusets */ > > - s->start_bh = qemu_bh_new(start_data_plane_bh, s); > > - qemu_bh_schedule(s->start_bh); > > + /* use QEMU main loop/context by default */ > > + if (!s->blk->context) { > > + s->blk->context = g_strdup("main"); > > + } > > Or rather create a device-specific context by default?
Yup, definitely. I think I did it this way to to give an idea of how a "normal" threaded device might look (i.e. reworked or written from the start to always be capable of being driven by a separate event loop) But x-data-plane=on should imply a new context be used so we don't break things, and I'll do it that way on the next pass. > > Paolo > > > + s->qctx = qcontext_find_by_name(s->blk->context, &err); > > + if (err) { > > + fprintf(stderr, "virtio-blk failed to start: %s", > > error_get_pretty(err)); > > + exit(1); > > + } > > + aio_context_attach(s->ctx, s->qctx); > > } > > > > void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s) > > @@ -517,15 +508,6 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane > > *s) > > s->stopping = true; > > trace_virtio_blk_data_plane_stop(s); > > > > - /* Stop thread or cancel pending thread creation BH */ > > - if (s->start_bh) { > > - qemu_bh_delete(s->start_bh); > > - s->start_bh = NULL; > > - } else { > > - aio_notify(s->ctx); > > - qemu_thread_join(&s->thread); > > - } > > - > > aio_set_event_notifier(s->ctx, &s->io_notifier, NULL, NULL); > > ioq_cleanup(&s->ioqueue); > > > > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > > index fc71853..c5514a4 100644 > > --- a/include/hw/virtio/virtio-blk.h > > +++ b/include/hw/virtio/virtio-blk.h > > @@ -110,6 +110,7 @@ struct VirtIOBlkConf > > uint32_t scsi; > > uint32_t config_wce; > > uint32_t data_plane; > > + char *context; > > }; > > > > struct VirtIOBlockDataPlane; > > @@ -138,13 +139,15 @@ typedef struct VirtIOBlock { > > DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf), > > \ > > DEFINE_PROP_STRING("serial", _state, _field.serial), > > \ > > DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true), > > \ > > - DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true) > > + DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true), > > \ > > + DEFINE_PROP_STRING("context", _state, _field.context) > > #else > > #define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field) > > \ > > DEFINE_BLOCK_PROPERTIES(_state, _field.conf), > > \ > > DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf), > > \ > > DEFINE_PROP_STRING("serial", _state, _field.serial), > > \ > > - DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true) > > + DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true), > > \ > > + DEFINE_PROP_STRING("context", _state, _field.context) > > #endif /* __linux__ */ > > > > void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); > > >