Re: [PATCH v3 mm-hotfixes 2/5] mm: introduce and use {pgd,p4d}_populate_kernel()

[Harry Yoo <harry.yoo@oracle.com>]

Adding some comment after looking at a kernel test robot report [1]
that seems to be rejected by linux-mm.

[1] https://lore.kernel.org/oe-kbuild-all/202507290917.T24WIcvt-lkp@intel.com

I will post the next version with it fixed and including only first
three patches that will be backported to -stable. (and post last 2
patches as a follow-up after that)

On Fri, Jul 25, 2025 at 10:21:03AM +0900, Harry Yoo wrote:
> Introduce and use {pgd,p4d}_populate_kernel() in core MM code when
> populating PGD and P4D entries for the kernel address space.
> These helpers ensure proper synchronization of page tables when
> updating the kernel portion of top-level page tables.
> 
> Until now, the kernel has relied on each architecture to handle
> synchronization of top-level page tables in an ad-hoc manner.
> For example, see commit 9b861528a801 ("x86-64, mem: Update all PGDs for
> direct mapping and vmemmap mapping changes").
> 
> However, this approach has proven fragile for following reasons:
> 
>   1) It is easy to forget to perform the necessary page table
>      synchronization when introducing new changes.
>      For instance, commit 4917f55b4ef9 ("mm/sparse-vmemmap: improve memory
>      savings for compound devmaps") overlooked the need to synchronize
>      page tables for the vmemmap area.
> 
>   2) It is also easy to overlook that the vmemmap and direct mapping areas
>      must not be accessed before explicit page table synchronization.
>      For example, commit 8d400913c231 ("x86/vmemmap: handle unpopulated
>      sub-pmd ranges")) caused crashes by accessing the vmemmap area
>      before calling sync_global_pgds().
> 
> To address this, as suggested by Dave Hansen, introduce _kernel() variants
> of the page table population helpers, which invoke architecture-specific
> hooks to properly synchronize page tables.
> 
> They reuse existing infrastructure for vmalloc and ioremap.
> Synchronization requirements are determined by ARCH_PAGE_TABLE_SYNC_MASK,
> and the actual synchronization is performed by arch_sync_kernel_mappings().
> 
> This change currently targets only x86_64, so only PGD and P4D level
> helpers are introduced. In theory, PUD and PMD level helpers can be added
> later if needed by other architectures.
> 
> Currently this is a no-op, since no architecture sets
> PGTBL_{PGD,P4D}_MODIFIED in ARCH_PAGE_TABLE_SYNC_MASK.
> 
> Cc: stable@vger.kernel.org
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Harry Yoo <harry.yoo@oracle.com>
> ---
>  include/asm-generic/pgalloc.h | 16 ++++++++++++++++
>  include/linux/pgtable.h       |  4 ++--
>  mm/kasan/init.c               | 10 +++++-----
>  mm/percpu.c                   |  4 ++--
>  mm/sparse-vmemmap.c           |  4 ++--
>  5 files changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
> index 3c8ec3bfea44..fc0ab8eed5a6 100644
> --- a/include/asm-generic/pgalloc.h
> +++ b/include/asm-generic/pgalloc.h
> @@ -4,6 +4,8 @@
>  
>  #ifdef CONFIG_MMU
>  
> +#include <linux/pgtable.h>
> +
>  #define GFP_PGTABLE_KERNEL	(GFP_KERNEL | __GFP_ZERO)
>  #define GFP_PGTABLE_USER	(GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
>  
> @@ -296,6 +298,20 @@ static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
>  }
>  #endif
>  
> +#define pgd_populate_kernel(addr, pgd, p4d)			\
> +do {								\
> +	pgd_populate(&init_mm, pgd, p4d);			\
> +	if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PGD_MODIFIED)	\
> +		arch_sync_kernel_mappings(addr, addr);		\
> +} while (0)
> +
> +#define p4d_populate_kernel(addr, p4d, pud)			\
> +do {								\
> +	p4d_populate(&init_mm, p4d, pud);			\
> +	if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_P4D_MODIFIED)	\
> +		arch_sync_kernel_mappings(addr, addr);		\
> +} while (0)
> +
>  #endif /* CONFIG_MMU */

The report [1] complains that p*d_populate_kernel() is not defined:

   mm/percpu.c: In function 'pcpu_populate_pte':
>> mm/percpu.c:3137:17: error: implicit declaration of function 'pgd_populate_kernel'; did you mean 'pmd_populate_kernel'? [-Wimplicit-function-declaration]
    3137 |                 pgd_populate_kernel(addr, pgd, p4d);
         |                 ^~~~~~~~~~~~~~~~~~~
         |                 pmd_populate_kernel
>> mm/percpu.c:3143:17: error: implicit declaration of function 'p4d_populate_kernel'; did you mean 'pmd_populate_kernel'? [-Wimplicit-function-declaration]
    3143 |                 p4d_populate_kernel(addr, p4d, pud);
         |                 ^~~~~~~~~~~~~~~~~~~
         |                 pmd_populate_kernel
--
   mm/sparse-vmemmap.c: In function 'vmemmap_p4d_populate':
>> mm/sparse-vmemmap.c:232:17: error: implicit declaration of function 'p4d_populate_kernel'; did you mean 'pmd_populate_kernel'? [-Wimplicit-function-declaration]
     232 |                 p4d_populate_kernel(addr, p4d, p);
         |                 ^~~~~~~~~~~~~~~~~~~
         |                 pmd_populate_kernel
   mm/sparse-vmemmap.c: In function 'vmemmap_pgd_populate':
>> mm/sparse-vmemmap.c:244:17: error: implicit declaration of function 'pgd_populate_kernel'; did you mean 'pmd_populate_kernel'? [-Wimplicit-function-declaration]
     244 |                 pgd_populate_kernel(addr, pgd, p);
         |                 ^~~~~~~~~~~~~~~~~~~
         |                 pmd_populate_kernel


I had incorrectly assumed that asm/pgalloc.h in all architecture would
include asm-generic/pgalloc.h. That's true for most architectures,
but a few architectures (sparc, powerpc, s390) don't do that.

As it turns out the assumption isn't valid on all arches, I think the
right thing to do now is to introduce include/linux/pgalloc.h and put
these helpers there, and include it from common code.

-- 
Cheers,
Harry / Hyeonggon