Re: [PATCH V5 2/2] io_uring: uring_cmd: add multishot support

[Jens Axboe <axboe@kernel.dk>]

On 8/21/25 10:23 AM, Caleb Sander Mateos wrote:
>> @@ -333,3 +351,54 @@ bool io_uring_cmd_post_mshot_cqe32(struct io_uring_cmd *cmd,
>>                 return false;
>>         return io_req_post_cqe32(req, cqe);
>>  }
>> +
>> +/*
>> + * Work with io_uring_mshot_cmd_post_cqe() together for committing the
>> + * provided buffer upfront
>> + */
>> +struct io_br_sel io_uring_cmd_buffer_select(struct io_uring_cmd *ioucmd,
>> +                                           unsigned buf_group, size_t *len,
>> +                                           unsigned int issue_flags)
>> +{
>> +       struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
>> +
>> +       if (!(ioucmd->flags & IORING_URING_CMD_MULTISHOT))
>> +               return (struct io_br_sel) { .val = -EINVAL };
> 
> Would this condition make more sense as a WARN_ON()? When would this
> be called for a non-IORING_URING_CMD_MULTISHOT io_uring_cmd?
>
>> +
>> +       if (WARN_ON_ONCE(!io_do_buffer_select(req)))
>> +               return (struct io_br_sel) { .val = -EINVAL };
>> +
>> +       return io_buffer_select(req, len, buf_group, issue_flags);
>> +}
>> +EXPORT_SYMBOL_GPL(io_uring_cmd_buffer_select);
>> +
>> +/*
>> + * Return true if this multishot uring_cmd needs to be completed, otherwise
>> + * the event CQE is posted successfully.
>> + *
>> + * This function must use `struct io_br_sel` returned from
>> + * io_uring_cmd_buffer_select() for committing the buffer in the same
>> + * uring_cmd submission context.
>> + */
>> +bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
>> +                                struct io_br_sel *sel, unsigned int issue_flags)
>> +{
>> +       struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
>> +       unsigned int cflags = 0;
>> +
>> +       if (!(ioucmd->flags & IORING_URING_CMD_MULTISHOT))
>> +               return true;
> 
> Same here, a WARN_ON() seems like it would make more sense.

Honestly, I think the existing WARN_ON_ONCE() needs to go rather. I
guess the point is that it's an internal misuse, eg it should not
happen. But I think just passing the error should be enough, you'd know
something is wrong anyway. The reasoning for getting rid of them is that
if these end up being somehow triggerable, then a WARN_ON() is a
potential DOS issue.

IOW, anything WARN_*() should probably just go on that side...

-- 
Jens Axboe