Re: [RFC PATCH 05/12] fs/proc/task_mmu: refactor pagemap_pmd_range()

Lorenzo Stoakes
2025-10-28T18:24:05Z

On Tue, Oct 28, 2025 at 09:52:44AM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 27, 2025 at 04:38:05PM +0000, Lorenzo Stoakes wrote:
> > On Mon, Oct 27, 2025 at 05:31:54PM +0100, David Hildenbrand wrote:
> > > >
> > > > I don't love the union.
> > > >
> > > > How would we determine what type it is, we'd have to have some
> > > > generic_leaf_entry_t type or something to contain the swap type field and then
> > > > cast and... is it worth it?
> > > >
> > > > Intent of non-present was to refer to not-swap swapentry. It's already a
> > > > convention that exists, e.g. is_pmd_non_present_folio_entry().
> > >
> > > Just noting that this was a recent addition (still not upstream) that
> > > essentially says "there is a folio here, but it's not in an ordinary present
> > > page table entry.
> > >
> > > So we could change that to something better.
> >
> > Yeah but leaf_entry_t encapsulates BOTH swap and non-swap entries.
> >
> > So that's nice.
> >
> > What do you propose calling non-swap leaf entries? It starts spiralling down a
> > bit there.
>
> You don't even ask that question.
>
> You have a leaf entry. It has a type.
>
> What you are calling a "swap entry" is a "leaf entry of swap type".

I think this is pretty well covered in the other thread tbh.

>
> The union helps encode in the type system what code is operating on
> what type of the leaf entry.
>
> It seems pretty simple.

As Gregory says, this requires reworking a lot of code. We at the very least
need to defer this, and I remain unconvinced it's really worth it.

So yeah, let's come back to this later.

>
> > And it's really common to have logic asserting it's actually a swap entry
> > vs. not etc.
>
> leafent_is_swap(ent) - meaning is a "leaf entry of swap type".

I mean, we already have a function that does this in this series with a
different name :)

But sure I'll rename it to this so we're good.

>
> > 1. we keep the non-present terminology as a better way of referring
> > to non-swap entries.
>
> I vastly prefer you leap ahead and start using leaf_entry
> terminology. We don't need a temporary name we are going to throw
> away.

Right well we agree on the other series to use the leafent_xxx() prefix so lets'
do that.

>
> Jason

Cheers, Lorenzo