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

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

On 2/7/25 15:59, Keith Busch wrote:
> On Fri, Feb 07, 2025 at 12:41:17PM +0000, Pavel Begunkov wrote:
>> On 2/3/25 15:45, Keith Busch wrote:
>>> +struct io_alloc_cache {
>>> +	void			**entries;
>>> +	unsigned int		nr_cached;
>>> +	unsigned int		max_cached;
>>> +	size_t			elem_size;
>>> +};
>>> +
>>>    struct io_buf_table {
>>>    	struct io_rsrc_data	data;
>>> +	struct io_alloc_cache	node_cache;
>>> +	struct io_alloc_cache	imu_cache;
>>
>> We can avoid all churn if you kill patch 5/6 and place put the
>> caches directly into struct io_ring_ctx. It's a bit better for
>> future cache improvements and we can even reuse the node cache
>> for files.
> 
> I had this that way in an earlier version. The cache is tightly
> connected to the buf table, though, so splitting them up makes for some
> awkward cleanup. Grouping them together makes it clear their lifetimes
> are as a single unit.

I'd say it's tightly couple with the context as depends on the
ctx sync, and the table doesn't really care from where memory
comes.

Anyway, there are basically two reasons. If we're adding node cache,
we need to reuse it for files as well. And I'll likely need to pull
it all back to ctx for cleaning up caches in general, and I'd rather
avoid such back and forths as it's not so great for backports and
things like that.

Is there a good reason why it needs to be in there?


> The filetable could have moved its bitmap into io_ring_ctx too, but it's
> in its own structure like this, and it conceptually makes sense. This is
> following in that same pattern.

-- 
Pavel Begunkov