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

[Kefeng Wang <wangkefeng.wang@huawei.com>]

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()? if there are no better options,
we'll leave it as is.

>>
>> But again, I won't fight for it if you have strong opinions on it.
> 
> It's not the biggest issue in the world. Equally if you insist I won't lose
> sleep over this...
> 
>>
>> 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
> 
> At v2 I had just got back from holiday (+ being sick) and had a >1,000 mail
> backlog, sorry.
> 
>>
>> --
>> Cheers
>>
>> David / dhildenb
>>
>>
> 
> Cheers, Lorenzo
>