From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MJRXO-0006T2-MT for qemu-devel@nongnu.org; Wed, 24 Jun 2009 08:25:14 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MJRXK-0006Rg-PH for qemu-devel@nongnu.org; Wed, 24 Jun 2009 08:25:14 -0400 Received: from [199.232.76.173] (port=37065 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MJRXK-0006Rc-MR for qemu-devel@nongnu.org; Wed, 24 Jun 2009 08:25:10 -0400 Received: from mx2.redhat.com ([66.187.237.31]:35065) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MJRXK-00047Z-3v for qemu-devel@nongnu.org; Wed, 24 Jun 2009 08:25:10 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n5OCP8Hp013095 for ; Wed, 24 Jun 2009 08:25:08 -0400 Message-ID: <4A421B36.3000902@redhat.com> Date: Wed, 24 Jun 2009 15:25:26 +0300 From: Dor Laor MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH][STABLE] Call missing qemu_bh_delete at the end of raw_aio_em_cb callback. References: <4A23949B.3020900@redhat.com> In-Reply-To: <4A23949B.3020900@redhat.com> Content-Type: multipart/alternative; boundary="------------000106020803000807070409" Reply-To: dlaor@redhat.com List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-devel , Glauber Costa This is a multi-part message in MIME format. --------------000106020803000807070409 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 > 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 > --- > 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 --------------000106020803000807070409 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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

--------------000106020803000807070409--