linux-kernel - Re: [PATCH 18/18] mm, netmem: remove the page pool members in struct page

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: <CAHS8izNYmWTgb+QDA72RYAQaFC15Tfc59tK3Q2d670gHyyKJNQ@mail.gmail.com>
Date: Tue, 27 May 2025 10:38:43 -0700
From: Mina Almasry <almasrymina@...gle.com>
To: Pavel Begunkov <asml.silence@...il.com>
Cc: Byungchul Park <byungchul@...com>, willy@...radead.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, kernel_team@...ynix.com, 
	kuba@...nel.org, ilias.apalodimas@...aro.org, harry.yoo@...cle.com, 
	hawk@...nel.org, akpm@...ux-foundation.org, davem@...emloft.net, 
	john.fastabend@...il.com, andrew+netdev@...n.ch, toke@...hat.com, 
	tariqt@...dia.com, edumazet@...gle.com, pabeni@...hat.com, saeedm@...dia.com, 
	leon@...nel.org, ast@...nel.org, daniel@...earbox.net, 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, 
	linux-rdma@...r.kernel.org, bpf@...r.kernel.org, vishal.moola@...il.com
Subject: Re: [PATCH 18/18] mm, netmem: remove the page pool members in struct page

On Mon, May 26, 2025 at 10:29 PM Pavel Begunkov <asml.silence@...il.com> wrote:
>
> On 5/27/25 02:02, Byungchul Park wrote:
> ...>> Patch 1:
> >>
> >> struct page {
> >>      unsigned long flags;
> >>      union {
> >>              struct_group_tagged(netmem_desc, netmem_desc) {
> >>                      // same layout as before
> >>                      ...
> >>                      struct page_pool *pp;
> >>                      ...
> >>              };
> >
> > This part will be gone shortly.  The matters come from absence of this
> > part.
>
> Right, the problem is not having an explicit netmem_desc in struct
> page and not using struct netmem_desc in all relevant helpers.
>
> >> struct net_iov {
> >>      unsigned long flags_padding;
> >>      union {
> >>              struct {
> >>                      // same layout as in page + build asserts;
> >>                      ...
> >>                      struct page_pool *pp;
> >>                      ...
> >>              };
> >>              struct netmem_desc desc;
> >>      };
> >> };
> >>
> >> struct netmem_desc *page_to_netmem_desc(struct page *page)
> >> {
> >>      return &page->netmem_desc;
> >
> > page will not have any netmem things in it after this, that matters.
>
> Ok, the question is where are you going to stash the fields?
> We still need space to store them. Are you going to do the
> indirection mm folks want?
>

I think I see some confusion here. I'm not sure indirection is what mm
folks want. The memdesc effort has already been implemented for zpdesc
and ptdesc[1], and the approach they did is very different from this
series. zpdesc and ptdesc have created a struct that mirrors the
entirety of struct page, not a subfield of struct page with
indirection:

https://elixir.bootlin.com/linux/v6.14.3/source/mm/zpdesc.h#L29

I'm now a bit confused, because the code changes in this series do not
match the general approach that zpdesc and ptdesc have done.
Byungchul, is the deviation in approach from zpdesc and ptdecs
intentional? And if so why? Should we follow the zpdesc and ptdesc
lead and implement a new struct that mirrors the entirety of struct
page?

[1] https://kernelnewbies.org/MatthewWilcox/Memdescs/Path

--
Thanks,
Mina