diff mbox

[3/3] xen-disk: use an IOThread per instance

Message ID 20170620134756.9632-4-paul.durrant@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Durrant June 20, 2017, 1:47 p.m. UTC
This patch allocates an IOThread object for each xen_disk instance and
sets the AIO context appropriately on connect. This allows processing
of I/O to proceed in parallel.

The patch also adds tracepoints into xen_disk to make it possible to
follow the state transtions of an instance in the log.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
---
 hw/block/trace-events |  7 +++++++
 hw/block/xen_disk.c   | 44 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 1 deletion(-)

Comments

Paolo Bonzini June 20, 2017, 4:07 p.m. UTC | #1
On 20/06/2017 15:47, Paul Durrant wrote:
> This patch allocates an IOThread object for each xen_disk instance and
> sets the AIO context appropriately on connect. This allows processing
> of I/O to proceed in parallel.
> 
> The patch also adds tracepoints into xen_disk to make it possible to
> follow the state transtions of an instance in the log.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

The QEMU block layer is not yet thread safe, but code running in
IOThreads still has to take the AioContext lock.  You need to call
aio_context_acquire/release in blk_bh and qemu_aio_complete.

Paolo

> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> ---
>  hw/block/trace-events |  7 +++++++
>  hw/block/xen_disk.c   | 44 +++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index 65e83dc258..608b24ba66 100644
> --- a/hw/block/trace-events
> +++ b/hw/block/trace-events
> @@ -10,3 +10,10 @@ virtio_blk_submit_multireq(void *mrb, int start, int num_reqs, uint64_t offset,
>  # hw/block/hd-geometry.c
>  hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p LCHS %d %d %d"
>  hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int trans) "blk %p CHS %u %u %u trans %d"
> +
> +# hw/block/xen_disk.c
> +xen_disk_alloc(char *name) "%s"
> +xen_disk_init(char *name) "%s"
> +xen_disk_connect(char *name) "%s"
> +xen_disk_disconnect(char *name) "%s"
> +xen_disk_free(char *name) "%s"
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index a9942d32db..ec1085c802 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -27,10 +27,13 @@
>  #include "hw/xen/xen_backend.h"
>  #include "xen_blkif.h"
>  #include "sysemu/blockdev.h"
> +#include "sysemu/iothread.h"
>  #include "sysemu/block-backend.h"
>  #include "qapi/error.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qstring.h"
> +#include "qom/object_interfaces.h"
> +#include "trace.h"
>  
>  /* ------------------------------------------------------------- */
>  
> @@ -128,6 +131,9 @@ struct XenBlkDev {
>      DriveInfo           *dinfo;
>      BlockBackend        *blk;
>      QEMUBH              *bh;
> +
> +    IOThread            *iothread;
> +    AioContext          *ctx;
>  };
>  
>  /* ------------------------------------------------------------- */
> @@ -923,11 +929,31 @@ static void blk_bh(void *opaque)
>  static void blk_alloc(struct XenDevice *xendev)
>  {
>      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
> +    Object *obj;
> +    char *name;
> +    Error *err = NULL;
> +
> +    trace_xen_disk_alloc(xendev->name);
>  
>      QLIST_INIT(&blkdev->inflight);
>      QLIST_INIT(&blkdev->finished);
>      QLIST_INIT(&blkdev->freelist);
> -    blkdev->bh = qemu_bh_new(blk_bh, blkdev);
> +
> +    obj = object_new(TYPE_IOTHREAD);
> +    name = g_strdup_printf("iothread-%s", xendev->name);
> +
> +    object_property_add_child(object_get_objects_root(), name, obj, &err);
> +    assert(!err);
> +
> +    g_free(name);
> +
> +    user_creatable_complete(obj, &err);
> +    assert(!err);
> +
> +    blkdev->iothread = (IOThread *)object_dynamic_cast(obj, TYPE_IOTHREAD);
> +    blkdev->ctx = iothread_get_aio_context(blkdev->iothread);
> +    blkdev->bh = aio_bh_new(blkdev->ctx, blk_bh, blkdev);
> +
>      if (xen_mode != XEN_EMULATE) {
>          batch_maps = 1;
>      }
> @@ -954,6 +980,8 @@ static int blk_init(struct XenDevice *xendev)
>      int info = 0;
>      char *directiosafe = NULL;
>  
> +    trace_xen_disk_init(xendev->name);
> +
>      /* read xenstore entries */
>      if (blkdev->params == NULL) {
>          char *h = NULL;
> @@ -1069,6 +1097,8 @@ static int blk_connect(struct XenDevice *xendev)
>      unsigned int i;
>      uint32_t *domids;
>  
> +    trace_xen_disk_connect(xendev->name);
> +
>      /* read-only ? */
>      if (blkdev->directiosafe) {
>          qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> @@ -1285,6 +1315,8 @@ static int blk_connect(struct XenDevice *xendev)
>          blkdev->persistent_gnt_count = 0;
>      }
>  
> +    blk_set_aio_context(blkdev->blk, blkdev->ctx);
> +
>      xen_be_bind_evtchn(&blkdev->xendev);
>  
>      xen_pv_printf(&blkdev->xendev, 1, "ok: proto %s, nr-ring-ref %u, "
> @@ -1298,13 +1330,20 @@ static void blk_disconnect(struct XenDevice *xendev)
>  {
>      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
>  
> +    trace_xen_disk_disconnect(xendev->name);
> +
> +    aio_context_acquire(blkdev->ctx);
> +
>      if (blkdev->blk) {
> +        blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
>          blk_detach_dev(blkdev->blk, blkdev);
>          blk_unref(blkdev->blk);
>          blkdev->blk = NULL;
>      }
>      xen_pv_unbind_evtchn(&blkdev->xendev);
>  
> +    aio_context_release(blkdev->ctx);
> +
>      if (blkdev->sring) {
>          xengnttab_unmap(blkdev->xendev.gnttabdev, blkdev->sring,
>                          blkdev->nr_ring_ref);
> @@ -1338,6 +1377,8 @@ static int blk_free(struct XenDevice *xendev)
>      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
>      struct ioreq *ioreq;
>  
> +    trace_xen_disk_free(xendev->name);
> +
>      if (blkdev->blk || blkdev->sring) {
>          blk_disconnect(xendev);
>      }
> @@ -1355,6 +1396,7 @@ static int blk_free(struct XenDevice *xendev)
>      g_free(blkdev->dev);
>      g_free(blkdev->devtype);
>      qemu_bh_delete(blkdev->bh);
> +    object_unparent(OBJECT(blkdev->iothread));
>      return 0;
>  }
>  
>
Paul Durrant June 20, 2017, 4:09 p.m. UTC | #2
> -----Original Message-----

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo

> Bonzini

> Sent: 20 June 2017 17:08

> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org;

> qemu-devel@nongnu.org; qemu-block@nongnu.org

> Cc: Anthony Perard <anthony.perard@citrix.com>; Kevin Wolf

> <kwolf@redhat.com>; Stefano Stabellini <sstabellini@kernel.org>; Max Reitz

> <mreitz@redhat.com>

> Subject: Re: [PATCH 3/3] xen-disk: use an IOThread per instance

> 

> On 20/06/2017 15:47, Paul Durrant wrote:

> > This patch allocates an IOThread object for each xen_disk instance and

> > sets the AIO context appropriately on connect. This allows processing

> > of I/O to proceed in parallel.

> >

> > The patch also adds tracepoints into xen_disk to make it possible to

> > follow the state transtions of an instance in the log.

> >

> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

> 

> The QEMU block layer is not yet thread safe, but code running in

> IOThreads still has to take the AioContext lock.  You need to call

> aio_context_acquire/release in blk_bh and qemu_aio_complete.

> 


Ok, thanks. I'll update the patch and re-test.

Cheers,

  Paul

> Paolo

> 

> > ---

> > Cc: Stefano Stabellini <sstabellini@kernel.org>

> > Cc: Anthony Perard <anthony.perard@citrix.com>

> > Cc: Kevin Wolf <kwolf@redhat.com>

> > Cc: Max Reitz <mreitz@redhat.com>

> > ---

> >  hw/block/trace-events |  7 +++++++

> >  hw/block/xen_disk.c   | 44

> +++++++++++++++++++++++++++++++++++++++++++-

> >  2 files changed, 50 insertions(+), 1 deletion(-)

> >

> > diff --git a/hw/block/trace-events b/hw/block/trace-events

> > index 65e83dc258..608b24ba66 100644

> > --- a/hw/block/trace-events

> > +++ b/hw/block/trace-events

> > @@ -10,3 +10,10 @@ virtio_blk_submit_multireq(void *mrb, int start, int

> num_reqs, uint64_t offset,

> >  # hw/block/hd-geometry.c

> >  hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p

> LCHS %d %d %d"

> >  hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t

> secs, int trans) "blk %p CHS %u %u %u trans %d"

> > +

> > +# hw/block/xen_disk.c

> > +xen_disk_alloc(char *name) "%s"

> > +xen_disk_init(char *name) "%s"

> > +xen_disk_connect(char *name) "%s"

> > +xen_disk_disconnect(char *name) "%s"

> > +xen_disk_free(char *name) "%s"

> > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c

> > index a9942d32db..ec1085c802 100644

> > --- a/hw/block/xen_disk.c

> > +++ b/hw/block/xen_disk.c

> > @@ -27,10 +27,13 @@

> >  #include "hw/xen/xen_backend.h"

> >  #include "xen_blkif.h"

> >  #include "sysemu/blockdev.h"

> > +#include "sysemu/iothread.h"

> >  #include "sysemu/block-backend.h"

> >  #include "qapi/error.h"

> >  #include "qapi/qmp/qdict.h"

> >  #include "qapi/qmp/qstring.h"

> > +#include "qom/object_interfaces.h"

> > +#include "trace.h"

> >

> >  /* ------------------------------------------------------------- */

> >

> > @@ -128,6 +131,9 @@ struct XenBlkDev {

> >      DriveInfo           *dinfo;

> >      BlockBackend        *blk;

> >      QEMUBH              *bh;

> > +

> > +    IOThread            *iothread;

> > +    AioContext          *ctx;

> >  };

> >

> >  /* ------------------------------------------------------------- */

> > @@ -923,11 +929,31 @@ static void blk_bh(void *opaque)

> >  static void blk_alloc(struct XenDevice *xendev)

> >  {

> >      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev,

> xendev);

> > +    Object *obj;

> > +    char *name;

> > +    Error *err = NULL;

> > +

> > +    trace_xen_disk_alloc(xendev->name);

> >

> >      QLIST_INIT(&blkdev->inflight);

> >      QLIST_INIT(&blkdev->finished);

> >      QLIST_INIT(&blkdev->freelist);

> > -    blkdev->bh = qemu_bh_new(blk_bh, blkdev);

> > +

> > +    obj = object_new(TYPE_IOTHREAD);

> > +    name = g_strdup_printf("iothread-%s", xendev->name);

> > +

> > +    object_property_add_child(object_get_objects_root(), name, obj,

> &err);

> > +    assert(!err);

> > +

> > +    g_free(name);

> > +

> > +    user_creatable_complete(obj, &err);

> > +    assert(!err);

> > +

> > +    blkdev->iothread = (IOThread *)object_dynamic_cast(obj,

> TYPE_IOTHREAD);

> > +    blkdev->ctx = iothread_get_aio_context(blkdev->iothread);

> > +    blkdev->bh = aio_bh_new(blkdev->ctx, blk_bh, blkdev);

> > +

> >      if (xen_mode != XEN_EMULATE) {

> >          batch_maps = 1;

> >      }

> > @@ -954,6 +980,8 @@ static int blk_init(struct XenDevice *xendev)

> >      int info = 0;

> >      char *directiosafe = NULL;

> >

> > +    trace_xen_disk_init(xendev->name);

> > +

> >      /* read xenstore entries */

> >      if (blkdev->params == NULL) {

> >          char *h = NULL;

> > @@ -1069,6 +1097,8 @@ static int blk_connect(struct XenDevice *xendev)

> >      unsigned int i;

> >      uint32_t *domids;

> >

> > +    trace_xen_disk_connect(xendev->name);

> > +

> >      /* read-only ? */

> >      if (blkdev->directiosafe) {

> >          qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;

> > @@ -1285,6 +1315,8 @@ static int blk_connect(struct XenDevice *xendev)

> >          blkdev->persistent_gnt_count = 0;

> >      }

> >

> > +    blk_set_aio_context(blkdev->blk, blkdev->ctx);

> > +

> >      xen_be_bind_evtchn(&blkdev->xendev);

> >

> >      xen_pv_printf(&blkdev->xendev, 1, "ok: proto %s, nr-ring-ref %u, "

> > @@ -1298,13 +1330,20 @@ static void blk_disconnect(struct XenDevice

> *xendev)

> >  {

> >      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev,

> xendev);

> >

> > +    trace_xen_disk_disconnect(xendev->name);

> > +

> > +    aio_context_acquire(blkdev->ctx);

> > +

> >      if (blkdev->blk) {

> > +        blk_set_aio_context(blkdev->blk, qemu_get_aio_context());

> >          blk_detach_dev(blkdev->blk, blkdev);

> >          blk_unref(blkdev->blk);

> >          blkdev->blk = NULL;

> >      }

> >      xen_pv_unbind_evtchn(&blkdev->xendev);

> >

> > +    aio_context_release(blkdev->ctx);

> > +

> >      if (blkdev->sring) {

> >          xengnttab_unmap(blkdev->xendev.gnttabdev, blkdev->sring,

> >                          blkdev->nr_ring_ref);

> > @@ -1338,6 +1377,8 @@ static int blk_free(struct XenDevice *xendev)

> >      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev,

> xendev);

> >      struct ioreq *ioreq;

> >

> > +    trace_xen_disk_free(xendev->name);

> > +

> >      if (blkdev->blk || blkdev->sring) {

> >          blk_disconnect(xendev);

> >      }

> > @@ -1355,6 +1396,7 @@ static int blk_free(struct XenDevice *xendev)

> >      g_free(blkdev->dev);

> >      g_free(blkdev->devtype);

> >      qemu_bh_delete(blkdev->bh);

> > +    object_unparent(OBJECT(blkdev->iothread));

> >      return 0;

> >  }

> >

> >
diff mbox

Patch

diff --git a/hw/block/trace-events b/hw/block/trace-events
index 65e83dc258..608b24ba66 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -10,3 +10,10 @@  virtio_blk_submit_multireq(void *mrb, int start, int num_reqs, uint64_t offset,
 # hw/block/hd-geometry.c
 hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p LCHS %d %d %d"
 hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int trans) "blk %p CHS %u %u %u trans %d"
+
+# hw/block/xen_disk.c
+xen_disk_alloc(char *name) "%s"
+xen_disk_init(char *name) "%s"
+xen_disk_connect(char *name) "%s"
+xen_disk_disconnect(char *name) "%s"
+xen_disk_free(char *name) "%s"
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index a9942d32db..ec1085c802 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -27,10 +27,13 @@ 
 #include "hw/xen/xen_backend.h"
 #include "xen_blkif.h"
 #include "sysemu/blockdev.h"
+#include "sysemu/iothread.h"
 #include "sysemu/block-backend.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
+#include "qom/object_interfaces.h"
+#include "trace.h"
 
 /* ------------------------------------------------------------- */
 
@@ -128,6 +131,9 @@  struct XenBlkDev {
     DriveInfo           *dinfo;
     BlockBackend        *blk;
     QEMUBH              *bh;
+
+    IOThread            *iothread;
+    AioContext          *ctx;
 };
 
 /* ------------------------------------------------------------- */
@@ -923,11 +929,31 @@  static void blk_bh(void *opaque)
 static void blk_alloc(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
+    Object *obj;
+    char *name;
+    Error *err = NULL;
+
+    trace_xen_disk_alloc(xendev->name);
 
     QLIST_INIT(&blkdev->inflight);
     QLIST_INIT(&blkdev->finished);
     QLIST_INIT(&blkdev->freelist);
-    blkdev->bh = qemu_bh_new(blk_bh, blkdev);
+
+    obj = object_new(TYPE_IOTHREAD);
+    name = g_strdup_printf("iothread-%s", xendev->name);
+
+    object_property_add_child(object_get_objects_root(), name, obj, &err);
+    assert(!err);
+
+    g_free(name);
+
+    user_creatable_complete(obj, &err);
+    assert(!err);
+
+    blkdev->iothread = (IOThread *)object_dynamic_cast(obj, TYPE_IOTHREAD);
+    blkdev->ctx = iothread_get_aio_context(blkdev->iothread);
+    blkdev->bh = aio_bh_new(blkdev->ctx, blk_bh, blkdev);
+
     if (xen_mode != XEN_EMULATE) {
         batch_maps = 1;
     }
@@ -954,6 +980,8 @@  static int blk_init(struct XenDevice *xendev)
     int info = 0;
     char *directiosafe = NULL;
 
+    trace_xen_disk_init(xendev->name);
+
     /* read xenstore entries */
     if (blkdev->params == NULL) {
         char *h = NULL;
@@ -1069,6 +1097,8 @@  static int blk_connect(struct XenDevice *xendev)
     unsigned int i;
     uint32_t *domids;
 
+    trace_xen_disk_connect(xendev->name);
+
     /* read-only ? */
     if (blkdev->directiosafe) {
         qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
@@ -1285,6 +1315,8 @@  static int blk_connect(struct XenDevice *xendev)
         blkdev->persistent_gnt_count = 0;
     }
 
+    blk_set_aio_context(blkdev->blk, blkdev->ctx);
+
     xen_be_bind_evtchn(&blkdev->xendev);
 
     xen_pv_printf(&blkdev->xendev, 1, "ok: proto %s, nr-ring-ref %u, "
@@ -1298,13 +1330,20 @@  static void blk_disconnect(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
 
+    trace_xen_disk_disconnect(xendev->name);
+
+    aio_context_acquire(blkdev->ctx);
+
     if (blkdev->blk) {
+        blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
         blk_detach_dev(blkdev->blk, blkdev);
         blk_unref(blkdev->blk);
         blkdev->blk = NULL;
     }
     xen_pv_unbind_evtchn(&blkdev->xendev);
 
+    aio_context_release(blkdev->ctx);
+
     if (blkdev->sring) {
         xengnttab_unmap(blkdev->xendev.gnttabdev, blkdev->sring,
                         blkdev->nr_ring_ref);
@@ -1338,6 +1377,8 @@  static int blk_free(struct XenDevice *xendev)
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
     struct ioreq *ioreq;
 
+    trace_xen_disk_free(xendev->name);
+
     if (blkdev->blk || blkdev->sring) {
         blk_disconnect(xendev);
     }
@@ -1355,6 +1396,7 @@  static int blk_free(struct XenDevice *xendev)
     g_free(blkdev->dev);
     g_free(blkdev->devtype);
     qemu_bh_delete(blkdev->bh);
+    object_unparent(OBJECT(blkdev->iothread));
     return 0;
 }