public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch
@ 2025-10-22 23:13 Caleb Sander Mateos
  2025-10-22 23:13 ` [PATCH 1/3] io_uring: expose io_should_terminate_tw() Caleb Sander Mateos
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Caleb Sander Mateos @ 2025-10-22 23:13 UTC (permalink / raw)
  To: Jens Axboe, Miklos Szeredi, Ming Lei, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, Chris Mason, David Sterba
  Cc: io-uring, linux-fsdevel, linux-block, linux-nvme, linux-btrfs,
	linux-kernel, Caleb Sander Mateos

Define a io_req_tw_func_t wrapper function around each io_uring_cmd_tw_t
function to avoid the additional indirect call and save 8 bytes in
struct io_uring_cmd. Additionally avoid the io_should_terminate_tw()
computation in uring_cmd task work callbacks that don't need it.

Caleb Sander Mateos (3):
  io_uring: expose io_should_terminate_tw()
  io_uring/uring_cmd: call io_should_terminate_tw() when needed
  io_uring/uring_cmd: avoid double indirect call in task work dispatch

 block/ioctl.c                  |  1 +
 drivers/block/ublk_drv.c       |  3 +++
 drivers/nvme/host/ioctl.c      |  1 +
 fs/btrfs/ioctl.c               |  1 +
 fs/fuse/dev_uring.c            |  3 ++-
 include/linux/io_uring.h       | 14 +++++++++++
 include/linux/io_uring/cmd.h   | 46 ++++++++++++++++++++++------------
 include/linux/io_uring_types.h |  1 -
 io_uring/io_uring.h            | 13 ----------
 io_uring/uring_cmd.c           | 17 ++-----------
 10 files changed, 54 insertions(+), 46 deletions(-)

-- 
2.45.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] io_uring: expose io_should_terminate_tw()
  2025-10-22 23:13 [PATCH 0/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
@ 2025-10-22 23:13 ` Caleb Sander Mateos
  2025-10-22 23:13 ` [PATCH 2/3] io_uring/uring_cmd: call io_should_terminate_tw() when needed Caleb Sander Mateos
  2025-10-22 23:13 ` [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
  2 siblings, 0 replies; 8+ messages in thread
From: Caleb Sander Mateos @ 2025-10-22 23:13 UTC (permalink / raw)
  To: Jens Axboe, Miklos Szeredi, Ming Lei, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, Chris Mason, David Sterba
  Cc: io-uring, linux-fsdevel, linux-block, linux-nvme, linux-btrfs,
	linux-kernel, Caleb Sander Mateos

A subsequent commit will call io_should_terminate_tw() from an inline
function in include/linux/io_uring/cmd.h, so move it from an io_uring
internal header to include/linux/io_uring.h. Callers outside io_uring
should not call it directly.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 include/linux/io_uring.h | 14 ++++++++++++++
 io_uring/io_uring.h      | 13 -------------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
index 85fe4e6b275c..c2a12287b821 100644
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
@@ -1,13 +1,27 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 #ifndef _LINUX_IO_URING_H
 #define _LINUX_IO_URING_H
 
+#include <linux/io_uring_types.h>
 #include <linux/sched.h>
 #include <linux/xarray.h>
 #include <uapi/linux/io_uring.h>
 
+/*
+ * Terminate the request if either of these conditions are true:
+ *
+ * 1) It's being executed by the original task, but that task is marked
+ *    with PF_EXITING as it's exiting.
+ * 2) PF_KTHREAD is set, in which case the invoker of the task_work is
+ *    our fallback task_work.
+ */
+static inline bool io_should_terminate_tw(struct io_ring_ctx *ctx)
+{
+	return (current->flags & (PF_KTHREAD | PF_EXITING)) || percpu_ref_is_dying(&ctx->refs);
+}
+
 #if defined(CONFIG_IO_URING)
 void __io_uring_cancel(bool cancel_all);
 void __io_uring_free(struct task_struct *tsk);
 void io_uring_unreg_ringfd(void);
 const char *io_uring_get_opcode(u8 opcode);
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index 46d9141d772a..78777bf1ea4b 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -556,23 +556,10 @@ static inline bool io_allowed_run_tw(struct io_ring_ctx *ctx)
 {
 	return likely(!(ctx->flags & IORING_SETUP_DEFER_TASKRUN) ||
 		      ctx->submitter_task == current);
 }
 
-/*
- * Terminate the request if either of these conditions are true:
- *
- * 1) It's being executed by the original task, but that task is marked
- *    with PF_EXITING as it's exiting.
- * 2) PF_KTHREAD is set, in which case the invoker of the task_work is
- *    our fallback task_work.
- */
-static inline bool io_should_terminate_tw(struct io_ring_ctx *ctx)
-{
-	return (current->flags & (PF_KTHREAD | PF_EXITING)) || percpu_ref_is_dying(&ctx->refs);
-}
-
 static inline void io_req_queue_tw_complete(struct io_kiocb *req, s32 res)
 {
 	io_req_set_res(req, res, 0);
 	req->io_task_work.func = io_req_task_complete;
 	io_req_task_work_add(req);
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] io_uring/uring_cmd: call io_should_terminate_tw() when needed
  2025-10-22 23:13 [PATCH 0/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
  2025-10-22 23:13 ` [PATCH 1/3] io_uring: expose io_should_terminate_tw() Caleb Sander Mateos
@ 2025-10-22 23:13 ` Caleb Sander Mateos
  2025-10-22 23:13 ` [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
  2 siblings, 0 replies; 8+ messages in thread
From: Caleb Sander Mateos @ 2025-10-22 23:13 UTC (permalink / raw)
  To: Jens Axboe, Miklos Szeredi, Ming Lei, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, Chris Mason, David Sterba
  Cc: io-uring, linux-fsdevel, linux-block, linux-nvme, linux-btrfs,
	linux-kernel, Caleb Sander Mateos

Most uring_cmd task work callbacks don't check IO_URING_F_TASK_DEAD. But
it's computed unconditionally in io_uring_cmd_work(). Add a helper
io_uring_cmd_should_terminate_tw() and call it instead of checking
IO_URING_F_TASK_DEAD in the one callback, fuse_uring_send_in_task().
Remove the now unused IO_URING_F_TASK_DEAD.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 fs/fuse/dev_uring.c            | 2 +-
 include/linux/io_uring/cmd.h   | 7 ++++++-
 include/linux/io_uring_types.h | 1 -
 io_uring/uring_cmd.c           | 6 +-----
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index f6b12aebb8bb..71b0c9662716 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -1214,11 +1214,11 @@ static void fuse_uring_send_in_task(struct io_uring_cmd *cmd,
 {
 	struct fuse_ring_ent *ent = uring_cmd_to_ring_ent(cmd);
 	struct fuse_ring_queue *queue = ent->queue;
 	int err;
 
-	if (!(issue_flags & IO_URING_F_TASK_DEAD)) {
+	if (!io_uring_cmd_should_terminate_tw(cmd)) {
 		err = fuse_uring_prepare_send(ent, ent->fuse_req);
 		if (err) {
 			fuse_uring_next_fuse_req(ent, queue, issue_flags);
 			return;
 		}
diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
index 7509025b4071..b84b97c21b43 100644
--- a/include/linux/io_uring/cmd.h
+++ b/include/linux/io_uring/cmd.h
@@ -1,11 +1,11 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 #ifndef _LINUX_IO_URING_CMD_H
 #define _LINUX_IO_URING_CMD_H
 
 #include <uapi/linux/io_uring.h>
-#include <linux/io_uring_types.h>
+#include <linux/io_uring.h>
 #include <linux/blk-mq.h>
 
 /* only top 8 bits of sqe->uring_cmd_flags for kernel internal use */
 #define IORING_URING_CMD_CANCELABLE	(1U << 30)
 /* io_uring_cmd is being issued again */
@@ -143,10 +143,15 @@ static inline void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
 			io_uring_cmd_tw_t task_work_cb)
 {
 	__io_uring_cmd_do_in_task(ioucmd, task_work_cb, 0);
 }
 
+static inline bool io_uring_cmd_should_terminate_tw(struct io_uring_cmd *cmd)
+{
+	return io_should_terminate_tw(cmd_to_io_kiocb(cmd)->ctx);
+}
+
 static inline struct task_struct *io_uring_cmd_get_task(struct io_uring_cmd *cmd)
 {
 	return cmd_to_io_kiocb(cmd)->tctx->task;
 }
 
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index c2ea6280901d..278c4a25c9e8 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -37,11 +37,10 @@ enum io_uring_cmd_flags {
 	IO_URING_F_IOPOLL		= (1 << 10),
 
 	/* set when uring wants to cancel a previously issued command */
 	IO_URING_F_CANCEL		= (1 << 11),
 	IO_URING_F_COMPAT		= (1 << 12),
-	IO_URING_F_TASK_DEAD		= (1 << 13),
 };
 
 struct io_wq_work_node {
 	struct io_wq_work_node *next;
 };
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index d1e3ba62ee8e..35bdac35cf4d 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -114,17 +114,13 @@ void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
 EXPORT_SYMBOL_GPL(io_uring_cmd_mark_cancelable);
 
 static void io_uring_cmd_work(struct io_kiocb *req, io_tw_token_t tw)
 {
 	struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
-	unsigned int flags = IO_URING_F_COMPLETE_DEFER;
-
-	if (io_should_terminate_tw(req->ctx))
-		flags |= IO_URING_F_TASK_DEAD;
 
 	/* task_work executor checks the deffered list completion */
-	ioucmd->task_work_cb(ioucmd, flags);
+	ioucmd->task_work_cb(ioucmd, IO_URING_F_COMPLETE_DEFER);
 }
 
 void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
 			io_uring_cmd_tw_t task_work_cb,
 			unsigned flags)
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch
  2025-10-22 23:13 [PATCH 0/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
  2025-10-22 23:13 ` [PATCH 1/3] io_uring: expose io_should_terminate_tw() Caleb Sander Mateos
  2025-10-22 23:13 ` [PATCH 2/3] io_uring/uring_cmd: call io_should_terminate_tw() when needed Caleb Sander Mateos
@ 2025-10-22 23:13 ` Caleb Sander Mateos
  2025-10-23 12:05   ` kernel test robot
                     ` (2 more replies)
  2 siblings, 3 replies; 8+ messages in thread
From: Caleb Sander Mateos @ 2025-10-22 23:13 UTC (permalink / raw)
  To: Jens Axboe, Miklos Szeredi, Ming Lei, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, Chris Mason, David Sterba
  Cc: io-uring, linux-fsdevel, linux-block, linux-nvme, linux-btrfs,
	linux-kernel, Caleb Sander Mateos

io_uring task work dispatch makes an indirect call to struct io_kiocb's
io_task_work.func field to allow running arbitrary task work functions.
In the uring_cmd case, this calls io_uring_cmd_work(), which immediately
makes another indirect call to struct io_uring_cmd's task_work_cb field.
Introduce a macro DEFINE_IO_URING_CMD_TASK_WORK() to define a
io_req_tw_func_t function wrapping an io_uring_cmd_tw_t. Convert the
io_uring_cmd_tw_t function to the io_req_tw_func_t function in
io_uring_cmd_complete_in_task() and io_uring_cmd_do_in_task_lazy().
Use DEFINE_IO_URING_CMD_TASK_WORK() to define a io_req_tw_func_t
function for each existing io_uring_cmd_tw_t function. Now uring_cmd
task work dispatch makes a single indirect call to the io_req_tw_func_t
wrapper function, which can inline the io_uring_cmd_tw_t function. This
also allows removing the task_work_cb field from struct io_uring_cmd,
freeing up some additional storage space.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 block/ioctl.c                |  1 +
 drivers/block/ublk_drv.c     |  3 +++
 drivers/nvme/host/ioctl.c    |  1 +
 fs/btrfs/ioctl.c             |  1 +
 fs/fuse/dev_uring.c          |  1 +
 include/linux/io_uring/cmd.h | 45 +++++++++++++++++++++---------------
 io_uring/uring_cmd.c         | 13 ++---------
 7 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index d7489a56b33c..7a3756863c9b 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -776,10 +776,11 @@ static void blk_cmd_complete(struct io_uring_cmd *cmd, unsigned int issue_flags)
 	if (bic->res == -EAGAIN && bic->nowait)
 		io_uring_cmd_issue_blocking(cmd);
 	else
 		io_uring_cmd_done(cmd, bic->res, issue_flags);
 }
+static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
 
 static void bio_cmd_bio_end_io(struct bio *bio)
 {
 	struct io_uring_cmd *cmd = bio->bi_private;
 	struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 0c74a41a6753..829b049c7c75 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1354,10 +1354,11 @@ static void ublk_cmd_tw_cb(struct io_uring_cmd *cmd,
 	struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
 	struct ublk_queue *ubq = pdu->ubq;
 
 	ublk_dispatch_req(ubq, pdu->req, issue_flags);
 }
+static DEFINE_IO_URING_CMD_TASK_WORK(ublk_cmd_tw_cb)
 
 static void ublk_queue_cmd(struct ublk_queue *ubq, struct request *rq)
 {
 	struct io_uring_cmd *cmd = ubq->ios[rq->tag].cmd;
 	struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
@@ -1378,10 +1379,11 @@ static void ublk_cmd_list_tw_cb(struct io_uring_cmd *cmd,
 		rq->rq_next = NULL;
 		ublk_dispatch_req(rq->mq_hctx->driver_data, rq, issue_flags);
 		rq = next;
 	} while (rq);
 }
+static DEFINE_IO_URING_CMD_TASK_WORK(ublk_cmd_list_tw_cb)
 
 static void ublk_queue_cmd_list(struct ublk_io *io, struct rq_list *l)
 {
 	struct io_uring_cmd *cmd = io->cmd;
 	struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
@@ -2529,10 +2531,11 @@ static void ublk_ch_uring_cmd_cb(struct io_uring_cmd *cmd,
 	int ret = ublk_ch_uring_cmd_local(cmd, issue_flags);
 
 	if (ret != -EIOCBQUEUED)
 		io_uring_cmd_done(cmd, ret, issue_flags);
 }
+static DEFINE_IO_URING_CMD_TASK_WORK(ublk_ch_uring_cmd_cb)
 
 static int ublk_ch_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
 	if (unlikely(issue_flags & IO_URING_F_CANCEL)) {
 		ublk_uring_cmd_cancel_fn(cmd, issue_flags);
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index c212fa952c0f..d4ca46b3abc7 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -405,10 +405,11 @@ static void nvme_uring_task_cb(struct io_uring_cmd *ioucmd,
 
 	if (pdu->bio)
 		blk_rq_unmap_user(pdu->bio);
 	io_uring_cmd_done32(ioucmd, pdu->status, pdu->result, issue_flags);
 }
+static DEFINE_IO_URING_CMD_TASK_WORK(nvme_uring_task_cb)
 
 static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
 						blk_status_t err)
 {
 	struct io_uring_cmd *ioucmd = req->end_io_data;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 185bef0df1c2..9d395f034403 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4704,10 +4704,11 @@ static void btrfs_uring_read_finished(struct io_uring_cmd *cmd, unsigned int iss
 	kfree(priv->pages);
 	kfree(priv->iov);
 	kfree(priv);
 	kfree(bc->data);
 }
+static DEFINE_IO_URING_CMD_TASK_WORK(btrfs_uring_read_finished)
 
 void btrfs_uring_read_extent_endio(void *ctx, int err)
 {
 	struct btrfs_uring_priv *priv = ctx;
 	struct io_btrfs_cmd *bc = io_uring_cmd_to_pdu(priv->cmd, struct io_btrfs_cmd);
diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 71b0c9662716..e2c87c01e021 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -1226,10 +1226,11 @@ static void fuse_uring_send_in_task(struct io_uring_cmd *cmd,
 		err = -ECANCELED;
 	}
 
 	fuse_uring_send(ent, cmd, err, issue_flags);
 }
+static DEFINE_IO_URING_CMD_TASK_WORK(fuse_uring_send_in_task)
 
 static struct fuse_ring_queue *fuse_uring_task_to_queue(struct fuse_ring *ring)
 {
 	unsigned int qid;
 	struct fuse_ring_queue *queue;
diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
index b84b97c21b43..5d6e30cc9b0b 100644
--- a/include/linux/io_uring/cmd.h
+++ b/include/linux/io_uring/cmd.h
@@ -9,18 +9,13 @@
 /* only top 8 bits of sqe->uring_cmd_flags for kernel internal use */
 #define IORING_URING_CMD_CANCELABLE	(1U << 30)
 /* io_uring_cmd is being issued again */
 #define IORING_URING_CMD_REISSUE	(1U << 31)
 
-typedef void (*io_uring_cmd_tw_t)(struct io_uring_cmd *cmd,
-				  unsigned issue_flags);
-
 struct io_uring_cmd {
 	struct file	*file;
 	const struct io_uring_sqe *sqe;
-	/* callback to defer completions to task context */
-	io_uring_cmd_tw_t task_work_cb;
 	u32		cmd_op;
 	u32		flags;
 	u8		pdu[32]; /* available inline for free use */
 };
 
@@ -58,13 +53,29 @@ int io_uring_cmd_import_fixed_vec(struct io_uring_cmd *ioucmd,
  */
 void __io_uring_cmd_done(struct io_uring_cmd *cmd, s32 ret, u64 res2,
 			 unsigned issue_flags, bool is_cqe32);
 
 void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
-			    io_uring_cmd_tw_t task_work_cb,
+			    io_req_tw_func_t task_work_cb,
 			    unsigned flags);
 
+/*
+ * uring_cmd_cb should be a function with the signature
+ * void (struct io_uring_cmd *cmd, unsigned issue_flags)
+ */
+#define IO_URING_CMD_TASK_WORK(uring_cmd_cb) uring_cmd_cb##_tw
+
+#define DEFINE_IO_URING_CMD_TASK_WORK(uring_cmd_cb)				\
+void										\
+IO_URING_CMD_TASK_WORK(uring_cmd_cb)(struct io_kiocb *req, io_tw_token_t tw)	\
+{										\
+	struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);\
+										\
+	/* task_work executor checks the deferred list completion */		\
+	uring_cmd_cb(ioucmd, IO_URING_F_COMPLETE_DEFER);			\
+}
+
 /*
  * Note: the caller should never hard code @issue_flags and only use the
  * mask provided by the core io_uring code.
  */
 void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
@@ -107,11 +118,11 @@ static inline int io_uring_cmd_import_fixed_vec(struct io_uring_cmd *ioucmd,
 static inline void __io_uring_cmd_done(struct io_uring_cmd *cmd, s32 ret,
 		u64 ret2, unsigned issue_flags, bool is_cqe32)
 {
 }
 static inline void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
-			    io_uring_cmd_tw_t task_work_cb, unsigned flags)
+			    io_req_tw_func_t task_work_cb, unsigned flags)
 {
 }
 static inline void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
 		unsigned int issue_flags)
 {
@@ -131,21 +142,19 @@ static inline bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
 	return true;
 }
 #endif
 
 /* users must follow the IOU_F_TWQ_LAZY_WAKE semantics */
-static inline void io_uring_cmd_do_in_task_lazy(struct io_uring_cmd *ioucmd,
-			io_uring_cmd_tw_t task_work_cb)
-{
-	__io_uring_cmd_do_in_task(ioucmd, task_work_cb, IOU_F_TWQ_LAZY_WAKE);
-}
-
-static inline void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
-			io_uring_cmd_tw_t task_work_cb)
-{
-	__io_uring_cmd_do_in_task(ioucmd, task_work_cb, 0);
-}
+#define io_uring_cmd_do_in_task_lazy(ioucmd, uring_cmd_cb)			\
+	__io_uring_cmd_do_in_task((ioucmd),					\
+				  IO_URING_CMD_TASK_WORK(uring_cmd_cb),		\
+				  IOU_F_TWQ_LAZY_WAKE)
+
+#define io_uring_cmd_complete_in_task(ioucmd, uring_cmd_cb)			\
+	__io_uring_cmd_do_in_task((ioucmd),					\
+				  IO_URING_CMD_TASK_WORK(uring_cmd_cb),		\
+				  0)
 
 static inline bool io_uring_cmd_should_terminate_tw(struct io_uring_cmd *cmd)
 {
 	return io_should_terminate_tw(cmd_to_io_kiocb(cmd)->ctx);
 }
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 35bdac35cf4d..5a80d35658dc 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -111,29 +111,20 @@ void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
 		io_ring_submit_unlock(ctx, issue_flags);
 	}
 }
 EXPORT_SYMBOL_GPL(io_uring_cmd_mark_cancelable);
 
