[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250801160805.28fa1e05@kernel.org>
Date: Fri, 1 Aug 2025 16:08:05 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Byungchul Park <byungchul@...com>
Cc: linux-mm@...ck.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel_team@...ynix.com,
harry.yoo@...cle.com, ast@...nel.org, daniel@...earbox.net,
davem@...emloft.net, hawk@...nel.org, john.fastabend@...il.com,
sdf@...ichev.me, saeedm@...dia.com, leon@...nel.org, tariqt@...dia.com,
mbloch@...dia.com, andrew+netdev@...n.ch, edumazet@...gle.com,
pabeni@...hat.com, akpm@...ux-foundation.org, david@...hat.com,
lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com, vbabka@...e.cz,
rppt@...nel.org, surenb@...gle.com, mhocko@...e.com, horms@...nel.org,
jackmanb@...gle.com, hannes@...xchg.org, ziy@...dia.com,
ilias.apalodimas@...aro.org, willy@...radead.org, brauner@...nel.org,
kas@...nel.org, yuzhao@...gle.com, usamaarif642@...il.com,
baolin.wang@...ux.alibaba.com, almasrymina@...gle.com, toke@...hat.com,
asml.silence@...il.com, bpf@...r.kernel.org, linux-rdma@...r.kernel.org,
sfr@...b.auug.org.au
Subject: Re: [PATCH linux-next v3] mm, page_pool: introduce a new page type
for page pool in page type
On Tue, 29 Jul 2025 20:02:10 +0900 Byungchul Park wrote:
> [PATCH linux-next v3] mm, page_pool: introduce a new page type for page pool in page type
linux-next does not accept patches. This has to go either via networking or MM.
> - if (unlikely(page_has_type(page)))
> + if (unlikely(page_has_type(page))) {
Maybe add :
/* networking expects to clear its page type before releasing */
> + WARN_ON_ONCE(PageNetpp(page));
> /* Reset the page_type (which overlays _mapcount) */
> page->page_type = UINT_MAX;
> + }
> static inline bool netmem_is_pp(netmem_ref netmem)
> {
> - return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) == PP_SIGNATURE;
> + /* Use ->pp for net_iov to identify if it's pp,
Please try to use precise language, this code is confusing as is.
net_iov may _belong_ to a page pool.
* which requires that non-pp net_iov should have ->pp NULL'd.
I don't think this adds any information.
> + */
> + if (netmem_is_net_iov(netmem))
> + return !!__netmem_clear_lsb(netmem)->pp;
> +
> + /* For system memory, page type bit in struct page can be used
"page type bit" -> "page type", it's not a bit.
> + * to identify if it's pp.
... to identify pages which belong to a page pool.
> + */
> + return PageNetpp(__netmem_to_page(netmem));
> }
>
> static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 05e2e22a8f7c..37eeab76c41c 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -654,7 +654,6 @@ s32 page_pool_inflight(const struct page_pool *pool, bool strict)
> void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
> {
> netmem_set_pp(netmem, pool);
> - netmem_or_pp_magic(netmem, PP_SIGNATURE);
>
> /* Ensuring all pages have been split into one fragment initially:
> * page_pool_set_pp_info() is only called once for every page when it
> @@ -665,12 +664,19 @@ void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
> page_pool_fragment_netmem(netmem, 1);
> if (pool->has_init_callback)
> pool->slow.init_callback(netmem, pool->slow.init_arg);
> +
> + /* If it's page-backed */
Please don't add obvious comments.
> + if (!netmem_is_net_iov(netmem))
> + __SetPageNetpp(__netmem_to_page(netmem));
Powered by blists - more mailing lists