[PATCH] mm: Standardize printing for pgtable entries

[PATCH] 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>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This patch applies on v7.2-rc2

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

diff --git a/mm/memory.c b/mm/memory.c
index ff338c2abe92..f0c3478f1a63 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -519,9 +519,52 @@ static bool is_bad_page_map_ratelimited(void)
 	return false;
 }
 
+#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
+
+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): {
+		const u64 *val = entry;
+
+		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+			snprintf(buf, buf_size, "%016llx%016llx", val[0], val[1]);
+		else
+			snprintf(buf, buf_size, "%016llx%016llx", val[1], val[0]);
+		break;
+	}
+#endif
+	default:
+		snprintf(buf, buf_size, "unsupported");
+		break;
+	}
+}
+
+#define ptval_to_str(buf, val)								\
+	do {										\
+		__auto_type __val = (val);						\
+											\
+		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
+	} while (0)
+
 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_type 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_type 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_type 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_type 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] mm: Standardize printing for pgtable entries

[Matthew Wilcox]

On Tue, Jul 07, 2026 at 09:47:03AM +0530, Anshuman Khandual wrote:
>  	if (!pgd_present(*pgdp) || pgd_leaf(*pgdp)) {
> -		pr_alert("pgd:%08llx\n", pgdv);
> +		pr_alert("pgd: %s\n", pgd_str);

I don't like the extra space being introduced after the colon.
It's normal practice to not do that.

But in general, this looks a lot better.

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

[Anshuman Khandual]

On 07/07/26 10:23 AM, Matthew Wilcox wrote:
> On Tue, Jul 07, 2026 at 09:47:03AM +0530, Anshuman Khandual wrote:
>>  	if (!pgd_present(*pgdp) || pgd_leaf(*pgdp)) {
>> -		pr_alert("pgd:%08llx\n", pgdv);
>> +		pr_alert("pgd: %s\n", pgd_str);
> 
> I don't like the extra space being introduced after the colon.
> It's normal practice to not do that.

Sure will remove that.
> 
> But in general, this looks a lot better.

Thanks !

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

[David Hildenbrand (Arm)]

On 7/7/26 06:17, 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
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>

You should add a Co-developed-by here :)

> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---

[...]

> +
> +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): {
> +		const u64 *val = entry;
> +
> +		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
> +			snprintf(buf, buf_size, "%016llx%016llx", val[0], val[1]);
> +		else
> +			snprintf(buf, buf_size, "%016llx%016llx", val[1], val[0]);
> +		break;

As discussed, can we simplify that by casting to (u128) and then shifting the
values into place?

-- 
Cheers,

David

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

[Anshuman Khandual]

On 07/07/26 12:38 PM, David Hildenbrand (Arm) wrote:
> On 7/7/26 06:17, 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
>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> 
> You should add a Co-developed-by here :)

Have not done much changes from your original patch :)

> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
> 
> [...]
> 
>> +
>> +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): {
>> +		const u64 *val = entry;
>> +
>> +		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
>> +			snprintf(buf, buf_size, "%016llx%016llx", val[0], val[1]);
>> +		else
>> +			snprintf(buf, buf_size, "%016llx%016llx", val[1], val[0]);
>> +		break;
> 
> As discussed, can we simplify that by casting to (u128) and then shifting the
> values into place?

which would be something like the following - had similar
construct in D128 RFC V2 series as well.

snprintf(buf, buf_size, "%016llx%016llx",
	(unsigned long long)*(const u128 *)entry >> 64),
	(unsigned long long)*(const u128 *)entry;

I did think about it but is not the proposed chunk bit cleaner
instead and easier to follow. Do you have concern regarding u64
pointers into u128 then accessed in chunks ? Regardless don't
have a strong opinion either way.

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

[David Hildenbrand (Arm)]

On 7/7/26 10:58, Anshuman Khandual wrote:
> 
> 
> On 07/07/26 12:38 PM, David Hildenbrand (Arm) wrote:
>> On 7/7/26 06:17, 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
>>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>>
>> You should add a Co-developed-by here :)
> 
> Have not done much changes from your original patch :)

Well, you did some :)

> 
>>> ---
>>
>> [...]
>>
>>> +
>>> +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): {
>>> +		const u64 *val = entry;
>>> +
>>> +		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
>>> +			snprintf(buf, buf_size, "%016llx%016llx", val[0], val[1]);
>>> +		else
>>> +			snprintf(buf, buf_size, "%016llx%016llx", val[1], val[0]);
>>> +		break;
>>
>> As discussed, can we simplify that by casting to (u128) and then shifting the
>> values into place?
> 
> which would be something like the following - had similar
> construct in D128 RFC V2 series as well.
> 
> snprintf(buf, buf_size, "%016llx%016llx",
> 	(unsigned long long)*(const u128 *)entry >> 64),
> 	(unsigned long long)*(const u128 *)entry;
> 
> I did think about it but is not the proposed chunk bit cleaner
> instead and easier to follow. Do you have concern regarding u64
> pointers into u128 then accessed in chunks ? Regardless don't
> have a strong opinion either way.

I think any removed endianess dependency is a thing worth considering :)

-- 
Cheers,

David