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

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