Re: [PATCH v4 5/7] mm/khugepaged: Refactor the PTE state checks into a helper

On 13 Aug 2026, at 3:13, David Hildenbrand (Arm) wrote:

>>> +	if (!pte_present(pteval)) {
>>> +		if (ctx->max_ptes_swap < 0)
>>> +			return SCAN_PTE_NON_PRESENT;
>>> +		if (++ctx->nonpresent_ptes > ctx->max_ptes_swap) {
>>
>> So max_ptes_swap is actually max_ptes_nonpresent. But due to
>> khugepaged's max_ptes_swap config name, we just keep the variable and
>> related function names that way?
>
> Yes, that's how we handle it today, unfortunately, for historical reasons.
> Mistakes were made.
>
> I think we might be able to convert this code to actually only count swap
> entries, and simply reject any other ones (migration, hwpoison, ...), but that's
> something for another patch.

I agree. pte_none covers not faulted in PTEs (zero covers read-only anon)
and swap covers swapped out PTEs. Other non-present cases could be
1. transient (like migration) and they can wait until they are stable or
2. never considered (like hwpoison).

>
> [...]
>
>>
>> SCAN_SUCCEED + folio != NULL means to proceed with the pte, while
>> SCAN_SUCCEED + folio == NULL means to skip the pte.
>>
>> collapse_anon_pte_check() probably needs to document this?
>
> Not really, as you say it's straight forward if you look at it from what's
> actually happening:
>
> SCAN_SUCCEED + folio != NULL: check folio
> SCAN_SUCCEED + folio == NULL: don't check folio (no folio to check?)
>

Sounds good.

For this patch, feel free to add

Reviewed-by: Zi Yan <[email protected]>

Best Regards,
Yan, Zi