Re: [RFC] mm: Drop pxd_ERROR()

From: Mike Rapoport

Date: Sun Aug 23 2026 - 02:51:08 EST

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?

> 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.