Re: [RFC] mm: Drop pxd_ERROR()
Anshuman Khandual <[email protected]>
| Newsgroups | org.kernel.vger.linux-arch,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <on4gs65rttqkuw2zomuswauqrzal33wel7cmeep4yw3hpodyqd@zkrqwu4sxmnz> |
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) <[email protected]> > > Cc: Andrew Morton <[email protected]> > > Cc: [email protected] > > Cc: [email protected] > > Cc: [email protected] > > Signed-off-by: Anshuman Khandual <[email protected]> > > --- > > This applies on v7.2-rc6 but after the following patch > > > > https://lore.kernel.org/all/[email protected]/ > > > > 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/[email protected]/ - 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.