-static void io_uring_cmd_work(struct io_kiocb *req, io_tw_token_t tw)
-{
-	struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
-
-	/* task_work executor checks the deffered list completion */
-	ioucmd->task_work_cb(ioucmd, IO_URING_F_COMPLETE_DEFER);
-}
-
 void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
-			io_uring_cmd_tw_t task_work_cb,
+			io_req_tw_func_t task_work_cb,
 			unsigned flags)
 {
 	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
 
 	if (WARN_ON_ONCE(req->flags & REQ_F_APOLL_MULTISHOT))
 		return;
 
-	ioucmd->task_work_cb = task_work_cb;
-	req->io_task_work.func = io_uring_cmd_work;
+	req->io_task_work.func = task_work_cb;
 	__io_req_task_work_add(req, flags);
 }
 EXPORT_SYMBOL_GPL(__io_uring_cmd_do_in_task);
 
 static inline void io_req_set_cqe32_extra(struct io_kiocb *req,
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch
  2025-10-22 23:13 ` [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
@ 2025-10-23 12:05   ` kernel test robot
  2025-10-23 13:40   ` Christoph Hellwig
  2025-10-23 19:49   ` kernel test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2025-10-23 12:05 UTC (permalink / raw)
  To: Caleb Sander Mateos, Jens Axboe, Miklos Szeredi, Ming Lei,
	Keith Busch, Christoph Hellwig, Sagi Grimberg, Chris Mason,
	David Sterba
  Cc: llvm, oe-kbuild-all, io-uring, linux-fsdevel, linux-block,
	linux-nvme, linux-btrfs, linux-kernel, Caleb Sander Mateos

Hi Caleb,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on kdave/for-next linus/master v6.18-rc2]
[cannot apply to mszeredi-fuse/for-next linux-nvme/for-next next-20251023]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Caleb-Sander-Mateos/io_uring-expose-io_should_terminate_tw/20251023-071617
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20251022231326.2527838-4-csander%40purestorage.com
patch subject: [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch
config: x86_64-buildonly-randconfig-005-20251023 (https://download.01.org/0day-ci/archive/20251023/202510231952.gAXMcT2A-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251023/202510231952.gAXMcT2A-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510231952.gAXMcT2A-lkp@intel.com/

All errors (new ones prefixed by >>):

>> block/ioctl.c:783:1: error: invalid storage class specifier in function declarator
     783 | static void bio_cmd_bio_end_io(struct bio *bio)
         | ^
>> block/ioctl.c:783:13: error: parameter named 'bio_cmd_bio_end_io' is missing
     783 | static void bio_cmd_bio_end_io(struct bio *bio)
         |             ^
>> block/ioctl.c:783:48: error: expected ';' at end of declaration
     783 | static void bio_cmd_bio_end_io(struct bio *bio)
         |                                                ^
         |                                                ;
>> block/ioctl.c:781:38: error: parameter 'blk_cmd_complete' was not declared, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
     781 | static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
         |                                      ^
     782 | 
     783 | static void bio_cmd_bio_end_io(struct bio *bio)
     784 | {
>> block/ioctl.c:781:8: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
     781 | static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
         | ~~~~~~ ^
         | int
>> block/ioctl.c:785:29: error: use of undeclared identifier 'bio'
     785 |         struct io_uring_cmd *cmd = bio->bi_private;
         |                                    ^
   block/ioctl.c:788:15: error: use of undeclared identifier 'bio'
     788 |         if (unlikely(bio->bi_status) && !bic->res)
         |                      ^
   block/ioctl.c:789:34: error: use of undeclared identifier 'bio'
     789 |                 bic->res = blk_status_to_errno(bio->bi_status);
         |                                                ^
>> block/ioctl.c:791:2: error: call to undeclared function 'IO_URING_CMD_TASK_WORK'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     791 |         io_uring_cmd_do_in_task_lazy(cmd, blk_cmd_complete);
         |         ^
   include/linux/io_uring/cmd.h:149:7: note: expanded from macro 'io_uring_cmd_do_in_task_lazy'
     149 |                                   IO_URING_CMD_TASK_WORK(uring_cmd_cb),         \
         |                                   ^
   block/ioctl.c:791:2: note: did you mean 'DEFINE_IO_URING_CMD_TASK_WORK'?
   include/linux/io_uring/cmd.h:149:7: note: expanded from macro 'io_uring_cmd_do_in_task_lazy'
     149 |                                   IO_URING_CMD_TASK_WORK(uring_cmd_cb),         \
         |                                   ^
   block/ioctl.c:781:8: note: 'DEFINE_IO_URING_CMD_TASK_WORK' declared here
     781 | static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
         |        ^
>> block/ioctl.c:791:2: error: incompatible integer to pointer conversion passing 'int' to parameter of type 'io_req_tw_func_t' (aka 'void (*)(struct io_kiocb *, struct io_tw_state)') [-Wint-conversion]
     791 |         io_uring_cmd_do_in_task_lazy(cmd, blk_cmd_complete);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/io_uring/cmd.h:149:7: note: expanded from macro 'io_uring_cmd_do_in_task_lazy'
     149 |                                   IO_URING_CMD_TASK_WORK(uring_cmd_cb),         \
         |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/io_uring/cmd.h:123:25: note: passing argument to parameter 'task_work_cb' here
     123 |                             io_req_tw_func_t task_work_cb, unsigned flags)
         |                                              ^
   block/ioctl.c:792:10: error: use of undeclared identifier 'bio'; did you mean 'bic'?
     792 |         bio_put(bio);
         |                 ^~~
         |                 bic
   block/ioctl.c:786:22: note: 'bic' declared here
     786 |         struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
         |                             ^
>> block/ioctl.c:781:8: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C23 [-Werror,-Wdeprecated-non-prototype]
     781 | static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
         |        ^
>> block/ioctl.c:847:20: error: use of undeclared identifier 'bio_cmd_bio_end_io'
     847 |         prev->bi_end_io = bio_cmd_bio_end_io;
         |                           ^
   13 errors generated.
--
>> drivers/nvme/host/ioctl.c:412:1: error: invalid storage class specifier in function declarator
     412 | static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
         | ^
>> drivers/nvme/host/ioctl.c:412:27: error: parameter named 'nvme_uring_cmd_end_io' is missing
     412 | static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
         |                           ^
>> drivers/nvme/host/ioctl.c:413:24: error: expected ';' at end of declaration
     413 |                                                 blk_status_t err)
         |                                                                  ^
         |                                                                  ;
>> drivers/nvme/host/ioctl.c:410:38: error: parameter 'nvme_uring_task_cb' was not declared, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
     410 | static DEFINE_IO_URING_CMD_TASK_WORK(nvme_uring_task_cb)
         |                                      ^
     411 | 
     412 | static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
     413 |                                                 blk_status_t err)
     414 | {
>> drivers/nvme/host/ioctl.c:410:8: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
     410 | static DEFINE_IO_URING_CMD_TASK_WORK(nvme_uring_task_cb)
         | ~~~~~~ ^
         | int
>> drivers/nvme/host/ioctl.c:415:32: error: use of undeclared identifier 'req'
     415 |         struct io_uring_cmd *ioucmd = req->end_io_data;
         |                                       ^
   drivers/nvme/host/ioctl.c:418:15: error: use of undeclared identifier 'req'
     418 |         if (nvme_req(req)->flags & NVME_REQ_CANCELLED) {
         |                      ^
   drivers/nvme/host/ioctl.c:421:26: error: use of undeclared identifier 'req'
     421 |                 pdu->status = nvme_req(req)->status;
         |                                        ^
>> drivers/nvme/host/ioctl.c:423:38: error: use of undeclared identifier 'err'
     423 |                         pdu->status = blk_status_to_errno(err);
         |                                                           ^
   drivers/nvme/host/ioctl.c:425:37: error: use of undeclared identifier 'req'
     425 |         pdu->result = le64_to_cpu(nvme_req(req)->result.u64);
         |                                            ^
   include/linux/byteorder/generic.h:87:21: note: expanded from macro 'le64_to_cpu'
      87 | #define le64_to_cpu __le64_to_cpu
         |                     ^
>> drivers/nvme/host/ioctl.c:435:2: error: call to undeclared function 'IO_URING_CMD_TASK_WORK'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     435 |         io_uring_cmd_do_in_task_lazy(ioucmd, nvme_uring_task_cb);
         |         ^
   include/linux/io_uring/cmd.h:149:7: note: expanded from macro 'io_uring_cmd_do_in_task_lazy'
     149 |                                   IO_URING_CMD_TASK_WORK(uring_cmd_cb),         \
         |                                   ^
   drivers/nvme/host/ioctl.c:435:2: note: did you mean 'DEFINE_IO_URING_CMD_TASK_WORK'?
   include/linux/io_uring/cmd.h:149:7: note: expanded from macro 'io_uring_cmd_do_in_task_lazy'
     149 |                                   IO_URING_CMD_TASK_WORK(uring_cmd_cb),         \
         |                                   ^
   drivers/nvme/host/ioctl.c:410:8: note: 'DEFINE_IO_URING_CMD_TASK_WORK' declared here
     410 | static DEFINE_IO_URING_CMD_TASK_WORK(nvme_uring_task_cb)
         |        ^
>> drivers/nvme/host/ioctl.c:435:2: error: incompatible integer to pointer conversion passing 'int' to parameter of type 'io_req_tw_func_t' (aka 'void (*)(struct io_kiocb *, struct io_tw_state)') [-Wint-conversion]
     435 |         io_uring_cmd_do_in_task_lazy(ioucmd, nvme_uring_task_cb);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/io_uring/cmd.h:149:7: note: expanded from macro 'io_uring_cmd_do_in_task_lazy'
     149 |                                   IO_URING_CMD_TASK_WORK(uring_cmd_cb),         \
         |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/io_uring/cmd.h:123:25: note: passing argument to parameter 'task_work_cb' here
     123 |                             io_req_tw_func_t task_work_cb, unsigned flags)
         |                                              ^
>> drivers/nvme/host/ioctl.c:410:8: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C23 [-Werror,-Wdeprecated-non-prototype]
     410 | static DEFINE_IO_URING_CMD_TASK_WORK(nvme_uring_task_cb)
         |        ^
>> drivers/nvme/host/ioctl.c:524:16: error: use of undeclared identifier 'nvme_uring_cmd_end_io'; did you mean 'nvme_uring_cmd_io'?
     524 |         req->end_io = nvme_uring_cmd_end_io;
         |                       ^~~~~~~~~~~~~~~~~~~~~
         |                       nvme_uring_cmd_io
   drivers/nvme/host/ioctl.c:439:12: note: 'nvme_uring_cmd_io' declared here
     439 | static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
         |            ^
   14 errors generated.


vim +783 block/ioctl.c

50c52250e2d74b Pavel Begunkov      2024-09-11  771  
50c52250e2d74b Pavel Begunkov      2024-09-11  772  static void blk_cmd_complete(struct io_uring_cmd *cmd, unsigned int issue_flags)
50c52250e2d74b Pavel Begunkov      2024-09-11  773  {
50c52250e2d74b Pavel Begunkov      2024-09-11  774  	struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
50c52250e2d74b Pavel Begunkov      2024-09-11  775  
50c52250e2d74b Pavel Begunkov      2024-09-11  776  	if (bic->res == -EAGAIN && bic->nowait)
50c52250e2d74b Pavel Begunkov      2024-09-11  777  		io_uring_cmd_issue_blocking(cmd);
50c52250e2d74b Pavel Begunkov      2024-09-11  778  	else
ef9f603fd3d4b7 Caleb Sander Mateos 2025-09-22  779  		io_uring_cmd_done(cmd, bic->res, issue_flags);
50c52250e2d74b Pavel Begunkov      2024-09-11  780  }
c004e50b1d8661 Caleb Sander Mateos 2025-10-22 @781  static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
50c52250e2d74b Pavel Begunkov      2024-09-11  782  
50c52250e2d74b Pavel Begunkov      2024-09-11 @783  static void bio_cmd_bio_end_io(struct bio *bio)
50c52250e2d74b Pavel Begunkov      2024-09-11  784  {
50c52250e2d74b Pavel Begunkov      2024-09-11 @785  	struct io_uring_cmd *cmd = bio->bi_private;
50c52250e2d74b Pavel Begunkov      2024-09-11  786  	struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
50c52250e2d74b Pavel Begunkov      2024-09-11  787  
50c52250e2d74b Pavel Begunkov      2024-09-11  788  	if (unlikely(bio->bi_status) && !bic->res)
50c52250e2d74b Pavel Begunkov      2024-09-11  789  		bic->res = blk_status_to_errno(bio->bi_status);
50c52250e2d74b Pavel Begunkov      2024-09-11  790  
50c52250e2d74b Pavel Begunkov      2024-09-11 @791  	io_uring_cmd_do_in_task_lazy(cmd, blk_cmd_complete);
50c52250e2d74b Pavel Begunkov      2024-09-11  792  	bio_put(bio);
50c52250e2d74b Pavel Begunkov      2024-09-11  793  }
50c52250e2d74b Pavel Begunkov      2024-09-11  794  
50c52250e2d74b Pavel Begunkov      2024-09-11  795  static int blkdev_cmd_discard(struct io_uring_cmd *cmd,
50c52250e2d74b Pavel Begunkov      2024-09-11  796  			      struct block_device *bdev,
50c52250e2d74b Pavel Begunkov      2024-09-11  797  			      uint64_t start, uint64_t len, bool nowait)
50c52250e2d74b Pavel Begunkov      2024-09-11  798  {
50c52250e2d74b Pavel Begunkov      2024-09-11  799  	struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
50c52250e2d74b Pavel Begunkov      2024-09-11  800  	gfp_t gfp = nowait ? GFP_NOWAIT : GFP_KERNEL;
50c52250e2d74b Pavel Begunkov      2024-09-11  801  	sector_t sector = start >> SECTOR_SHIFT;
50c52250e2d74b Pavel Begunkov      2024-09-11  802  	sector_t nr_sects = len >> SECTOR_SHIFT;
50c52250e2d74b Pavel Begunkov      2024-09-11  803  	struct bio *prev = NULL, *bio;
50c52250e2d74b Pavel Begunkov      2024-09-11  804  	int err;
50c52250e2d74b Pavel Begunkov      2024-09-11  805  
50c52250e2d74b Pavel Begunkov      2024-09-11  806  	if (!bdev_max_discard_sectors(bdev))
50c52250e2d74b Pavel Begunkov      2024-09-11  807  		return -EOPNOTSUPP;
50c52250e2d74b Pavel Begunkov      2024-09-11  808  	if (!(file_to_blk_mode(cmd->file) & BLK_OPEN_WRITE))
50c52250e2d74b Pavel Begunkov      2024-09-11  809  		return -EBADF;
50c52250e2d74b Pavel Begunkov      2024-09-11  810  	if (bdev_read_only(bdev))
50c52250e2d74b Pavel Begunkov      2024-09-11  811  		return -EPERM;
50c52250e2d74b Pavel Begunkov      2024-09-11  812  	err = blk_validate_byte_range(bdev, start, len);
50c52250e2d74b Pavel Begunkov      2024-09-11  813  	if (err)
50c52250e2d74b Pavel Begunkov      2024-09-11  814  		return err;
50c52250e2d74b Pavel Begunkov      2024-09-11  815  
50c52250e2d74b Pavel Begunkov      2024-09-11  816  	err = filemap_invalidate_pages(bdev->bd_mapping, start,
50c52250e2d74b Pavel Begunkov      2024-09-11  817  					start + len - 1, nowait);
50c52250e2d74b Pavel Begunkov      2024-09-11  818  	if (err)
50c52250e2d74b Pavel Begunkov      2024-09-11  819  		return err;
50c52250e2d74b Pavel Begunkov      2024-09-11  820  
50c52250e2d74b Pavel Begunkov      2024-09-11  821  	while (true) {
50c52250e2d74b Pavel Begunkov      2024-09-11  822  		bio = blk_alloc_discard_bio(bdev, &sector, &nr_sects, gfp);
50c52250e2d74b Pavel Begunkov      2024-09-11  823  		if (!bio)
50c52250e2d74b Pavel Begunkov      2024-09-11  824  			break;
50c52250e2d74b Pavel Begunkov      2024-09-11  825  		if (nowait) {
50c52250e2d74b Pavel Begunkov      2024-09-11  826  			/*
50c52250e2d74b Pavel Begunkov      2024-09-11  827  			 * Don't allow multi-bio non-blocking submissions as
50c52250e2d74b Pavel Begunkov      2024-09-11  828  			 * subsequent bios may fail but we won't get a direct
50c52250e2d74b Pavel Begunkov      2024-09-11  829  			 * indication of that. Normally, the caller should
50c52250e2d74b Pavel Begunkov      2024-09-11  830  			 * retry from a blocking context.
50c52250e2d74b Pavel Begunkov      2024-09-11  831  			 */
50c52250e2d74b Pavel Begunkov      2024-09-11  832  			if (unlikely(nr_sects)) {
50c52250e2d74b Pavel Begunkov      2024-09-11  833  				bio_put(bio);
50c52250e2d74b Pavel Begunkov      2024-09-11  834  				return -EAGAIN;
50c52250e2d74b Pavel Begunkov      2024-09-11  835  			}
50c52250e2d74b Pavel Begunkov      2024-09-11  836  			bio->bi_opf |= REQ_NOWAIT;
50c52250e2d74b Pavel Begunkov      2024-09-11  837  		}
50c52250e2d74b Pavel Begunkov      2024-09-11  838  
50c52250e2d74b Pavel Begunkov      2024-09-11  839  		prev = bio_chain_and_submit(prev, bio);
50c52250e2d74b Pavel Begunkov      2024-09-11  840  	}
50c52250e2d74b Pavel Begunkov      2024-09-11  841  	if (unlikely(!prev))
50c52250e2d74b Pavel Begunkov      2024-09-11  842  		return -EAGAIN;
50c52250e2d74b Pavel Begunkov      2024-09-11  843  	if (unlikely(nr_sects))
50c52250e2d74b Pavel Begunkov      2024-09-11  844  		bic->res = -EAGAIN;
50c52250e2d74b Pavel Begunkov      2024-09-11  845  
50c52250e2d74b Pavel Begunkov      2024-09-11  846  	prev->bi_private = cmd;
50c52250e2d74b Pavel Begunkov      2024-09-11 @847  	prev->bi_end_io = bio_cmd_bio_end_io;
50c52250e2d74b Pavel Begunkov      2024-09-11  848  	submit_bio(prev);
50c52250e2d74b Pavel Begunkov      2024-09-11  849  	return -EIOCBQUEUED;
50c52250e2d74b Pavel Begunkov      2024-09-11  850  }
50c52250e2d74b Pavel Begunkov      2024-09-11  851  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch
  2025-10-22 23:13 ` [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
  2025-10-23 12:05   ` kernel test robot
@ 2025-10-23 13:40   ` Christoph Hellwig
  2025-10-23 19:12     ` Caleb Sander Mateos
  2025-10-23 19:49   ` kernel test robot
  2 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2025-10-23 13:40 UTC (permalink / raw)
  To: Caleb Sander Mateos
  Cc: Jens Axboe, Miklos Szeredi, Ming Lei, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, Chris Mason, David Sterba,
	io-uring, linux-fsdevel, linux-block, linux-nvme, linux-btrfs,
	linux-kernel

On Wed, Oct 22, 2025 at 05:13:26PM -0600, Caleb Sander Mateos wrote:
> io_uring task work dispatch makes an indirect call to struct io_kiocb's
> io_task_work.func field to allow running arbitrary task work functions.
> In the uring_cmd case, this calls io_uring_cmd_work(), which immediately
> makes another indirect call to struct io_uring_cmd's task_work_cb field.
> Introduce a macro DEFINE_IO_URING_CMD_TASK_WORK() to define a
> io_req_tw_func_t function wrapping an io_uring_cmd_tw_t. Convert the
> io_uring_cmd_tw_t function to the io_req_tw_func_t function in
> io_uring_cmd_complete_in_task() and io_uring_cmd_do_in_task_lazy().
> Use DEFINE_IO_URING_CMD_TASK_WORK() to define a io_req_tw_func_t
> function for each existing io_uring_cmd_tw_t function. Now uring_cmd
> task work dispatch makes a single indirect call to the io_req_tw_func_t
> wrapper function, which can inline the io_uring_cmd_tw_t function. This
> also allows removing the task_work_cb field from struct io_uring_cmd,
> freeing up some additional storage space.

Please just open code the logic instead of the symbol-hiding multi-level
macro indirection.  Everyone who will have to touch the code in the
future will thank you.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch
  2025-10-23 13:40   ` Christoph Hellwig
@ 2025-10-23 19:12     ` Caleb Sander Mateos
  0 siblings, 0 replies; 8+ messages in thread
From: Caleb Sander Mateos @ 2025-10-23 19:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Miklos Szeredi, Ming Lei, Keith Busch, Sagi Grimberg,
	Chris Mason, David Sterba, io-uring, linux-fsdevel, linux-block,
	linux-nvme, linux-btrfs, linux-kernel

On Thu, Oct 23, 2025 at 6:40 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Oct 22, 2025 at 05:13:26PM -0600, Caleb Sander Mateos wrote:
> > io_uring task work dispatch makes an indirect call to struct io_kiocb's
> > io_task_work.func field to allow running arbitrary task work functions.
> > In the uring_cmd case, this calls io_uring_cmd_work(), which immediately
> > makes another indirect call to struct io_uring_cmd's task_work_cb field.
> > Introduce a macro DEFINE_IO_URING_CMD_TASK_WORK() to define a
> > io_req_tw_func_t function wrapping an io_uring_cmd_tw_t. Convert the
> > io_uring_cmd_tw_t function to the io_req_tw_func_t function in
> > io_uring_cmd_complete_in_task() and io_uring_cmd_do_in_task_lazy().
> > Use DEFINE_IO_URING_CMD_TASK_WORK() to define a io_req_tw_func_t
> > function for each existing io_uring_cmd_tw_t function. Now uring_cmd
> > task work dispatch makes a single indirect call to the io_req_tw_func_t
> > wrapper function, which can inline the io_uring_cmd_tw_t function. This
> > also allows removing the task_work_cb field from struct io_uring_cmd,
> > freeing up some additional storage space.
>
> Please just open code the logic instead of the symbol-hiding multi-level
> macro indirection.  Everyone who will have to touch the code in the
> future will thank you.

Sure, I can send out a v2 with that. My concern was that
io_kiocb_to_cmd() isn't really meant to be used outside the io_uring
internals. But I agree hiding the function definition in a macro is
less than ideal.

Thanks,
Caleb

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch
  2025-10-22 23:13 ` [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
  2025-10-23 12:05   ` kernel test robot
  2025-10-23 13:40   ` Christoph Hellwig
@ 2025-10-23 19:49   ` kernel test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2025-10-23 19:49 UTC (permalink / raw)
  To: Caleb Sander Mateos, Jens Axboe, Miklos Szeredi, Ming Lei,
	Keith Busch, Christoph Hellwig, Sagi Grimberg, Chris Mason,
	David Sterba
  Cc: oe-kbuild-all, io-uring, linux-fsdevel, linux-block, linux-nvme,
	linux-btrfs, linux-kernel, Caleb Sander Mateos

Hi Caleb,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on kdave/for-next linus/master v6.18-rc2]
[cannot apply to mszeredi-fuse/for-next linux-nvme/for-next next-20251023]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Caleb-Sander-Mateos/io_uring-expose-io_should_terminate_tw/20251023-071617
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20251022231326.2527838-4-csander%40purestorage.com
patch subject: [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch
config: arm-randconfig-002-20251024 (https://download.01.org/0day-ci/archive/20251024/202510240319.bLypyxx1-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251024/202510240319.bLypyxx1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510240319.bLypyxx1-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> block/ioctl.c:781:8: error: return type defaults to 'int' [-Wimplicit-int]
     781 | static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> block/ioctl.c:781:8: error: function declaration isn't a prototype [-Werror=strict-prototypes]
   block/ioctl.c: In function 'DEFINE_IO_URING_CMD_TASK_WORK':
>> block/ioctl.c:784:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     784 | {
         | ^
   block/ioctl.c:798:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     798 | {
         | ^
   block/ioctl.c:853:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     853 | {
         | ^
>> block/ioctl.c:781:8: error: type of 'blk_cmd_complete' defaults to 'int' [-Wimplicit-int]
     781 | static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> block/ioctl.c:876: error: expected '{' at end of input
   block/ioctl.c: At top level:
>> block/ioctl.c:781:8: warning: 'DEFINE_IO_URING_CMD_TASK_WORK' defined but not used [-Wunused-function]
     781 | static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> block/ioctl.c:772:13: warning: 'blk_cmd_complete' defined but not used [-Wunused-function]
     772 | static void blk_cmd_complete(struct io_uring_cmd *cmd, unsigned int issue_flags)
         |             ^~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
>> drivers/nvme/host/ioctl.c:410:8: error: return type defaults to 'int' [-Wimplicit-int]
     410 | static DEFINE_IO_URING_CMD_TASK_WORK(nvme_uring_task_cb)
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/nvme/host/ioctl.c:410:8: error: function declaration isn't a prototype [-Werror=strict-prototypes]
   drivers/nvme/host/ioctl.c: In function 'DEFINE_IO_URING_CMD_TASK_WORK':
>> drivers/nvme/host/ioctl.c:414:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     414 | {
         | ^
   drivers/nvme/host/ioctl.c:441:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     441 | {
         | ^
   drivers/nvme/host/ioctl.c:534:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     534 | {
         | ^
   drivers/nvme/host/ioctl.c:544:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     544 | {
         | ^
   drivers/nvme/host/ioctl.c:575:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     575 | {
         | ^
   drivers/nvme/host/ioctl.c:605:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     605 | {
         | ^
   drivers/nvme/host/ioctl.c:620:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     620 | {
         | ^
   drivers/nvme/host/ioctl.c:632:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     632 | {
         | ^
   drivers/nvme/host/ioctl.c:643:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     643 | {
         | ^
   drivers/nvme/host/ioctl.c:666:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     666 | {
         | ^
   drivers/nvme/host/ioctl.c:676:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     676 | {
         | ^
   drivers/nvme/host/ioctl.c:777:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     777 | {
         | ^
   drivers/nvme/host/ioctl.c:805:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     805 | {
         | ^
   drivers/nvme/host/ioctl.c:842:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     842 | {
         | ^
>> drivers/nvme/host/ioctl.c:410:8: error: type of 'nvme_uring_task_cb' defaults to 'int' [-Wimplicit-int]
     410 | static DEFINE_IO_URING_CMD_TASK_WORK(nvme_uring_task_cb)
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/nvme/host/ioctl.c:872: error: expected '{' at end of input
   drivers/nvme/host/ioctl.c: At top level:
>> drivers/nvme/host/ioctl.c:410:8: warning: 'DEFINE_IO_URING_CMD_TASK_WORK' defined but not used [-Wunused-function]
     410 | static DEFINE_IO_URING_CMD_TASK_WORK(nvme_uring_task_cb)
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/nvme/host/ioctl.c:401:13: warning: 'nvme_uring_task_cb' defined but not used [-Wunused-function]
     401 | static void nvme_uring_task_cb(struct io_uring_cmd *ioucmd,
         |             ^~~~~~~~~~~~~~~~~~
>> drivers/nvme/host/ioctl.c:329:12: warning: 'nvme_user_cmd64' defined but not used [-Wunused-function]
     329 | static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
         |            ^~~~~~~~~~~~~~~
>> drivers/nvme/host/ioctl.c:281:12: warning: 'nvme_user_cmd' defined but not used [-Wunused-function]
     281 | static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
         |            ^~~~~~~~~~~~~
>> drivers/nvme/host/ioctl.c:206:12: warning: 'nvme_submit_io' defined but not used [-Wunused-function]
     206 | static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
         |            ^~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/int +781 block/ioctl.c

   771	
 > 772	static void blk_cmd_complete(struct io_uring_cmd *cmd, unsigned int issue_flags)
   773	{
   774		struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
   775	
   776		if (bic->res == -EAGAIN && bic->nowait)
   777			io_uring_cmd_issue_blocking(cmd);
   778		else
   779			io_uring_cmd_done(cmd, bic->res, issue_flags);
   780	}
 > 781	static DEFINE_IO_URING_CMD_TASK_WORK(blk_cmd_complete)
   782	
   783	static void bio_cmd_bio_end_io(struct bio *bio)
 > 784	{
   785		struct io_uring_cmd *cmd = bio->bi_private;
   786		struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
   787	
   788		if (unlikely(bio->bi_status) && !bic->res)
   789			bic->res = blk_status_to_errno(bio->bi_status);
   790	
   791		io_uring_cmd_do_in_task_lazy(cmd, blk_cmd_complete);
   792		bio_put(bio);
   793	}
   794	
   795	static int blkdev_cmd_discard(struct io_uring_cmd *cmd,
   796				      struct block_device *bdev,
   797				      uint64_t start, uint64_t len, bool nowait)
 > 798	{
   799		struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
   800		gfp_t gfp = nowait ? GFP_NOWAIT : GFP_KERNEL;
   801		sector_t sector = start >> SECTOR_SHIFT;
   802		sector_t nr_sects = len >> SECTOR_SHIFT;
   803		struct bio *prev = NULL, *bio;
   804		int err;
   805	
   806		if (!bdev_max_discard_sectors(bdev))
   807			return -EOPNOTSUPP;
   808		if (!(file_to_blk_mode(cmd->file) & BLK_OPEN_WRITE))
   809			return -EBADF;
   810		if (bdev_read_only(bdev))
   811			return -EPERM;
   812		err = blk_validate_byte_range(bdev, start, len);
   813		if (err)
   814			return err;
   815	
   816		err = filemap_invalidate_pages(bdev->bd_mapping, start,
   817						start + len - 1, nowait);
   818		if (err)
   819			return err;
   820	
   821		while (true) {
   822			bio = blk_alloc_discard_bio(bdev, &sector, &nr_sects, gfp);
   823			if (!bio)
   824				break;
   825			if (nowait) {
   826				/*
   827				 * Don't allow multi-bio non-blocking submissions as
   828				 * subsequent bios may fail but we won't get a direct
   829				 * indication of that. Normally, the caller should
   830				 * retry from a blocking context.
   831				 */
   832				if (unlikely(nr_sects)) {
   833					bio_put(bio);
   834					return -EAGAIN;
   835				}
   836				bio->bi_opf |= REQ_NOWAIT;
   837			}
   838	
   839			prev = bio_chain_and_submit(prev, bio);
   840		}
   841		if (unlikely(!prev))
   842			return -EAGAIN;
   843		if (unlikely(nr_sects))
   844			bic->res = -EAGAIN;
   845	
   846		prev->bi_private = cmd;
   847		prev->bi_end_io = bio_cmd_bio_end_io;
   848		submit_bio(prev);
   849		return -EIOCBQUEUED;
   850	}
   851	
   852	int blkdev_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 > 853	{
   854		struct block_device *bdev = I_BDEV(cmd->file->f_mapping->host);
   855		struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
   856		const struct io_uring_sqe *sqe = cmd->sqe;
   857		u32 cmd_op = cmd->cmd_op;
   858		uint64_t start, len;
   859	
   860		if (unlikely(sqe->ioprio || sqe->__pad1 || sqe->len ||
   861			     sqe->rw_flags || sqe->file_index))
   862			return -EINVAL;
   863	
   864		bic->res = 0;
   865		bic->nowait = issue_flags & IO_URING_F_NONBLOCK;
   866	
   867		start = READ_ONCE(sqe->addr);
   868		len = READ_ONCE(sqe->addr3);
   869	
   870		switch (cmd_op) {
   871		case BLOCK_URING_CMD_DISCARD:
   872			return blkdev_cmd_discard(cmd, bdev, start, len, bic->nowait);
   873		}
   874		return -EINVAL;
   875	}

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-10-23 19:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-22 23:13 [PATCH 0/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
2025-10-22 23:13 ` [PATCH 1/3] io_uring: expose io_should_terminate_tw() Caleb Sander Mateos
2025-10-22 23:13 ` [PATCH 2/3] io_uring/uring_cmd: call io_should_terminate_tw() when needed Caleb Sander Mateos
2025-10-22 23:13 ` [PATCH 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch Caleb Sander Mateos
2025-10-23 12:05   ` kernel test robot
2025-10-23 13:40   ` Christoph Hellwig
2025-10-23 19:12     ` Caleb Sander Mateos
2025-10-23 19:49   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox