Re: [PATCH V8 5/7] io_uring: support leased group buffer with REQ_F_GROUP_KBUF

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

On 10/30/24 00:45, Ming Lei wrote:
> On Tue, Oct 29, 2024 at 04:47:59PM +0000, Pavel Begunkov wrote:
>> On 10/25/24 13:22, Ming Lei wrote:
>> ...
>>> diff --git a/io_uring/rw.c b/io_uring/rw.c
>>> index 4bc0d762627d..5a2025d48804 100644
>>> --- a/io_uring/rw.c
>>> +++ b/io_uring/rw.c
>>> @@ -245,7 +245,8 @@ static int io_prep_rw_setup(struct io_kiocb *req, int ddir, bool do_import)
>>>    	if (io_rw_alloc_async(req))
>>>    		return -ENOMEM;
>>> -	if (!do_import || io_do_buffer_select(req))
>>> +	if (!do_import || io_do_buffer_select(req) ||
>>> +	    io_use_leased_grp_kbuf(req))
>>>    		return 0;
>>>    	rw = req->async_data;
>>> @@ -489,6 +490,11 @@ static bool __io_complete_rw_common(struct io_kiocb *req, long res)
>>>    		}
>>>    		req_set_fail(req);
>>>    		req->cqe.res = res;
>>> +		if (io_use_leased_grp_kbuf(req)) {
>>
>> That's what I'm talking about, we're pushing more and
>> into the generic paths (or patching every single hot opcode
>> there is). You said it's fine for ublk the way it was, i.e.
>> without tracking, so let's then pretend it's a ublk specific
>> feature, kill that addition and settle at that if that's the
>> way to go.
> 
> As I mentioned before, it isn't ublk specific, zeroing is required
> because the buffer is kernel buffer, that is all. Any other approach
> needs this kind of handling too. The coming fuse zc need it.
> 
> And it can't be done in driver side, because driver has no idea how
> to consume the kernel buffer.
> 
> Also it is only required in case of short read/recv, and it isn't
> hot path, not mention it is just one check on request flag.

I agree, it's not hot, it's a failure path, and the recv side
is of medium hotness, but the main concern is that the feature
is too actively leaking into other requests.

-- 
Pavel Begunkov