linux-kernel - Re: [RFC 1/2] lib/vsprintf: Add support for pte

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aFP7wwKD_yeRRuI_@black.fi.intel.com>
Date: Thu, 19 Jun 2025 15:00:03 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Anshuman Khandual <anshuman.khandual@....com>
Cc: linux-mm@...ck.org, Rasmus Villemoes <linux@...musvillemoes.dk>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Petr Mladek <pmladek@...e.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Jonathan Corbet <corbet@....net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Hildenbrand <david@...hat.com>, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org
Subject: Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

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@...e.com>
> >> Cc: Steven Rostedt <rostedt@...dmis.org>
> >> Cc: Jonathan Corbet <corbet@....net>
> >> Cc: Andrew Morton <akpm@...ux-foundation.org>
> >> Cc: David Hildenbrand <david@...hat.com>
> >> Cc: linux-doc@...r.kernel.org
> >> Cc: linux-kernel@...r.kernel.org
> >> Cc: linux-mm@...ck.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@....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