Re: [RFC] mm: Drop pxd_ERROR()

From: Anshuman Khandual

Date: Sun Aug 23 2026 - 22:10:19 EST

On Sun, Aug 23, 2026 at 09:50:51AM +0300, Mike Rapoport wrote:
> On Thu, Aug 06, 2026 at 01:36:39PM +0530, Anshuman Khandual wrote:
> > pxd_ERROR() has been used in generic mm just to print the page table entry
> > in pxd_clear_bad() before clearing those out with pxd_clear() later. These
> > pxd_ERROR() macros have been provided by all platforms which basically did
> > the same thing.
> >
> > Make pxd_clear_bad() use recently added ptval_to_str() instead for printing
> > page table entries thus completely dropping dependency on platform provided
> > pxd_ERROR() macros which can then be dropped off. First move all required
> > helpers in core MM into a header file which could then be used else where.
> >
> > Although some platforms still use those macros internally as well. In which
> > case just move these macros inside the platform for now.
> >
> > Cc: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Cc: linux-arch@xxxxxxxxxxxxxxx
> > Cc: linux-mm@xxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> > ---
> > This applies on v7.2-rc6 but after the following patch
> >
> > https://lore.kernel.org/all/20260709044334.1741263-1-anshuman.khandual@xxxxxxx/
> >
> > Would it also make sense to just drop __FILE__ and __LINE__ from the output
> > in pxd_clear_bad() helpers as they always print the same details regardless
> > the error path and does not add much value.
>
> Maybe replace them with the caller address?

Hi Mike,

Thanks for your review but the latest series can be found here.

https://lore.kernel.org/all/20260818040041.1891769-1-anshuman.khandual@xxxxxxx/

- Anshuman

>
> > Build tested for various architectures.
> >
> > arch/alpha/include/asm/pgtable.h | 7 ----
> > arch/arc/include/asm/pgtable-levels.h | 11 -----
> > arch/arm/include/asm/pgtable.h | 7 ----
> > arch/arm/kernel/traps.c | 17 --------
> > arch/arm64/include/asm/pgtable.h | 15 -------
> > arch/csky/include/asm/pgtable.h | 4 --
> > arch/hexagon/include/asm/pgtable.h | 3 --
> > arch/loongarch/include/asm/pgtable.h | 10 -----
> > arch/m68k/include/asm/mcf_pgtable.h | 6 ---
> > arch/m68k/include/asm/motorola_pgtable.h | 8 ----
> > arch/m68k/include/asm/sun3_pgtable.h | 7 ----
> > arch/microblaze/include/asm/pgtable.h | 7 ----
> > arch/mips/include/asm/pgtable-32.h | 10 -----
> > arch/mips/include/asm/pgtable-64.h | 13 ------
> > arch/nios2/include/asm/pgtable.h | 7 ----
> > arch/openrisc/include/asm/pgtable.h | 7 ----
> > arch/parisc/include/asm/pgtable.h | 3 +-
> > arch/powerpc/include/asm/book3s/32/pgtable.h | 2 -
> > arch/powerpc/include/asm/book3s/64/pgtable.h | 7 ----
> > arch/powerpc/include/asm/nohash/32/pgtable.h | 2 -
> > .../include/asm/nohash/64/pgtable-4k.h | 3 --
> > arch/powerpc/include/asm/nohash/64/pgtable.h | 5 ---
> > arch/riscv/include/asm/pgtable-64.h | 9 ----
> > arch/riscv/include/asm/pgtable.h | 4 --
> > arch/s390/include/asm/pgtable.h | 11 -----
> > arch/sh/include/asm/pgtable-3level.h | 6 +++
> > arch/sparc/include/asm/pgtable_32.h | 3 --
> > arch/sparc/include/asm/pgtable_64.h | 10 -----
> > arch/um/include/asm/pgtable-2level.h | 7 ----
> > arch/um/include/asm/pgtable-4level.h | 13 ------
> > arch/x86/include/asm/pgtable-2level.h | 5 ---
> > arch/x86/include/asm/pgtable-3level.h | 11 -----
> > arch/x86/include/asm/pgtable_64.h | 18 --------
> > arch/xtensa/include/asm/pgtable.h | 4 --
> > include/asm-generic/pgtable-nop4d.h | 1 -
> > include/asm-generic/pgtable-nopmd.h | 1 -
> > include/asm-generic/pgtable-nopud.h | 1 -
> > include/linux/mm_types.h | 40 ++++++++++++++++++
> > mm/memory.c | 41 +------------------
> > mm/pgtable-generic.c | 21 ++++++++--
> > 40 files changed, 65 insertions(+), 302 deletions(-)
>
> This is nice :)
>
> > diff --git a/arch/parisc/include/asm/pgtable.h b/arch/parisc/include/asm/pgtable.h
> > index 467b8547ac8b..cca30bf11b4c 100644
> > --- a/arch/parisc/include/asm/pgtable.h
> > +++ b/arch/parisc/include/asm/pgtable.h
> > @@ -75,12 +75,11 @@ extern void __update_cache(pte_t pte);
> >
> > #endif /* !__ASSEMBLER__ */
> >
> > -#define pte_ERROR(e) \
> > - printk("%s:%d: bad pte %08lx.\n", __FILE__, __LINE__, pte_val(e))
> > #if CONFIG_PGTABLE_LEVELS == 3
> > #define pmd_ERROR(e) \
> > printk("%s:%d: bad pmd %08lx.\n", __FILE__, __LINE__, (unsigned long)pmd_val(e))
> > #endif
> > +
>
> Stray whitespace change?
>
> > #define pgd_ERROR(e) \
> > printk("%s:%d: bad pgd %08lx.\n", __FILE__, __LINE__, (unsigned long)pgd_val(e))
> >
> > diff --git a/arch/sh/include/asm/pgtable-3level.h b/arch/sh/include/asm/pgtable-3level.h
> > index d1ce73f3bd85..3cfbec15de2f 100644
> > --- a/arch/sh/include/asm/pgtable-3level.h
> > +++ b/arch/sh/include/asm/pgtable-3level.h
> > @@ -28,6 +28,12 @@
> > #define pmd_ERROR(e) \
> > printk("%s:%d: bad pmd %016llx.\n", __FILE__, __LINE__, pmd_val(e))
> >
> > +#define pud_ERROR(e) \
> > + printk("%s:%d: bad pud %016llx.\n", __FILE__, __LINE__, pud_val(e))
> > +
> > +#define p4d_ERROR(e) \
> > + printk("%s:%d: bad p4d %016llx.\n", __FILE__, __LINE__, p4d_val(e))
> > +
>
> I'd expect that all arch changes would only remove code. Why sh needs
> those?
>
> > typedef union {
> > struct {
> > unsigned long pmd_low;
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index b18c2b2e7d2c..1025b9140c0b 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -2001,4 +2001,44 @@ static inline unsigned long mmf_init_legacy_flags(unsigned long flags)
> > return flags & MMF_INIT_LEGACY_MASK;
> > }
> >
> > +static inline void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size)
>
> include/linux/pgtable.h seems more natural home for this
>
> > +{
> > + if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) {
> > + snprintf(buf, buf_size, "overflow");
> > + return;
> > + }
>
> --
> Sincerely yours,
> Mike.