[PATCH] io_uring/wq: avoid indirect do_work/free_work calls

[PATCH] io_uring/wq: avoid indirect do_work/free_work calls

[Caleb Sander Mateos]

struct io_wq stores do_work and free_work function pointers which are
called on each work item. But these function pointers are always set to
io_wq_submit_work and io_wq_free_work, respectively. So remove these
function pointers and just call the functions directly.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 io_uring/io-wq.c    | 15 ++++-----------
 io_uring/io-wq.h    |  5 -----
 io_uring/io_uring.c |  2 +-
 io_uring/tctx.c     |  2 --
 4 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
index 04a75d666195..d52069b1177b 100644
--- a/io_uring/io-wq.c
+++ b/io_uring/io-wq.c
@@ -112,13 +112,10 @@ enum {
  * Per io_wq state
   */
 struct io_wq {
 	unsigned long state;
 
-	free_work_fn *free_work;
-	io_wq_work_fn *do_work;
-
 	struct io_wq_hash *hash;
 
 	atomic_t worker_refs;
 	struct completion worker_done;
 
@@ -610,14 +607,14 @@ static void io_worker_handle_work(struct io_wq_acct *acct,
 			next_hashed = wq_next_work(work);
 
 			if (do_kill &&
 			    (work_flags & IO_WQ_WORK_UNBOUND))
 				atomic_or(IO_WQ_WORK_CANCEL, &work->flags);
-			wq->do_work(work);
+			io_wq_submit_work(work);
 			io_assign_current_work(worker, NULL);
 
-			linked = wq->free_work(work);
+			linked = io_wq_free_work(work);
 			work = next_hashed;
 			if (!work && linked && !io_wq_is_hashed(linked)) {
 				work = linked;
 				linked = NULL;
 			}
@@ -932,12 +929,12 @@ static bool io_wq_worker_wake(struct io_worker *worker, void *data)
 
 static void io_run_cancel(struct io_wq_work *work, struct io_wq *wq)
 {
 	do {
 		atomic_or(IO_WQ_WORK_CANCEL, &work->flags);
-		wq->do_work(work);
-		work = wq->free_work(work);
+		io_wq_submit_work(work);
+		work = io_wq_free_work(work);
 	} while (work);
 }
 
 static void io_wq_insert_work(struct io_wq *wq, struct io_wq_acct *acct,
 			      struct io_wq_work *work, unsigned int work_flags)
@@ -1193,23 +1190,19 @@ static int io_wq_hash_wake(struct wait_queue_entry *wait, unsigned mode,
 struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 {
 	int ret, i;
 	struct io_wq *wq;
 
-	if (WARN_ON_ONCE(!data->free_work || !data->do_work))
-		return ERR_PTR(-EINVAL);
 	if (WARN_ON_ONCE(!bounded))
 		return ERR_PTR(-EINVAL);
 
 	wq = kzalloc(sizeof(struct io_wq), GFP_KERNEL);
 	if (!wq)
 		return ERR_PTR(-ENOMEM);
 
 	refcount_inc(&data->hash->refs);
 	wq->hash = data->hash;
-	wq->free_work = data->free_work;
-	wq->do_work = data->do_work;
 
 	ret = -ENOMEM;
 
 	if (!alloc_cpumask_var(&wq->cpu_mask, GFP_KERNEL))
 		goto err;
diff --git a/io_uring/io-wq.h b/io_uring/io-wq.h
index d4fb2940e435..774abab54732 100644
--- a/io_uring/io-wq.h
+++ b/io_uring/io-wq.h
@@ -19,13 +19,10 @@ enum io_wq_cancel {
 	IO_WQ_CANCEL_OK,	/* cancelled before started */
 	IO_WQ_CANCEL_RUNNING,	/* found, running, and attempted cancelled */
 	IO_WQ_CANCEL_NOTFOUND,	/* work not found */
 };
 
-typedef struct io_wq_work *(free_work_fn)(struct io_wq_work *);
-typedef void (io_wq_work_fn)(struct io_wq_work *);
-
 struct io_wq_hash {
 	refcount_t refs;
 	unsigned long map;
 	struct wait_queue_head wait;
 };
@@ -37,12 +34,10 @@ static inline void io_wq_put_hash(struct io_wq_hash *hash)
 }
 
 struct io_wq_data {
 	struct io_wq_hash *hash;
 	struct task_struct *task;
-	io_wq_work_fn *do_work;
-	free_work_fn *free_work;
 };
 
 struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data);
 void io_wq_exit_start(struct io_wq *wq);
 void io_wq_put_and_exit(struct io_wq *wq);
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index e4484a03e033..1d8d1b0e92f2 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1808,11 +1808,11 @@ void io_wq_submit_work(struct io_wq_work *work)
 	const struct io_issue_def *def = &io_issue_defs[req->opcode];
 	unsigned int issue_flags = IO_URING_F_UNLOCKED | IO_URING_F_IOWQ;
 	bool needs_poll = false;
 	int ret = 0, err = -ECANCELED;
 
-	/* one will be dropped by ->io_wq_free_work() after returning to io-wq */
+	/* one will be dropped by io_wq_free_work() after returning to io-wq */
 	if (!(req->flags & REQ_F_REFCOUNT))
 		__io_req_set_refcount(req, 2);
 	else
 		req_ref_get(req);
 
diff --git a/io_uring/tctx.c b/io_uring/tctx.c
index adc6e42c14df..5b66755579c0 100644
--- a/io_uring/tctx.c
+++ b/io_uring/tctx.c
@@ -33,12 +33,10 @@ static struct io_wq *io_init_wq_offload(struct io_ring_ctx *ctx,
 	}
 	mutex_unlock(&ctx->uring_lock);
 
 	data.hash = hash;
 	data.task = task;
-	data.free_work = io_wq_free_work;
-	data.do_work = io_wq_submit_work;
 
 	/* Do QD, or 4 * CPUS, whatever is smallest */
 	concurrency = min(ctx->sq_entries, 4 * num_online_cpus());
 
 	return io_wq_create(concurrency, &data);
-- 
2.45.2

Re: [PATCH] io_uring/wq: avoid indirect do_work/free_work calls

[Jens Axboe]

On 3/29/25 10:15 AM, Caleb Sander Mateos wrote:
> struct io_wq stores do_work and free_work function pointers which are
> called on each work item. But these function pointers are always set to
> io_wq_submit_work and io_wq_free_work, respectively. So remove these
> function pointers and just call the functions directly.

Was going to say that the indirect call here is not something
I'd be worried about in terms of performance, but it's also kind of
pointless to have them when we have just the single do_work/free_work
callback. And hence it'd reduce the struct footprint, which is always
useful. So yeah, I do think the change makes sense.

-- 
Jens Axboe

Re: [PATCH] io_uring/wq: avoid indirect do_work/free_work calls

[Jens Axboe]

On Sat, 29 Mar 2025 10:15:24 -0600, Caleb Sander Mateos wrote:
> struct io_wq stores do_work and free_work function pointers which are
> called on each work item. But these function pointers are always set to
> io_wq_submit_work and io_wq_free_work, respectively. So remove these
> function pointers and just call the functions directly.
> 
> 

Applied, thanks!

[1/1] io_uring/wq: avoid indirect do_work/free_work calls
      commit: 842b5d5f87039d978a9748f8728cabe07a676252

Best regards,
-- 
Jens Axboe