Re: [PATCH v3 03/16] mm: avoid unnecessary uses of is_swap_pte()

Zi Yan
2025-11-12T16:11:21Z

On 11 Nov 2025, at 21:58, Zi Yan wrote:

> On 10 Nov 2025, at 17:21, Lorenzo Stoakes wrote:
>
>> There's an established convention in the kernel that we treat PTEs as
>> containing swap entries (and the unfortunately named non-swap swap entries)
>> should they be neither empty (i.e. pte_none() evaluating true) nor present
>> (i.e. pte_present() evaluating true).
>>
>> However, there is some inconsistency in how this is applied, as we also
>> have the is_swap_pte() helper which explicitly performs this check:
>>
>> /* check whether a pte points to a swap entry */
>> static inline int is_swap_pte(pte_t pte)
>> {
>> return !pte_none(pte) && !pte_present(pte);
>> }
>>
>> As this represents a predicate, and it's logical to assume that in order to
>> establish that a PTE entry can correctly be manipulated as a swap/non-swap
>> entry, this predicate seems as if it must first be checked.
>>
>> But we instead, we far more often utilise the established convention of
>> checking pte_none() / pte_present() before operating on entries as if they
>> were swap/non-swap.
>>
>> This patch works towards correcting this inconsistency by removing all uses
>> of is_swap_pte() where we are already in a position where we perform
>> pte_none()/pte_present() checks anyway or otherwise it is clearly logical
>> to do so.

BTW, I wonder if we could use switch + enum and compiler to prevent future
inconsistencies.

Basically,

enum PTE_State {
PTE_PRESENT,
PTE_NONE,
PTE_SOFTLEAF,
};

enum PTE_State get_pte_state(pte_t pte)
{
if (pte_present(pte))
return PTE_PRESENT;
if (pte_none(pte))
return PTE_NONE;
return PTE_SOFTLEAF;
}

in any code handling pte:

switch (get_pte_state(pte)):
case PTE_PRESENT:
break;
case PTE_NONE:
break;
case PTE_SOFTLEAF:
break;
}

And compiler will yell at you if any enum is missing in the switch case.

Just an idea came to my mind when I am reading the commit message.
Feel free to ignore it. :)

Best Regards,
Yan, Zi