Re: [RFC V2 3/3] mm: Replace pgtable entry prints with new format

[Anshuman Khandual]

On 02/07/26 1:02 PM, David Hildenbrand (Arm) wrote:
> On 7/2/26 06:29, Anshuman Khandual wrote:
>>
>>
>> On 30/06/26 7:06 PM, David Hildenbrand (Arm) wrote:
>>> On 6/16/26 08:19, Hugh Dickins wrote:
>>>>
>>>> Yes, that's what it's for. What we really want is to understand what went
>>>> wrong: that's too much to ask of a printk, but it can give us a good clue.
>>>>
>>>>
>>>> Page table entry and pmd entry are good enough: higher levels got
>>>> added at some stage, but they are unlikely to be useful here.
>>>
>>> Yes, I added them when we're processing PUD entries we'd also want
>>> P4D entry + PUD entry.
>>>
>>> This is one approach of having the printing be done mostly
>>> manually, supporting 32, 64 and 128bit pte_val(). As raised by Ryan,
>>> using local bufs to store the data to not involve printk.
>>>
>>>
>>> I played with printing the byte stream manually, but didn't really like it.
>>>
>>> Gave it a quick test and it seems to do its trick. I have the feeling that
>>> this can be beautified a bit more.
>>>
>>>
>>> From 05af7317b126991a61b0a3d01c2863ce5a578d1b Mon Sep 17 00:00:00 2001
>>> From: "David Hildenbrand (Arm)" <david@xxxxxxxxxx>
>>> Date: Tue, 30 Jun 2026 15:23:02 +0200
>>> Subject: [PATCH] tmp
>>>
>>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>>> ---
>>>  mm/memory.c | 110 +++++++++++++++++++++++++++++++++++++++++-----------
>>>  1 file changed, 87 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index ff338c2abe923..ad39cafe110f9 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -519,9 +519,57 @@ static bool is_bad_page_map_ratelimited(void)
>>>  	return false;
>>>  }
>>>  
>>> +#define PTVAL_STR_MAX	(sizeof(u64) * 4 + 1)
>>> +
>>> +static void ptval_bytes_to_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",
>>> +			 (unsigned long long)*(const u64 *)entry);
>>> +		break;
>>> +	case sizeof(u64) * 2: {
>>
>> Could this be made sizeof(u128) instead ? But overall this
>> approach looks good.
> 
> The would be cleaner. We might have to protect this case by something like
> 
> #defined(__SIZEOF_INT128__)
> 	case sizeof(u128):
> 		...
> 		break;
> #endif
> 	default:

Right - realized that just a bit later :) Not all
platforms and corresponding tool chains might not
support u128.

> 
> 	...
> 
> Can you take over this approach and refine it (and address Andy's comments)?

Sure will do that.
> 
> I'm not quite happy about the
> 
> 	typeof(pud_val(pud)) entry = pud_val(pud);
> 
> stuff, but I didn't see an easy (less ugly) way to avoid it. Maybe there is one :)
>

Could __auto_type be an alternative ?