Re: [PATCH v2 2/3] mm: rmap: support batched checks of the references for large folios

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 2025/12/18 20:08, Ryan Roberts wrote:
>>>>    +#define clear_flush_young_ptes clear_flush_young_ptes
>>>> +static inline int clear_flush_young_ptes(struct vm_area_struct *vma,
>>>> +                    unsigned long addr, pte_t *ptep,
>>>> +                    unsigned int nr)
>>>> +{
>>>> +    if (likely(nr == 1))
>>>> +        return __ptep_clear_flush_young(vma, addr, ptep);
>>>
>>> Bug: This is broken if core-mm tries to call this for nr=1 on a pte that is part
>>> of a contpte mapping.
>>>
>>> The similar fastpaths are here to prevent regressing the common small folio case.
>>
>> Thanks for catching this. I had considered this before, but I still missed it.
>>
>>> I guess here the best approach is (note no leading underscores):
>>>
>>>      if (likely(nr == 1))
>>>          return ptep_clear_flush_young(vma, addr, ptep);
>>
>> However, I prefer to use pte_cont() to check it. Later, I plan to clean up the
>> ptep_clear_flush_young().
>>
>>      if (nr == 1 && !pte_cont(__ptep_get(ptep))
>>          return __ptep_clear_flush_young(vma, addr, ptep);
> 
> Sure. That would follow the pattern in clear_young_dirty_ptes(). Please use the
> likely() hint as is done everywhere else:
> 
> 	if (likely(nr == 1 && !pte_cont(__ptep_get(ptep))))

Sure.

> I notice that ptep_test_and_clear_young() and ptep_clear_flush_young() are both
> testing aginst pte_valid_cont(). These could probably be relaxed to pte_cont()
> since it is implicit the the pte must be valid?

Yes, I think so. I can do a cleanup later in a separate patch.