Re: [PATCH net-next v4 2/5] page_pool: unify frag_count handling in page_pool_is_last_frag()
From: Jakub Kicinski <kuba@kernel.org>
Date: 2023-06-14 04:33:29
Also in:
lkml
On Mon, 12 Jun 2023 21:02:53 +0800 Yunsheng Lin wrote:
static inline void page_pool_fragment_page(struct page *page, long nr)
{
- atomic_long_set(&page->pp_frag_count, nr);
+ if (!PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
+ atomic_long_set(&page->pp_frag_count, nr);why not let the driver take references on the page count in this case? I'm not saying it's worth the effort, but a comment may be useful?
quoted hunk ↗ jump to hunk
diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 9c4118c62997..69e3c5175236 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c@@ -352,6 +352,14 @@ static void page_pool_set_pp_info(struct page_pool *pool, { page->pp = pool; page->pp_magic |= PP_SIGNATURE; + + /* Ensuring all pages have been split into one big frag initially:
Again, I find the "one big frag" slightly confusing. Maybe we should rename pp_frag_cnt into pp_refcnt? After this series is looks even more like a page pool reference count rather than some form of number of fragments.
+ * page_pool_set_pp_info() is only called once for every page when it + * is allocated from the page allocator and page_pool_fragment_page() + * is dirtying the same cache line as the page->pp_magic above, so + * the overhead is negligible. + */ + page_pool_fragment_page(page, 1); if (pool->p.init_callback) pool->p.init_callback(page, pool->p.init_arg); }