Re: [PATCH] mm: fix KASAN build error due to p*d_populate_kernel()

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Thu, Aug 21, 2025 at 06:35:42PM +0900, Harry Yoo wrote:
> KASAN unconditionally references kasan_early_shadow_{p4d,pud}.
> However, these global variables may not exist depending on the number of
> page table levels. For example, if CONFIG_PGTABLE_LEVELS=3, both
> variables do not exist. Although KASAN may refernce non-existent
> variables, it didn't break builds because calls to {pgd,p4d}_populate()
> are optimized away at compile time.
>
> However, {pgd,p4d}_populate_kernel() is defined as a function regardless
> of the number of page table levels, so the compiler may not optimize
> them away. In this case, the following linker error occurs:
>
> ld.lld: error: undefined symbol: kasan_early_shadow_p4d
> >>> referenced by init.c:260 (/home/hyeyoo/mm-new/mm/kasan/init.c:260)
> >>>               mm/kasan/init.o:(kasan_populate_early_shadow) in archive vmlinux.a
> >>> referenced by init.c:260 (/home/hyeyoo/mm-new/mm/kasan/init.c:260)
> >>>               mm/kasan/init.o:(kasan_populate_early_shadow) in archive vmlinux.a
> >>> did you mean: kasan_early_shadow_pmd
> >>> defined in: vmlinux.a(mm/kasan/init.o)
>
> ld.lld: error: undefined symbol: kasan_early_shadow_pud
> >>> referenced by init.c:263 (/home/hyeyoo/mm-new/mm/kasan/init.c:263)
> >>>               mm/kasan/init.o:(kasan_populate_early_shadow) in archive vmlinux.a
> >>> referenced by init.c:263 (/home/hyeyoo/mm-new/mm/kasan/init.c:263)
> >>>               mm/kasan/init.o:(kasan_populate_early_shadow) in archive vmlinux.a
> >>> referenced by init.c:200 (/home/hyeyoo/mm-new/mm/kasan/init.c:200)
> >>>               mm/kasan/init.o:(zero_p4d_populate) in archive vmlinux.a
> >>> referenced 1 more times
>
> Therefore, to allow calls to {pgd,p4d}_populate_kernel() to be optimized
> out at compile time, define {pgd,p4d}_populate_kernel() as macros.
> This way, when pgd_populate() or p4d_populate() are simply empty macros,
> the corresponding *_populate_kernel() functions can also be optimized
> away.
>
> Signed-off-by: Harry Yoo <harry.yoo@oracle.com>

This looks good, other than the nit below re: a comment, I think when we
are doing this kind of thing it's necessary to spell out plainly why
exactly we're doing it because it's not obvious at first glance.

Anyway have checked locally and all good and LGTM code-wise so aside from
above:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>
> While the description is quite verbose, it is intended to be fold-merged
> into patch [1] of the page table sync series V5.
>
> [1] https://lore.kernel.org/linux-mm/20250818020206.4517-3-harry.yoo@oracle.com/
>
>  include/linux/pgalloc.h | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/pgalloc.h b/include/linux/pgalloc.h
> index 290ab864320f..8812f842978f 100644
> --- a/include/linux/pgalloc.h
> +++ b/include/linux/pgalloc.h
> @@ -5,20 +5,18 @@
>  #include <linux/pgtable.h>
>  #include <asm/pgalloc.h>
>
> -static inline void pgd_populate_kernel(unsigned long addr, pgd_t *pgd,
> -				       p4d_t *p4d)
> -{
> -	pgd_populate(&init_mm, pgd, p4d);
> -	if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_PGD_MODIFIED)
> -		arch_sync_kernel_mappings(addr, addr);
> -}
> +#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)
>
> -static inline void p4d_populate_kernel(unsigned long addr, p4d_t *p4d,
> -				       pud_t *pud)
> -{
> -	p4d_populate(&init_mm, p4d, pud);
> -	if (ARCH_PAGE_TABLE_SYNC_MASK & PGTBL_P4D_MODIFIED)
> -		arch_sync_kernel_mappings(addr, addr);
> -}
> +#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)
>

Can we have a quick comment above these explaining why they have to be
macros? Thanks!

>  #endif /* _LINUX_PGALLOC_H */
> --
> 2.43.0
>