[Qemu-devel] [PATCH][STABLE] Call missing qemu_bh_delete at the end of raw_aio_em_cb callback.

[Qemu-devel] [PATCH][STABLE] Call missing qemu_bh_delete at the end of raw_aio_em_cb callback.

[Dor Laor]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0001--Call-missing-qemu_bh_delete-at-the-end-of-raw_aio_.patch --]
[-- Type: text/plain, Size: 2287 bytes --]

>From 8e2e406e1220f9eedb042919c431c963af1433e4 Mon Sep 17 00:00:00 2001
From: Dor Laor <dor@redhat.com>
Date: Mon, 1 Jun 2009 00:22:28 +0300
Subject: [PATCH] [PATCH] Call missing qemu_bh_delete at the end of raw_aio_em_cb callback.

Without it, these cb's does not get cleaned and it slows
down qemu more and more. (to test, the # of bh->next chain
is huge when using cache=off).
Thanks Avi for helping with the debug.

Signed-off-by: Dor Laor <dor@redhat.com>
---
 block-raw-posix.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/block-raw-posix.c b/block-raw-posix.c
index 85ca704..8fce9e6 100644
--- a/block-raw-posix.c
+++ b/block-raw-posix.c
@@ -439,6 +439,7 @@ typedef struct RawAIOCB {
     BlockDriverAIOCB common;
     struct qemu_paiocb aiocb;
     struct RawAIOCB *next;
+    QEMUBH *bh;
     int ret;
 } RawAIOCB;
 
@@ -593,6 +594,8 @@ static void raw_aio_em_cb(void* opaque)
 {
     RawAIOCB *acb = opaque;
     acb->common.cb(acb->common.opaque, acb->ret);
+    qemu_bh_delete(acb->bh);
+    acb->bh = NULL;
     qemu_aio_release(acb);
 }
 
@@ -628,11 +631,12 @@ static BlockDriverAIOCB *raw_aio_read(BlockDriverState *bs,
     BDRVRawState *s = bs->opaque;
 
     if (unlikely(s->aligned_buf != NULL && ((uintptr_t) buf % 512))) {
-        QEMUBH *bh;
         acb = qemu_aio_get(bs, cb, opaque);
         acb->ret = raw_pread(bs, 512 * sector_num, buf, 512 * nb_sectors);
-        bh = qemu_bh_new(raw_aio_em_cb, acb);
-        qemu_bh_schedule(bh);
+        acb->bh = qemu_bh_new(raw_aio_em_cb, acb);
+        if (!acb->bh)
+            return NULL;
+        qemu_bh_schedule(acb->bh);
         return &acb->common;
     }
 
@@ -659,11 +663,12 @@ static BlockDriverAIOCB *raw_aio_write(BlockDriverState *bs,
     BDRVRawState *s = bs->opaque;
 
     if (unlikely(s->aligned_buf != NULL && ((uintptr_t) buf % 512))) {
-        QEMUBH *bh;
         acb = qemu_aio_get(bs, cb, opaque);
         acb->ret = raw_pwrite(bs, 512 * sector_num, buf, 512 * nb_sectors);
-        bh = qemu_bh_new(raw_aio_em_cb, acb);
-        qemu_bh_schedule(bh);
+        acb->bh = qemu_bh_new(raw_aio_em_cb, acb);
+        if (!acb->bh)
+            return NULL;
+        qemu_bh_schedule(acb->bh);
         return &acb->common;
     }
 
-- 
1.5.6.6

Re: [Qemu-devel] [PATCH][STABLE] Call missing qemu_bh_delete at the end of raw_aio_em_cb callback.

[Dor Laor]

[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]

Now that Avi found an issue with the qemu_bh_delete patch,
I checkout the stable branch and this one was forgotten.
Actually this one does the right thing, when forward porting of it
to master I dropped the acb->bh =null.

On the stable it is a real issue, so please commit it.
Thanks,
Dor

On 06/01/2009 11:43 AM, Dor Laor wrote:
>
> ------------------------------------------------------------------------
>
>  From 8e2e406e1220f9eedb042919c431c963af1433e4 Mon Sep 17 00:00:00 2001
> From: Dor Laor<dor@redhat.com>
> Date: Mon, 1 Jun 2009 00:22:28 +0300
> Subject: [PATCH] [PATCH] Call missing qemu_bh_delete at the end of raw_aio_em_cb callback.
>
> Without it, these cb's does not get cleaned and it slows
> down qemu more and more. (to test, the # of bh->next chain
> is huge when using cache=off).
> Thanks Avi for helping with the debug.
>
> Signed-off-by: Dor Laor<dor@redhat.com>
> ---
>   block-raw-posix.c |   17 +++++++++++------
>   1 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/block-raw-posix.c b/block-raw-posix.c
> index 85ca704..8fce9e6 100644
> --- a/block-raw-posix.c
> +++ b/block-raw-posix.c
> @@ -439,6 +439,7 @@ typedef struct RawAIOCB {
>       BlockDriverAIOCB common;
>       struct qemu_paiocb aiocb;
>       struct RawAIOCB *next;
> +    QEMUBH *bh;
>       int ret;
>   } RawAIOCB;
>
> @@ -593,6 +594,8 @@ static void raw_aio_em_cb(void* opaque)
>   {
>       RawAIOCB *acb = opaque;
>       acb->common.cb(acb->common.opaque, acb->ret);
> +    qemu_bh_delete(acb->bh);
> +    acb->bh = NULL;
>       qemu_aio_release(acb);
>   }
>
> @@ -628,11 +631,12 @@ static BlockDriverAIOCB *raw_aio_read(BlockDriverState *bs,
>       BDRVRawState *s = bs->opaque;
>
>       if (unlikely(s->aligned_buf != NULL&&  ((uintptr_t) buf % 512))) {
> -        QEMUBH *bh;
>           acb = qemu_aio_get(bs, cb, opaque);
>           acb->ret = raw_pread(bs, 512 * sector_num, buf, 512 * nb_sectors);
> -        bh = qemu_bh_new(raw_aio_em_cb, acb);
> -        qemu_bh_schedule(bh);
> +        acb->bh = qemu_bh_new(raw_aio_em_cb, acb);
> +        if (!acb->bh)
> +            return NULL;
> +        qemu_bh_schedule(acb->bh);
>           return&acb->common;
>       }
>
> @@ -659,11 +663,12 @@ static BlockDriverAIOCB *raw_aio_write(BlockDriverState *bs,
>       BDRVRawState *s = bs->opaque;
>
>       if (unlikely(s->aligned_buf != NULL&&  ((uintptr_t) buf % 512))) {
> -        QEMUBH *bh;
>           acb = qemu_aio_get(bs, cb, opaque);
>           acb->ret = raw_pwrite(bs, 512 * sector_num, buf, 512 * nb_sectors);
> -        bh = qemu_bh_new(raw_aio_em_cb, acb);
> -        qemu_bh_schedule(bh);
> +        acb->bh = qemu_bh_new(raw_aio_em_cb, acb);
> +        if (!acb->bh)
> +            return NULL;
> +        qemu_bh_schedule(acb->bh);
>           return&acb->common;
>       }
>
> -- 1.5.6.6


[-- Attachment #2: Type: text/html, Size: 3570 bytes --]