[PATCH 0/3] eventfd signalling cleanup

[PATCH 0/3] eventfd signalling cleanup

[Pavel Begunkov]

Get rid of of conditional rcu locking in io_uring/eventfd.c,
fix a sparse warning and clean it up.

Pavel Begunkov (3):
  io_uring/eventfd: dedup signalling helpers
  io_uring/eventfd: clean up rcu locking
  io_uring/eventfd: open code io_eventfd_grab()

 io_uring/eventfd.c  | 66 ++++++++++-----------------------------------
 io_uring/eventfd.h  |  3 +--
 io_uring/io_uring.c |  4 +--
 3 files changed, 17 insertions(+), 56 deletions(-)

-- 
2.48.1

[PATCH 1/3] io_uring/eventfd: dedup signalling helpers

[Pavel Begunkov]

Consolidate io_eventfd_flush_signal() and io_eventfd_signal(). Not much
of a difference for now, but it prepares it for following changes.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/eventfd.c  | 26 +++++++++-----------------
 io_uring/eventfd.h  |  3 +--
 io_uring/io_uring.c |  4 ++--
 3 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/io_uring/eventfd.c b/io_uring/eventfd.c
index 100d5da94cb9..a9da2d0d7510 100644
--- a/io_uring/eventfd.c
+++ b/io_uring/eventfd.c
@@ -112,23 +112,16 @@ static struct io_ev_fd *io_eventfd_grab(struct io_ring_ctx *ctx)
 	return NULL;
 }
 
