Re: [PATCH v2 3/3] mm: huge_memory: use folio_skip_prot_numa() for pmd folio

[David Hildenbrand <david@redhat.com>]

On 15.10.25 13:04, Kefeng Wang wrote:
> 
> 
> On 2025/10/15 17:54, David Hildenbrand wrote:
>> On 15.10.25 11:21, Kefeng Wang wrote:
>>>
>>>
>>> On 2025/10/15 15:30, Dev Jain wrote:
>>>>
>>>> On 14/10/25 5:03 pm, Kefeng Wang wrote:
>>>>> Rename prot_numa_skip() to folio_skip_prot_numa(), and remove
>>>>> ret by directly return value instead of goto style.
>>>>>
>>>>> The folio skip checks for prot numa should be suitable for pmd
>>>>> folio too, which helps to avoid unnecessary pmd change and folio
>>>>> migration attempts.
>>>>>
>>>>> Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
>>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>>
>>>> In the review of my mprotect pte batching series, reviewers had
>>>> noted that the branch "if (folio_use_access_time(folio))" in
>>>> folio_skip_prot_numa() did not belong there - it should be done
>>>> outside of the function. But I see that that would duplicate a line
>>>> now that this function has two users. So in light of that, would you
>>>> mind changing the name of this function to
>>>> folio_skip_or_process_prot_numa()?
>>>>
>>>
>>> The name is a bit long, and it only update access_time not change the
>>> pte, so maybe we leave it as is?
>>
>> Any such name might make the return value weird (which indicates whether
>> to skip) I'm afraid.
>>
>> We could invert the meaning and call it something like
>>
>>       folio_apply_prot_numa()
>>
>> And return whether we have to protect it.
>>
>> Maybe that's better? Other naming suggestions welcome :)
>>
> 
> That's better, or folio_needs_prot_numa() as there are already some
> similar names with folio_needs_ prefix ?

Yeah, why not. Then we can also add kerneldoc to describe that it will 
adjust the access time if the function returns true.

-- 
Cheers

David / dhildenb