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

[David Hildenbrand <david@redhat.com>]

On 21.10.25 10:41, Kefeng Wang wrote:
> 
> 
> On 2025/10/21 2:56, Lorenzo Stoakes wrote:
>> On Mon, Oct 20, 2025 at 08:12:08PM +0200, David Hildenbrand wrote:
>>> 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_*.
>>
>> folio_is_zone_device()
>> folio_is_zone_movable()
>> folio_needs_release()
>> folio_needs_cow_for_dma()
>> folio_memcg_kmem()
>> folio_memcg_charged()
>> folio_pgoff()
>> folio_pos()
>> folio_contains()
>> folio_mapcount()
>>
>> etc. etc. etc.
>>
>> All properties of the folio in particular, not folio_has_*() or folio_test_*().
>>
>> I mean the pattern is established in the kernel. Including folio_needs_*()!
>>
>> I honestly wonder if the original formulation was correct - check for exemptions.
>>
>> So something like folio_exempt_from_prot_numa()?
>>
>> Or this way around folio_suitable_prot_numa()?
>>
>> I think this is an English thing. 'folio needs prot numa' reads as 'this folio
>> _needs_ prot numa' right?
>>
>> Oh - folio_can_map_prot_numa()? Something like this?
>>
>>>
>>> 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.
>>
>> OK, I guess folio_needs_cow_for_dma() does the same thing with MMF_HAS_PINNED.
>>
>>>
>>>> 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.
>>
>> Again I think it's an English thing. As I said above.
>>
>>>
>>> And keep disliking prot_numa_hint_needed() ;)
>>
>> Well you are very particular about naming :)
>>
>> I similarly dislike folio_needs_prot_numa()...
>>
> 
> Maybe folio_needs_protnone_mapping()?

I think prot_numa should be in the name one way or the other, because we 
also have ordinary PROT_NONE unrealted to NUMA faults.

-- 
Cheers

David / dhildenb