From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:44167)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1X2dtF-0007tl-7w
	for qemu-devel@nongnu.org; Thu, 03 Jul 2014 06:05:23 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1X2dt7-0008Et-OI
	for qemu-devel@nongnu.org; Thu, 03 Jul 2014 06:05:17 -0400
Received: from mx1.redhat.com ([209.132.183.28]:2141)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1X2dt7-0008Ej-Ga
	for qemu-devel@nongnu.org; Thu, 03 Jul 2014 06:05:09 -0400
Message-ID: <53B52ACA.4000900@redhat.com>
Date: Thu, 03 Jul 2014 12:04:58 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1404303528-7115-1-git-send-email-ming.lei@canonical.com>	<1404303528-7115-3-git-send-email-ming.lei@canonical.com>	<20140703094009.GC4322@noname.redhat.com>
	<CACVXFVMAXs6FK3Vz0bKJJ-to8M68dN_5n5KTWfadOhKfP+JfQg@mail.gmail.com>
In-Reply-To: <CACVXFVMAXs6FK3Vz0bKJJ-to8M68dN_5n5KTWfadOhKfP+JfQg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v4 2/3] linux-aio: implement io plug,
 unplug and flush io 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: Ming Lei <ming.lei@canonical.com>, Kevin Wolf <kwolf@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, Fam Zheng <famz@redhat.com>, qemu-devel <qemu-devel@nongnu.org>, Stefan Hajnoczi <stefanha@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>

Il 03/07/2014 11:51, Ming Lei ha scritto:
>>> >> +int laio_io_unplug(BlockDriverState *bs, void *aio_ctx, bool unplug)
>>> >> +{
>>> >> +    struct qemu_laio_state *s = aio_ctx;
>>> >> +    int ret = 0;
>>> >> +
>> >
>> > How about an assert(s->io_q.plugged > 0); here?
> how about just adding a warning because flush io queue uses
> the function too?

This suggest that there is a smaller piece of code that both 
laio_io_unplug ad flush io queue should use.  However, not that flush io 
queue is not necessary.  The only place where you're using it (before a 
call to bdrv_aio_flush) does not need it.

Paolo


> Also that is why 'plugged' is defined as signed.
>
>> >
>>> >> +    if (unplug && --s->io_q.plugged > 0)
>>> >> +        return 0;
>> >
>> > Missing braces.
>> >
>>> >> +
>>> >> +    if (s->io_q.idx > 0) {
>>> >> +        ret = ioq_submit(s);
>>> >> +    }
>>> >> +
>>> >> +    return ret;