Re: [PATCH v5 03/25] mm: Make pte_next_pfn() a wrapper around pte_advance_pfn()

David Hildenbrand
2024-02-12T12:14:29Z

On 02.02.24 09:07, Ryan Roberts wrote:
> The goal is to be able to advance a PTE by an arbitrary number of PFNs.
> So introduce a new API that takes a nr param.
>
> We are going to remove pte_next_pfn() and replace it with
> pte_advance_pfn(). As a first step, implement pte_next_pfn() as a
> wrapper around pte_advance_pfn() so that we can incrementally switch the
> architectures over. Once all arches are moved over, we will change all
> the core-mm callers to call pte_advance_pfn() directly and remove the
> wrapper.
>
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
> include/linux/pgtable.h | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 5e7eaf8f2b97..815d92dcb96b 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -214,9 +214,15 @@ static inline int pmd_dirty(pmd_t pmd)
>
>
> #ifndef pte_next_pfn
> +#ifndef pte_advance_pfn
> +static inline pte_t pte_advance_pfn(pte_t pte, unsigned long nr)
> +{
> + return __pte(pte_val(pte) + (nr << PFN_PTE_SHIFT));
> +}
> +#endif
> static inline pte_t pte_next_pfn(pte_t pte)
> {
> - return __pte(pte_val(pte) + (1UL << PFN_PTE_SHIFT));
> + return pte_advance_pfn(pte, 1);
> }
> #endif
>

I do wonder if we simply want to leave pte_next_pfn() around? Especially
patch #4, #6 don't really benefit from the change? So are the other
set_ptes() implementations.

That is, only convert all pte_next_pfn()->pte_advance_pfn(), and leave a
pte_next_pfn() macro in place.

Any downsides to that? This patch here would become:

#ifndef pte_advance_pfn
static inline pte_t pte_advance_pfn(pte_t pte, unsigned long nr)
{
return __pte(pte_val(pte) + (nr << PFN_PTE_SHIFT));
}
#endif

#ifndef pte_next_pfn
#define pte_next_pfn(pte) pte_advance_pfn(pte, 1)
#endif

As you convert the three arches, make them define pte_advance_pfn and
udnefine pte_next_pfn. in the end, you can drop the #ifdef around
pte_next_pfn here.

--
Cheers,

David / dhildenb