[RFC 0/2] lib/vsprintf: Add support for pte_t

[RFC 0/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

Printing page table entries has been a common requirement both in generic
and platform memory management for various purposes. Let's just create a
dedicated printk format for such entries which will also help standardize
printing across different platforms.

Also add a test for this new print format in lib/tests/printf_kunit.c via
CONFIG_PRINTF_KUNIT_TEST.

This series applies on v6.16-rc2

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-mm@kvack.org

Anshuman Khandual (2):
  lib/vsprintf: Add support for pte_t
  kunit: printf: Add test case for pte_t

 Documentation/core-api/printk-formats.rst | 14 ++++++++++++
 lib/tests/printf_kunit.c                  | 26 +++++++++++++++++++++++
 lib/vsprintf.c                            | 20 +++++++++++++++++
 mm/memory.c                               |  5 ++---
 scripts/checkpatch.pl                     |  2 +-
 5 files changed, 63 insertions(+), 4 deletions(-)

-- 
2.30.2

[RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

Add a new format for printing page table entries.

Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
 lib/vsprintf.c                            | 20 ++++++++++++++++++++
 mm/memory.c                               |  5 ++---
 scripts/checkpatch.pl                     |  2 +-
 4 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 4b7f3646ec6ce..75a110b059ee1 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -689,6 +689,20 @@ Rust
 Only intended to be used from Rust code to format ``core::fmt::Arguments``.
 Do *not* use it from C.
 
+Page Table Entry
+----------------
+
+::
+        %ppte
+
+Print standard page table entry pte_t.
+
+Passed by reference.
+
+Examples for a 64 bit page table entry, given &(u64)0xc0ffee::
+
+        %ppte   0x00c0ffee
+
 Thanks
 ======
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3d85800757aa5..005490202ffb5 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2433,6 +2433,9 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
  *		Without an option prints the full name of the node
  *		f full name
  *		P node name, including a possible unit address
+ * - 'pte'	For a 64 bit page table entry, this prints its contents in
+ *              a hexa decimal format
+ *
  * - 'x' For printing the address unmodified. Equivalent to "%lx".
  *       Please read the documentation (path below) before using!
  * - '[ku]s' For a BPF/tracing related format specifier, e.g. used out of
@@ -2542,6 +2545,23 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		default:
 			return error_string(buf, end, "(einval)", spec);
 		}
+	case 'p':
+		if (fmt[1] == 't' && fmt[2] == 'e') {
+			pte_t *pte = (pte_t *)ptr;
+
+			spec.field_width = 10;
+			spec.precision = 8;
+			spec.base = 16;
+			spec.flags = SPECIAL | SMALL | ZEROPAD;
+			if (sizeof(pte_t) == sizeof(u64)) {
+				u64 val = pte_val(*pte);
+
+				return number(buf, end, val, spec);
+			}
+			WARN_ONCE(1, "Non standard pte_t\n");
+			return error_string(buf, end, "(einval)", spec);
+		}
+		fallthrough;
 	default:
 		return default_pointer(buf, end, ptr, spec);
 	}
diff --git a/mm/memory.c b/mm/memory.c
index 8eba595056fe3..c9a057b371eae 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -522,9 +522,8 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
 	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  pte:%08llx pmd:%08llx\n",
-		 current->comm,
-		 (long long)pte_val(pte), (long long)pmd_val(*pmd));
+	pr_alert("BUG: Bad page map in process %s  pte:%ppte pmd:%ppte\n",
+		 current->comm, &pte, pmd);
 	if (page)
 		dump_page(page, "bad pte");
 	pr_alert("addr:%px vm_flags:%08lx anon_vma:%px mapping:%px index:%lx\n",
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 664f7b7a622c2..fb8abc5dbfb8e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6906,7 +6906,7 @@ sub process {
 				my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
 				$fmt =~ s/%%//g;
 
-				while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*))/g) {
+				while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*)(\te))/g) {
 					$specifier = $1;
 					$extension = $2;
 					$qualifier = $3;
-- 
2.30.2

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[David Hildenbrand]

On 18.06.25 06:12, Anshuman Khandual wrote:
> Add a new format for printing page table entries.
> 
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mm@kvack.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
>   lib/vsprintf.c                            | 20 ++++++++++++++++++++
>   mm/memory.c                               |  5 ++---
>   scripts/checkpatch.pl                     |  2 +-
>   4 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 4b7f3646ec6ce..75a110b059ee1 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -689,6 +689,20 @@ Rust
>   Only intended to be used from Rust code to format ``core::fmt::Arguments``.
>   Do *not* use it from C.
>   
> +Page Table Entry
> +----------------
> +
> +::
> +        %ppte
> +
> +Print standard page table entry pte_t.
> +
> +Passed by reference.

Curious, why the decision to pass by reference?


-- 
Cheers,

David / dhildenb

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 18/06/25 1:48 PM, David Hildenbrand wrote:
> On 18.06.25 06:12, Anshuman Khandual wrote:
>> Add a new format for printing page table entries.
>>
>> Cc: Petr Mladek <pmladek@suse.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-mm@kvack.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
>>   lib/vsprintf.c                            | 20 ++++++++++++++++++++
>>   mm/memory.c                               |  5 ++---
>>   scripts/checkpatch.pl                     |  2 +-
>>   4 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>> index 4b7f3646ec6ce..75a110b059ee1 100644
>> --- a/Documentation/core-api/printk-formats.rst
>> +++ b/Documentation/core-api/printk-formats.rst
>> @@ -689,6 +689,20 @@ Rust
>>   Only intended to be used from Rust code to format ``core::fmt::Arguments``.
>>   Do *not* use it from C.
>>   +Page Table Entry
>> +----------------
>> +
>> +::
>> +        %ppte
>> +
>> +Print standard page table entry pte_t.
>> +
>> +Passed by reference.
> 
> Curious, why the decision to pass by reference?

Just to make this via %p<> based address mechanism. But wondering
will it be better for the pte to be represented via value instead
of reference ?

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[David Hildenbrand]

On 18.06.25 10:37, Anshuman Khandual wrote:
> 
> 
> On 18/06/25 1:48 PM, David Hildenbrand wrote:
>> On 18.06.25 06:12, Anshuman Khandual wrote:
>>> Add a new format for printing page table entries.
>>>
>>> Cc: Petr Mladek <pmladek@suse.com>
>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: linux-doc@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: linux-mm@kvack.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>    Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
>>>    lib/vsprintf.c                            | 20 ++++++++++++++++++++
>>>    mm/memory.c                               |  5 ++---
>>>    scripts/checkpatch.pl                     |  2 +-
>>>    4 files changed, 37 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>>> index 4b7f3646ec6ce..75a110b059ee1 100644
>>> --- a/Documentation/core-api/printk-formats.rst
>>> +++ b/Documentation/core-api/printk-formats.rst
>>> @@ -689,6 +689,20 @@ Rust
>>>    Only intended to be used from Rust code to format ``core::fmt::Arguments``.
>>>    Do *not* use it from C.
>>>    +Page Table Entry
>>> +----------------
>>> +
>>> +::
>>> +        %ppte
>>> +
>>> +Print standard page table entry pte_t.
>>> +
>>> +Passed by reference.
>>
>> Curious, why the decision to pass by reference?
> 
> Just to make this via %p<> based address mechanism. But wondering
> will it be better for the pte to be represented via value instead
> of reference ?

We commonly pass ptes to functions through value, not reference, that's 
why I am asking.

-- 
Cheers,

David / dhildenb

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Pedro Falcato]

