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. These are introduced in a new header file, include/linux/pgalloc.h, so they can be called from common code.
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 Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") Suggested-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Harry Yoo harry.yoo@oracle.com --- include/linux/pgalloc.h | 24 ++++++++++++++++++++++++ include/linux/pgtable.h | 4 ++-- mm/kasan/init.c | 12 ++++++------ mm/percpu.c | 6 +++--- mm/sparse-vmemmap.c | 6 +++--- 5 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 include/linux/pgalloc.h
diff --git a/include/linux/pgalloc.h b/include/linux/pgalloc.h new file mode 100644 index 000000000000..290ab864320f --- /dev/null +++ b/include/linux/pgalloc.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_PGALLOC_H +#define _LINUX_PGALLOC_H + +#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); +} + +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); +} + +#endif /* _LINUX_PGALLOC_H */ diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index ba699df6ef69..0cf5c6c3e483 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -1469,8 +1469,8 @@ static inline void modify_prot_commit_ptes(struct vm_area_struct *vma, unsigned
/* * Architectures can set this mask to a combination of PGTBL_P?D_MODIFIED values - * and let generic vmalloc and ioremap code know when arch_sync_kernel_mappings() - * needs to be called. + * and let generic vmalloc, ioremap and page table update code know when + * arch_sync_kernel_mappings() needs to be called. */ #ifndef ARCH_PAGE_TABLE_SYNC_MASK #define ARCH_PAGE_TABLE_SYNC_MASK 0 diff --git a/mm/kasan/init.c b/mm/kasan/init.c index ced6b29fcf76..8fce3370c84e 100644 --- a/mm/kasan/init.c +++ b/mm/kasan/init.c @@ -13,9 +13,9 @@ #include <linux/mm.h> #include <linux/pfn.h> #include <linux/slab.h> +#include <linux/pgalloc.h>
#include <asm/page.h> -#include <asm/pgalloc.h>
#include "kasan.h"
@@ -191,7 +191,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr, pud_t *pud; pmd_t *pmd;
- p4d_populate(&init_mm, p4d, + p4d_populate_kernel(addr, p4d, lm_alias(kasan_early_shadow_pud)); pud = pud_offset(p4d, addr); pud_populate(&init_mm, pud, @@ -212,7 +212,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr, } else { p = early_alloc(PAGE_SIZE, NUMA_NO_NODE); pud_init(p); - p4d_populate(&init_mm, p4d, p); + p4d_populate_kernel(addr, p4d, p); } } zero_pud_populate(p4d, addr, next); @@ -251,10 +251,10 @@ int __ref kasan_populate_early_shadow(const void *shadow_start, * puds,pmds, so pgd_populate(), pud_populate() * is noops. */ - pgd_populate(&init_mm, pgd, + pgd_populate_kernel(addr, pgd, lm_alias(kasan_early_shadow_p4d)); p4d = p4d_offset(pgd, addr); - p4d_populate(&init_mm, p4d, + p4d_populate_kernel(addr, p4d, lm_alias(kasan_early_shadow_pud)); pud = pud_offset(p4d, addr); pud_populate(&init_mm, pud, @@ -273,7 +273,7 @@ int __ref kasan_populate_early_shadow(const void *shadow_start, if (!p) return -ENOMEM; } else { - pgd_populate(&init_mm, pgd, + pgd_populate_kernel(addr, pgd, early_alloc(PAGE_SIZE, NUMA_NO_NODE)); } } diff --git a/mm/percpu.c b/mm/percpu.c index d9cbaee92b60..a56f35dcc417 100644 --- a/mm/percpu.c +++ b/mm/percpu.c @@ -3108,7 +3108,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, #endif /* BUILD_EMBED_FIRST_CHUNK */
#ifdef BUILD_PAGE_FIRST_CHUNK -#include <asm/pgalloc.h> +#include <linux/pgalloc.h>
#ifndef P4D_TABLE_SIZE #define P4D_TABLE_SIZE PAGE_SIZE @@ -3134,13 +3134,13 @@ void __init __weak pcpu_populate_pte(unsigned long addr)
if (pgd_none(*pgd)) { p4d = memblock_alloc_or_panic(P4D_TABLE_SIZE, P4D_TABLE_SIZE); - pgd_populate(&init_mm, pgd, p4d); + pgd_populate_kernel(addr, pgd, p4d); }
p4d = p4d_offset(pgd, addr); if (p4d_none(*p4d)) { pud = memblock_alloc_or_panic(PUD_TABLE_SIZE, PUD_TABLE_SIZE); - p4d_populate(&init_mm, p4d, pud); + p4d_populate_kernel(addr, p4d, pud); }
pud = pud_offset(p4d, addr); diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c index 41aa0493eb03..dbd8daccade2 100644 --- a/mm/sparse-vmemmap.c +++ b/mm/sparse-vmemmap.c @@ -27,9 +27,9 @@ #include <linux/spinlock.h> #include <linux/vmalloc.h> #include <linux/sched.h> +#include <linux/pgalloc.h>
#include <asm/dma.h> -#include <asm/pgalloc.h> #include <asm/tlbflush.h>
#include "hugetlb_vmemmap.h" @@ -229,7 +229,7 @@ p4d_t * __meminit vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node) if (!p) return NULL; pud_init(p); - p4d_populate(&init_mm, p4d, p); + p4d_populate_kernel(addr, p4d, p); } return p4d; } @@ -241,7 +241,7 @@ pgd_t * __meminit vmemmap_pgd_populate(unsigned long addr, int node) void *p = vmemmap_alloc_block_zero(PAGE_SIZE, node); if (!p) return NULL; - pgd_populate(&init_mm, pgd, p); + pgd_populate_kernel(addr, pgd, p); } return pgd; }
On Mon, Aug 11, 2025 at 02:34:19PM +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:
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.
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. These are introduced in a new header file, include/linux/pgalloc.h, so they can be called from common code.
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 Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") Suggested-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Harry Yoo harry.yoo@oracle.com
Reviewed-by: Mike Rapoport (Microsoft) rppt@kernel.org
include/linux/pgalloc.h | 24 ++++++++++++++++++++++++ include/linux/pgtable.h | 4 ++-- mm/kasan/init.c | 12 ++++++------ mm/percpu.c | 6 +++--- mm/sparse-vmemmap.c | 6 +++--- 5 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 include/linux/pgalloc.h
diff --git a/include/linux/pgalloc.h b/include/linux/pgalloc.h new file mode 100644 index 000000000000..290ab864320f --- /dev/null +++ b/include/linux/pgalloc.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_PGALLOC_H +#define _LINUX_PGALLOC_H
+#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);+}
+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);+}
+#endif /* _LINUX_PGALLOC_H */ diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index ba699df6ef69..0cf5c6c3e483 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -1469,8 +1469,8 @@ static inline void modify_prot_commit_ptes(struct vm_area_struct *vma, unsigned /*
- Architectures can set this mask to a combination of PGTBL_P?D_MODIFIED values
- and let generic vmalloc and ioremap code know when arch_sync_kernel_mappings()
- needs to be called.
- and let generic vmalloc, ioremap and page table update code know when
*/
- arch_sync_kernel_mappings() needs to be called.
#ifndef ARCH_PAGE_TABLE_SYNC_MASK #define ARCH_PAGE_TABLE_SYNC_MASK 0 diff --git a/mm/kasan/init.c b/mm/kasan/init.c index ced6b29fcf76..8fce3370c84e 100644 --- a/mm/kasan/init.c +++ b/mm/kasan/init.c @@ -13,9 +13,9 @@ #include <linux/mm.h> #include <linux/pfn.h> #include <linux/slab.h> +#include <linux/pgalloc.h> #include <asm/page.h> -#include <asm/pgalloc.h> #include "kasan.h" @@ -191,7 +191,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr, pud_t *pud; pmd_t *pmd;
p4d_populate(&init_mm, p4d,
p4d_populate_kernel(addr, p4d, lm_alias(kasan_early_shadow_pud)); pud = pud_offset(p4d, addr); pud_populate(&init_mm, pud,@@ -212,7 +212,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr, } else { p = early_alloc(PAGE_SIZE, NUMA_NO_NODE); pud_init(p);
p4d_populate(&init_mm, p4d, p);
p4d_populate_kernel(addr, p4d, p); }@@ -251,10 +251,10 @@ int __ref kasan_populate_early_shadow(const void *shadow_start, * puds,pmds, so pgd_populate(), pud_populate() * is noops. */
pgd_populate(&init_mm, pgd,
pgd_populate_kernel(addr, pgd, lm_alias(kasan_early_shadow_p4d)); p4d = p4d_offset(pgd, addr);
p4d_populate(&init_mm, p4d,
p4d_populate_kernel(addr, p4d, lm_alias(kasan_early_shadow_pud)); pud = pud_offset(p4d, addr); pud_populate(&init_mm, pud,@@ -273,7 +273,7 @@ int __ref kasan_populate_early_shadow(const void *shadow_start, if (!p) return -ENOMEM; } else {
pgd_populate(&init_mm, pgd,
pgd_populate_kernel(addr, pgd, early_alloc(PAGE_SIZE, NUMA_NO_NODE)); }diff --git a/mm/percpu.c b/mm/percpu.c index d9cbaee92b60..a56f35dcc417 100644 --- a/mm/percpu.c +++ b/mm/percpu.c @@ -3108,7 +3108,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, #endif /* BUILD_EMBED_FIRST_CHUNK */ #ifdef BUILD_PAGE_FIRST_CHUNK -#include <asm/pgalloc.h> +#include <linux/pgalloc.h> #ifndef P4D_TABLE_SIZE #define P4D_TABLE_SIZE PAGE_SIZE @@ -3134,13 +3134,13 @@ void __init __weak pcpu_populate_pte(unsigned long addr) if (pgd_none(*pgd)) { p4d = memblock_alloc_or_panic(P4D_TABLE_SIZE, P4D_TABLE_SIZE);
pgd_populate(&init_mm, pgd, p4d);
pgd_populate_kernel(addr, pgd, p4d);p4d = p4d_offset(pgd, addr); if (p4d_none(*p4d)) { pud = memblock_alloc_or_panic(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
p4d_populate(&init_mm, p4d, pud);
p4d_populate_kernel(addr, p4d, pud);pud = pud_offset(p4d, addr); diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c index 41aa0493eb03..dbd8daccade2 100644 --- a/mm/sparse-vmemmap.c +++ b/mm/sparse-vmemmap.c @@ -27,9 +27,9 @@ #include <linux/spinlock.h> #include <linux/vmalloc.h> #include <linux/sched.h> +#include <linux/pgalloc.h> #include <asm/dma.h> -#include <asm/pgalloc.h> #include <asm/tlbflush.h> #include "hugetlb_vmemmap.h" @@ -229,7 +229,7 @@ p4d_t * __meminit vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node) if (!p) return NULL; pud_init(p);
p4d_populate(&init_mm, p4d, p);
p4d_populate_kernel(addr, p4d, p);} @@ -241,7 +241,7 @@ pgd_t * __meminit vmemmap_pgd_populate(unsigned long addr, int node) void *p = vmemmap_alloc_block_zero(PAGE_SIZE, node); if (!p) return NULL;
pgd_populate(&init_mm, pgd, p);
pgd_populate_kernel(addr, pgd, p);}
2.43.0
On Mon, Aug 11, 2025 at 02:34:19PM +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:
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.
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. These are introduced in a new header file, include/linux/pgalloc.h, so they can be called from common code.
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 Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") Suggested-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Harry Yoo harry.yoo@oracle.com
include/linux/pgalloc.h | 24 ++++++++++++++++++++++++
Could we put this in the correct place in MAINTAINERS please? I think MEMORY MANAGEMENT - CORE is correct, given the below file is there.
include/linux/pgtable.h | 4 ++--
Thanks!
On Mon, Aug 11, 2025 at 10:10:58AM +0100, Lorenzo Stoakes wrote:
On Mon, Aug 11, 2025 at 02:34:19PM +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:
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.
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. These are introduced in a new header file, include/linux/pgalloc.h, so they can be called from common code.
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 Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") Suggested-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Harry Yoo harry.yoo@oracle.com
include/linux/pgalloc.h | 24 ++++++++++++++++++++++++
Could we put this in the correct place in MAINTAINERS please?
Definitely yes!
Since this series will be backported to about five -stable kernels (v5.13.x and later), I will add that as part of a follow-up series that is not intended for backporting.
Does that sound okay?
I think MEMORY MANAGEMENT - CORE is correct, given the below file is there.
Thanks for confirming that!
On Mon, Aug 11, 2025 at 07:36:46PM +0900, Harry Yoo wrote:
include/linux/pgalloc.h | 24 ++++++++++++++++++++++++
Could we put this in the correct place in MAINTAINERS please?
Definitely yes!
Since this series will be backported to about five -stable kernels (v5.13.x and later), I will add that as part of a follow-up series that is not intended for backporting.
Does that sound okay?
Yes that's fine thanks!
On Mon, Aug 11, 2025 at 02:34:19PM +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:
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.
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. These are introduced in a new header file, include/linux/pgalloc.h, so they can be called from common code.
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
Well, arm defines ARCH_PAGE_TABLE_SYNC_MASK in arch/arm/include/asm/page.h. But it aliases this to PGTBL_PMD_MODIFIED so will remain unaffected :)
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 Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") Suggested-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Harry Yoo harry.yoo@oracle.com
include/linux/pgalloc.h | 24 ++++++++++++++++++++++++ include/linux/pgtable.h | 4 ++-- mm/kasan/init.c | 12 ++++++------ mm/percpu.c | 6 +++--- mm/sparse-vmemmap.c | 6 +++--- 5 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 include/linux/pgalloc.h
diff --git a/include/linux/pgalloc.h b/include/linux/pgalloc.h new file mode 100644 index 000000000000..290ab864320f --- /dev/null +++ b/include/linux/pgalloc.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_PGALLOC_H +#define _LINUX_PGALLOC_H
+#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)
Hm, ARCH_PAGE_TABLE_SYNC_MASK is only defined for x86 2, 3 page level and arm. I see:
#ifndef ARCH_PAGE_TABLE_SYNC_MASK #define ARCH_PAGE_TABLE_SYNC_MASK 0 #endif
In linux/vmalloc.h, but you're not importing that?
It sucks that that there is there, but maybe you need to #include <linux/vmalloc.h> for this otherwise this could be broken on other arches?
You may be getting lucky with nested header includes that causes this to be picked up somewhere for you, or having it only declared for arches that define it, but we should probably make this explicit.
Also arch_sync_kernel_mappings() is defined in linux/vmalloc.h so seems sensible.
arch_sync_kernel_mappings(addr, addr);+}
+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);
It's kind of weird we don't have this defined as a function for many arches, (weird as well that we declare it in... vmalloc.h but I guess one for follow up cleanups that).
But I see from the comment:
/* * There is no default implementation for arch_sync_kernel_mappings(). It is * relied upon the compiler to optimize calls out if ARCH_PAGE_TABLE_SYNC_MASK * is 0. */
So this seems intended... :)
The rest of this seems sensible, nice cleanup!
+}
+#endif /* _LINUX_PGALLOC_H */ diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index ba699df6ef69..0cf5c6c3e483 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -1469,8 +1469,8 @@ static inline void modify_prot_commit_ptes(struct vm_area_struct *vma, unsigned
/*
- Architectures can set this mask to a combination of PGTBL_P?D_MODIFIED values
- and let generic vmalloc and ioremap code know when arch_sync_kernel_mappings()
- needs to be called.
- and let generic vmalloc, ioremap and page table update code know when
*/
- arch_sync_kernel_mappings() needs to be called.
#ifndef ARCH_PAGE_TABLE_SYNC_MASK #define ARCH_PAGE_TABLE_SYNC_MASK 0 diff --git a/mm/kasan/init.c b/mm/kasan/init.c index ced6b29fcf76..8fce3370c84e 100644 --- a/mm/kasan/init.c +++ b/mm/kasan/init.c @@ -13,9 +13,9 @@ #include <linux/mm.h> #include <linux/pfn.h> #include <linux/slab.h> +#include <linux/pgalloc.h>
#include <asm/page.h> -#include <asm/pgalloc.h>
#include "kasan.h"
@@ -191,7 +191,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr, pud_t *pud; pmd_t *pmd;
p4d_populate(&init_mm, p4d,
p4d_populate_kernel(addr, p4d, lm_alias(kasan_early_shadow_pud)); pud = pud_offset(p4d, addr); pud_populate(&init_mm, pud,@@ -212,7 +212,7 @@ static int __ref zero_p4d_populate(pgd_t *pgd, unsigned long addr, } else { p = early_alloc(PAGE_SIZE, NUMA_NO_NODE); pud_init(p);
p4d_populate(&init_mm, p4d, p);
p4d_populate_kernel(addr, p4d, p); }@@ -251,10 +251,10 @@ int __ref kasan_populate_early_shadow(const void *shadow_start, * puds,pmds, so pgd_populate(), pud_populate() * is noops. */
pgd_populate(&init_mm, pgd,
pgd_populate_kernel(addr, pgd, lm_alias(kasan_early_shadow_p4d)); p4d = p4d_offset(pgd, addr);
p4d_populate(&init_mm, p4d,
p4d_populate_kernel(addr, p4d, lm_alias(kasan_early_shadow_pud)); pud = pud_offset(p4d, addr); pud_populate(&init_mm, pud,@@ -273,7 +273,7 @@ int __ref kasan_populate_early_shadow(const void *shadow_start, if (!p) return -ENOMEM; } else {
pgd_populate(&init_mm, pgd,
pgd_populate_kernel(addr, pgd, early_alloc(PAGE_SIZE, NUMA_NO_NODE)); }diff --git a/mm/percpu.c b/mm/percpu.c index d9cbaee92b60..a56f35dcc417 100644 --- a/mm/percpu.c +++ b/mm/percpu.c @@ -3108,7 +3108,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, #endif /* BUILD_EMBED_FIRST_CHUNK */
#ifdef BUILD_PAGE_FIRST_CHUNK -#include <asm/pgalloc.h> +#include <linux/pgalloc.h>
#ifndef P4D_TABLE_SIZE #define P4D_TABLE_SIZE PAGE_SIZE @@ -3134,13 +3134,13 @@ void __init __weak pcpu_populate_pte(unsigned long addr)
if (pgd_none(*pgd)) { p4d = memblock_alloc_or_panic(P4D_TABLE_SIZE, P4D_TABLE_SIZE);
pgd_populate(&init_mm, pgd, p4d);
pgd_populate_kernel(addr, pgd, p4d);}
p4d = p4d_offset(pgd, addr); if (p4d_none(*p4d)) { pud = memblock_alloc_or_panic(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
p4d_populate(&init_mm, p4d, pud);
p4d_populate_kernel(addr, p4d, pud);}
pud = pud_offset(p4d, addr);
diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c index 41aa0493eb03..dbd8daccade2 100644 --- a/mm/sparse-vmemmap.c +++ b/mm/sparse-vmemmap.c @@ -27,9 +27,9 @@ #include <linux/spinlock.h> #include <linux/vmalloc.h> #include <linux/sched.h> +#include <linux/pgalloc.h>
#include <asm/dma.h> -#include <asm/pgalloc.h> #include <asm/tlbflush.h>
#include "hugetlb_vmemmap.h" @@ -229,7 +229,7 @@ p4d_t * __meminit vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node) if (!p) return NULL; pud_init(p);
p4d_populate(&init_mm, p4d, p);
p4d_populate_kernel(addr, p4d, p);} @@ -241,7 +241,7 @@ pgd_t * __meminit vmemmap_pgd_populate(unsigned long addr, int node) void *p = vmemmap_alloc_block_zero(PAGE_SIZE, node); if (!p) return NULL;
pgd_populate(&init_mm, pgd, p);
pgd_populate_kernel(addr, pgd, p);}
2.43.0
On Mon, Aug 11, 2025 at 12:38:37PM +0100, Lorenzo Stoakes wrote:
On Mon, Aug 11, 2025 at 02:34:19PM +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:
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.
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. These are introduced in a new header file, include/linux/pgalloc.h, so they can be called from common code.
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
Hi Lorenzo, thanks for looking at this!
Well, arm defines ARCH_PAGE_TABLE_SYNC_MASK in arch/arm/include/asm/page.h. But it aliases this to PGTBL_PMD_MODIFIED so will remain unaffected :)
Oh, here I just intended to explain why I didn't implement {pud,pmd}_populate_kernel().
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 Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") Suggested-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Harry Yoo harry.yoo@oracle.com
include/linux/pgalloc.h | 24 ++++++++++++++++++++++++ include/linux/pgtable.h | 4 ++-- mm/kasan/init.c | 12 ++++++------ mm/percpu.c | 6 +++--- mm/sparse-vmemmap.c | 6 +++--- 5 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 include/linux/pgalloc.h
diff --git a/include/linux/pgalloc.h b/include/linux/pgalloc.h new file mode 100644 index 000000000000..290ab864320f --- /dev/null +++ b/include/linux/pgalloc.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_PGALLOC_H +#define _LINUX_PGALLOC_H
+#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)
Hm, ARCH_PAGE_TABLE_SYNC_MASK is only defined for x86 2, 3 page level and arm. I see:
#ifndef ARCH_PAGE_TABLE_SYNC_MASK #define ARCH_PAGE_TABLE_SYNC_MASK 0 #endif
In linux/vmalloc.h, but you're not importing that?
Patch 1 moves it from linux/vmalloc.h to linux/pgtable.h, and linux/pgalloc.h includes linux/pgtable.h.
It sucks that that there is there, but maybe you need to #include <linux/vmalloc.h> for this otherwise this could be broken on other arches?
You may be getting lucky with nested header includes that causes this to be picked up somewhere for you, or having it only declared for arches that define it, but we should probably make this explicit.
...so I don't think I'm missing necessary header includes even on other architectures?
Also arch_sync_kernel_mappings() is defined in linux/vmalloc.h so seems sensible.
Also moved to linux/pgtable.h.
arch_sync_kernel_mappings(addr, addr);+}
+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);It's kind of weird we don't have this defined as a function for many arches,
That's really a mystery :)
I have no idea why other architectures don't handle this.
(At least on 64 bit arches) In theory I think only a few architectures (like arm64 where a kernel page table is shared between tasks) don't have to implement this.
Probably because it's a bit niche bug to hit? (vmemmap, direct mapping, vmalloc/vmap area can span multiple PGD ranges) AND (populating some PGD entries is done after boot process (e.g. memory hot-plug or vmalloc())).
(weird as well that we declare it in... vmalloc.h but I guess one for follow up cleanups that).
But I see from the comment:
/*
- There is no default implementation for arch_sync_kernel_mappings(). It is
- relied upon the compiler to optimize calls out if ARCH_PAGE_TABLE_SYNC_MASK
- is 0.
*/
So this seems intended... :)
The rest of this seems sensible, nice cleanup!
Thanks for looking at!
On Mon, Aug 11, 2025 at 09:12:08PM +0900, Harry Yoo wrote:
On Mon, Aug 11, 2025 at 12:38:37PM +0100, Lorenzo Stoakes wrote:
On Mon, Aug 11, 2025 at 02:34:19PM +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:
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.
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. These are introduced in a new header file, include/linux/pgalloc.h, so they can be called from common code.
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
Hi Lorenzo, thanks for looking at this!
Well, arm defines ARCH_PAGE_TABLE_SYNC_MASK in arch/arm/include/asm/page.h. But it aliases this to PGTBL_PMD_MODIFIED so will remain unaffected :)
Oh, here I just intended to explain why I didn't implement {pud,pmd}_populate_kernel().
I'd add that arm handles PGTBL_PMD_MODIFIED and therefore remains unaffected just to be super clear.
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 Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") Suggested-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Harry Yoo harry.yoo@oracle.com
Given that I missed you fixed the vmalloc.h thing, this LGTM so:
Reviewed-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
include/linux/pgalloc.h | 24 ++++++++++++++++++++++++ include/linux/pgtable.h | 4 ++-- mm/kasan/init.c | 12 ++++++------ mm/percpu.c | 6 +++--- mm/sparse-vmemmap.c | 6 +++--- 5 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 include/linux/pgalloc.h
diff --git a/include/linux/pgalloc.h b/include/linux/pgalloc.h new file mode 100644 index 000000000000..290ab864320f --- /dev/null +++ b/include/linux/pgalloc.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_PGALLOC_H +#define _LINUX_PGALLOC_H
+#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)
Hm, ARCH_PAGE_TABLE_SYNC_MASK is only defined for x86 2, 3 page level and arm. I see:
#ifndef ARCH_PAGE_TABLE_SYNC_MASK #define ARCH_PAGE_TABLE_SYNC_MASK 0 #endif
In linux/vmalloc.h, but you're not importing that?
Patch 1 moves it from linux/vmalloc.h to linux/pgtable.h, and linux/pgalloc.h includes linux/pgtable.h.
It sucks that that there is there, but maybe you need to #include <linux/vmalloc.h> for this otherwise this could be broken on other arches?
You may be getting lucky with nested header includes that causes this to be picked up somewhere for you, or having it only declared for arches that define it, but we should probably make this explicit.
...so I don't think I'm missing necessary header includes even on other architectures?
Also arch_sync_kernel_mappings() is defined in linux/vmalloc.h so seems sensible.
Also moved to linux/pgtable.h.
Ah yeah damn, I missed that you do that there, ok well that's fine then :)
arch_sync_kernel_mappings(addr, addr);+}
+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);It's kind of weird we don't have this defined as a function for many arches,
That's really a mystery :)
I have no idea why other architectures don't handle this.
(At least on 64 bit arches) In theory I think only a few architectures (like arm64 where a kernel page table is shared between tasks) don't have to implement this.
Probably because it's a bit niche bug to hit? (vmemmap, direct mapping, vmalloc/vmap area can span multiple PGD ranges) AND (populating some PGD entries is done after boot process (e.g. memory hot-plug or vmalloc())).
No comment is more why we don't just do a standard:
#ifndef xxx #define xxx (0) #endif
Or something. Just odd.
(weird as well that we declare it in... vmalloc.h but I guess one for follow up cleanups that).
But I see from the comment:
/*
- There is no default implementation for arch_sync_kernel_mappings(). It is
- relied upon the compiler to optimize calls out if ARCH_PAGE_TABLE_SYNC_MASK
- is 0.
*/
So this seems intended... :)
The rest of this seems sensible, nice cleanup!
Thanks for looking at!
-- Cheers, Harry / Hyeonggon
On Mon, Aug 11, 2025 at 01:18:12PM +0100, Lorenzo Stoakes wrote:
On Mon, Aug 11, 2025 at 09:12:08PM +0900, Harry Yoo wrote:
On Mon, Aug 11, 2025 at 12:38:37PM +0100, Lorenzo Stoakes wrote:
On Mon, Aug 11, 2025 at 02:34:19PM +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:
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.
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. These are introduced in a new header file, include/linux/pgalloc.h, so they can be called from common code.
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
Hi Lorenzo, thanks for looking at this!
Well, arm defines ARCH_PAGE_TABLE_SYNC_MASK in arch/arm/include/asm/page.h. But it aliases this to PGTBL_PMD_MODIFIED so will remain unaffected :)
Oh, here I just intended to explain why I didn't implement {pud,pmd}_populate_kernel().
I'd add that arm handles PGTBL_PMD_MODIFIED and therefore remains unaffected just to be super clear.
Will do:
This change currently targets only x86_64, so only PGD and P4D level helpers are introduced. Currently, these helpers are no-ops since no architecture sets PGTBL_{PGD,P4D}_MODIFIED in ARCH_PAGE_TABLE_SYNC_MASK.
In theory, PUD and PMD level helpers can be added later if needed by other architectures. For now, 32-bit architectures (x86-32 and arm) only handle PGTBL_PMD_MODIFIED, so p*d_populate_kernel() will never affect them unless we introduce a PMD level helper.
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 Fixes: 8d400913c231 ("x86/vmemmap: handle unpopulated sub-pmd ranges") Suggested-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Harry Yoo harry.yoo@oracle.com
Given that I missed you fixed the vmalloc.h thing, this LGTM so:
Reviewed-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
Thanks!
On Tue, Aug 12, 2025 at 06:53:49PM +0900, Harry Yoo wrote:
I'd add that arm handles PGTBL_PMD_MODIFIED and therefore remains unaffected just to be super clear.
Will do:
This change currently targets only x86_64, so only PGD and P4D level helpers are introduced. Currently, these helpers are no-ops since no architecture sets PGTBL_{PGD,P4D}_MODIFIED in ARCH_PAGE_TABLE_SYNC_MASK.
In theory, PUD and PMD level helpers can be added later if needed by other architectures. For now, 32-bit architectures (x86-32 and arm) only handle PGTBL_PMD_MODIFIED, so p*d_populate_kernel() will never affect them unless we introduce a PMD level helper.
Sounds good!
Cheers, Lorenzo
linux-stable-mirror@lists.linaro.org
-
Harry Yoo -
Lorenzo Stoakes -
Mike Rapoport