Re: [PATCH 2/4] io_uring: add struct io_cold_def->sqe_copy() method

[Jens Axboe <axboe@kernel.dk>]

On 6/6/25 11:36 AM, Caleb Sander Mateos wrote:
> On Thu, Jun 5, 2025 at 12:47?PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> Will be called by the core of io_uring, if inline issue is not going
>> to be tried for a request. Opcodes can define this handler to defer
>> copying of SQE data that should remain stable.
>>
>> Called with IO_URING_F_INLINE set if this is an inline issue, and that
>> flag NOT set if it's an out-of-line call. The handler can use this to
>> determine if it's still safe to copy the SQE. The core should always
>> guarantee that it will be safe, but by having this flag available the
>> handler is able to check and fail.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  io_uring/io_uring.c | 32 ++++++++++++++++++++++++--------
>>  io_uring/opdef.h    |  1 +
>>  2 files changed, 25 insertions(+), 8 deletions(-)
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index 079a95e1bd82..fdf23e81c4ff 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -147,7 +147,7 @@ static bool io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
>>                                          bool cancel_all,
>>                                          bool is_sqpoll_thread);
>>
>> -static void io_queue_sqe(struct io_kiocb *req);
>> +static void io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe);
>>  static void __io_req_caches_free(struct io_ring_ctx *ctx);
>>
>>  static __read_mostly DEFINE_STATIC_KEY_FALSE(io_key_has_sqarray);
>> @@ -1377,7 +1377,7 @@ void io_req_task_submit(struct io_kiocb *req, io_tw_token_t tw)
>>         else if (req->flags & REQ_F_FORCE_ASYNC)
>>                 io_queue_iowq(req);
>>         else
>> -               io_queue_sqe(req);
>> +               io_queue_sqe(req, NULL);
> 
> Passing NULL here is a bit weird. As I mentioned on patch 1, I think
> it would make more sense to consider this task work path a
> "non-inline" issue.

It very much _is_ a non-inline issue, the sqe being NULL is how the
->sqe_copy() would treat it. But it is a bit confusing in that the
callback would need to check

if (!sqe || !(flags & INLINE))

as the condition for whether or not the sqe is valid. Probably better to
make the INLINE flag dependent on sqe != NULL as well, and then that's
all the callback would need to check for.

-- 
Jens Axboe