On Wed, Jun 18, 2025 at 10:44:20AM +0200, David Hildenbrand wrote:
> On 18.06.25 10:37, Anshuman Khandual wrote:
> > 
> > 
> > On 18/06/25 1:48 PM, David Hildenbrand wrote:
> > > On 18.06.25 06:12, Anshuman Khandual wrote:
> > > > Add a new format for printing page table entries.
> > > > 
> > > > Cc: Petr Mladek <pmladek@suse.com>
> > > > Cc: Steven Rostedt <rostedt@goodmis.org>
> > > > Cc: Jonathan Corbet <corbet@lwn.net>
> > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > Cc: David Hildenbrand <david@redhat.com>
> > > > Cc: linux-doc@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Cc: linux-mm@kvack.org
> > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > > > ---
> > > >    Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
> > > >    lib/vsprintf.c                            | 20 ++++++++++++++++++++
> > > >    mm/memory.c                               |  5 ++---
> > > >    scripts/checkpatch.pl                     |  2 +-
> > > >    4 files changed, 37 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> > > > index 4b7f3646ec6ce..75a110b059ee1 100644
> > > > --- a/Documentation/core-api/printk-formats.rst
> > > > +++ b/Documentation/core-api/printk-formats.rst
> > > > @@ -689,6 +689,20 @@ Rust
> > > >    Only intended to be used from Rust code to format ``core::fmt::Arguments``.
> > > >    Do *not* use it from C.
> > > >    +Page Table Entry
> > > > +----------------
> > > > +
> > > > +::
> > > > +        %ppte
> > > > +
> > > > +Print standard page table entry pte_t.
> > > > +
> > > > +Passed by reference.
> > > 
> > > Curious, why the decision to pass by reference?
> > 
> > Just to make this via %p<> based address mechanism. But wondering
> > will it be better for the pte to be represented via value instead
> > of reference ?
> 
> We commonly pass ptes to functions through value, not reference, that's why
> I am asking.


All printf/printk extensions in the kernel follow %p<some letters> and use
pointers because %p takes pointers, so it lets us use -Wformat with no issues.

So yes, taking a pte_t * is required.

-- 
Pedro

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Petr Mladek]

On Wed 2025-06-18 19:16:00, Pedro Falcato wrote:
> On Wed, Jun 18, 2025 at 10:44:20AM +0200, David Hildenbrand wrote:
> > On 18.06.25 10:37, Anshuman Khandual wrote:
> > > 
> > > 
> > > On 18/06/25 1:48 PM, David Hildenbrand wrote:
> > > > On 18.06.25 06:12, Anshuman Khandual wrote:
> > > > > Add a new format for printing page table entries.
> > > > > 
> > > > > Cc: Petr Mladek <pmladek@suse.com>
> > > > > Cc: Steven Rostedt <rostedt@goodmis.org>
> > > > > Cc: Jonathan Corbet <corbet@lwn.net>
> > > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > > Cc: David Hildenbrand <david@redhat.com>
> > > > > Cc: linux-doc@vger.kernel.org
> > > > > Cc: linux-kernel@vger.kernel.org
> > > > > Cc: linux-mm@kvack.org
> > > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > > > > ---
> > > > >    Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
> > > > >    lib/vsprintf.c                            | 20 ++++++++++++++++++++
> > > > >    mm/memory.c                               |  5 ++---
> > > > >    scripts/checkpatch.pl                     |  2 +-
> > > > >    4 files changed, 37 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> > > > > index 4b7f3646ec6ce..75a110b059ee1 100644
> > > > > --- a/Documentation/core-api/printk-formats.rst
> > > > > +++ b/Documentation/core-api/printk-formats.rst
> > > > > @@ -689,6 +689,20 @@ Rust
> > > > >    Only intended to be used from Rust code to format ``core::fmt::Arguments``.
> > > > >    Do *not* use it from C.
> > > > >    +Page Table Entry
> > > > > +----------------
> > > > > +
> > > > > +::
> > > > > +        %ppte
> > > > > +
> > > > > +Print standard page table entry pte_t.
> > > > > +
> > > > > +Passed by reference.
> > > > 
> > > > Curious, why the decision to pass by reference?
> > > 
> > > Just to make this via %p<> based address mechanism. But wondering
> > > will it be better for the pte to be represented via value instead
> > > of reference ?
> > 
> > We commonly pass ptes to functions through value, not reference, that's why
> > I am asking.
> 
> 
> All printf/printk extensions in the kernel follow %p<some letters> and use
> pointers because %p takes pointers, so it lets us use -Wformat with no issues.
> 
> So yes, taking a pte_t * is required.

