@@ -71,8 +71,6 @@ typedef struct {
* removed
*
* Add/remove/modify a monitored file descriptor.
- *
- * Called with ctx->list_lock acquired.
*/
void (*update)(AioContext *ctx, AioHandler *old_node, AioHandler *new_node);
@@ -84,7 +82,7 @@ typedef struct {
*
* Wait for file descriptors to become ready and place them on ready_list.
*
- * Called with ctx->list_lock incremented but not locked.
+ * Called with ctx->walking_handlers incremented.
*
* Returns: number of ready file descriptors.
*/
@@ -136,10 +134,10 @@ struct AioContext {
*/
BdrvGraphRWlock *bdrv_graph;
- /* The list of registered AIO handlers. Protected by ctx->list_lock. */
+ /* The list of registered AIO handlers. */
AioHandlerList aio_handlers;
- /* The list of AIO handlers to be deleted. Protected by ctx->list_lock. */
+ /* The list of AIO handlers to be deleted. */
AioHandlerList deleted_aio_handlers;
/* Used to avoid unnecessary event_notifier_set calls in aio_notify;
@@ -171,11 +169,8 @@ struct AioContext {
*/
uint32_t notify_me;
- /* A lock to protect between QEMUBH and AioHandler adders and deleter,
- * and to ensure that no callbacks are removed while we're walking and
- * dispatching them.
- */
- QemuLockCnt list_lock;
+ /* Re-entrancy counter for iterating over aio_handlers */
+ uint32_t walking_handlers;
/* Bottom Halves pending aio_bh_poll() processing */
BHList bh_list;
@@ -236,12 +231,7 @@ struct AioContext {
/* AIO engine parameters */
int64_t aio_max_batch; /* maximum number of requests in a batch */
- /*
- * List of handlers participating in userspace polling. Protected by
- * ctx->list_lock. Iterated and modified mostly by the event loop thread
- * from aio_poll() with ctx->list_lock incremented. aio_set_fd_handler()
- * only touches the list to delete nodes if ctx->list_lock's count is zero.
- */
+ /* List of handlers participating in userspace polling. */
AioHandlerList poll_aio_handlers;
/* Are we in polling mode or monitoring file descriptors? */
@@ -84,7 +84,7 @@ static bool aio_remove_fd_handler(AioContext *ctx, AioHandler *node)
}
/* If a read is in progress, just mark the node as deleted */
- if (qemu_lockcnt_count(&ctx->list_lock)) {
+ if (ctx->walking_handlers > 0) {
QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted);
return false;
}
@@ -116,14 +116,11 @@ static void aio_set_fd_handler_local(AioContext *ctx,
io_poll = NULL; /* polling only makes sense if there is a handler */
}
- qemu_lockcnt_lock(&ctx->list_lock);
-
node = find_aio_handler(ctx, fd);
/* Are we deleting the fd handler? */
if (!io_read && !io_write && !io_poll) {
if (node == NULL) {
- qemu_lockcnt_unlock(&ctx->list_lock);
return;
}
/* Clean events in order to unregister fd from the ctx epoll. */
@@ -171,7 +168,6 @@ static void aio_set_fd_handler_local(AioContext *ctx,
if (node) {
deleted = aio_remove_fd_handler(ctx, node);
}
- qemu_lockcnt_unlock(&ctx->list_lock);
aio_notify(ctx);
if (deleted) {
@@ -317,7 +313,7 @@ static bool poll_set_started(AioContext *ctx, AioHandlerList *ready_list,
ctx->poll_started = started;
- qemu_lockcnt_inc(&ctx->list_lock);
+ ctx->walking_handlers++;
QLIST_FOREACH(node, &ctx->poll_aio_handlers, node_poll) {
IOHandler *fn;
@@ -341,7 +337,7 @@ static bool poll_set_started(AioContext *ctx, AioHandlerList *ready_list,
progress = true;
}
}
- qemu_lockcnt_dec(&ctx->list_lock);
+ ctx->walking_handlers--;
return progress;
}
@@ -363,12 +359,7 @@ bool aio_pending(AioContext *ctx)
AioHandler *node;
bool result = false;
- /*
- * We have to walk very carefully in case aio_set_fd_handler is
- * called while we're walking.
- */
- qemu_lockcnt_inc(&ctx->list_lock);
-
+ ctx->walking_handlers++;
QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
int revents;
@@ -383,19 +374,17 @@ bool aio_pending(AioContext *ctx)
break;
}
}
- qemu_lockcnt_dec(&ctx->list_lock);
+ ctx->walking_handlers--;
return result;
}
+/* Caller must not have ctx->walking_handlers incremented */
static void aio_free_deleted_handlers(AioContext *ctx)
{
AioHandler *node;
- if (QLIST_EMPTY_RCU(&ctx->deleted_aio_handlers)) {
- return;
- }
- if (!qemu_lockcnt_dec_if_lock(&ctx->list_lock)) {
+ if (ctx->walking_handlers > 0) {
return; /* we are nested, let the parent do the freeing */
}
@@ -405,8 +394,6 @@ static void aio_free_deleted_handlers(AioContext *ctx)
QLIST_SAFE_REMOVE(node, node_poll);
g_free(node);
}
-
- qemu_lockcnt_inc_and_unlock(&ctx->list_lock);
}
static bool aio_dispatch_handler(AioContext *ctx, AioHandler *node)
@@ -511,11 +498,12 @@ static bool aio_dispatch_handlers(AioContext *ctx)
void aio_dispatch(AioContext *ctx)
{
- qemu_lockcnt_inc(&ctx->list_lock);
+ ctx->walking_handlers++;
aio_bh_poll(ctx);
aio_dispatch_handlers(ctx);
+ ctx->walking_handlers--;
+
aio_free_deleted_handlers(ctx);
- qemu_lockcnt_dec(&ctx->list_lock);
timerlistgroup_run_timers(&ctx->tlg);
}
@@ -607,7 +595,7 @@ static bool remove_idle_poll_handlers(AioContext *ctx,
*
* Polls for a given time.
*
- * Note that the caller must have incremented ctx->list_lock.
+ * Note that the caller must have incremented ctx->walking_handlers.
*
* Returns: true if progress was made, false otherwise
*/
@@ -617,7 +605,7 @@ static bool run_poll_handlers(AioContext *ctx, AioHandlerList *ready_list,
bool progress;
int64_t start_time, elapsed_time;
- assert(qemu_lockcnt_count(&ctx->list_lock) > 0);
+ assert(&ctx->walking_handlers > 0);
trace_run_poll_handlers_begin(ctx, max_ns, *timeout);
@@ -663,7 +651,7 @@ static bool run_poll_handlers(AioContext *ctx, AioHandlerList *ready_list,
* @timeout: timeout for blocking wait, computed by the caller and updated if
* polling succeeds.
*
- * Note that the caller must have incremented ctx->list_lock.
+ * Note that the caller must have incremented ctx->walking_handlers.
*
* Returns: true if progress was made, false otherwise
*/
@@ -711,7 +699,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
assert(in_aio_context_home_thread(ctx == iohandler_get_aio_context() ?
qemu_get_aio_context() : ctx));
- qemu_lockcnt_inc(&ctx->list_lock);
+ ctx->walking_handlers++;
if (ctx->poll_max_ns) {
start = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
@@ -814,9 +802,9 @@ bool aio_poll(AioContext *ctx, bool blocking)
progress |= aio_bh_poll(ctx);
progress |= aio_dispatch_ready_handlers(ctx, &ready_list);
- aio_free_deleted_handlers(ctx);
+ ctx->walking_handlers--;
- qemu_lockcnt_dec(&ctx->list_lock);
+ aio_free_deleted_handlers(ctx);
progress |= timerlistgroup_run_timers(&ctx->tlg);
@@ -412,7 +412,6 @@ aio_ctx_finalize(GSource *source)
aio_set_event_notifier(ctx, &ctx->notifier, NULL, NULL, NULL);
event_notifier_cleanup(&ctx->notifier);
qemu_rec_mutex_destroy(&ctx->lock);
- qemu_lockcnt_destroy(&ctx->list_lock);
timerlistgroup_deinit(&ctx->tlg);
unregister_aiocontext(ctx);
aio_context_destroy(ctx);
@@ -585,7 +584,6 @@ AioContext *aio_context_new(Error **errp)
goto fail;
}
g_source_set_can_recurse(&ctx->source, true);
- qemu_lockcnt_init(&ctx->list_lock);
ctx->co_schedule_bh = aio_bh_new(ctx, co_schedule_bh_cb, ctx);
QSLIST_INIT(&ctx->scheduled_coroutines);
@@ -132,15 +132,11 @@ bool fdmon_epoll_try_upgrade(AioContext *ctx, unsigned npfd)
return false;
}
- /* The list must not change while we add fds to epoll */
- if (!qemu_lockcnt_dec_if_lock(&ctx->list_lock)) {
+ if (ctx->walking_handlers > 1) {
return false;
}
ok = fdmon_epoll_try_enable(ctx);
-
- qemu_lockcnt_inc_and_unlock(&ctx->list_lock);
-
if (!ok) {
fdmon_epoll_disable(ctx);
}
TODO further simplifications may be possible, like using none _RCU() macros for the aio_handlers QLIST Now that aio_set_fd_handler() uses a BH to schedule itself in remote AioContexts it is no longer necessary to worry about multi-threading. Replace the ctx->list_lock locked counter with a plain uint32_t counter called ctx->walking_handlers. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/block/aio.h | 22 ++++++---------------- util/aio-posix.c | 44 ++++++++++++++++---------------------------- util/async.c | 2 -- util/fdmon-epoll.c | 6 +----- 4 files changed, 23 insertions(+), 51 deletions(-)