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@xxxxxxxx> >>>> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> >>>> Cc: Jonathan Corbet <corbet@xxxxxxx> >>>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >>>> Cc: David Hildenbrand <david@xxxxxxxxxx> >>>> Cc: linux-doc@xxxxxxxxxxxxxxx >>>> Cc: linux-kernel@xxxxxxxxxxxxxxx >>>> Cc: linux-mm@xxxxxxxxx >>> >>> 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@xxxxxxx> >>>> --- >>> >>> (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.