Correct. But the pointer is usually needed because the %pxx format
need to access a structure.

Passing a pointer is another potential source of errors. I mean that
the callers might pass an invalid pointer by mistake...

Another aspect is performance. It is likely not a big deal for classic
printk() which is a slow path. But trace_printk() tries to optimize
the speed by deferred formatting where possible, see vbin_printf()
and bstr_printf().

I think that this is not a blocker for this patchset. But you should
know that using %pxx has a cost.

Best Regards,
Petr

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 19/06/25 6:38 PM, Petr Mladek wrote:
> On Wed 2025-06-18 19:16:00, Pedro Falcato wrote:
>> On Wed, Jun 18, 2025 at 10:44:20AM +0200, David Hildenbrand wrote:
>>> On 18.06.25 10:37, Anshuman Khandual wrote:
>>>>
>>>>
>>>> On 18/06/25 1:48 PM, David Hildenbrand wrote:
>>>>> On 18.06.25 06:12, Anshuman Khandual wrote:
>>>>>> Add a new format for printing page table entries.
>>>>>>
>>>>>> Cc: Petr Mladek <pmladek@suse.com>
>>>>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>>>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>>>> Cc: David Hildenbrand <david@redhat.com>
>>>>>> Cc: linux-doc@vger.kernel.org
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: linux-mm@kvack.org
>>>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>>> ---
>>>>>>    Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
>>>>>>    lib/vsprintf.c                            | 20 ++++++++++++++++++++
>>>>>>    mm/memory.c                               |  5 ++---
>>>>>>    scripts/checkpatch.pl                     |  2 +-
>>>>>>    4 files changed, 37 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>>>>>> index 4b7f3646ec6ce..75a110b059ee1 100644
>>>>>> --- a/Documentation/core-api/printk-formats.rst
>>>>>> +++ b/Documentation/core-api/printk-formats.rst
>>>>>> @@ -689,6 +689,20 @@ Rust
>>>>>>    Only intended to be used from Rust code to format ``core::fmt::Arguments``.
>>>>>>    Do *not* use it from C.
>>>>>>    +Page Table Entry
>>>>>> +----------------
>>>>>> +
>>>>>> +::
>>>>>> +        %ppte
>>>>>> +
>>>>>> +Print standard page table entry pte_t.
>>>>>> +
>>>>>> +Passed by reference.
>>>>>
>>>>> Curious, why the decision to pass by reference?
>>>>
>>>> Just to make this via %p<> based address mechanism. But wondering
>>>> will it be better for the pte to be represented via value instead
>>>> of reference ?
>>>
>>> We commonly pass ptes to functions through value, not reference, that's why
>>> I am asking.
>>
>>
>> All printf/printk extensions in the kernel follow %p<some letters> and use
>> pointers because %p takes pointers, so it lets us use -Wformat with no issues.
>>
>> So yes, taking a pte_t * is required.
> 
> Correct. But the pointer is usually needed because the %pxx format
> need to access a structure.

Right.

> 
> Passing a pointer is another potential source of errors. I mean that
> the callers might pass an invalid pointer by mistake...

Agreed - could be a source of error when not used properly.

> 
> Another aspect is performance. It is likely not a big deal for classic
> printk() which is a slow path. But trace_printk() tries to optimize
> the speed by deferred formatting where possible, see vbin_printf()
> and bstr_printf().
> 
> I think that this is not a blocker for this patchset. But you should
> know that using %pxx has a cost.
Got it - thanks for the explanation.

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[David Hildenbrand]

