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

[Jens Axboe <axboe@kernel.dk>]

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