Re: [PATCH v4 3/4] mm: mprotect: convert to folio_needs_prot_numa()

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Mon, Oct 20, 2025 at 07:34:51PM +0200, David Hildenbrand wrote:
> On 20.10.25 17:14, Kefeng Wang wrote:
> >
> >
> > On 2025/10/20 21:10, Lorenzo Stoakes wrote:
> > > On Mon, Oct 20, 2025 at 02:18:44PM +0800, Kefeng Wang wrote:
> > > > The prot_numa_skip() naming is not good since it updates the folio
> > > > access time except checking whether to skip prot NUMA, so rename it
> > > > to folio_needs_prot_numa(), and cleanup it a bit, remove ret by
> > >
> > > Hmm not sure 'folio_needs_prot_numa()' is any better in terms of indicating
> > > that you're updating the access time to be honest.
> > >
> > > Also it seems to suggest that you're determining whether a mapping of the
> > > folio should be made a NUMA hint by the folio alone rather than the reality
> > > that the mapping is being considered for NUMA hinting and you're checking
> > > to see if you actually have to do it.
>
> ... because the folio doesn't need prot_none protection for NUMA hitning? :)

folio_xxx() impliles to me that the folio independently has proeprty 'xxx'.

So it's like saying 'here's a folio, does it NEED prot numa?' right?

But that's not what we're doing, we're actually kinda doing the opposite, we're
saying 'ok we want NUMA hinting, but do we actually need it?'

The sematics are hard yes.

>
> > >
> > > prot_numa_hint_needed() seems better to me?
> > >
> > > folio_xxx() stuff all seems to be derived from the folio alone.
> >
> > Naming is hard, David suggested it with folio_ prefix in v2,
> > the most check are derived from folio, but I could update it if
> > we like the new prot_numa_hint_needed().
>
> I prefer what we have here, but I will not fight for it.

I also dislike the 'folio' in the name. We're not only basing it on folio, but
alsoo vma, targe_node.

It's hard to put all that across in the name, but I'm personally not a big fan
of the original for these reasons.

>
> --
> Cheers
>
> David / dhildenb
>
>

Cheers, Lorenzo