> +	case 'p':
> +		if (fmt[1] == 't' && fmt[2] == 'e') {
> +			pte_t *pte = (pte_t *)ptr;
> +
> +			spec.field_width = 10;
> +			spec.precision = 8;
> +			spec.base = 16;
> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
> +			if (sizeof(pte_t) == sizeof(u64)) {
> +				u64 val = pte_val(*pte);
> +
> +				return number(buf, end, val, spec);
> +			}
> +			WARN_ONCE(1, "Non standard pte_t\n");

What about 32bit with 32bit pte_t?

-- 
Cheers,

David / dhildenb

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 18/06/25 1:49 PM, David Hildenbrand wrote:
>> +    case 'p':
>> +        if (fmt[1] == 't' && fmt[2] == 'e') {
>> +            pte_t *pte = (pte_t *)ptr;
>> +
>> +            spec.field_width = 10;
>> +            spec.precision = 8;
>> +            spec.base = 16;
>> +            spec.flags = SPECIAL | SMALL | ZEROPAD;
>> +            if (sizeof(pte_t) == sizeof(u64)) {
>> +                u64 val = pte_val(*pte);
>> +
>> +                return number(buf, end, val, spec);
>> +            }
>> +            WARN_ONCE(1, "Non standard pte_t\n");
> 
> What about 32bit with 32bit pte_t?

Ahh, missed that. Just wondering which all platforms might
care about the 32 bit pte representation.

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[David Hildenbrand]

On 18.06.25 10:33, Anshuman Khandual wrote:
> 
> 
> On 18/06/25 1:49 PM, David Hildenbrand wrote:
>>> +    case 'p':
>>> +        if (fmt[1] == 't' && fmt[2] == 'e') {
>>> +            pte_t *pte = (pte_t *)ptr;
>>> +
>>> +            spec.field_width = 10;
>>> +            spec.precision = 8;
>>> +            spec.base = 16;
>>> +            spec.flags = SPECIAL | SMALL | ZEROPAD;
>>> +            if (sizeof(pte_t) == sizeof(u64)) {
>>> +                u64 val = pte_val(*pte);
>>> +
>>> +                return number(buf, end, val, spec);
>>> +            }
>>> +            WARN_ONCE(1, "Non standard pte_t\n");
>>
>> What about 32bit with 32bit pte_t?
> 
> Ahh, missed that. Just wondering which all platforms might
> care about the 32 bit pte representation.

I think e.g., 32bit arm has 32bit ptes?

arch/arm/include/asm/pgtable-2level-types.h

typedef u32 pteval_t;
...
typedef struct { pteval_t pte; } pte_t;

-- 
Cheers,

David / dhildenb

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 18/06/25 2:13 PM, David Hildenbrand wrote:
> On 18.06.25 10:33, Anshuman Khandual wrote:
>>
>>
>> On 18/06/25 1:49 PM, David Hildenbrand wrote:
>>>> +    case 'p':
>>>> +        if (fmt[1] == 't' && fmt[2] == 'e') {
>>>> +            pte_t *pte = (pte_t *)ptr;
>>>> +
>>>> +            spec.field_width = 10;
>>>> +            spec.precision = 8;
>>>> +            spec.base = 16;
>>>> +            spec.flags = SPECIAL | SMALL | ZEROPAD;
>>>> +            if (sizeof(pte_t) == sizeof(u64)) {
>>>> +                u64 val = pte_val(*pte);
>>>> +
>>>> +                return number(buf, end, val, spec);
>>>> +            }
>>>> +            WARN_ONCE(1, "Non standard pte_t\n");
>>>
>>> What about 32bit with 32bit pte_t?
>>
>> Ahh, missed that. Just wondering which all platforms might
>> care about the 32 bit pte representation.
> 
> I think e.g., 32bit arm has 32bit ptes?
> 
> arch/arm/include/asm/pgtable-2level-types.h
> 
> typedef u32 pteval_t;
> ...
> typedef struct { pteval_t pte; } pte_t;

Right, missed that. I will accommodate 32 bit representations.

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Andy Shevchenko]

On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
> Add a new format for printing page table entries.

> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mm@kvack.org

Please. move these to be after the '---' cutter line below. Just leave SoB tag
alone. This will have the same effect w/o polluting commit message.

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

(somewhere here is a good place for all your Cc: tags)

...

> +        %ppte

I believe you can take %pte.

...

> +Print standard page table entry pte_t.
> +
> +Passed by reference.
> +
> +Examples for a 64 bit page table entry, given &(u64)0xc0ffee::

What does this mean?

> +        %ppte   0x00c0ffee

Can it be ever 64-bit?

...

> +			spec.field_width = 10;
> +			spec.precision = 8;
> +			spec.base = 16;
> +			spec.flags = SPECIAL | SMALL | ZEROPAD;

Do not duplicate code we have already in the file.

> +			if (sizeof(pte_t) == sizeof(u64)) {
> +				u64 val = pte_val(*pte);
> +
> +				return number(buf, end, val, spec);
> +			}

Ditto.

> +			WARN_ONCE(1, "Non standard pte_t\n");

(almost) Ditto,

> +			return error_string(buf, end, "(einval)", spec);

Ditto.

> +		}
> +		fallthrough;

Please, avoid this, it makes code much harder to read and maintain.
See above how.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 18/06/25 11:16 PM, Andy Shevchenko wrote:
> On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
>> Add a new format for printing page table entries.
> 
>> Cc: Petr Mladek <pmladek@suse.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-mm@kvack.org
> 
> Please. move these to be after the '---' cutter line below. Just leave SoB tag
> alone. This will have the same effect w/o polluting commit message.
> 
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
> 
> (somewhere here is a good place for all your Cc: tags)

Is not it better to also capture the Cc: list in the commit message.
Seems like such has been the practice for various patches on the MM
list. But not sure if that is an expected standard for all patches.

> 
> ...
> 
>> +        %ppte
> 
> I believe you can take %pte.

Yes - that should be possible.

> 
> ...
> 
>> +Print standard page table entry pte_t.
>> +
>> +Passed by reference.
>> +
>> +Examples for a 64 bit page table entry, given &(u64)0xc0ffee::
> 
> What does this mean?

64 bit address containing value the 0xc0ffee

> 
>> +        %ppte   0x00c0ffee
> 
> Can it be ever 64-bit?
I am sorry - did not get that. pte_t contained value can be 64
bits if that's what you meant.

> 
> ...
> 
>> +			spec.field_width = 10;
>> +			spec.precision = 8;
>> +			spec.base = 16;
>> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
> 
> Do not duplicate code we have already in the file.
I am sorry - did not get that. Is the above flag combination some
how wrong ?

> 
>> +			if (sizeof(pte_t) == sizeof(u64)) {
>> +				u64 val = pte_val(*pte);
>> +
>> +				return number(buf, end, val, spec);
>> +			}
> 
> Ditto.
> 
>> +			WARN_ONCE(1, "Non standard pte_t\n");
> 
> (almost) Ditto,
> 
>> +			return error_string(buf, end, "(einval)", spec);
> 
> Ditto.
> 
>> +		}
>> +		fallthrough;
> 
> Please, avoid this, it makes code much harder to read and maintain.
> See above how.
> 

Could you please kindly elaborate on the code duplication problem
you have mentioned earlier. I might not understand your concern
here correctly.

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Andy Shevchenko]

On Thu, Jun 19, 2025 at 03:05:10PM +0530, Anshuman Khandual wrote:
> On 18/06/25 11:16 PM, Andy Shevchenko wrote:
> > On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
> >> Add a new format for printing page table entries.
> > 
> >> Cc: Petr Mladek <pmladek@suse.com>
> >> Cc: Steven Rostedt <rostedt@goodmis.org>
> >> Cc: Jonathan Corbet <corbet@lwn.net>
> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> Cc: David Hildenbrand <david@redhat.com>
> >> Cc: linux-doc@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: linux-mm@kvack.org
> > 
> > Please. move these to be after the '---' cutter line below. Just leave SoB tag
> > alone. This will have the same effect w/o polluting commit message.
> > 
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> > 
> > (somewhere here is a good place for all your Cc: tags)
> 
> Is not it better to also capture the Cc: list in the commit message.

No it's worse. One may easily get the same from lore. Can you give a good
justification for the polluting message with 8 lines over a single line of the
useful information, please?

> Seems like such has been the practice for various patches on the MM
> list. But not sure if that is an expected standard for all patches.

