Re: [PATCH 6/6] io_uring: cache nodes and mapped buffers

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

On 2/7/25 15:33, Keith Busch wrote:
...
>>> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
>>> index 864c2eabf8efd..5434b0d992d62 100644
>>> --- a/io_uring/rsrc.c
>>> +++ b/io_uring/rsrc.c
>>> @@ -117,23 +117,39 @@ static void io_buffer_unmap(struct io_ring_ctx *ctx, struct io_rsrc_node *node)
>>>    				unpin_user_page(imu->bvec[i].bv_page);
>>>    		if (imu->acct_pages)
>>>    			io_unaccount_mem(ctx, imu->acct_pages);
>>> -		kvfree(imu);
>>> +		if (struct_size(imu, bvec, imu->nr_bvecs) >
>>> +				ctx->buf_table.imu_cache.elem_size ||
>>
>> It could be quite a large allocation, let's not cache it if
>> it hasn't came from the cache for now. We can always improve
>> on top.
> 
> Eh? This already skips inserting into the cache if it wasn't allocated
> out of the cache.

Ah yes, you're right, I overlooked it.

> 
> I picked an arbitrary size, 512b, as the threshold for caching. If you
> need more bvecs than fit in that, it falls back to a kvmalloc/kvfree.
> The allocation overhead is pretty insignificant when you're transferring
> large payloads like that, and 14 vectors was chosen as the tipping point
> because it fits in a nice round number.
>   
>> And can we invert how it's calculated? See below. You'll have
>> fewer calculations in the fast path, and I don't really like
>> users looking at ->elem_size when it's not necessary.
>>
>>
>> #define IO_CACHED_BVEC_SEGS	N
> 
> Yah, that's fine.

-- 
Pavel Begunkov