Re: [RFC PATCH 02/57] mm: add pte_none_or_zero()

[David Hildenbrand (Arm)]

On 8/17/26 00:45, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
> 
> A PTE that is none and one that maps the shared zeropage both stand for
> a page of zeroes the mapping does not own.  Code that cares only about
> the contents can treat the two alike.
> 
> Move khugepaged's local helper for that test to pgtable.h, below the
> is_zero_pfn() it is built on.
> 
> migrate_vma_insert_page() open-codes the same test on the slot it is
> about to fill.  Convert it.  It still tells none from the zeropage, but
> only to decide whether there is an old mapping to flush.
> 
> No functional change intended.
> 
> Assisted-by: Claude-Code:claude-opus-5
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> ---
>  include/linux/pgtable.h | 17 +++++++++++++++++
>  mm/khugepaged.c         |  7 -------
>  mm/migrate_device.c     |  9 ++-------
>  3 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 8c093c119e5a..bbee6d31f015 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -2064,6 +2064,23 @@ static inline struct page *_zero_page(unsigned long addr)
>  
>  #ifdef CONFIG_MMU
>  
> +/**
> + * pte_none_or_zero - Does this PTE map nothing, or the shared zeropage?
> + * @pte: The page table entry to test.
> + *
> + * A PTE that is none and one that maps the shared zeropage both stand for a
> + * page of zeroes the mapping does not own, so code that only cares about the
> + * contents can treat them alike.
> + *
> + * Return: %true if @pte is none or maps the shared zeropage.
> + */
> +static inline bool pte_none_or_zero(pte_t pte)
> +{
> +	if (pte_none(pte))
> +		return true;
> +	return pte_present(pte) && is_zero_pfn(pte_pfn(pte));
> +}

(casually skimming over some patches)

That's just a horrible function. :)

If there is no pte_zero() then there also shouldn't be a pte_none_or_zero().

And in the code-base we have "pte_t pte_zero = {0}" which actually makes sense,
but is not what we care about here.


Just have an additional helper like:

	pte_maps_zero_page()
	pte_is_zero_page()
	pte_maps_zero_folio()
	pte_is_zero_folio()
	pte_zero_page()
	pte_zero_folio()

And let the callers still spell both cases out.

if (pte_none(pte) || pte_is_zero_page(pte))
	/* Do something amazing */

(we have both is_zero_page() and is_zero_folio() I assume "zero page" is cleaner
as this thing might soon no longer be a folio)

-- 
Cheers,

David