| Message ID | 1367597032-28934-10-git-send-email-mdroth@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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 <mdroth@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? 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); >
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 <mdroth@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); > > >
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"); + } + 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);
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 <mdroth@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(-)