Patchwork [RFC] performance improvement for windows guests, running on top of virtio block device

login
register
mail settings
Submitter Vadim Rozenfeld
Date Jan. 11, 2010, 7:40 a.m.
Message ID <1263195647.2005.44.camel@localhost>
Download mbox | patch
Permalink /patch/42590/
State New
Headers show

Comments

Vadim Rozenfeld - Jan. 11, 2010, 7:40 a.m.
The following patch allows us to improve Windows virtio 
block driver performance on small size requests. 
Additionally, it leads to reducing of cpu usage on write IOs  

repository: /home/vadimr/work/win7/qemu
branch: master
commit 68290c4e9c96f345d544ca5d2b89f27a1e67e27a
Author: Vadim Rozenfeld <vadimr@localhost.localdomain>
Date:   Mon Jan 11 09:00:21 2010 +0200

[RFC][PATCH] small IOs performance for windows guests, running on top of
virtio block device

     VirtIOBlock *s = req->dev;
@@ -95,6 +98,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);
 
+    if(--s->pending == 0) { 
+        virtio_queue_set_notification(s->vq, 1);
+        virtio_blk_handle_output(&s->vdev, s->vq);
+    }
+
     qemu_free(req);
 }
 
@@ -340,6 +348,9 @@ static void virtio_blk_handle_output(VirtIODevice
*vdev, VirtQueue *vq)
             exit(1);
         }
 
+        if(++s->pending == 1)
+            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;
Avi Kivity - Jan. 11, 2010, 8:30 a.m.
On 01/11/2010 09:40 AM, Vadim Rozenfeld wrote:
> The following patch allows us to improve Windows virtio
> block driver performance on small size requests.
> Additionally, it leads to reducing of cpu usage on write IOs
>
>    

Note, this is not an improvement for Windows specifically.

> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index a2f0639..0e3a8d5 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -28,6 +28,7 @@ typedef struct VirtIOBlock
>       char serial_str[BLOCK_SERIAL_STRLEN + 1];
>       QEMUBH *bh;
>       size_t config_size;
> +    unsigned int pending;
>   } VirtIOBlock;
>
>   static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
> @@ -87,6 +88,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 +98,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);
>
> +    if(--s->pending == 0) {
> +        virtio_queue_set_notification(s->vq, 1);
> +        virtio_blk_handle_output(&s->vdev, s->vq);
> +    }
> +
>    

Coding style: space after if.  See the CODING_STYLE file.

> @@ -340,6 +348,9 @@ static void virtio_blk_handle_output(VirtIODevice
> *vdev, VirtQueue *vq)
>               exit(1);
>           }
>
> +        if(++s->pending == 1)
> +            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;
>
>    

Coding style: space after if, braces after if.

Your patch is word wrapped, please send it correctly.  Easiest using git 
send-email.

The patch has potential to reduce performance on volumes with multiple 
spindles.  Consider two processes issuing sequential reads into a RAID 
array.  With this patch, the reads will be executed sequentially rather 
than in parallel, so I think a follow-on patch to make the minimum depth 
a parameter (set by the guest? the host?) would be helpful.
Dor Laor - Jan. 11, 2010, 9:19 a.m.
On 01/11/2010 11:03 AM, Dor Laor wrote:
> On 01/11/2010 10:30 AM, Avi Kivity wrote:
>> On 01/11/2010 09:40 AM, Vadim Rozenfeld wrote:
>>> The following patch allows us to improve Windows virtio
>>> block driver performance on small size requests.
>>> Additionally, it leads to reducing of cpu usage on write IOs
>>>
>>
>> Note, this is not an improvement for Windows specifically.
>>
>>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
>>> index a2f0639..0e3a8d5 100644
>>> --- a/hw/virtio-blk.c
>>> +++ b/hw/virtio-blk.c
>>> @@ -28,6 +28,7 @@ typedef struct VirtIOBlock
>>> char serial_str[BLOCK_SERIAL_STRLEN + 1];
>>> QEMUBH *bh;
>>> size_t config_size;
>>> + unsigned int pending;
>>> } VirtIOBlock;
>>>
>>> static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
>>> @@ -87,6 +88,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 +98,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);
>>>
>>> + if(--s->pending == 0) {
>>> + virtio_queue_set_notification(s->vq, 1);
>>> + virtio_blk_handle_output(&s->vdev, s->vq);
>
> The above line should be moved out of the 'if'.
>
> Attached results with rhel5.4 (qemu0.11) for win2k8 32bit guest. Note
> the drastic reduction in cpu consumption.

Attachment did not survive the email server, so you'll have to trust me 
saying that cpu consumption was done from 65% -> 40% for reads and from 
80% --> 30% for writes

>
>>> + }
>>> +
>>
>> Coding style: space after if. See the CODING_STYLE file.
>>
>>> @@ -340,6 +348,9 @@ static void virtio_blk_handle_output(VirtIODevice
>>> *vdev, VirtQueue *vq)
>>> exit(1);
>>> }
>>>
>>> + if(++s->pending == 1)
>>> + 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;
>>>
>>
>> Coding style: space after if, braces after if.
>>
>> Your patch is word wrapped, please send it correctly. Easiest using git
>> send-email.
>>
>> The patch has potential to reduce performance on volumes with multiple
>> spindles. Consider two processes issuing sequential reads into a RAID
>> array. With this patch, the reads will be executed sequentially rather
>> than in parallel, so I think a follow-on patch to make the minimum depth
>> a parameter (set by the guest? the host?) would be helpful.
>>
>

Patch

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index a2f0639..0e3a8d5 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -28,6 +28,7 @@  typedef struct VirtIOBlock
     char serial_str[BLOCK_SERIAL_STRLEN + 1];
     QEMUBH *bh;
     size_t config_size;
+    unsigned int pending;
 } VirtIOBlock;
 
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
@@ -87,6 +88,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)
 {