Re: [PATCH v2 1/2] smaps: fill missing fields for vma(VM_HUGETLB)

Naoya Horiguchi
2015-08-11T23:39:17Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3585 bytes --]

On Mon, Aug 10, 2015 at 05:37:54PM -0700, David Rientjes wrote:
> On Fri, 7 Aug 2015, Naoya Horiguchi wrote:
>
> > Currently smaps reports many zero fields for vma(VM_HUGETLB), which is
> > inconvenient when we want to know per-task or per-vma base hugetlb usage.
> > This patch enables these fields by introducing smaps_hugetlb_range().
> >
> > before patch:
> >
> > 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
> > Swap: 0 kB
> > KernelPageSize: 2048 kB
> > MMUPageSize: 2048 kB
> > Locked: 0 kB
> > VmFlags: rd wr mr mw me de ht
> >
> > after patch:
> >
> > Size: 20480 kB
> > Rss: 18432 kB
> > Pss: 18432 kB
> > Shared_Clean: 0 kB
> > Shared_Dirty: 0 kB
> > Private_Clean: 0 kB
> > Private_Dirty: 18432 kB
> > Referenced: 18432 kB
> > Anonymous: 18432 kB
> > AnonHugePages: 0 kB
> > Swap: 0 kB
> > KernelPageSize: 2048 kB
> > MMUPageSize: 2048 kB
> > Locked: 0 kB
> > VmFlags: rd wr mr mw me de ht
> >
>
> I think this will lead to breakage, unfortunately, specifically for users
> who are concerned with resource management.
>
> An example: we use memcg hierarchies to charge memory for individual jobs,
> specific users, and system overhead. Memcg is a cgroup, so this is done
> for an aggregate of processes, and we often have to monitor their memory
> usage. Each process isn't assigned to its own memcg, and I don't believe
> common users of memcg assign individual processes to their own memcgs.
>
> When a memcg is out of memory, we need to track the memory usage of
> processes attached to its memcg hierarchy to determine what is unexpected,
> either as a result of a new rollout or because of a memory leak. To do
> that, we use the rss exported by smaps that is now changed with this
> patch. By using smaps rather than /proc/pid/status, we can report where
> memory usage is unexpected.
>
> This would cause our process that manages all memcgs on our systems to
> break. Perhaps I haven't been as convincing in my previous messages of
> this, but it's quite an obvious userspace regression.

OK, this version assumes that userspace distinguishes vma(VM_HUGETLB) with
"VmFlags" field, which is unrealistic. So I'll keep all existing fields
untouched by introducing hugetlb usage info.

> This memory was not included in rss originally because memory in the
> hugetlb persistent pool is always resident. Unmapping the memory does not
> free memory. For this reason, hugetlb memory has always been treated as
> its own type of memory.

Right, so it might be better not to use the word "RSS" for hugetlb, maybe
something like "HugetlbPages:" seems better to me.

Thanks,
Naoya Horiguchi

> It would have been arguable back when hugetlbfs was introduced whether it
> should be included. I'm afraid the ship has sailed on that since a decade
> has past and it would cause userspace to break if existing metrics are
> used that already have cleared defined semantics.ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éÝ¶\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dÊ‡Ú™ë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