netdev - Re: [PATCH net-next v7 1/2] page_pool: Move pp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <D8ZSA9FSRHX2.2Q6MA2HLESONR@nvidia.com>
Date: Sun, 06 Apr 2025 14:56:51 -0400
From: "Zi Yan" <ziy@...dia.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>, "David
 S. Miller" <davem@...emloft.net>, "Jakub Kicinski" <kuba@...nel.org>,
 "Jesper Dangaard Brouer" <hawk@...nel.org>, "Saeed Mahameed"
 <saeedm@...dia.com>, "Leon Romanovsky" <leon@...nel.org>, "Tariq Toukan"
 <tariqt@...dia.com>, "Andrew Lunn" <andrew+netdev@...n.ch>, "Eric Dumazet"
 <edumazet@...gle.com>, "Paolo Abeni" <pabeni@...hat.com>, "Ilias
 Apalodimas" <ilias.apalodimas@...aro.org>, "Simon Horman"
 <horms@...nel.org>, "Andrew Morton" <akpm@...ux-foundation.org>, "Mina
 Almasry" <almasrymina@...gle.com>, "Yonglong Liu" <liuyonglong@...wei.com>,
 "Yunsheng Lin" <linyunsheng@...wei.com>, "Pavel Begunkov"
 <asml.silence@...il.com>, "Matthew Wilcox" <willy@...radead.org>
Cc: <netdev@...r.kernel.org>, <bpf@...r.kernel.org>,
 <linux-rdma@...r.kernel.org>, <linux-mm@...ck.org>
Subject: Re: [PATCH net-next v7 1/2] page_pool: Move pp_magic check into
 helper functions

On Fri Apr 4, 2025 at 6:18 AM EDT, Toke Høiland-Jørgensen wrote:
> Since we are about to stash some more information into the pp_magic
> field, let's move the magic signature checks into a pair of helper
> functions so it can be changed in one place.
>
> Reviewed-by: Mina Almasry <almasrymina@...gle.com>
> Tested-by: Yonglong Liu <liuyonglong@...wei.com>
> Acked-by: Jesper Dangaard Brouer <hawk@...nel.org>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@...aro.org>
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c |  4 ++--
>  include/net/page_pool/types.h                    | 18 ++++++++++++++++++
>  mm/page_alloc.c                                  |  9 +++------
>  net/core/netmem_priv.h                           |  5 +++++
>  net/core/skbuff.c                                | 16 ++--------------
>  net/core/xdp.c                                   |  4 ++--
>  6 files changed, 32 insertions(+), 24 deletions(-)
>

<snip>

> diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
> index 36eb57d73abc6cfc601e700ca08be20fb8281055..df0d3c1608929605224feb26173135ff37951ef8 100644
> --- a/include/net/page_pool/types.h
> +++ b/include/net/page_pool/types.h
> @@ -54,6 +54,14 @@ struct pp_alloc_cache {
>  	netmem_ref cache[PP_ALLOC_CACHE_SIZE];
>  };
>  
> +/* Mask used for checking in page_pool_page_is_pp() below. page->pp_magic is
> + * OR'ed with PP_SIGNATURE after the allocation in order to preserve bit 0 for
> + * the head page of compound page and bit 1 for pfmemalloc page.
> + * page_is_pfmemalloc() is checked in __page_pool_put_page() to avoid recycling
> + * the pfmemalloc page.
> + */
> +#define PP_MAGIC_MASK ~0x3UL
> +
>  /**
>   * struct page_pool_params - page pool parameters
>   * @fast:	params accessed frequently on hotpath
> @@ -264,6 +272,11 @@ void page_pool_destroy(struct page_pool *pool);
>  void page_pool_use_xdp_mem(struct page_pool *pool, void (*disconnect)(void *),
>  			   const struct xdp_mem_info *mem);
>  void page_pool_put_netmem_bulk(netmem_ref *data, u32 count);
> +
> +static inline bool page_pool_page_is_pp(struct page *page)
> +{
> +	return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> +}
>  #else
>  static inline void page_pool_destroy(struct page_pool *pool)
>  {
> @@ -278,6 +291,11 @@ static inline void page_pool_use_xdp_mem(struct page_pool *pool,
>  static inline void page_pool_put_netmem_bulk(netmem_ref *data, u32 count)
>  {
>  }
> +
> +static inline bool page_pool_page_is_pp(struct page *page)
> +{
> +	return false;
> +}
>  #endif
>  
>  void page_pool_put_unrefed_netmem(struct page_pool *pool, netmem_ref netmem,
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f51aa6051a99867d2d7d8c70aa7c30e523629951..347a3cc2c188f4a9ced85e0d198947be7c503526 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -55,6 +55,7 @@
>  #include <linux/delayacct.h>
>  #include <linux/cacheinfo.h>
>  #include <linux/pgalloc_tag.h>
> +#include <net/page_pool/types.h>
>  #include <asm/div64.h>
>  #include "internal.h"
>  #include "shuffle.h"
> @@ -897,9 +898,7 @@ static inline bool page_expected_state(struct page *page,
>  #ifdef CONFIG_MEMCG
>  			page->memcg_data |
>  #endif
> -#ifdef CONFIG_PAGE_POOL
> -			((page->pp_magic & ~0x3UL) == PP_SIGNATURE) |
> -#endif
> +			page_pool_page_is_pp(page) |
>  			(page->flags & check_flags)))
>  		return false;
>  
> @@ -926,10 +925,8 @@ static const char *page_bad_reason(struct page *page, unsigned long flags)
>  	if (unlikely(page->memcg_data))
>  		bad_reason = "page still charged to cgroup";
>  #endif
> -#ifdef CONFIG_PAGE_POOL
> -	if (unlikely((page->pp_magic & ~0x3UL) == PP_SIGNATURE))
> +	if (unlikely(page_pool_page_is_pp(page)))
>  		bad_reason = "page_pool leak";
> -#endif
>  	return bad_reason;
>  }
>  

I wonder if it is OK to make page allocation depend on page_pool from net/page_pool.
Would linux/mm.h be a better place for page_pool_page_is_pp()?

-- 
Best Regards,
Yan, Zi