linux-kernel - Re: [PATCH v2 3/3] io_uring/uring_cmd: avoid double indirect call in task work dispatch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aPr1i-k0byzYjv8G@fedora>
Date: Fri, 24 Oct 2025 11:42:03 +0800
From: Ming Lei <ming.lei@...hat.com>
To: Caleb Sander Mateos <csander@...estorage.com>
Cc: Jens Axboe <axboe@...nel.dk>, Miklos Szeredi <miklos@...redi.hu>,
	Keith Busch <kbusch@...nel.org>, Christoph Hellwig <hch@....de>,
	Sagi Grimberg <sagi@...mberg.me>, Chris Mason <clm@...com>,
	David Sterba <dsterba@...e.com>, io-uring@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-block@...r.kernel.org,
	linux-nvme@...ts.infradead.org, linux-btrfs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] io_uring/uring_cmd: avoid double indirect call in
 task work dispatch

On Thu, Oct 23, 2025 at 02:18:30PM -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.
> Define the uring_cmd task work callbacks as functions whose signatures
> match io_req_tw_func_t. Define a IO_URING_CMD_TASK_WORK_ISSUE_FLAGS
> constant in io_uring/cmd.h to avoid manufacturing issue_flags in the
> uring_cmd task work callbacks. Now uring_cmd task work dispatch makes a
> single indirect call to the uring_cmd implementation's callback. This
> also allows removing the task_work_cb field from struct io_uring_cmd,
> freeing up some additional storage space.

The idea looks good.

> 
> Signed-off-by: Caleb Sander Mateos <csander@...estorage.com>
> ---
>  block/ioctl.c                |  4 +++-
>  drivers/block/ublk_drv.c     | 15 +++++++++------
>  drivers/nvme/host/ioctl.c    |  5 +++--
>  fs/btrfs/ioctl.c             |  4 +++-
>  fs/fuse/dev_uring.c          |  5 +++--
>  include/linux/io_uring/cmd.h | 16 +++++++---------
>  io_uring/uring_cmd.c         | 13 ++-----------
>  7 files changed, 30 insertions(+), 32 deletions(-)
> 
> diff --git a/block/ioctl.c b/block/ioctl.c
> index d7489a56b33c..5c10d48fab27 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -767,13 +767,15 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>  struct blk_iou_cmd {
>  	int res;
>  	bool nowait;
>  };
>  
> -static void blk_cmd_complete(struct io_uring_cmd *cmd, unsigned int issue_flags)
> +static void blk_cmd_complete(struct io_kiocb *req, io_tw_token_t tw)
>  {
> +	struct io_uring_cmd *cmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
>  	struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
> +	unsigned int issue_flags = IO_URING_CMD_TASK_WORK_ISSUE_FLAGS;

Now `io_kiocb` is exposed to driver, it could be perfect if 'io_uring_cmd'
is kept in kernel API interface, IMO.

...

> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> index b84b97c21b43..3efad93404f9 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 */

pdu[40]



Thanks,
Ming