Re: [RFC PATCH v1] tools/mm: Add thpmaps script to dump THP usage info

Ryan Roberts
2024-01-05T08:35:08Z

On 04/01/2024 22:48, John Hubbard wrote:
> On 1/3/24 02:20, Ryan Roberts wrote:
>> On 03/01/2024 10:09, William Kucharski wrote:
> ...
>>>> The reason is that it is also possible to invoke the tool with --cgroup instead
>>>> of --pid. In this case, the tool will iterate over all the pids in the
>>>> cgroup so
>>>> (when --summary is not specified) having the pid associated with each vma is
>>>> useful.
>>>>
>>>> I could change it to conditionally output the pid only when --cgroup is
>>>> specified?
>>>
>>> You could, or perhaps emit a colon after the pid to delineate it, e.g.:
>>>
>>>> 000000ce: 0000aaaadbb20000-0000aaaadbb21000 r-xp 00000000 fe:0000426969
>>>> /root/a.out
>>
>> Yeah that sounds like the least worst option. Let's go with that.
>
> I'm trying this out and had the exact same issue with pid. I'd suggest:
>
> a) pid should always be printed in decimal, because that's what ps(1) uses
>    and no one expects to see it in other formats such as hex.

right aligned with 0 or ' ' as the pad? I guess ' ' if you want it to look like
ps? But given pid is the first column, I think it will look weird right aligned.
Perhaps left aligned, followed by colon, followed by pad? Here are the 3 options:

00000206: 0000aaaadbb20000-0000aaaadbb21000 r-xp 00000000 fe:00 00426969
206: 0000aaaadbb20000-0000aaaadbb21000 r-xp 00000000 fe:00 00426969
206: 0000aaaadbb20000-0000aaaadbb21000 r-xp 00000000 fe:00 00426969

My personal preference is the first option; right aligned with 0 pad.

>
> b) In fact, perhaps a header row would help. There could be a --no-header-row
>    option for cases that want to feed this to other scripts, but the default
>    would be to include a human-friendly header.

How about this for a header (with example first data row):

PID START END PROT OFF MJ:MN INODE FILE
00000206: 0000aaaadbb20000-0000aaaadbb21000 r-xp 00000000 fe:00 00426969

Personally I wouldn't bother with a --no-header option; just keep it always on.

>
> c) pid should probably be suppressed if --pid is specified, but that's
>    less important than the other points.

If we have the header then I think its clear what it is and I'd prefer to keep
the data format consistent between --pid and --cgroup. So prefer to leave pid in
always.

>
> In a day or two I'll get a chance to run this on something that allocates
> lots of mTHPs, and give a closer look.

Thanks - it would be great to get some feedback on the usefulness of the actual
counters! :)

I'm considering adding an --ignore-folio-boundaries option, which would modify
the way the cont counters work, to only look for contiguity and alignment and
ignore any folio boundaries. At the moment, if you have multiple contiguous
folios, they don't count, because the memory doesn't all belong to the same
folio. I think this could be useful in some (limited) circumstances.

>
>
> thanks,