Patchwork [RFC,v2] suppressing queue notification with queue depth parameter (was: [RFC][PATCH] Queue notify support for virtio block device.)

login
register
mail settings
Submitter Vadim Rozenfeld
Date Jan. 14, 2010, 10:33 a.m.
Message ID <1263465217.7273.74.camel@localhost>
Download mbox | patch
Permalink /patch/42867/
State New
Headers show

Comments

Vadim Rozenfeld - Jan. 14, 2010, 10:33 a.m.
The following patch allows to suppress virtio queue notification with
the number of requests passed as parameter.

Changes from v1:
- code styling,
- parameter "x-queue-depth-suppress-notify" for queue depth adjustment.

repository: /home/vadimr/work/upstream/qemu
branch: master
commit d23a1c2fbd23e3da9a671a35c95324d2c630e4c9
Author: Vadim Rozenfeld <vrozenfe@redhat.com>
Date:   Thu Jan 14 08:09:26 2010 +0200

    [RFC][PATCH v2] suppressing queue notification with queue depth
parameter

 {
@@ -2110,6 +2122,7 @@ DriveInfo *drive_init(QemuOpts *opts, void
*opaque,
     const char *devaddr;
     DriveInfo *dinfo;
     int snapshot = 0;
+    int queue_depth = 1;
 
     *fatal_error = 1;
 
@@ -2141,6 +2154,7 @@ DriveInfo *drive_init(QemuOpts *opts, void
*opaque,
 
     file = qemu_opt_get(opts, "file");
     serial = qemu_opt_get(opts, "serial");
+    queue_depth = qemu_opt_get_number(opts,
"x-queue-depth-suppress-notify",1);
 
     if ((buf = qemu_opt_get(opts, "if")) != NULL) {
         pstrcpy(devname, sizeof(devname), buf);
@@ -2375,6 +2389,7 @@ DriveInfo *drive_init(QemuOpts *opts, void
*opaque,
     dinfo->on_read_error = on_read_error;
     dinfo->on_write_error = on_write_error;
     dinfo->opts = opts;
+    dinfo->queue_depth = queue_depth;
     if (serial)
         strncpy(dinfo->serial, serial, sizeof(serial));
     QTAILQ_INSERT_TAIL(&drives, dinfo, next);
Anthony Liguori - Jan. 14, 2010, 2:30 p.m.
On 01/14/2010 04:33 AM, Vadim Rozenfeld wrote:
> The following patch allows to suppress virtio queue notification with
> the number of requests passed as parameter.
>
> Changes from v1:
> - code styling,
> - parameter "x-queue-depth-suppress-notify" for queue depth adjustment.
>
> repository: /home/vadimr/work/upstream/qemu
> branch: master
> commit d23a1c2fbd23e3da9a671a35c95324d2c630e4c9
> Author: Vadim Rozenfeld<vrozenfe@redhat.com>
> Date:   Thu Jan 14 08:09:26 2010 +0200
>
>      [RFC][PATCH v2] suppressing queue notification with queue depth
> parameter
>
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index cb1ae1f..93b584d 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -28,6 +28,8 @@ typedef struct VirtIOBlock
>       char serial_str[BLOCK_SERIAL_STRLEN + 1];
>       QEMUBH *bh;
>       size_t config_size;
> +    unsigned int queue_depth;
> +    unsigned int requests;
>   } VirtIOBlock;
>
>   static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
> @@ -87,6 +89,8 @@ typedef struct VirtIOBlockReq
>       struct VirtIOBlockReq *next;
>   } VirtIOBlockReq;
>
> +static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue
> *vq);
> +
>   static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
>   {
>       VirtIOBlock *s = req->dev;
> @@ -95,6 +99,11 @@ static void virtio_blk_req_complete(VirtIOBlockReq
> *req, int status)
>       virtqueue_push(s->vq,&req->elem, req->qiov.size +
> sizeof(*req->in));
>       virtio_notify(&s->vdev, s->vq);
>
> +    virtio_blk_handle_output(&s->vdev, s->vq);
> +    if (--s->requests == (s->queue_depth - 1)) {
> +        virtio_queue_set_notification(s->vq, 1);
> +    }
> +
>       qemu_free(req);
>   }
>
> @@ -340,6 +349,10 @@ static void virtio_blk_handle_output(VirtIODevice
> *vdev, VirtQueue *vq)
>               exit(1);
>           }
>
> +        if (++s->requests == s->queue_depth) {
> +            virtio_queue_set_notification(s->vq, 0);
> +        }
> +
>           req->out = (void *)req->elem.out_sg[0].iov_base;
>           req->in = (void *)req->elem.in_sg[req->elem.in_num -
> 1].iov_base;
>
> @@ -502,6 +515,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> DriveInfo *dinfo)
>       s->vdev.reset = virtio_blk_reset;
>       s->bs = dinfo->bdrv;
>       s->rq = NULL;
> +    s->queue_depth = drive_get_queue_depth(dinfo->bdrv);
>       if (strlen(ps))
>           strncpy(s->serial_str, ps, sizeof(s->serial_str));
>       else
> diff --git a/qemu-config.c b/qemu-config.c
> index c3203c8..6fcf958 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -78,6 +78,10 @@ QemuOptsList qemu_drive_opts = {
>           },{
>               .name = "readonly",
>               .type = QEMU_OPT_BOOL,
> +        },{
> +            .name = "x-queue-depth-suppress-notify",
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "number of requests in queueu before suppressing
> notify",
>           },
>           { /* end if list */ }
>       },
> diff --git a/sysemu.h b/sysemu.h
> index 9d80bb2..6eecbe1 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -183,6 +183,7 @@ typedef struct DriveInfo {
>       BlockInterfaceErrorAction on_read_error;
>       BlockInterfaceErrorAction on_write_error;
>       char serial[BLOCK_SERIAL_STRLEN + 1];
> +    int queue_depth;
>       QTAILQ_ENTRY(DriveInfo) next;
>   } DriveInfo;
>
> @@ -198,6 +199,7 @@ extern DriveInfo *drive_get_by_id(const char *id);
>   extern int drive_get_max_bus(BlockInterfaceType type);
>   extern void drive_uninit(DriveInfo *dinfo);
>   extern const char *drive_get_serial(BlockDriverState *bdrv);
> +extern int drive_get_queue_depth(BlockDriverState *bdrv);
>
>   extern BlockInterfaceErrorAction drive_get_on_error(
>       BlockDriverState *bdrv, int is_read);
> diff --git a/vl.c b/vl.c
> index b048e89..517d290 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2043,6 +2043,18 @@ const char *drive_get_serial(BlockDriverState
> *bdrv)
>       return "\0";
>   }
>
> +int drive_get_queue_depth(BlockDriverState *bdrv)
> +{
> +    DriveInfo *dinfo;
> +
> +    QTAILQ_FOREACH(dinfo,&drives, next) {
> +        if (dinfo->bdrv == bdrv)
> +            return dinfo->queue_depth;
> +    }
> +
> +    return 1;
> +}
> +
>   BlockInterfaceErrorAction drive_get_on_error(
>       BlockDriverState *bdrv, int is_read)
>   {
> @@ -2110,6 +2122,7 @@ DriveInfo *drive_init(QemuOpts *opts, void
> *opaque,
>       const char *devaddr;
>       DriveInfo *dinfo;
>       int snapshot = 0;
> +    int queue_depth = 1;
>    

It should be disabled by default.  queue_depth=1 by default is going to 
be devastating for multi-spindle set-ups.

Regards,

Anthony Liguori

Patch

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index cb1ae1f..93b584d 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -28,6 +28,8 @@  typedef struct VirtIOBlock
     char serial_str[BLOCK_SERIAL_STRLEN + 1];
     QEMUBH *bh;
     size_t config_size;
+    unsigned int queue_depth;
+    unsigned int requests;
 } VirtIOBlock;
 
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
@@ -87,6 +89,8 @@  typedef struct VirtIOBlockReq
     struct VirtIOBlockReq *next;
 } VirtIOBlockReq;
 
