* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Thu, 6 Mar 2025 at 00:13, tip-bot2 for Andy Shevchenko > <tip-bot2@xxxxxxxxxxxxx> wrote: > > > > x86/mm: Check if PTRS_PER_PMD is defined before use > > I'm not at all happy with this one. > > > -#if PTRS_PER_PMD > 1 > > +#if defined(PTRS_PER_PMD) && (PTRS_PER_PMD > 1) > > Honestly, I feel that if PTRS_PER_PMD isn't defined, we've missed some > include, and now the code is making random decisions based on lack of > information. Yeah, so <asm/pgtable-2level_types.h> hasn't defined it historically, because 2-level paging only has PGDs and PTE tables - and it relies on <asm-generic/pgtable-nopmd.h> doing it: #define PTRS_PER_PMD 1 <asm/pgtable_types.h> includes <asm-generic/pgtable-nopmd.h>, and with it most of the MM headers. But: > It should be defined either by the architecture pgtable_types.h > header, or if the PMD is folded away, the architecture should have > included <asm-generic/pgtable-nopmd.h>. > > So I'm *really* thinking this patch is completely bogus and is hiding > a serious problem, and making PAGE_TABLE_SIZE() have random values. Yeah, so the MM headers cover the C case - but the bugreport was about the assembly side (head_32.S): In file included from arch/x86/kernel/head_32.S:29: arch/x86/include/asm/pgtable_32.h:59:5: error: "PTRS_PER_PMD" is not defined, evaluates to 0 [-Werror=undef] 59 | #if PTRS_PER_PMD > 1 and AFAICS the assembly version of these headers doesn't define PTRS_PER_PMD. Separating out the assembler-compatible defines from the types headers appears to be a bigger patch, since it's all mixed in with C syntax: <=-----------------------------------=============================== typedef struct { pud_t pud; } pmd_t; #define PMD_SHIFT PUD_SHIFT #define PTRS_PER_PMD 1 #define PMD_SIZE (1UL << PMD_SHIFT) #define PMD_MASK (~(PMD_SIZE-1)) /* * The "pud_xxx()" functions here are trivial for a folded two-level * setup: the pmd is never bad, and a pmd always exists (as it's folded * into the pud entry) */ static inline int pud_none(pud_t pud) { return 0; } static inline int pud_bad(pud_t pud) { return 0; } static inline int pud_present(pud_t pud) { return 1; } ================================================================> In any case I've removed the commit for the time being until this all is cleared up. Thanks, Ingo
![]() |