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

[David Hildenbrand <david@redhat.com>]

On 20.10.25 19:49, Lorenzo Stoakes wrote:
> 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'.
> 

I would agree if it would be a folio_has_* or folio_test_*.

To me the folio is really just the main entity we are querying 
information about. Not the VMA, not the target node, but the folio.

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

I'd say: "here is a folio, does it need numa protection in this vma". So 
I still don't understand your point, unfortunately.

And keep disliking prot_numa_hint_needed() ;)

But again, I won't fight for it if you have strong opinions on it.

It would be better if we could have discussed that as part of v2 to 
avoid having Kefeng go back and forth. Maybe v3+v4 were sent out a bit 
too quickly

-- 
Cheers

David / dhildenb