Re: [PATCH v5 1/2] mm: hugetlb: proc: add HugetlbPages field to /proc/PID/smaps

[Naoya Horiguchi]

On Mon, Sep 07, 2015 at 02:23:44AM +0000, Horiguchi Naoya(åå çä) wrote:
> On Mon, Sep 07, 2015 at 02:29:53AM +0100, PÃdraig Brady wrote:
> > On 20/08/15 09:26, Naoya Horiguchi wrote:
> > > Currently /proc/PID/smaps provides no usage info for vma(VM_HUGETLB), which
> > > is inconvenient when we want to know per-task or per-vma base hugetlb usage.
> > > To solve this, this patch adds a new line for hugetlb usage like below:
> > > 
> > >   Size:              20480 kB
> > >   Rss:                   0 kB
> > >   Pss:                   0 kB
> > >   Shared_Clean:          0 kB
> > >   Shared_Dirty:          0 kB
> > >   Private_Clean:         0 kB
> > >   Private_Dirty:         0 kB
> > >   Referenced:            0 kB
> > >   Anonymous:             0 kB
> > >   AnonHugePages:         0 kB
> > >   HugetlbPages:      18432 kB
> > >   Swap:                  0 kB
> > >   KernelPageSize:     2048 kB
> > >   MMUPageSize:        2048 kB
> > >   Locked:                0 kB
> > >   VmFlags: rd wr mr mw me de ht
> > > 
> > > Signed-off-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
> > > Acked-by: Joern Engel <joern@xxxxxxxxx>
> > > Acked-by: David Rientjes <rientjes@xxxxxxxxxx>
> > > ---
> > > v3 -> v4:
> > > - suspend Acked-by tag because v3->v4 change is not trivial
> > > - I stated in previous discussion that HugetlbPages line can contain page
> > >   size info, but that's not necessary because we already have KernelPageSize
> > >   info.
> > > - merged documentation update, where the current documentation doesn't mention
> > >   AnonHugePages, so it's also added.
> > > ---
> > >  Documentation/filesystems/proc.txt |  7 +++++--
> > >  fs/proc/task_mmu.c                 | 29 +++++++++++++++++++++++++++++
> > >  2 files changed, 34 insertions(+), 2 deletions(-)
> > > 
> > > diff --git v4.2-rc4/Documentation/filesystems/proc.txt v4.2-rc4_patched/Documentation/filesystems/proc.txt
> > > index 6f7fafde0884..22e40211ef64 100644
> > > --- v4.2-rc4/Documentation/filesystems/proc.txt
> > > +++ v4.2-rc4_patched/Documentation/filesystems/proc.txt
> > > @@ -423,6 +423,8 @@ Private_Clean:         0 kB
> > >  Private_Dirty:         0 kB
> > >  Referenced:          892 kB
> > >  Anonymous:             0 kB
> > > +AnonHugePages:         0 kB
> > > +HugetlbPages:          0 kB
> > >  Swap:                  0 kB
> > >  KernelPageSize:        4 kB
> > >  MMUPageSize:           4 kB
> > > @@ -440,8 +442,9 @@ indicates the amount of memory currently marked as referenced or accessed.
> > >  "Anonymous" shows the amount of memory that does not belong to any file.  Even
> > >  a mapping associated with a file may contain anonymous pages: when MAP_PRIVATE
> > >  and a page is modified, the file page is replaced by a private anonymous copy.
> > > -"Swap" shows how much would-be-anonymous memory is also used, but out on
> > > -swap.
> > > +"AnonHugePages" shows the ammount of memory backed by transparent hugepage.
> > > +"HugetlbPages" shows the ammount of memory backed by hugetlbfs page.
> > > +"Swap" shows how much would-be-anonymous memory is also used, but out on swap.
> > 
> > There is no distinction between "private" and "shared" in this "huge page" accounting right?
> 
> Right for current version. And I think that private/shared distinction
> gives some help.
> 
> > Would it be possible to account for the huge pages in the {Private,Shared}_{Clean,Dirty} fields?
> > Or otherwise split the huge page accounting into shared/private?

Sorry, I didn't catch you properly.
I think that accounting for hugetlb pages should be done only with HugetlbPages
or any other new field for hugetlb, in order not to break the behavior of existing
fields. So splitting HugetlbPages into shared/private looks good to me.

Thanks,
Naoya Horiguchi

> As for clean/dirty distinction, I'm not sure how it's worthwhile because
> hugetlb pages are always on memory and never swapped out (userspace doesn't
> care about dirtiness of hugetlb?).
> 
> According to commit log of commit b4d1d99fdd8b ("hugetlb: handle updating
> of ACCESSED and DIRTY in hugetlb_fault()"), dirty bit of hugetlb is maintained
> to make arch-specific TLB handling convenient. It looks purely kernel-internal,
> so I think we don't have to expose it.
> 
> Thanks,
> Naoya Horiguchi