Re: [RFC V1 PATCH mm-hotfixes 1/3] mm: introduce and use {pgd,p4d}_populate_kernel()

Mike Rapoport
2025-07-13T17:56:28Z

On Sun, Jul 13, 2025 at 08:39:53PM +0900, Harry Yoo wrote:
> On Fri, Jul 11, 2025 at 06:18:44PM +0200, David Hildenbrand wrote:
> > On 09.07.25 15:16, Harry Yoo wrote:
> > > Intrdocue and use {pgd,p4d}_pouplate_kernel() in core MM code when
> > > populating PGD and P4D entries corresponding to the kernel address
> > > space. The main purpose of these helpers is to ensure synchronization of
> > > the kernel portion of the top-level page tables whenever such an entry
> > > is populated.
> > >
> > > 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, as it's easy to forget to
> > > perform the necessary synchronization when introducing new changes.
> > >
> > > To address this, introduce _kernel() varients of the page table
> >
> > s/varients/variants/
>
> Will fix. Thanks.
>
> > > population helpers that invoke architecture-specific hooks to properly
> > > synchronize the page tables.
> >
> > I was expecting to see the sync be done in common code -- such that it
> > cannot be missed :)
>
> You mean something like an arch-independent implementation of
> sync_global_pgds()?
>
> That would be a "much more robust" approach ;)
>
> To do that, the kernel would need to maintain a list of page tables that
> have kernel portion mapped and perform the sync in the common code.
>
> But determining which page tables to add to the list would be highly
> architecture-specific. For example, I think some architectures use separate
> page tables for kernel space, unlike x86 (e.g., arm64 TTBR1, SPARC) and
> user page tables should not be affected.

sync_global_pgds() can be still implemented per architecture, but it can be
called from the common code.
We already have something like that for vmalloc that calls
arch_sync_kernel_mappings(). It's implemented only by x86-32 and arm, other
architectures do not define it.

> While doing the sync in common code might be a more robust option
> in the long term, I'm afraid that making it work correctly across
> all architectures would be challenging, due to differences in how each
> architecture manages the kernel address space.
>
> > But it's really just rerouting to the arch code where the sync can be done,
> > correct?
>
> Yes, that's correct.
>
> Thanks for taking a look!
>
> --
> Cheers,
> Harry / Hyeonggon

--
Sincerely yours,
Mike.