+static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue
*vq);
+
 static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 {
     VirtIOBlock *s = req->dev;
@@ -95,6 +99,11 @@  static void virtio_blk_req_complete(VirtIOBlockReq
*req, int status)
     virtqueue_push(s->vq, &req->elem, req->qiov.size +
sizeof(*req->in));
     virtio_notify(&s->vdev, s->vq);
 
+    virtio_blk_handle_output(&s->vdev, s->vq);
+    if (--s->requests == (s->queue_depth - 1)) {
+        virtio_queue_set_notification(s->vq, 1);
+    }
+
     qemu_free(req);
 }
 
@@ -340,6 +349,10 @@  static void virtio_blk_handle_output(VirtIODevice
*vdev, VirtQueue *vq)
             exit(1);
         }
 
+        if (++s->requests == s->queue_depth) {
+            virtio_queue_set_notification(s->vq, 0);
+        }
+
         req->out = (void *)req->elem.out_sg[0].iov_base;
         req->in = (void *)req->elem.in_sg[req->elem.in_num -
1].iov_base;
 
@@ -502,6 +515,7 @@  VirtIODevice *virtio_blk_init(DeviceState *dev,
DriveInfo *dinfo)
     s->vdev.reset = virtio_blk_reset;
     s->bs = dinfo->bdrv;
     s->rq = NULL;
+    s->queue_depth = drive_get_queue_depth(dinfo->bdrv);
     if (strlen(ps))
         strncpy(s->serial_str, ps, sizeof(s->serial_str));
     else
diff --git a/qemu-config.c b/qemu-config.c
index c3203c8..6fcf958 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -78,6 +78,10 @@  QemuOptsList qemu_drive_opts = {
         },{
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
+        },{
+            .name = "x-queue-depth-suppress-notify",
+            .type = QEMU_OPT_NUMBER,
+            .help = "number of requests in queueu before suppressing
notify",
         },
         { /* end if list */ }
     },
diff --git a/sysemu.h b/sysemu.h
index 9d80bb2..6eecbe1 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -183,6 +183,7 @@  typedef struct DriveInfo {
     BlockInterfaceErrorAction on_read_error;
     BlockInterfaceErrorAction on_write_error;
     char serial[BLOCK_SERIAL_STRLEN + 1];
+    int queue_depth;
     QTAILQ_ENTRY(DriveInfo) next;
 } DriveInfo;
 
@@ -198,6 +199,7 @@  extern DriveInfo *drive_get_by_id(const char *id);
 extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
 extern const char *drive_get_serial(BlockDriverState *bdrv);
+extern int drive_get_queue_depth(BlockDriverState *bdrv);
 
 extern BlockInterfaceErrorAction drive_get_on_error(
     BlockDriverState *bdrv, int is_read);
diff --git a/vl.c b/vl.c
index b048e89..517d290 100644
--- a/vl.c
+++ b/vl.c
@@ -2043,6 +2043,18 @@  const char *drive_get_serial(BlockDriverState
*bdrv)
     return "\0";
 }
 
+int drive_get_queue_depth(BlockDriverState *bdrv)
+{
+    DriveInfo *dinfo;
+
+    QTAILQ_FOREACH(dinfo, &drives, next) {
+        if (dinfo->bdrv == bdrv)
+            return dinfo->queue_depth;
+    }
+
+    return 1;
+}
+
 BlockInterfaceErrorAction drive_get_on_error(
     BlockDriverState *bdrv, int is_read)