From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55980) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQyNY-0001ny-K8 for qemu-devel@nongnu.org; Thu, 26 Feb 2015 08:21:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQyNV-00052u-EJ for qemu-devel@nongnu.org; Thu, 26 Feb 2015 08:21:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35387) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQyNV-00052n-6x for qemu-devel@nongnu.org; Thu, 26 Feb 2015 08:21:21 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1QDLJTd016077 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 26 Feb 2015 08:21:20 -0500 Message-ID: <54EF1DCA.4020202@redhat.com> Date: Thu, 26 Feb 2015 14:21:14 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1424449612-18215-1-git-send-email-pbonzini@redhat.com> <1424449612-18215-3-git-send-email-pbonzini@redhat.com> <20150225054541.GA5293@ad.nay.redhat.com> In-Reply-To: <20150225054541.GA5293@ad.nay.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 2/3] AioContext: acquire/release AioContext during aio_poll List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: qemu-devel@nongnu.org, stefanha@redhat.com On 25/02/2015 06:45, Fam Zheng wrote: > On Fri, 02/20 17:26, Paolo Bonzini wrote: >> This is the first step in pushing down acquire/release, and will let >> rfifolock drop the contention callback feature. >> >> Signed-off-by: Paolo Bonzini >> --- >> aio-posix.c | 9 +++++++++ >> aio-win32.c | 8 ++++++++ >> include/block/aio.h | 15 ++++++++------- >> 3 files changed, 25 insertions(+), 7 deletions(-) >> >> diff --git a/aio-posix.c b/aio-posix.c >> index 4a30b77..292ae84 100644 >> --- a/aio-posix.c >> +++ b/aio-posix.c >> @@ -238,6 +238,7 @@ bool aio_poll(AioContext *ctx, bool blocking) >> bool progress; >> int64_t timeout; >> >> + aio_context_acquire(ctx); >> was_dispatching = ctx->dispatching; >> progress = false; >> >> @@ -267,7 +268,13 @@ bool aio_poll(AioContext *ctx, bool blocking) >> timeout = blocking ? aio_compute_timeout(ctx) : 0; >> >> /* wait until next event */ >> + if (timeout) { >> + aio_context_release(ctx); >> + } >> ret = qemu_poll_ns((GPollFD *)pollfds, npfd, timeout); >> + if (timeout) { >> + aio_context_acquire(ctx); >> + } >> >> /* if we have any readable fds, dispatch event */ >> if (ret > 0) { >> @@ -285,5 +292,7 @@ bool aio_poll(AioContext *ctx, bool blocking) >> } >> >> aio_set_dispatching(ctx, was_dispatching); >> + aio_context_release(ctx); >> + >> return progress; >> } >> diff --git a/aio-win32.c b/aio-win32.c >> index e6f4ced..233d8f5 100644 >> --- a/aio-win32.c >> +++ b/aio-win32.c >> @@ -283,6 +283,7 @@ bool aio_poll(AioContext *ctx, bool blocking) >> int count; >> int timeout; >> >> + aio_context_acquire(ctx); >> have_select_revents = aio_prepare(ctx); >> if (have_select_revents) { >> blocking = false; >> @@ -323,7 +324,13 @@ bool aio_poll(AioContext *ctx, bool blocking) >> >> timeout = blocking >> ? qemu_timeout_ns_to_ms(aio_compute_timeout(ctx)) : 0; >> + if (timeout) { >> + aio_context_release(ctx); > > Why are the unlock/lock pairs around poll conditional? Both iothread.c and os_host_main_loop_wait are doing it, IIRC it was measurably faster. In particular, iothread.c was completely avoiding acquire/release around non-blocking aio_poll and this patch does not have exactly the same behavior, but it is trying to be close: - aio_context_acquire(iothread->ctx); - blocking = true; - while (!iothread->stopping && aio_poll(iothread->ctx, blocking)) { - /* Progress was made, keep going */ - blocking = false; - } - aio_context_release(iothread->ctx); The exact same behavior can be done easily directly in aio_poll, for this RFC I'm keeping the code a little simpler. Paolo > > Fam > >> + } >> ret = WaitForMultipleObjects(count, events, FALSE, timeout); >> + if (timeout) { >> + aio_context_acquire(ctx); >> + } >> aio_set_dispatching(ctx, true); >> >> if (first && aio_bh_poll(ctx)) {