From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:60117)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1S0ug2-00082d-Ps
	for qemu-devel@nongnu.org; Fri, 24 Feb 2012 07:55:11 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1S0ufw-0003yk-R8
	for qemu-devel@nongnu.org; Fri, 24 Feb 2012 07:55:10 -0500
Received: from mail-pw0-f45.google.com ([209.85.160.45]:60670)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1S0ufw-0003xK-L3
	for qemu-devel@nongnu.org; Fri, 24 Feb 2012 07:55:04 -0500
Received: by pbbro12 with SMTP id ro12so3101749pbb.4
	for <qemu-devel@nongnu.org>; Fri, 24 Feb 2012 04:55:03 -0800 (PST)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <4F47889F.7030801@redhat.com>
Date: Fri, 24 Feb 2012 13:54:55 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1329713430-9209-1-git-send-email-zwu.kernel@gmail.com>
	<20120224084903.GB872@stefanha-thinkpad.localdomain>
In-Reply-To: <20120224084903.GB872@stefanha-thinkpad.localdomain>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/2] block: add the support for draining
 the throttled request queue
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, chris@arachsys.com, stefanha@linux.vnet.ibm.com, qemu-devel@nongnu.org, zwu.kernel@gmail.com, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

On 02/24/2012 09:49 AM, Stefan Hajnoczi wrote:
>> > +void bdrv_drain_request(BlockDriverState *throttled_bs)
>> >  {
>> >      BlockDriverState *bs;
>> >  
>> > +    QTAILQ_FOREACH(bs, &bdrv_states, list) {
>> > +        if (throttled_bs && throttled_bs != bs) {
>> > +            continue;
>> > +        }
>> > +        qemu_co_queue_restart_all(&bs->throttled_reqs);
>> > +    }
>> > +
>> >      qemu_aio_flush();
> Since I/O throttling is still enabled, the restarted requests could
> enqueue again if they exceed the limit.  We could still hit the assert.

Yes, and qemu_aio_flush() rightly doesn't know that there are pending
requests, because these are "not there" until the timer fires.

Perhaps the bug is simply that the assert is bogus.  If there are
throttled requests, we need to invoke qemu_aio_flush() again.  The
problem with this is that timers don't run inside qemu_aio_wait() so we
have to restart them all and we're effectively busy waiting.  Not a huge
problem since qemu_aio_flush() is rare, but not too nice either.

Paolo