From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NVQiI-00034A-JP for qemu-devel@nongnu.org; Thu, 14 Jan 2010 09:30:18 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NVQiE-00032Y-V4 for qemu-devel@nongnu.org; Thu, 14 Jan 2010 09:30:18 -0500 Received: from [199.232.76.173] (port=33849 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NVQiE-00032M-PN for qemu-devel@nongnu.org; Thu, 14 Jan 2010 09:30:14 -0500 Received: from mail-qy0-f194.google.com ([209.85.221.194]:55047) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NVQiD-0005w8-L4 for qemu-devel@nongnu.org; Thu, 14 Jan 2010 09:30:14 -0500 Received: by qyk32 with SMTP id 32so3924341qyk.4 for ; Thu, 14 Jan 2010 06:30:08 -0800 (PST) Message-ID: <4B4F2A6D.6030702@codemonkey.ws> Date: Thu, 14 Jan 2010 08:30:05 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [RFC][PATCH v2] suppressing queue notification with queue depth parameter (was: [RFC][PATCH] Queue notify support for virtio block device.) References: <1263465217.7273.74.camel@localhost> In-Reply-To: <1263465217.7273.74.camel@localhost> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vadim Rozenfeld Cc: Dor Laor , qemu-devel , Avi Kivity 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 > 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