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