Hi Dave, > On 9/2/26 04:56, Yeoreum Yun wrote: > > The behaviour of pXd_page() will change with generic compile-time folded > > page tables by disallowing its use and triggering a compile-time error > > when it's used improperly, ensuring that the actual pXd_page() is used > > instead. > > > > To prepare fot that, skip collapse_pud_page() when > > CONFIG_X86_DIRECT_GBPAGES is disabled. > > Nit: this doesn't explain how the change actually fixes anything or what > the specific problem being solved is. > > I think you want to say something along the lines of: > > collapse_pud_page() uses pud_page() in a way which will soon > trigger a compile-time error on configs that have a folded pud. > > The code which will generate that error is actually unreachable > on those configs because 'direct_gbpages' is always 0 there. > However, the compiler does not know that because > 'direct_gbpages' is a normal integer from a separate compilation > unit. > > Make the compiler aware when most of collapse_pud_page() is > unreachable by adding a Kconfig check. This ensures it will not > trip the errors when they are introduced. It probably also trims > the kernel image down a wee bit too as a side benefit.
Yes.. Sorry for my poor commit message. > > Maybe I should just merge something like the attached patch. I think it > would solve your problem and make things generally cleaner too. > > There is an existing variable (direct_gbpages) that says whether the > kernel can and should use 1G pages in the direct map. It is driven > by a bunch of other machinery. At least: > > 1. Hardware support for 1G pages > 2. Kconfig support for 1G direct mappings > 3. Kernel command line overrides > > Most code just checks the 'direct_gbpages' variable itself. But this > prevents compiler optimization in cases where 1G mappings are > compile-time disabled (via X86_DIRECT_GBPAGES). > > Add a helper to replace 'direct_gbpages' checks. Check the Kconfig > option and base CPU support before looking at the variable. > > This lets the compiler optimize things better, especially > collapse_pud_page() where most of the function can now be optimized > away. Yeap. I've tested with your patch and it works for me! Thanks! Reviewed-by: Yeoreum Yun <[email protected]> Tested-by: Yeoreum Yun <[email protected]> > > --- > > b/arch/x86/include/asm/pgtable.h | 14 ++++++++++++++ > b/arch/x86/kernel/cpu/common.c | 2 +- > b/arch/x86/kernel/machine_kexec_64.c | 2 +- > b/arch/x86/mm/init.c | 2 +- > b/arch/x86/mm/pat/set_memory.c | 4 ++-- > 5 files changed, 19 insertions(+), 5 deletions(-) > > diff -puN arch/x86/include/asm/pgtable.h~direct_gbpages-compiletime > arch/x86/include/asm/pgtable.h > --- a/arch/x86/include/asm/pgtable.h~direct_gbpages-compiletime > 2026-09-02 06:53:57.835733398 -0700 > +++ b/arch/x86/include/asm/pgtable.h 2026-09-02 08:17:01.434727771 -0700 > @@ -1163,6 +1163,20 @@ static inline int pgd_none(pgd_t pgd) > #ifndef __ASSEMBLER__ > > extern int direct_gbpages; > +static inline bool direct_gbpages_enabled(void) > +{ > + /* Check the direct map config option: */ > + if (!IS_ENABLED(CONFIG_X86_DIRECT_GBPAGES)) > + return false; > + > + /* Check the CPU feature: */ > + if (!cpu_feature_enabled(X86_FEATURE_GBPAGES)) > + return false; > + > + /* Check the command-line and early setup variable: */ > + return direct_gbpages; > +} > + > void init_mem_mapping(void); > void early_alloc_pgt_buf(void); > void __init poking_init(void); > diff -puN arch/x86/mm/init.c~direct_gbpages-compiletime arch/x86/mm/init.c > --- a/arch/x86/mm/init.c~direct_gbpages-compiletime 2026-09-02 > 06:55:01.004094924 -0700 > +++ b/arch/x86/mm/init.c 2026-09-02 08:20:36.759992562 -0700 > @@ -251,7 +251,7 @@ static void __init probe_page_size_mask( > __default_kernel_pte_mask &= ~_PAGE_GLOBAL; > > /* Enable 1 GB linear kernel mappings if available: */ > - if (direct_gbpages && boot_cpu_has(X86_FEATURE_GBPAGES)) { > + if (direct_gbpages_enabled()) { > printk(KERN_INFO "Using GB pages for direct mapping\n"); > page_size_mask |= 1 << PG_LEVEL_1G; > } else { > diff -puN arch/x86/kernel/cpu/common.c~direct_gbpages-compiletime > arch/x86/kernel/cpu/common.c > --- a/arch/x86/kernel/cpu/common.c~direct_gbpages-compiletime 2026-09-02 > 06:57:47.935849397 -0700 > +++ b/arch/x86/kernel/cpu/common.c 2026-09-02 06:57:57.299617535 -0700 > @@ -2660,7 +2660,7 @@ void __init arch_cpu_finalize_init(void) > * Right now we don't do that with gbpages because there seems > * very little benefit for that case. > */ > - if (!direct_gbpages) > + if (!direct_gbpages_enabled()) > set_memory_4k((unsigned long)__va(0), 1); > } else { > fpu__init_check_bugs(); > diff -puN arch/x86/kernel/machine_kexec_64.c~direct_gbpages-compiletime > arch/x86/kernel/machine_kexec_64.c > --- a/arch/x86/kernel/machine_kexec_64.c~direct_gbpages-compiletime > 2026-09-02 06:57:58.462712975 -0700 > +++ b/arch/x86/kernel/machine_kexec_64.c 2026-09-02 06:58:05.219267514 > -0700 > @@ -257,7 +257,7 @@ static int init_pgtable(struct kimage *i > info.kernpg_flag |= _PAGE_ENC; > } > > - if (direct_gbpages) > + if (direct_gbpages_enabled()) > info.direct_gbpages = true; > > for (i = 0; i < nr_pfn_mapped; i++) { > diff -puN arch/x86/mm/pat/set_memory.c~direct_gbpages-compiletime > arch/x86/mm/pat/set_memory.c > --- a/arch/x86/mm/pat/set_memory.c~direct_gbpages-compiletime 2026-09-02 > 06:58:43.518414555 -0700 > +++ b/arch/x86/mm/pat/set_memory.c 2026-09-02 06:59:27.252015369 -0700 > @@ -130,7 +130,7 @@ void arch_report_meminfo(struct seq_file > seq_printf(m, "DirectMap4M: %8lu kB\n", > direct_pages_count[PG_LEVEL_2M] << 12); > #endif > - if (direct_gbpages) > + if (direct_gbpages_enabled()) > seq_printf(m, "DirectMap1G: %8lu kB\n", > direct_pages_count[PG_LEVEL_1G] << 20); > } > @@ -1340,7 +1340,7 @@ static int collapse_pud_page(pud_t *pud, > pmd_t *pmd, first; > int i; > > - if (!direct_gbpages) > + if (!direct_gbpages_enabled()) > return 0; > > addr &= PUD_MASK; > _ -- Sincerely, Yeoreum Yun
