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

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

> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> Co-developed-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> ---
... 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