[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aB5DNqwP_LFV_ULL@casper.infradead.org>
Date: Fri, 9 May 2025 19:02:30 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Byungchul Park <byungchul@...com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, kernel_team@...ynix.com, kuba@...nel.org,
almasrymina@...gle.com, ilias.apalodimas@...aro.org,
harry.yoo@...cle.com, hawk@...nel.org, akpm@...ux-foundation.org,
ast@...nel.org, daniel@...earbox.net, davem@...emloft.net,
john.fastabend@...il.com, andrew+netdev@...n.ch,
edumazet@...gle.com, pabeni@...hat.com, vishal.moola@...il.com
Subject: Re: [RFC 19/19] mm, netmem: remove the page pool members in struct
page
On Fri, May 09, 2025 at 08:51:26PM +0900, Byungchul Park wrote:
> +++ b/include/linux/mm_types.h
> @@ -20,6 +20,7 @@
> #include <linux/seqlock.h>
> #include <linux/percpu_counter.h>
> #include <linux/types.h>
> +#include <net/netmem_type.h> /* for page pool */
>
> #include <asm/mmu.h>
>
> @@ -118,17 +119,7 @@ struct page {
> */
> unsigned long private;
> };
> - struct { /* page_pool used by netstack */
> - /**
> - * @pp_magic: magic value to avoid recycling non
> - * page_pool allocated pages.
> - */
> - unsigned long pp_magic;
> - struct page_pool *pp;
> - unsigned long _pp_mapping_pad;
> - unsigned long dma_addr;
> - atomic_long_t pp_ref_count;
> - };
> + struct __netmem_desc place_holder_1; /* for page pool */
> struct { /* Tail pages of compound page */
> unsigned long compound_head; /* Bit zero is set */
> };
The include and the place holder aren't needed.
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 00064e766b889..c414de6c6ab0d 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -10,6 +10,7 @@
>
> #include <linux/mm.h>
> #include <net/net_debug.h>
> +#include <net/netmem_type.h>
... which I think means you don't need the separate header file.
> /* net_iov */
>
> @@ -20,15 +21,6 @@ DECLARE_STATIC_KEY_FALSE(page_pool_mem_providers);
> */
> #define NET_IOV 0x01UL
>
> -struct netmem_desc {
> - unsigned long __unused_padding;
> - unsigned long pp_magic;
> - struct page_pool *pp;
> - struct net_iov_area *owner;
> - unsigned long dma_addr;
> - atomic_long_t pp_ref_count;
> -};
> -
> struct net_iov_area {
> /* Array of net_iovs for this area. */
> struct netmem_desc *niovs;
> @@ -38,31 +30,6 @@ struct net_iov_area {
> unsigned long base_virtual;
> };
>
> -/* These fields in struct page are used by the page_pool and net stack:
> - *
> - * struct {
> - * unsigned long pp_magic;
> - * struct page_pool *pp;
> - * unsigned long _pp_mapping_pad;
> - * unsigned long dma_addr;
> - * atomic_long_t pp_ref_count;
> - * };
> - *
> - * We mirror the page_pool fields here so the page_pool can access these fields
> - * without worrying whether the underlying fields belong to a page or net_iov.
> - *
> - * The non-net stack fields of struct page are private to the mm stack and must
> - * never be mirrored to net_iov.
> - */
> -#define NET_IOV_ASSERT_OFFSET(pg, iov) \
> - static_assert(offsetof(struct page, pg) == \
> - offsetof(struct netmem_desc, iov))
> -NET_IOV_ASSERT_OFFSET(pp_magic, pp_magic);
> -NET_IOV_ASSERT_OFFSET(pp, pp);
> -NET_IOV_ASSERT_OFFSET(dma_addr, dma_addr);
> -NET_IOV_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
> -#undef NET_IOV_ASSERT_OFFSET
... but you do want to keep asserting that netmem_desc and
net_iov have the same offsets. And you want to assert that struct page
is big enough to hold everything in netmem_desc, like we do for slab:
static_assert(sizeof(struct slab) <= sizeof(struct page));
Powered by blists - more mailing lists