It's not an MM subsystem.

...

> >> +Print standard page table entry pte_t.
> >> +
> >> +Passed by reference.
> >> +
> >> +Examples for a 64 bit page table entry, given &(u64)0xc0ffee::
> > 
> > What does this mean?
> 
> 64 bit address containing value the 0xc0ffee

Please, make it 64-bit address. The example as is is quite confusing.

> >> +        %ppte   0x00c0ffee
> > 
> > Can it be ever 64-bit?
> I am sorry - did not get that. pte_t contained value can be 64
> bits if that's what you meant.

Yes, see above why I have such a question.

...

> >> +			spec.field_width = 10;
> >> +			spec.precision = 8;
> >> +			spec.base = 16;
> >> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
> > 
> > Do not duplicate code we have already in the file.
> I am sorry - did not get that. Is the above flag combination some
> how wrong ?

It's dup. Please, take your time to find the very similar piece of code in one
of the helper functions we have.

I recommend you to look at the history of the changes in this file for when the
new specifier was added and how it is implemented.

...

> Could you please kindly elaborate on the code duplication problem
> you have mentioned earlier. I might not understand your concern
> here correctly.

Just find the same or similar pieces of code elsewhere in the same file.
Use them.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 19/06/25 5:30 PM, Andy Shevchenko wrote:
> On Thu, Jun 19, 2025 at 03:05:10PM +0530, Anshuman Khandual wrote:
>> On 18/06/25 11:16 PM, Andy Shevchenko wrote:
>>> On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
>>>> Add a new format for printing page table entries.
>>>
>>>> Cc: Petr Mladek <pmladek@suse.com>
>>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>> Cc: David Hildenbrand <david@redhat.com>
>>>> Cc: linux-doc@vger.kernel.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: linux-mm@kvack.org
>>>
>>> Please. move these to be after the '---' cutter line below. Just leave SoB tag
>>> alone. This will have the same effect w/o polluting commit message.
>>>
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>
>>> (somewhere here is a good place for all your Cc: tags)
>>
>> Is not it better to also capture the Cc: list in the commit message.
> 
> No it's worse. One may easily get the same from lore. Can you give a good
> justification for the polluting message with 8 lines over a single line of the
> useful information, please?

Will drop the Cc: list from the commit message and move it below '---'
cutter line as suggested earlier.

> 
>> Seems like such has been the practice for various patches on the MM
>> list. But not sure if that is an expected standard for all patches.
> 
> It's not an MM subsystem.
> 
> ...
> 
>>>> +Print standard page table entry pte_t.
>>>> +
>>>> +Passed by reference.
>>>> +
>>>> +Examples for a 64 bit page table entry, given &(u64)0xc0ffee::
>>>
>>> What does this mean?
>>
>> 64 bit address containing value the 0xc0ffee
> 
> Please, make it 64-bit address. The example as is is quite confusing.
Agreed it is some what confusing - will fix it.

> 
>>>> +        %ppte   0x00c0ffee
>>>
>>> Can it be ever 64-bit?
>> I am sorry - did not get that. pte_t contained value can be 64
>> bits if that's what you meant.
> 
> Yes, see above why I have such a question.

Got it.

> 
> ...
> 
>>>> +			spec.field_width = 10;
>>>> +			spec.precision = 8;
>>>> +			spec.base = 16;
>>>> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
>>>
>>> Do not duplicate code we have already in the file.
>> I am sorry - did not get that. Is the above flag combination some
>> how wrong ?
> 
> It's dup. Please, take your time to find the very similar piece of code in one
> of the helper functions we have.

Are you referring to special_hex_number() ?

> 
> I recommend you to look at the history of the changes in this file for when the
> new specifier was added and how it is implemented> 
> ...
> 
>> Could you please kindly elaborate on the code duplication problem
>> you have mentioned earlier. I might not understand your concern
>> here correctly.
> 
> Just find the same or similar pieces of code elsewhere in the same file.
> Use them.
> Will go through previous print format additions and re-work the patches
accommodating various suggestions. Thanks for your review.

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Andy Shevchenko]

Fri, Jun 20, 2025 at 12:23:47PM +0530, Anshuman Khandual kirjoitti:
> On 19/06/25 5:30 PM, Andy Shevchenko wrote:
> > On Thu, Jun 19, 2025 at 03:05:10PM +0530, Anshuman Khandual wrote:
> >> On 18/06/25 11:16 PM, Andy Shevchenko wrote:
> >>> On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:

...

> >>>> +			spec.field_width = 10;
> >>>> +			spec.precision = 8;
> >>>> +			spec.base = 16;
> >>>> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
> >>>
> >>> Do not duplicate code we have already in the file.
> >> I am sorry - did not get that. Is the above flag combination some
> >> how wrong ?
> > 
> > It's dup. Please, take your time to find the very similar piece of code in one
> > of the helper functions we have.
> 
> Are you referring to special_hex_number() ?

Bingo!

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Petr Mladek]

On Wed 2025-06-18 20:46:38, Andy Shevchenko wrote:
> On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
> > Add a new format for printing page table entries.
> 
> > Cc: Petr Mladek <pmladek@suse.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: linux-doc@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-mm@kvack.org
> 
> Please. move these to be after the '---' cutter line below. Just leave SoB tag
> alone. This will have the same effect w/o polluting commit message.
> 
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > ---
> 
> (somewhere here is a good place for all your Cc: tags)
> 
> ...
> 
> > +        %ppte
> 
> I believe you can take %pte.

We should think about the future. If we added "pte", people would want
to add also "pmd", "pud", ...

It might actually be a good idea to keep them under the %pp prefix.

