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

[David Hildenbrand (Arm)]

>> +	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.

[...]

> 
> 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?)

-- 
Cheers,

David