[PATCH V3] mm: Standardize printing for pgtable entries

[PATCH V3] mm: Standardize printing for pgtable entries

[Anshuman Khandual]

From: "David Hildenbrand (Arm)" <david@kernel.org>

Bad page map reporting currently stores page table entry values in an
unsigned long long and prints them with fixed 64-bit-oriented format
strings. This is inconsistent across call sites and does not work well for
architectures where page table entry values are not naturally represented
as 64-bit values, such as 32-bit or 128-bit entries.

Introduce a common helper to convert raw page table entry values into a
fixed-width hexadecimal string based on the actual entry size. Use it for
bad page map reporting and for dumping the page table walk in
__print_bad_page_map_pgtable().

Pass page table entry values to the reporting path as raw bytes together
with their size, instead of forcing them through an unsigned long long.
It keeps the printed output consistent and avoids truncation or misleading
formatting for non-64-bit page table entries.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Co-developed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This patch applies on v7.2-rc2

Changes in V3:

- Replaced __auto_type with auto per Andy
- Added Co-developed-by:

Changes in V2:

https://lore.kernel.org/all/20260708032824.969752-1-anshuman.khandual@arm.com/

- Dropped space after ":" during print per Matthew
- Dropped CONFIG_CPU_BIG_ENDIAN per David

Changes in V1:

https://lore.kernel.org/all/20260707041703.658021-1-anshuman.khandual@arm.com/

 mm/memory.c | 102 ++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 79 insertions(+), 23 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index ff338c2abe92..c8fe2f373fd2 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -519,9 +519,52 @@ static bool is_bad_page_map_ratelimited(void)
 	return false;
 }
 
+static void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size)
+{
+	if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) {
+		snprintf(buf, buf_size, "overflow");
+		return;
+	}
+
+	switch (entry_size) {
+	case sizeof(u32):
+		snprintf(buf, buf_size, "%08x", *(const u32 *)entry);
+		break;
+	case sizeof(u64):
+		snprintf(buf, buf_size, "%016llx", *(const u64 *)entry);
+		break;
+#if defined(__SIZEOF_INT128__)
+	case sizeof(u128):
+		snprintf(buf, buf_size, "%016llx%016llx",
+			 (unsigned long long)(*(const u128 *)entry >> 64),
+			 (unsigned long long)*(const u128 *)entry);
+		break;
+#endif
+	default:
+		snprintf(buf, buf_size, "unsupported");
+		break;
+	}
+}
+
+#define ptval_to_str(buf, val)								\
+	do {										\
+		auto __val = (val);							\
+											\
+		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
+	} while (0)
+
+#if defined(__SIZEOF_INT128__)
+#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
+#else
+#define PTVAL_STR_MAX	(16 + 1) /* Max 64-bit value in hex + NUL */
+#endif
+
 static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long addr)
 {
-	unsigned long long pgdv, p4dv, pudv, pmdv;
+	char pgd_str[PTVAL_STR_MAX];
+	char p4d_str[PTVAL_STR_MAX];
+	char pud_str[PTVAL_STR_MAX];
+	char pmd_str[PTVAL_STR_MAX];
 	p4d_t p4d, *p4dp;
 	pud_t pud, *pudp;
 	pmd_t pmd, *pmdp;
@@ -532,34 +575,34 @@ static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long add
 	 * see locking requirements for print_bad_page_map().
 	 */
 	pgdp = pgd_offset(mm, addr);
-	pgdv = pgd_val(*pgdp);
+	ptval_to_str(pgd_str, pgd_val(*pgdp));
 
 	if (!pgd_present(*pgdp) || pgd_leaf(*pgdp)) {
-		pr_alert("pgd:%08llx\n", pgdv);
+		pr_alert("pgd:%s\n", pgd_str);
 		return;
 	}
 
 	p4dp = p4d_offset(pgdp, addr);
 	p4d = p4dp_get(p4dp);
-	p4dv = p4d_val(p4d);
+	ptval_to_str(p4d_str, p4d_val(p4d));
 
 	if (!p4d_present(p4d) || p4d_leaf(p4d)) {
-		pr_alert("pgd:%08llx p4d:%08llx\n", pgdv, p4dv);
+		pr_alert("pgd:%s p4d:%s\n", pgd_str, p4d_str);
 		return;
 	}
 
 	pudp = pud_offset(p4dp, addr);
 	pud = pudp_get(pudp);
-	pudv = pud_val(pud);
+	ptval_to_str(pud_str, pud_val(pud));
 
 	if (!pud_present(pud) || pud_leaf(pud)) {
-		pr_alert("pgd:%08llx p4d:%08llx pud:%08llx\n", pgdv, p4dv, pudv);
+		pr_alert("pgd:%s p4d:%s pud:%s\n", pgd_str, p4d_str, pud_str);
 		return;
 	}
 
 	pmdp = pmd_offset(pudp, addr);
 	pmd = pmdp_get(pmdp);
-	pmdv = pmd_val(pmd);
+	ptval_to_str(pmd_str, pmd_val(pmd));
 
 	/*
 	 * Dumping the PTE would be nice, but it's tricky with CONFIG_HIGHPTE,
@@ -567,8 +610,7 @@ static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long add
 	 * doing another map would be bad. print_bad_page_map() should
 	 * already take care of printing the PTE.
 	 */
-	pr_alert("pgd:%08llx p4d:%08llx pud:%08llx pmd:%08llx\n", pgdv,
-		 p4dv, pudv, pmdv);
+	pr_alert("pgd:%s p4d:%s pud:%s pmd:%s\n", pgd_str, p4d_str, pud_str, pmd_str);
 }
 
 /*
@@ -584,10 +626,11 @@ static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long add
  * page table lock.
  */
 static void print_bad_page_map(struct vm_area_struct *vma,
-		unsigned long addr, unsigned long long entry, struct page *page,
-		enum pgtable_level level)
+		unsigned long addr, const void *entry, size_t entry_size,
+		struct page *page, enum pgtable_level level)
 {
 	struct address_space *mapping;
+	char entry_str[PTVAL_STR_MAX];
 	pgoff_t index;
 
 	if (is_bad_page_map_ratelimited())
@@ -596,8 +639,9 @@ static void print_bad_page_map(struct vm_area_struct *vma,
 	mapping = vma->vm_file ? vma->vm_file->f_mapping : NULL;
 	index = linear_page_index(vma, addr);
 
-	pr_alert("BUG: Bad page map in process %s  %s:%08llx", current->comm,
-		 pgtable_level_to_str(level), entry);
+	ptval_bytes_to_hex_str(entry_str, sizeof(entry_str), entry, entry_size);
+	pr_alert("BUG: Bad page map in process %s  %s:%s", current->comm,
+		 pgtable_level_to_str(level), entry_str);
 	__print_bad_page_map_pgtable(vma->vm_mm, addr);
 	if (page)
 		dump_page(page, "bad page map");
@@ -627,8 +671,13 @@ static inline bool pgtable_level_has_pxx_special(enum pgtable_level level)
 	}
 }
 
-#define print_bad_pte(vma, addr, pte, page) \
-	print_bad_page_map(vma, addr, pte_val(pte), page, PGTABLE_LEVEL_PTE)
+static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
+			  pte_t pte, struct page *page)
+{
+	auto entry = pte_val(pte);
+
+	print_bad_page_map(vma, addr, &entry, sizeof(entry), page, PGTABLE_LEVEL_PTE);
+}
 
 /**
  * __vm_normal_page() - Get the "struct page" associated with a page table entry.
@@ -636,8 +685,9 @@ static inline bool pgtable_level_has_pxx_special(enum pgtable_level level)
  * @addr: The address where the page table entry is mapped.
  * @pfn: The PFN stored in the page table entry.
  * @special: Whether the page table entry is marked "special".
- * @level: The page table level for error reporting purposes only.
  * @entry: The page table entry value for error reporting purposes only.
+ * @entry_size: The size of @entry.
+ * @level: The page table level for error reporting purposes only.
  *
  * "Special" mappings do not wish to be associated with a "struct page" (either
  * it doesn't exist, or it exists but they don't want to touch it). In this
@@ -697,7 +747,7 @@ static inline bool pgtable_level_has_pxx_special(enum pgtable_level level)
  */
 static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
 		unsigned long addr, unsigned long pfn, bool special,
-		unsigned long long entry, enum pgtable_level level)
+		const void *entry, size_t entry_size, enum pgtable_level level)
 {
 	if (pgtable_level_has_pxx_special(level)) {
 		if (unlikely(special)) {
@@ -710,7 +760,7 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
 			if (is_zero_pfn(pfn) || is_huge_zero_pfn(pfn))
 				return NULL;
 
-			print_bad_page_map(vma, addr, entry, NULL, level);
+			print_bad_page_map(vma, addr, entry, entry_size, NULL, level);
 			return NULL;
 		}
 		/*
@@ -741,7 +791,7 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
 
 	if (unlikely(pfn > highest_memmap_pfn)) {
 		/* Corrupted page table entry. */
-		print_bad_page_map(vma, addr, entry, NULL, level);
+		print_bad_page_map(vma, addr, entry, entry_size, NULL, level);
 		return NULL;
 	}
 	/*
@@ -767,8 +817,10 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
 struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
 			    pte_t pte)
 {
+	auto entry = pte_val(pte);
+
 	return __vm_normal_page(vma, addr, pte_pfn(pte), pte_special(pte),
-				pte_val(pte), PGTABLE_LEVEL_PTE);
+				&entry, sizeof(entry), PGTABLE_LEVEL_PTE);
 }
 
 /**
@@ -809,8 +861,10 @@ struct folio *vm_normal_folio(struct vm_area_struct *vma, unsigned long addr,
 struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
 				pmd_t pmd)
 {
+	auto entry = pmd_val(pmd);
+
 	return __vm_normal_page(vma, addr, pmd_pfn(pmd), pmd_special(pmd),
-				pmd_val(pmd), PGTABLE_LEVEL_PMD);
+				&entry, sizeof(entry), PGTABLE_LEVEL_PMD);
 }
 
 /**
@@ -850,8 +904,10 @@ struct folio *vm_normal_folio_pmd(struct vm_area_struct *vma,
 struct page *vm_normal_page_pud(struct vm_area_struct *vma,
 		unsigned long addr, pud_t pud)
 {
+	auto entry = pud_val(pud);
+
 	return __vm_normal_page(vma, addr, pud_pfn(pud), pud_special(pud),
-				pud_val(pud), PGTABLE_LEVEL_PUD);
+				&entry, sizeof(entry), PGTABLE_LEVEL_PUD);
 }
 #endif
 
-- 
2.43.0

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Andy Shevchenko]

+Petr, wondering your opinion about code dedup (see below).

On Thu, Jul 09, 2026 at 10:13:34AM +0530, Anshuman Khandual wrote:
> From: "David Hildenbrand (Arm)" <david@kernel.org>
> 
> Bad page map reporting currently stores page table entry values in an
> unsigned long long and prints them with fixed 64-bit-oriented format
> strings. This is inconsistent across call sites and does not work well for
> architectures where page table entry values are not naturally represented
> as 64-bit values, such as 32-bit or 128-bit entries.
> 
> Introduce a common helper to convert raw page table entry values into a
> fixed-width hexadecimal string based on the actual entry size. Use it for
> bad page map reporting and for dumping the page table walk in
> __print_bad_page_map_pgtable().
> 
> Pass page table entry values to the reporting path as raw bytes together
> with their size, instead of forcing them through an unsigned long long.
> It keeps the printed output consistent and avoids truncation or misleading
> formatting for non-64-bit page table entries.

> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org

FWIW, you may move these to be after...

> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Co-developed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
... this cutter line with the same effect on the email. The bonus is that
it will eliminate the huge noise and churn in the commit message.
Note, many maintainers switched to this schema (especially those, who use
`b4` tool, which allows to handle this nicely).

...

> +static void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size)
> +{
> +	if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) {
> +		snprintf(buf, buf_size, "overflow");
> +		return;
> +	}
> +
> +	switch (entry_size) {
> +	case sizeof(u32):
> +		snprintf(buf, buf_size, "%08x", *(const u32 *)entry);
> +		break;
> +	case sizeof(u64):
> +		snprintf(buf, buf_size, "%016llx", *(const u64 *)entry);
> +		break;
> +#if defined(__SIZEOF_INT128__)
> +	case sizeof(u128):
> +		snprintf(buf, buf_size, "%016llx%016llx",
> +			 (unsigned long long)(*(const u128 *)entry >> 64),
> +			 (unsigned long long)*(const u128 *)entry);
> +		break;
> +#endif
> +	default:
> +		snprintf(buf, buf_size, "unsupported");
> +		break;
> +	}
> +}
> +
> +#define ptval_to_str(buf, val)								\
> +	do {										\
> +		auto __val = (val);							\
> +											\
> +		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
> +	} while (0)
> +
> +#if defined(__SIZEOF_INT128__)
> +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
> +#else
> +#define PTVAL_STR_MAX	(16 + 1) /* Max 64-bit value in hex + NUL */
> +#endif

The above is quite duplicative with what we have in lib/vsprintf.c. Have you considered
using something from there instead? (Yes, it might require some functions to be wrapped
or dropped from static.)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[David Hildenbrand (Arm)]

On 7/9/26 09:40, Andy Shevchenko wrote:
> +Petr, wondering your opinion about code dedup (see below).
> 
> On Thu, Jul 09, 2026 at 10:13:34AM +0530, Anshuman Khandual wrote:
>> From: "David Hildenbrand (Arm)" <david@kernel.org>
>>
>> Bad page map reporting currently stores page table entry values in an
>> unsigned long long and prints them with fixed 64-bit-oriented format
>> strings. This is inconsistent across call sites and does not work well for
>> architectures where page table entry values are not naturally represented
>> as 64-bit values, such as 32-bit or 128-bit entries.
>>
>> Introduce a common helper to convert raw page table entry values into a
>> fixed-width hexadecimal string based on the actual entry size. Use it for
>> bad page map reporting and for dumping the page table walk in
>> __print_bad_page_map_pgtable().
>>
>> Pass page table entry values to the reporting path as raw bytes together
>> with their size, instead of forcing them through an unsigned long long.
>> It keeps the printed output consistent and avoids truncation or misleading
>> formatting for non-64-bit page table entries.
> 
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: linux-mm@kvack.org
>> Cc: linux-kernel@vger.kernel.org
> 
> FWIW, you may move these to be after...

We generally don't handle it like that in MM. Maybe one day we'll switch over
and document it accordingly.

For now, having selected patches doing this differently is not any helpful.

[...]

>> +static void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size)
>> +{
>> +	if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) {
>> +		snprintf(buf, buf_size, "overflow");
>> +		return;
>> +	}
>> +
>> +	switch (entry_size) {
>> +	case sizeof(u32):
>> +		snprintf(buf, buf_size, "%08x", *(const u32 *)entry);
>> +		break;
>> +	case sizeof(u64):
>> +		snprintf(buf, buf_size, "%016llx", *(const u64 *)entry);
>> +		break;
>> +#if defined(__SIZEOF_INT128__)
>> +	case sizeof(u128):
>> +		snprintf(buf, buf_size, "%016llx%016llx",
>> +			 (unsigned long long)(*(const u128 *)entry >> 64),
>> +			 (unsigned long long)*(const u128 *)entry);
>> +		break;
>> +#endif
>> +	default:
>> +		snprintf(buf, buf_size, "unsupported");
>> +		break;
>> +	}
>> +}
>> +
>> +#define ptval_to_str(buf, val)								\
>> +	do {										\
>> +		auto __val = (val);							\
>> +											\
>> +		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
>> +	} while (0)
>> +
>> +#if defined(__SIZEOF_INT128__)
>> +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
>> +#else
>> +#define PTVAL_STR_MAX	(16 + 1) /* Max 64-bit value in hex + NUL */
>> +#endif
> 
> The above is quite duplicative with what we have in lib/vsprintf.c. Have you considered
> using something from there instead? (Yes, it might require some functions to be wrapped
> or dropped from static.)

Which part in particular do you have in mind?

num_to_str() does not apply due to the u128.

We could have u128 variant that we would only provide with __SIZEOF_INT128__

int num128_to_str(char *buf, int size, u128 num, unsigned int width)

And then have the code pass the value instead of a pointer to the value. A bit
tricky to handle this based on conditional __SIZEOF_INT128__ support, but could
be done.

Not sure if that is really what we want here, though. ptval_bytes_to_hex_str()
is pretty ... simple :)

I didn't immediately spot a replacement for PTVAL_STR_MAX. E.g.,
of_unittest_printf what calls num_to_str() just hard-codes "16".

So it would be good if you could clarify what you had in mind, thanks!

-- 
Cheers,

David

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Petr Mladek]

On Thu 2026-07-09 10:40:43, Andy Shevchenko wrote:
> +Petr, wondering your opinion about code dedup (see below).
> 
> On Thu, Jul 09, 2026 at 10:13:34AM +0530, Anshuman Khandual wrote:
> > From: "David Hildenbrand (Arm)" <david@kernel.org>
> > 
> > Bad page map reporting currently stores page table entry values in an
> > unsigned long long and prints them with fixed 64-bit-oriented format
> > strings. This is inconsistent across call sites and does not work well for
> > architectures where page table entry values are not naturally represented
> > as 64-bit values, such as 32-bit or 128-bit entries.
> > 
> > Introduce a common helper to convert raw page table entry values into a
> > fixed-width hexadecimal string based on the actual entry size. Use it for
> > bad page map reporting and for dumping the page table walk in
> > __print_bad_page_map_pgtable().
> > 
> > Pass page table entry values to the reporting path as raw bytes together
> > with their size, instead of forcing them through an unsigned long long.
> > It keeps the printed output consistent and avoids truncation or misleading
> > formatting for non-64-bit page table entries.
> 
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: linux-mm@kvack.org
> > Cc: linux-kernel@vger.kernel.org
> 
> FWIW, you may move these to be after...
> 
> > Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> > Co-developed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > ---
> ... this cutter line with the same effect on the email. The bonus is that
> it will eliminate the huge noise and churn in the commit message.
> Note, many maintainers switched to this schema (especially those, who use
> `b4` tool, which allows to handle this nicely).
> 
> ...
> 
> > +static void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size)
> > +{
> > +	if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) {
> > +		snprintf(buf, buf_size, "overflow");
> > +		return;
> > +	}
> > +
> > +	switch (entry_size) {
> > +	case sizeof(u32):
> > +		snprintf(buf, buf_size, "%08x", *(const u32 *)entry);
> > +		break;
> > +	case sizeof(u64):
> > +		snprintf(buf, buf_size, "%016llx", *(const u64 *)entry);
> > +		break;
> > +#if defined(__SIZEOF_INT128__)
> > +	case sizeof(u128):
> > +		snprintf(buf, buf_size, "%016llx%016llx",
> > +			 (unsigned long long)(*(const u128 *)entry >> 64),
> > +			 (unsigned long long)*(const u128 *)entry);
> > +		break;
> > +#endif
> > +	default:
> > +		snprintf(buf, buf_size, "unsupported");
> > +		break;
> > +	}
> > +}
> > +
> > +#define ptval_to_str(buf, val)								\
> > +	do {										\
> > +		auto __val = (val);							\
> > +											\
> > +		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
> > +	} while (0)
> > +
> > +#if defined(__SIZEOF_INT128__)
> > +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
> > +#else
> > +#define PTVAL_STR_MAX	(16 + 1) /* Max 64-bit value in hex + NUL */
> > +#endif
> 
> The above is quite duplicative with what we have in lib/vsprintf.c. Have you considered
> using something from there instead? (Yes, it might require some functions to be wrapped
> or dropped from static.)

Honestly, I can't see any reasonable way to deduplicate the code.

Most of the magic in the above macros are in the value/buffer size
detection. I do not see anything similar in vsprintf.c. The sizes
are handled another way there.

Also the internal API used in vsprintf.c is a bit tricky because
the buffer size is passed via *end pointer and it counts the printed
characters even behind the *end pointer.

But maybe you had something particular in mind.

Best Regards,
Petr

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Andy Shevchenko]

On Thu, Jul 09, 2026 at 11:12:58AM +0200, David Hildenbrand (Arm) wrote:
> On 7/9/26 09:40, Andy Shevchenko wrote:
> > On Thu, Jul 09, 2026 at 10:13:34AM +0530, Anshuman Khandual wrote:

> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> Cc: linux-mm@kvack.org
> >> Cc: linux-kernel@vger.kernel.org
> > 
> > FWIW, you may move these to be after...
> 
> We generally don't handle it like that in MM. Maybe one day we'll switch over
> and document it accordingly.
> 
> For now, having selected patches doing this differently is not any helpful.

Can you elaborate "differently"? Except a big noise and churn in the commit message
I do not see any benefit of doing the "old" way. Lately I saw some patch-bot messages
from Andrew, I assume he updates his scripts towards newer tooling, which may be
a good sign that the (good) changes are still possible.

[...]

> >> +static void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size)
> >> +{
> >> +	if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) {
> >> +		snprintf(buf, buf_size, "overflow");
> >> +		return;
> >> +	}
> >> +
> >> +	switch (entry_size) {
> >> +	case sizeof(u32):
> >> +		snprintf(buf, buf_size, "%08x", *(const u32 *)entry);
> >> +		break;
> >> +	case sizeof(u64):
> >> +		snprintf(buf, buf_size, "%016llx", *(const u64 *)entry);
> >> +		break;
> >> +#if defined(__SIZEOF_INT128__)
> >> +	case sizeof(u128):
> >> +		snprintf(buf, buf_size, "%016llx%016llx",
> >> +			 (unsigned long long)(*(const u128 *)entry >> 64),
> >> +			 (unsigned long long)*(const u128 *)entry);
> >> +		break;
> >> +#endif
> >> +	default:
> >> +		snprintf(buf, buf_size, "unsupported");
> >> +		break;
> >> +	}
> >> +}
> >> +
> >> +#define ptval_to_str(buf, val)								\
> >> +	do {										\
> >> +		auto __val = (val);							\
> >> +											\
> >> +		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
> >> +	} while (0)
> >> +
> >> +#if defined(__SIZEOF_INT128__)
> >> +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
> >> +#else
> >> +#define PTVAL_STR_MAX	(16 + 1) /* Max 64-bit value in hex + NUL */
> >> +#endif
> > 
> > The above is quite duplicative with what we have in lib/vsprintf.c. Have you considered
> > using something from there instead? (Yes, it might require some functions to be wrapped
> > or dropped from static.)
> 
> Which part in particular do you have in mind?
> 
> num_to_str() does not apply due to the u128.
> 
> We could have u128 variant that we would only provide with __SIZEOF_INT128__
> 
> int num128_to_str(char *buf, int size, u128 num, unsigned int width)
> 
> And then have the code pass the value instead of a pointer to the value. A bit
> tricky to handle this based on conditional __SIZEOF_INT128__ support, but could
> be done.
> 
> Not sure if that is really what we want here, though. ptval_bytes_to_hex_str()
> is pretty ... simple :)
> 
> I didn't immediately spot a replacement for PTVAL_STR_MAX.

I referred mostly to special_hex_number().

> So it would be good if you could clarify what you had in mind, thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Andy Shevchenko]

On Thu, Jul 09, 2026 at 11:15:27AM +0200, Petr Mladek wrote:
> On Thu 2026-07-09 10:40:43, Andy Shevchenko wrote:
> > On Thu, Jul 09, 2026 at 10:13:34AM +0530, Anshuman Khandual wrote:

...

> > > +static void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size)
> > > +{
> > > +	if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) {
> > > +		snprintf(buf, buf_size, "overflow");
> > > +		return;
> > > +	}
> > > +
> > > +	switch (entry_size) {
> > > +	case sizeof(u32):
> > > +		snprintf(buf, buf_size, "%08x", *(const u32 *)entry);
> > > +		break;
> > > +	case sizeof(u64):
> > > +		snprintf(buf, buf_size, "%016llx", *(const u64 *)entry);
> > > +		break;
> > > +#if defined(__SIZEOF_INT128__)
> > > +	case sizeof(u128):
> > > +		snprintf(buf, buf_size, "%016llx%016llx",
> > > +			 (unsigned long long)(*(const u128 *)entry >> 64),
> > > +			 (unsigned long long)*(const u128 *)entry);
> > > +		break;
> > > +#endif
> > > +	default:
> > > +		snprintf(buf, buf_size, "unsupported");
> > > +		break;
> > > +	}
> > > +}
> > > +
> > > +#define ptval_to_str(buf, val)								\
> > > +	do {										\
> > > +		auto __val = (val);							\
> > > +											\
> > > +		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
> > > +	} while (0)
> > > +
> > > +#if defined(__SIZEOF_INT128__)
> > > +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
> > > +#else
> > > +#define PTVAL_STR_MAX	(16 + 1) /* Max 64-bit value in hex + NUL */
> > > +#endif
> > 
> > The above is quite duplicative with what we have in lib/vsprintf.c. Have you considered
> > using something from there instead? (Yes, it might require some functions to be wrapped
> > or dropped from static.)
> 
> Honestly, I can't see any reasonable way to deduplicate the code.
> 
> Most of the magic in the above macros are in the value/buffer size
> detection. I do not see anything similar in vsprintf.c. The sizes
> are handled another way there.
> 
> Also the internal API used in vsprintf.c is a bit tricky because
> the buffer size is passed via *end pointer and it counts the printed
> characters even behind the *end pointer.
> 
> But maybe you had something particular in mind.

Yes, I was thinking of a wrapper on top of special_hex_number(). It takes size
as an argument and hence will work even for 128-bit cases.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Andy Shevchenko]

On Thu, Jul 09, 2026 at 01:11:46PM +0300, Andy Shevchenko wrote:
> On Thu, Jul 09, 2026 at 11:15:27AM +0200, Petr Mladek wrote:
> > On Thu 2026-07-09 10:40:43, Andy Shevchenko wrote:
> > > On Thu, Jul 09, 2026 at 10:13:34AM +0530, Anshuman Khandual wrote:

...

> > But maybe you had something particular in mind.
> 
> Yes, I was thinking of a wrapper on top of special_hex_number(). It takes size
> as an argument and hence will work even for 128-bit cases.

Okay, the argument is limited to 64-bit in there. But I don't think it's a big
impediment.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[David Hildenbrand (Arm)]

On 7/9/26 12:09, Andy Shevchenko wrote:
> On Thu, Jul 09, 2026 at 11:12:58AM +0200, David Hildenbrand (Arm) wrote:
>> On 7/9/26 09:40, Andy Shevchenko wrote:
> 
>>>
>>> FWIW, you may move these to be after...
>>
>> We generally don't handle it like that in MM. Maybe one day we'll switch over
>> and document it accordingly.
>>
>> For now, having selected patches doing this differently is not any helpful.
> 
> Can you elaborate "differently"? Except a big noise and churn in the commit message
> I do not see any benefit of doing the "old" way. Lately I saw some patch-bot messages
> from Andrew, I assume he updates his scripts towards newer tooling, which may be
> a good sign that the (good) changes are still possible.

Take a look at the patches that go upstream from Andrew, where he still adds
additional Cc above his SOB.

We're planning a transition to a different mm model, as Andrew prepares for his
(partial :) ) retirement. We can discuss such things once the time has come, and
come to an agreement within the MM community.

This is not the time to have this discussion again, let's focus on the details
of the patch here, please.

> 
> [...]
> 
>>>
>>> The above is quite duplicative with what we have in lib/vsprintf.c. Have you considered
>>> using something from there instead? (Yes, it might require some functions to be wrapped
>>> or dropped from static.)
>>
>> Which part in particular do you have in mind?
>>
>> num_to_str() does not apply due to the u128.
>>
>> We could have u128 variant that we would only provide with __SIZEOF_INT128__
>>
>> int num128_to_str(char *buf, int size, u128 num, unsigned int width)
>>
>> And then have the code pass the value instead of a pointer to the value. A bit
>> tricky to handle this based on conditional __SIZEOF_INT128__ support, but could
>> be done.
>>
>> Not sure if that is really what we want here, though. ptval_bytes_to_hex_str()
>> is pretty ... simple :)
>>
>> I didn't immediately spot a replacement for PTVAL_STR_MAX.
> 
> I referred mostly to special_hex_number().

Only the .field_width = 2 + 2 * (size) in special_hex_spec() seems a bit related
(although different, because we don't want the leading 0x).

Like Petr says, I don't immediately see how this can easily be reused, but maybe
there is a way @Anshuman, @Petr

-- 
Cheers,

David

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[David Hildenbrand (Arm)]

On 7/9/26 12:13, Andy Shevchenko wrote:
> On Thu, Jul 09, 2026 at 01:11:46PM +0300, Andy Shevchenko wrote:
>> On Thu, Jul 09, 2026 at 11:15:27AM +0200, Petr Mladek wrote:
> 
> ...
> 
>>> But maybe you had something particular in mind.
>>
>> Yes, I was thinking of a wrapper on top of special_hex_number(). It takes size
>> as an argument and hence will work even for 128-bit cases.
> 
> Okay, the argument is limited to 64-bit in there. But I don't think it's a big
> impediment.

Rewrite number() / create a new 128bit variants to avoid 3 simple snprintf()
statements? :)

Hm ...

-- 
Cheers,

David

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Andy Shevchenko]

On Thu, Jul 09, 2026 at 12:24:08PM +0200, David Hildenbrand (Arm) wrote:
> On 7/9/26 12:13, Andy Shevchenko wrote:
> > On Thu, Jul 09, 2026 at 01:11:46PM +0300, Andy Shevchenko wrote:
> >> On Thu, Jul 09, 2026 at 11:15:27AM +0200, Petr Mladek wrote:

...

> >>> But maybe you had something particular in mind.
> >>
> >> Yes, I was thinking of a wrapper on top of special_hex_number(). It takes size
> >> as an argument and hence will work even for 128-bit cases.
> > 
> > Okay, the argument is limited to 64-bit in there. But I don't think it's a big
> > impediment.
> 
> Rewrite number() / create a new 128bit variants to avoid 3 simple snprintf()
> statements? :)
> 
> Hm ...

Since it's slowly spreading (I mean 128-bit types), I think sooner or later we
would do it anyway. For now we can just have a wrapper on top of number()
(yes, special_hex_number() probably too special for this case) to print it.
And yes, avoiding sprintf() in this case seems beneficial to me, we will have
one implementation of how we print pointers.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Anshuman Khandual]

On 09/07/26 4:13 PM, Andy Shevchenko wrote:
> On Thu, Jul 09, 2026 at 12:24:08PM +0200, David Hildenbrand (Arm) wrote:
>> On 7/9/26 12:13, Andy Shevchenko wrote:
>>> On Thu, Jul 09, 2026 at 01:11:46PM +0300, Andy Shevchenko wrote:
>>>> On Thu, Jul 09, 2026 at 11:15:27AM +0200, Petr Mladek wrote:
> 
> ...
> 
>>>>> But maybe you had something particular in mind.
>>>>
>>>> Yes, I was thinking of a wrapper on top of special_hex_number(). It takes size
>>>> as an argument and hence will work even for 128-bit cases.
>>>
>>> Okay, the argument is limited to 64-bit in there. But I don't think it's a big
>>> impediment.
>>
>> Rewrite number() / create a new 128bit variants to avoid 3 simple snprintf()
>> statements? :)
>>
>> Hm ...
> 
> Since it's slowly spreading (I mean 128-bit types), I think sooner or later we
> would do it anyway. For now we can just have a wrapper on top of number()
> (yes, special_hex_number() probably too special for this case) to print it.
> And yes, avoiding sprintf() in this case seems beneficial to me, we will have
> one implementation of how we print pointers.

We followed this approach in a self contained manner inside core
MM in order to avoid adding new printk format. TBH don't see too
much value in again trying to create factored common code between
with lib/vsprintf.c As David mentioned this may not be worth it.

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Andy Shevchenko]

On Thu, Jul 09, 2026 at 04:42:30PM +0530, Anshuman Khandual wrote:
> On 09/07/26 4:13 PM, Andy Shevchenko wrote:
> > On Thu, Jul 09, 2026 at 12:24:08PM +0200, David Hildenbrand (Arm) wrote:
> >> On 7/9/26 12:13, Andy Shevchenko wrote:
> >>> On Thu, Jul 09, 2026 at 01:11:46PM +0300, Andy Shevchenko wrote:
> >>>> On Thu, Jul 09, 2026 at 11:15:27AM +0200, Petr Mladek wrote:

...

> >>>>> But maybe you had something particular in mind.
> >>>>
> >>>> Yes, I was thinking of a wrapper on top of special_hex_number(). It takes size
> >>>> as an argument and hence will work even for 128-bit cases.
> >>>
> >>> Okay, the argument is limited to 64-bit in there. But I don't think it's a big
> >>> impediment.
> >>
> >> Rewrite number() / create a new 128bit variants to avoid 3 simple snprintf()
> >> statements? :)
> >>
> >> Hm ...
> > 
> > Since it's slowly spreading (I mean 128-bit types), I think sooner or later we
> > would do it anyway. For now we can just have a wrapper on top of number()
> > (yes, special_hex_number() probably too special for this case) to print it.
> > And yes, avoiding sprintf() in this case seems beneficial to me, we will have
> > one implementation of how we print pointers.
> 
> We followed this approach in a self contained manner inside core
> MM in order to avoid adding new printk format. TBH don't see too
> much value in again trying to create factored common code between
> with lib/vsprintf.c As David mentioned this may not be worth it.

I'm not fighting to death for this, just my observations, so we see
that majority of people agree with leaving this one independent on
lib/vsprintf.c implementation.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Andrew Morton]

On Thu,  9 Jul 2026 10:13:34 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:

> From: "David Hildenbrand (Arm)" <david@kernel.org>
> 
> Bad page map reporting currently stores page table entry values in an
> unsigned long long and prints them with fixed 64-bit-oriented format
> strings. This is inconsistent across call sites and does not work well for
> architectures where page table entry values are not naturally represented
> as 64-bit values, such as 32-bit or 128-bit entries.

Well grumble.  It's a lot of fuss for something which nobody is hurting
from.  Or are they?  What's the actual utility here?