Best Regards,
Petr

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 19/06/25 6:42 PM, Petr Mladek wrote:
> On Wed 2025-06-18 20:46:38, Andy Shevchenko wrote:
>> On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
>>> Add a new format for printing page table entries.
>>
>>> Cc: Petr Mladek <pmladek@suse.com>
>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: linux-doc@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: linux-mm@kvack.org
>>
>> Please. move these to be after the '---' cutter line below. Just leave SoB tag
>> alone. This will have the same effect w/o polluting commit message.
>>
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>
>> (somewhere here is a good place for all your Cc: tags)
>>
>> ...
>>
>>> +        %ppte
>>
>> I believe you can take %pte.
> 
> We should think about the future. If we added "pte", people would want
> to add also "pmd", "pud", ...
> 
> It might actually be a good idea to keep them under the %pp prefix.

Agreed.

The first 'p' here is for being pointer and second 'p' is from 'p'xx.
Then last two letters to differentiate between page table levels. 

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Pedro Falcato]

On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
> Add a new format for printing page table entries.
> 
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mm@kvack.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
>  lib/vsprintf.c                            | 20 ++++++++++++++++++++
>  mm/memory.c                               |  5 ++---
>  scripts/checkpatch.pl                     |  2 +-
>  4 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 4b7f3646ec6ce..75a110b059ee1 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -689,6 +689,20 @@ Rust
>  Only intended to be used from Rust code to format ``core::fmt::Arguments``.
>  Do *not* use it from C.
>  
> +Page Table Entry
> +----------------
> +
> +::
> +        %ppte
> +
> +Print standard page table entry pte_t.
> +
> +Passed by reference.
> +
> +Examples for a 64 bit page table entry, given &(u64)0xc0ffee::
> +
> +        %ppte   0x00c0ffee

Ok, so what's the point of this if you're just printing the number?

Could at least do something like:

%ppte 0xc0ff000|WRITE|DIRTY|PRESENT

no? Otherwise it's a not super useful wrapper around printing pte_val(*pte).

> +
>  Thanks
>  ======
>  
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3d85800757aa5..005490202ffb5 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -2433,6 +2433,9 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
>   *		Without an option prints the full name of the node
>   *		f full name
>   *		P node name, including a possible unit address
> + * - 'pte'	For a 64 bit page table entry, this prints its contents in
> + *              a hexa decimal format
> + *
>   * - 'x' For printing the address unmodified. Equivalent to "%lx".
>   *       Please read the documentation (path below) before using!
>   * - '[ku]s' For a BPF/tracing related format specifier, e.g. used out of
> @@ -2542,6 +2545,23 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>  		default:
>  			return error_string(buf, end, "(einval)", spec);
>  		}
> +	case 'p':
> +		if (fmt[1] == 't' && fmt[2] == 'e') {
> +			pte_t *pte = (pte_t *)ptr;
> +
> +			spec.field_width = 10;
> +			spec.precision = 8;
> +			spec.base = 16;
> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
> +			if (sizeof(pte_t) == sizeof(u64)) {
> +				u64 val = pte_val(*pte);
> +
> +				return number(buf, end, val, spec);
> +			}

As mentioned elsewhere in the thread, this obviously doesn't work for everything
32-bit, and 64-bit PAE, and all of the weird page table formats we have around.

-- 
Pedro

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 18/06/25 11:49 PM, Pedro Falcato wrote:
> On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
>> Add a new format for printing page table entries.
>>
>> Cc: Petr Mladek <pmladek@suse.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-mm@kvack.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  Documentation/core-api/printk-formats.rst | 14 ++++++++++++++
>>  lib/vsprintf.c                            | 20 ++++++++++++++++++++
>>  mm/memory.c                               |  5 ++---
>>  scripts/checkpatch.pl                     |  2 +-
>>  4 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>> index 4b7f3646ec6ce..75a110b059ee1 100644
>> --- a/Documentation/core-api/printk-formats.rst
>> +++ b/Documentation/core-api/printk-formats.rst
>> @@ -689,6 +689,20 @@ Rust
>>  Only intended to be used from Rust code to format ``core::fmt::Arguments``.
>>  Do *not* use it from C.
>>  
>> +Page Table Entry
>> +----------------
>> +
>> +::
>> +        %ppte
>> +
>> +Print standard page table entry pte_t.
>> +
>> +Passed by reference.
>> +
>> +Examples for a 64 bit page table entry, given &(u64)0xc0ffee::
>> +
>> +        %ppte   0x00c0ffee
> 
> Ok, so what's the point of this if you're just printing the number?

I might have got this wrong probably. The ideas is to represent
a 64 bit address containing a 64 bit value i.e 0xc0ffee - which
needs to be printed via the new print format.

> 
> Could at least do something like:
> 
> %ppte 0xc0ff000|WRITE|DIRTY|PRESENT
> 
> no? Otherwise it's a not super useful wrapper around printing pte_val(*pte).

Although it would be great to have PTE flags called out as well,
the proposed patch here just wanted to transparently extract 64
bit printable value from pte_t represented page table entries.

But coming back to your suggestion above.

%ppte 0xc0ff000|WRITE|DIRTY|PRESENT

Should all the generic page table entry flags and contained pfn
be extracted from the pte_t and printed via new format %ppte ?

> 
>> +
>>  Thanks
>>  ======
>>  
>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>> index 3d85800757aa5..005490202ffb5 100644
>> --- a/lib/vsprintf.c
>> +++ b/lib/vsprintf.c
>> @@ -2433,6 +2433,9 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
>>   *		Without an option prints the full name of the node
>>   *		f full name
>>   *		P node name, including a possible unit address
>> + * - 'pte'	For a 64 bit page table entry, this prints its contents in
>> + *              a hexa decimal format
>> + *
>>   * - 'x' For printing the address unmodified. Equivalent to "%lx".
>>   *       Please read the documentation (path below) before using!
>>   * - '[ku]s' For a BPF/tracing related format specifier, e.g. used out of
>> @@ -2542,6 +2545,23 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>>  		default:
>>  			return error_string(buf, end, "(einval)", spec);
>>  		}
>> +	case 'p':
>> +		if (fmt[1] == 't' && fmt[2] == 'e') {
>> +			pte_t *pte = (pte_t *)ptr;
>> +
>> +			spec.field_width = 10;
>> +			spec.precision = 8;
>> +			spec.base = 16;
>> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
>> +			if (sizeof(pte_t) == sizeof(u64)) {
>> +				u64 val = pte_val(*pte);
>> +
>> +				return number(buf, end, val, spec);
>> +			}
> 
> As mentioned elsewhere in the thread, this obviously doesn't work for everything
> 32-bit, and 64-bit PAE, and all of the weird page table formats we have around.
I will accommodate 32 bit formats.

But what about 64-bit PAE ? Would not pte_val() also return a printable
64 bit number for such cases. Could you please elaborate on the weird
page table formats you mentioned and why would not pte_val() work for
those as well.

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Matthew Wilcox]

On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
> +++ b/mm/memory.c
> @@ -522,9 +522,8 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
>  	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  pte:%08llx pmd:%08llx\n",
> -		 current->comm,
> -		 (long long)pte_val(pte), (long long)pmd_val(*pmd));
> +	pr_alert("BUG: Bad page map in process %s  pte:%ppte pmd:%ppte\n",
> +		 current->comm, &pte, pmd);

Unfortunately, the one example you've converted shows why this is a bad
idea.  You're passing a pmd_t pointer to a function which is assuming a
pte_t pointer.  And a pmd_t and a pte_t are sometimes different sizes!
(eg sometimes one is 64 bit and the other 32 bit).

So no, NACK.

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 19/06/25 6:56 PM, Matthew Wilcox wrote:
> On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
>> +++ b/mm/memory.c
>> @@ -522,9 +522,8 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
>>  	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  pte:%08llx pmd:%08llx\n",
>> -		 current->comm,
>> -		 (long long)pte_val(pte), (long long)pmd_val(*pmd));
>> +	pr_alert("BUG: Bad page map in process %s  pte:%ppte pmd:%ppte\n",
>> +		 current->comm, &pte, pmd);
> 
> Unfortunately, the one example you've converted shows why this is a bad
> idea.  You're passing a pmd_t pointer to a function which is assuming a
> pte_t pointer.  And a pmd_t and a pte_t are sometimes different sizes!
> (eg sometimes one is 64 bit and the other 32 bit).

As discussed on a separate thread, this might be addressed via separate
printf formats for each page table level e.g %ppte, %ppmd, and %ppud etc. 

> 
> So no, NACK.
> 

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Matthew Wilcox]

On Fri, Jun 20, 2025 at 01:42:53PM +0530, Anshuman Khandual wrote:
> On 19/06/25 6:56 PM, Matthew Wilcox wrote:
> > Unfortunately, the one example you've converted shows why this is a bad
> > idea.  You're passing a pmd_t pointer to a function which is assuming a
> > pte_t pointer.  And a pmd_t and a pte_t are sometimes different sizes!
> > (eg sometimes one is 64 bit and the other 32 bit).
> 
> As discussed on a separate thread, this might be addressed via separate
> printf formats for each page table level e.g %ppte, %ppmd, and %ppud etc. 

There's still no typechecking!

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Steven Rostedt]

On Tue, 24 Jun 2025 14:11:08 +0100
Matthew Wilcox <willy@infradead.org> wrote:

> On Fri, Jun 20, 2025 at 01:42:53PM +0530, Anshuman Khandual wrote:
> > On 19/06/25 6:56 PM, Matthew Wilcox wrote:  
> > > Unfortunately, the one example you've converted shows why this is a bad
> > > idea.  You're passing a pmd_t pointer to a function which is assuming a
> > > pte_t pointer.  And a pmd_t and a pte_t are sometimes different sizes!
> > > (eg sometimes one is 64 bit and the other 32 bit).  
> > 
> > As discussed on a separate thread, this might be addressed via separate
> > printf formats for each page table level e.g %ppte, %ppmd, and %ppud etc.   
> 
> There's still no typechecking!

There's lots of %pX formats that have no type checking. I think this is
an issue. Could we have one of the static checkers test these? Smatch,
sparse, whatever? Or maybe they do and I'm unaware of it?

-- Steve

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Petr Mladek]

On Wed 2025-06-18 09:42:34, Anshuman Khandual wrote:
> Add a new format for printing page table entries.

How many users do you explect, please?

This patch adds only one caller. It does not justify the added complexity.

> @@ -2542,6 +2545,23 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>  		default:
>  			return error_string(buf, end, "(einval)", spec);
>  		}
> +	case 'p':

Please, do not opencode this in the already very long switch().
Move it to a helper function.


> +		if (fmt[1] == 't' && fmt[2] == 'e') {
> +			pte_t *pte = (pte_t *)ptr;

If the value (pointer) gets dereferenced then please add a basic
check:

	if (check_pointer(&buf, end, ptr, spec))
		return buf;

> +			spec.field_width = 10;
> +			spec.precision = 8;

Is she precision = 8 really needed?
I guess that .field_width + ZEROPAD would do the trick.

And them maybe special_hex_number() might be used instead of number()
and safe a lot of code.

> +			spec.base = 16;
> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
> +			if (sizeof(pte_t) == sizeof(u64)) {
> +				u64 val = pte_val(*pte);
> +
> +				return number(buf, end, val, spec);
> +			}
> +			WARN_ONCE(1, "Non standard pte_t\n");

This is nasty. It should be a compile-time check. And the code should
get fixed on all architectures. If it is not easy then
it might be a signal that the generic %ppte flag is not a good idea.

> +			return error_string(buf, end, "(einval)", spec);
> +		}
> +		fallthrough;
>  	default:
>  		return default_pointer(buf, end, ptr, spec);
>  	}

Best Regards,
Petr

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Anshuman Khandual]

On 19/06/25 7:31 PM, Petr Mladek wrote:
> On Wed 2025-06-18 09:42:34, Anshuman Khandual wrote:
>> Add a new format for printing page table entries.
> 
> How many users do you explect, please?
> 
> This patch adds only one caller. It does not justify the added complexity.

Understood.

The idea is to convert all page table entry prints through out the tree
both in generic and platform code. Added just a single generic example
here for this being a RFC proposal. Will go through similar instances
and be back with more comprehensive change set.

