From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48830) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2NIC-0003OX-37 for qemu-devel@nongnu.org; Wed, 02 Jul 2014 12:22:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X2NI5-0005Bv-SE for qemu-devel@nongnu.org; Wed, 02 Jul 2014 12:21:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:29144) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2NI5-0005AA-Kt for qemu-devel@nongnu.org; Wed, 02 Jul 2014 12:21:49 -0400 Message-ID: <53B43194.80102@redhat.com> Date: Wed, 02 Jul 2014 18:21:40 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <20140627120129.GO12061@stefanha-thinkpad.muc.redhat.com> <53ADE769.3060903@redhat.com> <20140630080850.GB30969@stefanha-thinkpad.redhat.com> <53B2E69A.1090707@redhat.com> <20140702085453.GI4660@stefanha-thinkpad.redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [regression] dataplane: throughout -40% by commit 580b6b2aa2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei , Stefan Hajnoczi Cc: Kevin Wolf , Stefan Hajnoczi , Fam Zheng , qemu-devel , "Michael S. Tsirkin" Il 02/07/2014 17:45, Ming Lei ha scritto: > The attachment debug patch skips aio_notify() if qemu_bh_schedule > is running from current aio context, but looks there is still 120K > writes triggered. (without the patch, 400K can be observed in > same test) Nice. Another observation is that after aio_dispatch we'll always re-evaluate everything (bottom halves, file descriptors and timeouts), so we can skip the aio_notify if we're inside aio_dispatch. So what about this untested patch: diff --git a/aio-posix.c b/aio-posix.c index f921d4f..a23d85d 100644 --- a/aio-posix.c +++ b/aio-posix.c @@ -124,6 +124,9 @@ static bool aio_dispatch(AioContext *ctx) AioHandler *node; bool progress = false; + /* No need to set the event notifier during aio_notify. */ + ctx->running++; + /* * We have to walk very carefully in case qemu_aio_set_fd_handler is * called while we're walking. @@ -169,6 +171,11 @@ static bool aio_dispatch(AioContext *ctx) /* Run our timers */ progress |= timerlistgroup_run_timers(&ctx->tlg); + smp_wmb(); + ctx->iter_count++; + smp_wmb(); + ctx->running--; + return progress; } diff --git a/async.c b/async.c index 5b6fe6b..1f56afa 100644 --- a/async.c +++ b/async.c @@ -249,7 +249,19 @@ ThreadPool *aio_get_thread_pool(AioContext *ctx) void aio_notify(AioContext *ctx) { - event_notifier_set(&ctx->notifier); + uint32_t iter_count; + do { + iter_count = ctx->iter_count; + /* Read ctx->iter_count before ctx->running. */ + smb_rmb(); + if (!ctx->running) { + event_notifier_set(&ctx->notifier); + return; + } + /* Read ctx->running before ctx->iter_count. */ + smb_rmb(); + /* ctx might have gone to sleep. */ + } while (iter_count != ctx->iter_count); } static void aio_timerlist_notify(void *opaque) @@ -269,6 +279,7 @@ AioContext *aio_context_new(void) ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext)); ctx->pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD)); ctx->thread_pool = NULL; + ctx->iter_count = ctx->running = 0; qemu_mutex_init(&ctx->bh_lock); rfifolock_init(&ctx->lock, aio_rfifolock_cb, ctx); event_notifier_init(&ctx->notifier, false); diff --git a/include/block/aio.h b/include/block/aio.h index a92511b..9f51c4f 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -51,6 +51,9 @@ struct AioContext { /* Protects all fields from multi-threaded access */ RFifoLock lock; + /* Used to avoid aio_notify while dispatching event handlers. + * Writes protected by lock or BQL, reads are lockless. + */ + uint32_t iter_count, running; + /* The list of registered AIO handlers */ QLIST_HEAD(, AioHandler) aio_handlers; Please review carefully. > So is there still other writes not found in the path? What do perf or gdb say? :) Paolo