Re: [PATCH] x86/vmemmap: Synchronize with global pgds if populating init_mm's pgd

[Dave Hansen]

On 2/19/25 22:41, Gwan-gyeong Mun wrote:
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 01ea7c6df303..7935859bcc21 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1498,6 +1498,54 @@ static long __meminitdata addr_start, addr_end;
>  static void __meminitdata *p_start, *p_end;
>  static int __meminitdata node_start;
>  
> +static void * __meminit vmemmap_alloc_block_zero(unsigned long size, int node)
> +{
> +	void *p = vmemmap_alloc_block(size, node);
> +
> +	if (!p)
> +		return NULL;
> +	memset(p, 0, size);
> +
> +	return p;
> +}

This is a pure copy and paste of the generic function. I assume this is
because the mm/sparse-vmemmap.c is static. But this kind of copying is
really unfortunate.

...
> +pgd_t * __meminit vmemmap_pgd_populate(unsigned long addr, int node)
> +{
> +	pgd_t *pgd = pgd_offset_k(addr);
> +
> +	if (pgd_none(*pgd)) {
> +		void *p = vmemmap_alloc_block_zero(PAGE_SIZE, node);
> +
> +		if (!p)
> +			return NULL;
> +
> +		pgd_populate(&init_mm, pgd, p);
> +		sync_global_pgds(addr, addr);
> +	}
> +
> +	return pgd;
> +}

I'd _really_ like to find another way to do this. We really don't want
to add copy-and-paste versions of generic functions that we now need to
maintain on the x86 side.

The _best_ way is probably to create some p*d_populate_kernel() helpers:

void pgd_populate_kernel(unsigned long addr, pgd_t *pgd, p4d_t *p4d)
{
	pgd_populate(&init_mm, pgd, p4d);
	arch_sync_global_pgds(addr, addr+something);
}

and move over most of the callers of:

	p*d_populate(&init_mm, ...);

Because I suspect that'll fix your issue _and_ solve the generic class
of issues where folks populate a kernel page table entry but forget to
call sync_global_pgds().