Re: [PATCHv4 1/5] io_uring: move fixed buffer import to issue path

[Pavel Begunkov <asml.silence@gmail.com>]

On 2/19/25 16:48, Pavel Begunkov wrote:
> On 2/18/25 22:42, Keith Busch wrote:
>> From: Keith Busch <kbusch@kernel.org>
>>
>> Similar to the fixed file path, requests may depend on a previous one
>> to set up an index, so we need to allow linking them. The prep callback
>> happens too soon for linked commands, so the lookup needs to be deferred
>> to the issue path. Change the prep callbacks to just set the buf_index
>> and let generic io_uring code handle the fixed buffer node setup, just
>> like it already does for fixed files.
>>
>> Signed-off-by: Keith Busch <kbusch@kernel.org>
> 
> It wasn't great before, and it'd be harder to follow if we shove it
> into the issue path like that. Add additional overhead in the common
> path and that it's not super flexible, like the notification problem
> and what we need out of it for other features.
> 
> We're better to remove the lookup vs import split like below.
> Here is a branch, let's do it on top.
> 
> https://github.com/isilence/linux.git regbuf-import
> 
> 
> diff --git a/io_uring/net.c b/io_uring/net.c
> index ce0a39972cce..322cf023233a 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -1360,24 +1360,10 @@ static int io_send_zc_import(struct io_kiocb *req, unsigned int issue_flags)
...
> -int io_import_fixed(int ddir, struct iov_iter *iter,
> -               struct io_mapped_ubuf *imu,
> -               u64 buf_addr, size_t len)
> +static int io_import_fixed_imu(int ddir, struct iov_iter *iter,
> +                struct io_mapped_ubuf *imu,
> +                u64 buf_addr, size_t len)
>   {
>       u64 buf_end;
>       size_t offset;
> @@ -919,6 +919,35 @@ int io_import_fixed(int ddir, struct iov_iter *iter,
>       return 0;
>   }
> 
> +static inline struct io_rsrc_node *io_find_buf_node(struct io_kiocb *req,
> +                            unsigned issue_flags)
> +{
> +    struct io_ring_ctx *ctx = req->ctx;
> +    struct io_rsrc_node *node;
> +
> +    if (req->buf_node)

Seems it should be checking for REQ_F_BUF_NODE instead

> +        return req->buf_node;
> +
> +    io_ring_submit_lock(ctx, issue_flags);
> +    node = io_rsrc_node_lookup(&ctx->buf_table, req->buf_index);
> +    if (node)
> +        io_req_assign_buf_node(req, node);
> +    io_ring_submit_unlock(ctx, issue_flags);
> +    return node;
> +}

-- 
Pavel Begunkov