[PATCH] block: only read from sqe on initial invocation of blkdev_uring_cmd()

[PATCH] block: only read from sqe on initial invocation of blkdev_uring_cmd()

[Jens Axboe]

This passthrough helper currently only supports discards. Part of that
command is the start and length, which is read from the SQE. It does
so on every invocation, where it really should just make it stable
on the first invocation. This avoids needing to copy the SQE upfront,
as we only really need those two 8b values stored in our per-req
payload.

Cc: stable@vger.kernel.org # 6.17+
Signed-off-by: Jens Axboe <axboe@kernel.dk>

---

diff --git a/block/ioctl.c b/block/ioctl.c
index fc3be0549aa7..6aa02575e08d 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -857,6 +857,8 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 #endif
 
 struct blk_iou_cmd {
+	u64 start;
+	u64 len;
 	int res;
 	bool nowait;
 };
@@ -946,23 +948,27 @@ int blkdev_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
 	struct block_device *bdev = I_BDEV(cmd->file->f_mapping->host);
 	struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
-	const struct io_uring_sqe *sqe = cmd->sqe;
 	u32 cmd_op = cmd->cmd_op;
-	uint64_t start, len;
 
-	if (unlikely(sqe->ioprio || sqe->__pad1 || sqe->len ||
-		     sqe->rw_flags || sqe->file_index))
-		return -EINVAL;
+	/* Read what we need from the SQE on the first issue */
+	if (issue_flags & IO_URING_F_INLINE) {
+		const struct io_uring_sqe *sqe = cmd->sqe;
+
+		if (unlikely(sqe->ioprio || sqe->__pad1 || sqe->len ||
+			     sqe->rw_flags || sqe->file_index))
+			return -EINVAL;
+
+		bic->start = READ_ONCE(sqe->addr);
+		bic->len = READ_ONCE(sqe->addr3);
+	}
 
 	bic->res = 0;
 	bic->nowait = issue_flags & IO_URING_F_NONBLOCK;
 
-	start = READ_ONCE(sqe->addr);
-	len = READ_ONCE(sqe->addr3);
-
 	switch (cmd_op) {
 	case BLOCK_URING_CMD_DISCARD:
-		return blkdev_cmd_discard(cmd, bdev, start, len, bic->nowait);
+		return blkdev_cmd_discard(cmd, bdev, bic->start, bic->len,
+					  bic->nowait);
 	}
 	return -EINVAL;
 }

-- 
Jens Axboe

Re: [PATCH] block: only read from sqe on initial invocation of blkdev_uring_cmd()

[Caleb Sander Mateos]

On Mon, May 4, 2026 at 7:39 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> This passthrough helper currently only supports discards. Part of that
> command is the start and length, which is read from the SQE. It does
> so on every invocation, where it really should just make it stable
> on the first invocation. This avoids needing to copy the SQE upfront,
> as we only really need those two 8b values stored in our per-req
> payload.
>
> Cc: stable@vger.kernel.org # 6.17+
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>
> ---
>
> diff --git a/block/ioctl.c b/block/ioctl.c
> index fc3be0549aa7..6aa02575e08d 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -857,6 +857,8 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>  #endif
>
>  struct blk_iou_cmd {
> +       u64 start;
> +       u64 len;
>         int res;
>         bool nowait;
>  };
> @@ -946,23 +948,27 @@ int blkdev_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
>  {
>         struct block_device *bdev = I_BDEV(cmd->file->f_mapping->host);
>         struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
> -       const struct io_uring_sqe *sqe = cmd->sqe;
>         u32 cmd_op = cmd->cmd_op;
> -       uint64_t start, len;
>
> -       if (unlikely(sqe->ioprio || sqe->__pad1 || sqe->len ||
> -                    sqe->rw_flags || sqe->file_index))
> -               return -EINVAL;
> +       /* Read what we need from the SQE on the first issue */
> +       if (issue_flags & IO_URING_F_INLINE) {

I don't think IO_URING_F_INLINE is guaranteed to be set on the first
issue? For example, a subsequent command in a linked chain, or one
that sets REQ_F_FORCE_ASYNC or REQ_F_IO_DRAIN will be issued
asynchronously originally. Could you check !(issue_flags &
IORING_URING_CMD_REISSUE) instead?

Best,
Caleb

> +               const struct io_uring_sqe *sqe = cmd->sqe;
> +
> +               if (unlikely(sqe->ioprio || sqe->__pad1 || sqe->len ||
> +                            sqe->rw_flags || sqe->file_index))
> +                       return -EINVAL;
> +
> +               bic->start = READ_ONCE(sqe->addr);
> +               bic->len = READ_ONCE(sqe->addr3);
> +       }
>
>         bic->res = 0;
>         bic->nowait = issue_flags & IO_URING_F_NONBLOCK;
>
> -       start = READ_ONCE(sqe->addr);
> -       len = READ_ONCE(sqe->addr3);
> -
>         switch (cmd_op) {
>         case BLOCK_URING_CMD_DISCARD:
> -               return blkdev_cmd_discard(cmd, bdev, start, len, bic->nowait);
> +               return blkdev_cmd_discard(cmd, bdev, bic->start, bic->len,
> +                                         bic->nowait);
>         }
>         return -EINVAL;
>  }
>
> --
> Jens Axboe
>
>

Re: [PATCH] block: only read from sqe on initial invocation of blkdev_uring_cmd()

[Jens Axboe]

On 5/4/26 11:40 AM, Caleb Sander Mateos wrote:
> On Mon, May 4, 2026 at 7:39 AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> This passthrough helper currently only supports discards. Part of that
>> command is the start and length, which is read from the SQE. It does
>> so on every invocation, where it really should just make it stable
>> on the first invocation. This avoids needing to copy the SQE upfront,
>> as we only really need those two 8b values stored in our per-req
>> payload.
>>
>> Cc: stable@vger.kernel.org # 6.17+
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> ---
>>
>> diff --git a/block/ioctl.c b/block/ioctl.c
>> index fc3be0549aa7..6aa02575e08d 100644
>> --- a/block/ioctl.c
>> +++ b/block/ioctl.c
>> @@ -857,6 +857,8 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>>  #endif
>>
>>  struct blk_iou_cmd {
>> +       u64 start;
>> +       u64 len;
>>         int res;
>>         bool nowait;
>>  };
>> @@ -946,23 +948,27 @@ int blkdev_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
>>  {
>>         struct block_device *bdev = I_BDEV(cmd->file->f_mapping->host);
>>         struct blk_iou_cmd *bic = io_uring_cmd_to_pdu(cmd, struct blk_iou_cmd);
>> -       const struct io_uring_sqe *sqe = cmd->sqe;
>>         u32 cmd_op = cmd->cmd_op;
>> -       uint64_t start, len;
>>
>> -       if (unlikely(sqe->ioprio || sqe->__pad1 || sqe->len ||
>> -                    sqe->rw_flags || sqe->file_index))
>> -               return -EINVAL;
>> +       /* Read what we need from the SQE on the first issue */
>> +       if (issue_flags & IO_URING_F_INLINE) {
> 
> I don't think IO_URING_F_INLINE is guaranteed to be set on the first
> issue? For example, a subsequent command in a linked chain, or one
> that sets REQ_F_FORCE_ASYNC or REQ_F_IO_DRAIN will be issued
> asynchronously originally. Could you check !(issue_flags &
> IORING_URING_CMD_REISSUE) instead?

Good point, that is more self-explanatory too (and correct). I'll
make the change.

-- 
Jens Axboe