> 
>> @@ -2542,6 +2545,23 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>>  		default:
>>  			return error_string(buf, end, "(einval)", spec);
>>  		}
>> +	case 'p':
> 
> Please, do not opencode this in the already very long switch().
> Move it to a helper function.
Sure, will do.

> 
> 
>> +		if (fmt[1] == 't' && fmt[2] == 'e') {
>> +			pte_t *pte = (pte_t *)ptr;
> 
> If the value (pointer) gets dereferenced then please add a basic
> check:

Sure, will do.

> 
> 	if (check_pointer(&buf, end, ptr, spec))
> 		return buf;
> 
>> +			spec.field_width = 10;
>> +			spec.precision = 8;
> 
> Is she precision = 8 really needed?
> I guess that .field_width + ZEROPAD would do the trick.
> 
> And them maybe special_hex_number() might be used instead of number()
> and safe a lot of code.

Agreed. Andy also might have suggested about special_hex_number() helper
on the other thread. Will try and use the helper instead.

> 
>> +			spec.base = 16;
>> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
>> +			if (sizeof(pte_t) == sizeof(u64)) {
>> +				u64 val = pte_val(*pte);
>> +
>> +				return number(buf, end, val, spec);
>> +			}
>> +			WARN_ONCE(1, "Non standard pte_t\n");
> 
> This is nasty. It should be a compile-time check. And the code should

Something like BUILD_BUG_ON() against pte_t as either u64 or u32 aka all
the sizes the print format is going to support and it should pass on all
platforms ?

> get fixed on all architectures. If it is not easy then> it might be a signal that the generic %ppte flag is not a good idea.

Understood.

> 
>> +			return error_string(buf, end, "(einval)", spec);
>> +		}
>> +		fallthrough;
>>  	default:
>>  		return default_pointer(buf, end, ptr, spec);
>>  	}
> 
> Best Regards,
> Petr

Thanks for your review.

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Petr Mladek]

On Fri 2025-06-20 13:32:31, Anshuman Khandual wrote:
> On 19/06/25 7:31 PM, Petr Mladek wrote:
> > On Wed 2025-06-18 09:42:34, Anshuman Khandual wrote:
> >> Add a new format for printing page table entries.
> > 
> > How many users do you explect, please?
> > 
> > This patch adds only one caller. It does not justify the added complexity.
> 
> Understood.
> 
> The idea is to convert all page table entry prints through out the tree
> both in generic and platform code. Added just a single generic example
> here for this being a RFC proposal. Will go through similar instances
> and be back with more comprehensive change set.

You do not need to cover all cases at this stage. For me it is enough
to know that the printf format will have several (like >= 5) users.

> >> @@ -2542,6 +2545,23 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
[...]
> Agreed. Andy also might have suggested about special_hex_number() helper
> on the other thread. Will try and use the helper instead.
> 
> > 
> >> +			spec.base = 16;
> >> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
> >> +			if (sizeof(pte_t) == sizeof(u64)) {
> >> +				u64 val = pte_val(*pte);
> >> +
> >> +				return number(buf, end, val, spec);
> >> +			}
> >> +			WARN_ONCE(1, "Non standard pte_t\n");
> > 
> > This is nasty. It should be a compile-time check. And the code should
> 
> Something like BUILD_BUG_ON() against pte_t as either u64 or u32 aka all
> the sizes the print format is going to support and it should pass on all
> platforms ?

Yes, I had BUILD_BUG_ON() in mind. It would be nice if you check at
least the most known architectures. You could do it just by looking
into the code. We could rely on the bots, monitoring mailing list
and linux-next, for the less known architectures.

The more you check in advance the less surprises would come
from the bots.

Best Regards,
Petr

Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

[Andy Shevchenko]

On Tue, Jun 24, 2025 at 12:48:58PM +0200, Petr Mladek wrote:
> On Fri 2025-06-20 13:32:31, Anshuman Khandual wrote:
> > On 19/06/25 7:31 PM, Petr Mladek wrote:

...

> > Something like BUILD_BUG_ON() against pte_t as either u64 or u32 aka all
> > the sizes the print format is going to support and it should pass on all
> > platforms ?
> 
> Yes, I had BUILD_BUG_ON() in mind. It would be nice if you check at
> least the most known architectures.

FWIW, we use static_assert():s in the file, so may be better to use them.

-- 
With Best Regards,
Andy Shevchenko

[RFC 2/2] kunit: printf: Add test case for pte_t

[Anshuman Khandual]

This adds a new test case for pte_t printf format.

Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 lib/tests/printf_kunit.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
index bc54cca2d7a6f..d94c5067e8b0d 100644
--- a/lib/tests/printf_kunit.c
+++ b/lib/tests/printf_kunit.c
@@ -763,6 +763,31 @@ errptr(struct kunit *kunittest)
 #endif
 }
 
+struct pte_test {
+	u64 val;
+	const char *name;
+};
+
+static struct pte_test pte_test_cases[] = {
+	{ .val = 0xc0ffee,      .name = "0x00c0ffee"},
+	{ .val = 0xdeadbeef,    .name = "0xdeadbeef"},
+	{ .val = 0xaabbcc,      .name = "0x00aabbcc"},
+};
+
+static void
+pte(struct kunit *kunittest)
+{
+	char buf[64];
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pte_test_cases); i++) {
+		pte_t pte = __pte(pte_test_cases[i].val);
+
+		snprintf(buf, sizeof(buf), "%ppte", &pte);
+		KUNIT_EXPECT_STREQ(kunittest, buf, pte_test_cases[i].name);
+	}
+}
+
 static int printf_suite_init(struct kunit_suite *suite)
 {
 	total_tests = 0;
@@ -811,6 +836,7 @@ static struct kunit_case printf_test_cases[] = {
 	KUNIT_CASE(errptr),
 	KUNIT_CASE(fwnode_pointer),
 	KUNIT_CASE(fourcc_pointer),
+	KUNIT_CASE(pte),
 	{}
 };
 
-- 
2.30.2