Re: [PATCH 0/6] ublk zero-copy support

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

On 2/8/25 05:44, Ming Lei wrote:
> On Fri, Feb 07, 2025 at 07:06:54AM -0700, Keith Busch wrote:
>> On Fri, Feb 07, 2025 at 11:51:49AM +0800, Ming Lei wrote:
>>> On Mon, Feb 03, 2025 at 07:45:11AM -0800, Keith Busch wrote:
>>>>
>>>> The previous version from Ming can be viewed here:
>>>>
>>>>    https://lore.kernel.org/linux-block/20241107110149.890530-1-ming.lei@redhat.com/
>>>>
>>>> Based on the feedback from that thread, the desired io_uring interfaces
>>>> needed to be simpler, and the kernel registered resources need to behave
>>>> more similiar to user registered buffers.
>>>>
>>>> This series introduces a new resource node type, KBUF, which, like the
>>>> BUFFER resource, needs to be installed into an io_uring buf_node table
>>>> in order for the user to access it in a fixed buffer command. The
>>>> new io_uring kernel API provides a way for a user to register a struct
>>>> request's bvec to a specific index, and a way to unregister it.
>>>>
>>>> When the ublk server receives notification of a new command, it must
>>>> first select an index and register the zero copy buffer. It may use that
>>>> index for any number of fixed buffer commands, then it must unregister
>>>> the index when it's done. This can all be done in a single io_uring_enter
>>>> if desired, or it can be split into multiple enters if needed.
>>>
>>> I suspect it may not be done in single io_uring_enter() because there
>>> is strict dependency among the three OPs(register buffer, read/write,
>>> unregister buffer).
>>
>> The registration is synchronous. io_uring completes the SQE entirely
>> before it even looks at the read command in the next SQE.
> 
> Can you explain a bit "synchronous" here?

I'd believe synchronous here means "executed during submission from
the submit syscall path". And I agree that we can't rely on that.
That's an implementation detail and io_uring doesn't promise that,
but even now it relies on not using certain features like drain and
the async flag.
  
> In patch 4, two ublk uring_cmd(UBLK_U_IO_REGISTER_IO_BUF/UBLK_U_IO_UNREGISTER_IO_BUF)
> are added, and their handlers are called from uring_cmd's ->issue().
> 
>>
>> The read or write is asynchronous, but it's prep takes a reference on
>> the node before moving on to the next SQE..
> 
> The buffer is registered in ->issue() of UBLK_U_IO_REGISTER_IO_BUF,
> and it isn't done yet when calling ->prep() of read_fixed/write_fixed,
> in which buffer is looked up in ->prep().

I believe we should eventually move all such binding to ->issue
to be consistent with file handling. Not super happy about either
of those, but that's the kinds of problems coming from supporting
links.

-- 
Pavel Begunkov