-void io_eventfd_signal(struct io_ring_ctx *ctx)
+void io_eventfd_signal(struct io_ring_ctx *ctx, bool cqe_event)
 {
+	bool skip = false, put_ref = true;
 	struct io_ev_fd *ev_fd;
 
 	ev_fd = io_eventfd_grab(ctx);
-	if (ev_fd)
-		io_eventfd_release(ev_fd, __io_eventfd_signal(ev_fd));
-}
-
-void io_eventfd_flush_signal(struct io_ring_ctx *ctx)
-{
-	struct io_ev_fd *ev_fd;
-
-	ev_fd = io_eventfd_grab(ctx);
-	if (ev_fd) {
-		bool skip, put_ref = true;
+	if (!ev_fd)
+		return;
 
+	if (cqe_event) {
 		/*
 		 * Eventfd should only get triggered when at least one event
 		 * has been posted. Some applications rely on the eventfd
@@ -142,12 +135,11 @@ void io_eventfd_flush_signal(struct io_ring_ctx *ctx)
 		skip = ctx->cached_cq_tail == ev_fd->last_cq_tail;
 		ev_fd->last_cq_tail = ctx->cached_cq_tail;
 		spin_unlock(&ctx->completion_lock);
-
-		if (!skip)
-			put_ref = __io_eventfd_signal(ev_fd);
-
-		io_eventfd_release(ev_fd, put_ref);
 	}
+
+	if (!skip)
+		put_ref = __io_eventfd_signal(ev_fd);
+	io_eventfd_release(ev_fd, put_ref);
 }
 
 int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
diff --git a/io_uring/eventfd.h b/io_uring/eventfd.h
index d394f49c6321..e2f1985c2cf9 100644
--- a/io_uring/eventfd.h
+++ b/io_uring/eventfd.h
@@ -4,5 +4,4 @@ int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
 			unsigned int eventfd_async);
 int io_eventfd_unregister(struct io_ring_ctx *ctx);
 
-void io_eventfd_flush_signal(struct io_ring_ctx *ctx);
-void io_eventfd_signal(struct io_ring_ctx *ctx);
+void io_eventfd_signal(struct io_ring_ctx *ctx, bool cqe_event);
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 7099b488c5e1..33d1a6b29b46 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -584,7 +584,7 @@ void __io_commit_cqring_flush(struct io_ring_ctx *ctx)
 	if (ctx->drain_active)
 		io_queue_deferred(ctx);
 	if (ctx->has_evfd)
-		io_eventfd_flush_signal(ctx);
+		io_eventfd_signal(ctx, true);
 }
 
 static inline void __io_cq_lock(struct io_ring_ctx *ctx)
@@ -1204,7 +1204,7 @@ static void io_req_local_work_add(struct io_kiocb *req, unsigned flags)
 		if (ctx->flags & IORING_SETUP_TASKRUN_FLAG)
 			atomic_or(IORING_SQ_TASKRUN, &ctx->rings->sq_flags);
 		if (ctx->has_evfd)
-			io_eventfd_signal(ctx);
+			io_eventfd_signal(ctx, false);
 	}
 
 	nr_wait = atomic_read(&ctx->cq_wait_nr);
-- 
2.48.1

[PATCH 2/3] io_uring/eventfd: clean up rcu locking

[Pavel Begunkov]

Conditional locking is never welcome if there are better options. Move
rcu locking into io_eventfd_signal(), make it unconditional and use
guards. It also helps with sparse warnings.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/eventfd.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/io_uring/eventfd.c b/io_uring/eventfd.c
index a9da2d0d7510..8c2835ac17a0 100644
--- a/io_uring/eventfd.c
+++ b/io_uring/eventfd.c
@@ -47,13 +47,6 @@ static void io_eventfd_do_signal(struct rcu_head *rcu)
 	io_eventfd_put(ev_fd);
 }
 
-static void io_eventfd_release(struct io_ev_fd *ev_fd, bool put_ref)
-{
-	if (put_ref)
-		io_eventfd_put(ev_fd);
-	rcu_read_unlock();
-}
-
 /*
  * Returns true if the caller should put the ev_fd reference, false if not.
  */
@@ -89,11 +82,6 @@ static struct io_ev_fd *io_eventfd_grab(struct io_ring_ctx *ctx)
 {
 	struct io_ev_fd *ev_fd;
 
-	if (READ_ONCE(ctx->rings->cq_flags) & IORING_CQ_EVENTFD_DISABLED)
-		return NULL;
-
-	rcu_read_lock();
-
 	/*
 	 * rcu_dereference ctx->io_ev_fd once and use it for both for checking
 	 * and eventfd_signal
@@ -108,15 +96,18 @@ static struct io_ev_fd *io_eventfd_grab(struct io_ring_ctx *ctx)
 	if (io_eventfd_trigger(ev_fd) && refcount_inc_not_zero(&ev_fd->refs))
 		return ev_fd;
 
-	rcu_read_unlock();
 	return NULL;
 }
 
 void io_eventfd_signal(struct io_ring_ctx *ctx, bool cqe_event)
 {
-	bool skip = false, put_ref = true;
+	bool skip = false;
 	struct io_ev_fd *ev_fd;
 
+	if (READ_ONCE(ctx->rings->cq_flags) & IORING_CQ_EVENTFD_DISABLED)
+		return;
+
+	guard(rcu)();
 	ev_fd = io_eventfd_grab(ctx);
 	if (!ev_fd)
 		return;
@@ -137,9 +128,8 @@ void io_eventfd_signal(struct io_ring_ctx *ctx, bool cqe_event)
 		spin_unlock(&ctx->completion_lock);
 	}
 
-	if (!skip)
-		put_ref = __io_eventfd_signal(ev_fd);
-	io_eventfd_release(ev_fd, put_ref);
+	if (skip || __io_eventfd_signal(ev_fd))
+		io_eventfd_put(ev_fd);
 }
 
 int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
-- 
2.48.1

[PATCH 3/3] io_uring/eventfd: open code io_eventfd_grab()

[Pavel Begunkov]

io_eventfd_grab() doesn't help wit understanding the path, it'll be
simpler to keep the helper open coded.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/eventfd.c | 40 ++++++++++------------------------------
 1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/io_uring/eventfd.c b/io_uring/eventfd.c
index 8c2835ac17a0..78f8ab7db104 100644
--- a/io_uring/eventfd.c
+++ b/io_uring/eventfd.c
@@ -65,38 +65,11 @@ static bool __io_eventfd_signal(struct io_ev_fd *ev_fd)
 
 /*
  * Trigger if eventfd_async isn't set, or if it's set and the caller is
- * an async worker. If ev_fd isn't valid, obviously return false.
+ * an async worker.
  */
 static bool io_eventfd_trigger(struct io_ev_fd *ev_fd)
 {
-	if (ev_fd)
-		return !ev_fd->eventfd_async || io_wq_current_is_worker();
-	return false;
-}
-
-/*
- * On success, returns with an ev_fd reference grabbed and the RCU read
- * lock held.
- */
-static struct io_ev_fd *io_eventfd_grab(struct io_ring_ctx *ctx)
-{
-	struct io_ev_fd *ev_fd;
-
-	/*
-	 * rcu_dereference ctx->io_ev_fd once and use it for both for checking
-	 * and eventfd_signal
-	 */
-	ev_fd = rcu_dereference(ctx->io_ev_fd);
-
-	/*
-	 * Check again if ev_fd exists in case an io_eventfd_unregister call
-	 * completed between the NULL check of ctx->io_ev_fd at the start of
-	 * the function and rcu_read_lock.
-	 */
-	if (io_eventfd_trigger(ev_fd) && refcount_inc_not_zero(&ev_fd->refs))
-		return ev_fd;
-
-	return NULL;
+	return !ev_fd->eventfd_async || io_wq_current_is_worker();
 }
 
 void io_eventfd_signal(struct io_ring_ctx *ctx, bool cqe_event)
@@ -108,9 +81,16 @@ void io_eventfd_signal(struct io_ring_ctx *ctx, bool cqe_event)
 		return;
 
 	guard(rcu)();
-	ev_fd = io_eventfd_grab(ctx);
+	ev_fd = rcu_dereference(ctx->io_ev_fd);
+	/*
+	 * Check again if ev_fd exists in case an io_eventfd_unregister call
+	 * completed between the NULL check of ctx->io_ev_fd at the start of
+	 * the function and rcu_read_lock.
+	 */
 	if (!ev_fd)
 		return;
+	if (!io_eventfd_trigger(ev_fd) || !refcount_inc_not_zero(&ev_fd->refs))
+		return;
 
 	if (cqe_event) {
 		/*
-- 
2.48.1

Re: [PATCH 0/3] eventfd signalling cleanup

[Jens Axboe]

On Thu, 24 Apr 2025 12:31:15 +0100, Pavel Begunkov wrote:
> Get rid of of conditional rcu locking in io_uring/eventfd.c,
> fix a sparse warning and clean it up.
> 
> Pavel Begunkov (3):
>   io_uring/eventfd: dedup signalling helpers
>   io_uring/eventfd: clean up rcu locking
>   io_uring/eventfd: open code io_eventfd_grab()
> 
> [...]

Applied, thanks!

[1/3] io_uring/eventfd: dedup signalling helpers
      commit: 5de2c46e0f46328b5b35ea1f86299af3cf7163c3
[2/3] io_uring/eventfd: clean up rcu locking
      commit: b27f1209b3efe0e93b033533874e982d1925418f
[3/3] io_uring/eventfd: open code io_eventfd_grab()
      commit: 61dceb2a1c94b3e2c5ec8335bfb7acb83c6fca6d

Best regards,
-- 
Jens Axboe