From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47727)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1Un8bF-00088k-3d
	for qemu-devel@nongnu.org; Thu, 13 Jun 2013 10:34:09 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1Un8bE-00044K-0v
	for qemu-devel@nongnu.org; Thu, 13 Jun 2013 10:34:05 -0400
Received: from mail-qc0-x233.google.com ([2607:f8b0:400d:c01::233]:53967)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1Un8bD-000440-TC
	for qemu-devel@nongnu.org; Thu, 13 Jun 2013 10:34:03 -0400
Received: by mail-qc0-f179.google.com with SMTP id e11so1627477qcx.38
	for <qemu-devel@nongnu.org>; Thu, 13 Jun 2013 07:34:02 -0700 (PDT)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <51B9D856.20901@redhat.com>
Date: Thu, 13 Jun 2013 10:33:58 -0400
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1370867173-25755-1-git-send-email-stefanha@redhat.com>
	<1370867173-25755-2-git-send-email-stefanha@redhat.com>
	<20130610143842.GB4838@stefanha-thinkpad.redhat.com>
In-Reply-To: <20130610143842.GB4838@stefanha-thinkpad.redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3 01/17] block: stop relying on io_flush()
 in bdrv_drain_all()
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: Kevin Wolf <kwolf@redhat.com>, Anthony Liguori <aliguori@us.ibm.com>, Ping Fan Liu <pingfank@linux.vnet.ibm.com>, qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>

Il 10/06/2013 10:38, Stefan Hajnoczi ha scritto:
> On Mon, Jun 10, 2013 at 02:25:57PM +0200, Stefan Hajnoczi wrote:
>> @@ -1427,26 +1456,18 @@ void bdrv_close_all(void)
>>  void bdrv_drain_all(void)
>>  {
>>      BlockDriverState *bs;
>> -    bool busy;
>> -
>> -    do {
>> -        busy = qemu_aio_wait();
>>  
>> +    while (bdrv_requests_pending_all()) {
>>          /* FIXME: We do not have timer support here, so this is effectively
>>           * a busy wait.
>>           */
>>          QTAILQ_FOREACH(bs, &bdrv_states, list) {
>>              if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
>>                  qemu_co_queue_restart_all(&bs->throttled_reqs);
>> -                busy = true;
>>              }
>>          }
>> -    } while (busy);
>>  
>> -    /* If requests are still pending there is a bug somewhere */
>> -    QTAILQ_FOREACH(bs, &bdrv_states, list) {
>> -        assert(QLIST_EMPTY(&bs->tracked_requests));
>> -        assert(qemu_co_queue_empty(&bs->throttled_reqs));
>> +        qemu_aio_wait();
>>      }
>>  }
> 
> tests/ide-test found an issue here: block.c invokes callbacks from a BH
> so we may not yet have completed the request when this loop terminates.
> 
> Kevin: can you fold in this patch?
> 
> diff --git a/block.c b/block.c
> index 31f7231..e176215 100644
> --- a/block.c
> +++ b/block.c
> @@ -1469,6 +1469,9 @@ void bdrv_drain_all(void)
>  
>          qemu_aio_wait();
>      }
> +
> +    /* Process pending completion BHs */
> +    aio_poll(qemu_get_aio_context(), false);
>  }

aio_poll could require multiple iterations, this is why the old code was
starting with "busy = qemu_aio_wait()".  You can add a while loop around
the new call, or do something more similar to the old code, along the
lines of "do { QTAILQ_FOREACH(...) ...; busy =
bdrv_request_pending_all(); busy |= aio_poll(qemu_get_aio_context(),
busy); } while(busy);".