Re: [RFC PATCH net-next] page_pool: Track DMA-mapped pages and unmap them when destroying the pool

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Yunsheng Lin <linyunsheng@huawei.com> writes:

> On 2025/3/10 23:24, Toke Høiland-Jørgensen wrote:
>
>>>
>>> I guess that is one of the disadvantages that an advanced struct like
>>> Xarray is used:(
>> 
>> Sure, there will be some overhead from using xarray, but I think the
>> simplicity makes up for it; especially since we can limit this to the
>
> As my understanding, it is more complicated, it is just that
> complexity is hidden before xarray now.

Yes, which encapsulates the complexity into a shared abstraction that is
widely used in the kernel, so it does not add new complexity to the
kernel as a whole. Whereas your series adds a whole bunch of new
complexity to the kernel in the form of a new slab allocator.

> Even if there is no space in 'struct page' to store the id, the
> 'struct page' pointer itself can be used as id if the xarray can
> use pointer as id. But it might mean the memory utilization might
> not be as efficient as it should be, and performance hurts too if
> there is more memory to be allocated and freed.

I don't think it can. But sure, there can be other ways around this.

FWIW, I don't think your idea of allocating page_pool_items to use as an
indirection is totally crazy, but all the complexity around it (the
custom slab allocator etc) is way too much. And if we can avoid the item
indirection that is obviously better.

> It seems it is just a matter of choices between using tailor-made
> page_pool specific optimization and using some generic advanced
> struct like xarray.

Yup, basically.

> I chose the tailor-made one because it ensure least overhead as
> much as possibe from performance and memory utilization perspective,
> for example, the 'single producer, multiple consumer' guarantee
> offered by NAPI context can avoid some lock and atomic operation.

Right, and my main point is that the complexity of this is not worth it :)

>> cases where it's absolutely needed.
>
> The above can also be done for using page_pool_item too as the
> lower 2 bits can be used to indicate the pointer in 'struct page'
> is 'page_pool_item' or 'page_pool', I just don't think it is
> necessary yet as it might add more checking in the fast path.

Yup, did think about using the lower bits to distinguish if it does turn
out that we can't avoid an indirection. See above; it's not actually the
page_pool_item concept that is my main issue with your series.

-Toke