> Introduce a common helper to convert raw page table entry values into a
> fixed-width hexadecimal string based on the actual entry size. Use it for
> bad page map reporting and for dumping the page table walk in
> __print_bad_page_map_pgtable().
> 
> Pass page table entry values to the reporting path as raw bytes together
> with their size, instead of forcing them through an unsigned long long.
> It keeps the printed output consistent and avoids truncation or misleading
> formatting for non-64-bit page table entries.
> 
> --- a/mm/memory.c
> +++ b/mm/memory.c
>
> ...
>
> +#if defined(__SIZEOF_INT128__)
> +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
> +#else
> +#define PTVAL_STR_MAX	(16 + 1) /* Max 64-bit value in hex + NUL */
> +#endif
> +
>  static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long addr)
>  {
> -	unsigned long long pgdv, p4dv, pudv, pmdv;
> +	char pgd_str[PTVAL_STR_MAX];
> +	char p4d_str[PTVAL_STR_MAX];
> +	char pud_str[PTVAL_STR_MAX];
> +	char pmd_str[PTVAL_STR_MAX];

That's another 128b of stack, in a potentially deep code path.

Hopefully gcc can reuse the same stack space for some of these but it's
not been good at this in the past.

>  	p4d_t p4d, *p4dp;
>  	pud_t pud, *pudp;
>  	pmd_t pmd, *pmdp;
> @@ -532,34 +575,34 @@ static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long add
>  	 * see locking requirements for print_bad_page_map().
>  	 */
>  	pgdp = pgd_offset(mm, addr);
> -	pgdv = pgd_val(*pgdp);
> +	ptval_to_str(pgd_str, pgd_val(*pgdp));
>  
>  	if (!pgd_present(*pgdp) || pgd_leaf(*pgdp)) {
> -		pr_alert("pgd:%08llx\n", pgdv);
> +		pr_alert("pgd:%s\n", pgd_str);

can this do
		pr_alert("pgd:%s\n", ptval_to_str(pgd_str, pgd_val(*pgdp)));

and eliminate a few locals?

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Anshuman Khandual]

On 10/07/26 5:38 AM, Andrew Morton wrote:
> On Thu,  9 Jul 2026 10:13:34 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
>> From: "David Hildenbrand (Arm)" <david@kernel.org>
>>
>> Bad page map reporting currently stores page table entry values in an
>> unsigned long long and prints them with fixed 64-bit-oriented format
>> strings. This is inconsistent across call sites and does not work well for
>> architectures where page table entry values are not naturally represented
>> as 64-bit values, such as 32-bit or 128-bit entries.
> 
> Well grumble.  It's a lot of fuss for something which nobody is hurting
> from.  Or are they?  What's the actual utility here?

Current page table entries print does not work here for
128 bits format (arm64 D128) because neither 'unsigned
long long' can hold 128 bits nor printing extracts u64
from the 128 bit entries while printing with "%llx".

Proposed a separate printk format %pp[te|md|ud|p4d|gd]

https://lore.kernel.org/all/20260610043545.3725735-1-anshuman.khandual@arm.com/  
 
But there after it was decided to rather have a local
solution in mm/memory.c instead of adding a new print
format.

The proposed change here streamlines existing u32/u64
page table entries format prints but also enables for
u128 which will be added later.

> >> Introduce a common helper to convert raw page table entry values into a
>> fixed-width hexadecimal string based on the actual entry size. Use it for
>> bad page map reporting and for dumping the page table walk in
>> __print_bad_page_map_pgtable().
>>
>> Pass page table entry values to the reporting path as raw bytes together
>> with their size, instead of forcing them through an unsigned long long.
>> It keeps the printed output consistent and avoids truncation or misleading
>> formatting for non-64-bit page table entries.
>>
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>>
>> ...
>>
>> +#if defined(__SIZEOF_INT128__)
>> +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
>> +#else
>> +#define PTVAL_STR_MAX	(16 + 1) /* Max 64-bit value in hex + NUL */
>> +#endif
>> +
>>  static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long addr)
>>  {
>> -	unsigned long long pgdv, p4dv, pudv, pmdv;
>> +	char pgd_str[PTVAL_STR_MAX];
>> +	char p4d_str[PTVAL_STR_MAX];
>> +	char pud_str[PTVAL_STR_MAX];
>> +	char pmd_str[PTVAL_STR_MAX];
> 
> That's another 128b of stack, in a potentially deep code path.
> 
> Hopefully gcc can reuse the same stack space for some of these but it's
> not been good at this in the past.
> 
>>  	p4d_t p4d, *p4dp;
>>  	pud_t pud, *pudp;
>>  	pmd_t pmd, *pmdp;
>> @@ -532,34 +575,34 @@ static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long add
>>  	 * see locking requirements for print_bad_page_map().
>>  	 */
>>  	pgdp = pgd_offset(mm, addr);
>> -	pgdv = pgd_val(*pgdp);
>> +	ptval_to_str(pgd_str, pgd_val(*pgdp));
>>  
>>  	if (!pgd_present(*pgdp) || pgd_leaf(*pgdp)) {
>> -		pr_alert("pgd:%08llx\n", pgdv);
>> +		pr_alert("pgd:%s\n", pgd_str);
> 
> can this do
> 		pr_alert("pgd:%s\n", ptval_to_str(pgd_str, pgd_val(*pgdp)));
> 
> and eliminate a few locals?

But that would not eliminate any local variable.

The helper ptval_to_str() does not return but instead
fills up the provided buffer which later gets printed.

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[Andrew Morton]

On Fri, 10 Jul 2026 08:40:25 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:

> 
> 
> On 10/07/26 5:38 AM, Andrew Morton wrote:
> > On Thu,  9 Jul 2026 10:13:34 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> > 
> >> From: "David Hildenbrand (Arm)" <david@kernel.org>
> >>
> >> Bad page map reporting currently stores page table entry values in an
> >> unsigned long long and prints them with fixed 64-bit-oriented format
> >> strings. This is inconsistent across call sites and does not work well for
> >> architectures where page table entry values are not naturally represented
> >> as 64-bit values, such as 32-bit or 128-bit entries.
> > 
> > Well grumble.  It's a lot of fuss for something which nobody is hurting
> > from.  Or are they?  What's the actual utility here?
> 
> Current page table entries print does not work here for
> 128 bits format (arm64 D128) because neither 'unsigned
> long long' can hold 128 bits nor printing extracts u64
> from the 128 bit entries while printing with "%llx".
> 
> Proposed a separate printk format %pp[te|md|ud|p4d|gd]
> 
> https://lore.kernel.org/all/20260610043545.3725735-1-anshuman.khandual@arm.com/  
>  
> But there after it was decided to rather have a local
> solution in mm/memory.c instead of adding a new print
> format.

OK, thanks.  Let's please spell such things out in the changelogging. 
Explaining how a change helps our users (ie, improves Linux) is super
important!

> >>  	if (!pgd_present(*pgdp) || pgd_leaf(*pgdp)) {
> >> -		pr_alert("pgd:%08llx\n", pgdv);
> >> +		pr_alert("pgd:%s\n", pgd_str);
> > 
> > can this do
> > 		pr_alert("pgd:%s\n", ptval_to_str(pgd_str, pgd_val(*pgdp)));
> > 
> > and eliminate a few locals?
> 
> But that would not eliminate any local variable.
> 
> The helper ptval_to_str() does not return but instead
> fills up the provided buffer which later gets printed.

Well yes.  Changing it to return `buf' was kinda implicit!

Re: [PATCH V3] mm: Standardize printing for pgtable entries

[David Hildenbrand (Arm)]

On 7/10/26 02:08, Andrew Morton wrote:
> On Thu,  9 Jul 2026 10:13:34 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
>> From: "David Hildenbrand (Arm)" <david@kernel.org>
>>
>> Bad page map reporting currently stores page table entry values in an
>> unsigned long long and prints them with fixed 64-bit-oriented format
>> strings. This is inconsistent across call sites and does not work well for
>> architectures where page table entry values are not naturally represented
>> as 64-bit values, such as 32-bit or 128-bit entries.
> 
> Well grumble.  It's a lot of fuss for something which nobody is hurting
> from.  Or are they?  What's the actual utility here?

Hugh seems to rely on this information heavily when debugging

https://lore.kernel.org/r/3afa822d-3cc9-1068-9a10-94a5f2e4d29a@google.com

-- 
Cheers,

David