On 2024/9/27 17:58, Ilias Apalodimas wrote: ... >> >>> importantly, though, why does struct page need to know about this? >>> Can't we have the same information in page pool? >>> When the driver allocates pages it does via page_pool_dev_alloc_XXXXX >>> or something similar. Cant we do what you suggest here ? IOW when we >>> allocate a page we put it in a list, and when that page returns to >>> page_pool (and it's mapped) we remove it. >> >> Yes, that is the basic idea, but the important part is how to do that >> with less performance impact. > > Yes, but do you think that keeping that list of allocated pages in > struct page_pool will end up being more costly somehow compared to > struct page? I am not sure if I understand your above question here. I am supposing the question is about what's the cost between using single/doubly linked list for the inflight pages or using a array for the inflight pages like this patch does using pool->items? If I understand question correctly, the single/doubly linked list is more costly than array as the page_pool case as my understanding. For single linked list, it doesn't allow deleting a specific entry but only support deleting the first entry and all the entries. It does support lockless operation using llist, but have limitation as below: https://elixir.bootlin.com/linux/v6.7-rc8/source/include/linux/llist.h#L13 For doubly linked list, it needs two pointer to support deleting a specific entry and it does not support lockless operation. For pool->items, as the alloc side is protected by NAPI context, and the free side use item->pp_idx to ensure there is only one producer for each item, which means for each item in pool->items, there is only one consumer and one producer, which seems much like the case when the page is not recyclable in __page_pool_put_page, we don't need a lock protection when calling page_pool_return_page(), the 'struct page' is also one consumer and one producer as the pool->items[item->pp_idx] does: https://elixir.bootlin.com/linux/v6.7-rc8/source/net/core/page_pool.c#L645 We only need a lock protection when page_pool_destroy() is called to check if there is inflight page to be unmapped as a consumer, and the __page_pool_put_page() may also called to unmapped the inflight page as another consumer, there is why the 'destroy_lock' is added for protection when pool->destroy_cnt > 0. > > Thanks > /Ilias
