On Mon, May 26, 2025 at 10:29 PM Pavel Begunkov <asml.silence@xxxxxxxxx> 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
