Re: [tip: x86/mm] x86/mm: Check if PTRS_PER_PMD is defined before use

[Ingo Molnar]

* 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