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

[David Hildenbrand (Arm)]

On 7/8/26 13:22, Ryan Roberts wrote:
> On 08/07/2026 04:28, Anshuman Khandual wrote:
>> From: "David Hildenbrand (Arm)" <david@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxxxxxxx>
>> Cc: linux-mm@xxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
>> ---
>> This patch applies on v7.2-rc2
>>
>> Changes in V2:
>>
>> - 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@xxxxxxx/
>>
>>  mm/memory.c | 98 ++++++++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 75 insertions(+), 23 deletions(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index ff338c2abe92..a2b63af82792 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -519,9 +519,48 @@ static bool is_bad_page_map_ratelimited(void)
>>  	return false;
>>  }
>>  
>> +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */
> 
> Not sure if it's worth doing something like this?:
> 
> #define PTVAL_STR_MAX						\
> 	(MAX(MAX(MAX(MAX(sizeof(pteval_t), sizeof(pmdval_t)), 	\
> 	    	     sizeof(pudval_t)), 			\
> 		 sizeof(p4dval_t)), 				\
>     	     sizeof(pgdval_t)) + 1)
> 
> Would probably save stack space for 32bit arches?

Do we really care about that? This is about a corner cases error reporting right
now.

> 
>> +
>> +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_type __val = (val);						\
>> +											\
>> +		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
>> +	} while (0)
> 
> I think arm64 code also does pte printing, which you also need to fix up for
> D128 support. Perhaps this could be moved to a header for reuse?

We could do that as a second step, right?

> 
> Also not sure if it's worth returning buf so that this pattern would be possible:
> 
> char pmd_str[PTVAL_STR_MAX];
> pr_err("pmd=%s\n, ptval_to_str(pmd_str, pmd_val(*pmdp));
> 
> Although perhaps that's a bit busy and should be discouraged...

Yeah, let's not do that.

-- 
Cheers,

David