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

[Michal Hocko]

On Wed 26-08-15 15:02:49, David Rientjes wrote:
> On Wed, 26 Aug 2015, Michal Hocko wrote:
> 
> > I thought the purpose was to give the amount of hugetlb based
> > resident memory.
> 
> Persistent hugetlb memory is always resident, the goal is to show what is 
> currently mapped.
> 
> > At least this is what Jörn was asking for AFAIU.
> > /proc/<pid>/status should be as lightweight as possible. The current
> > implementation is quite heavy as already pointed out. So I am really
> > curious whether this is _really_ needed. I haven't heard about a real
> > usecase except for top displaying HRss which doesn't need the break
> > down values. You have brought that up already
> > http://marc.info/?l=linux-mm&m=143941143109335&w=2 and nobody actually
> > asked for it. "I do not mind having it" is not an argument for inclusion
> > especially when the implementation is more costly and touches hot paths.
> > 
> 
> It iterates over HUGE_MAX_HSTATE and reads atomic usage counters twice.  

I am not worried about /proc/<pid>/status read path. That one is indeed
trivial.

> On x86, HUGE_MAX_HSTATE == 2.  I don't consider that to be expensive.
> 
> If you are concerned about the memory allocation of struct hugetlb_usage, 
> it could easily be embedded directly in struct mm_struct.

Yes I am concerned about that and
9 files changed, 112 insertions(+), 1 deletion(-)
for something that is even not clear to be really required. And I still
haven't heard any strong usecase to justify it.

Can we go with the single and much simpler cumulative number first and
only add the break down list if it is _really_ required? We can even
document that the future version of /proc/<pid>/status might add an
additional information to prepare all the parsers to be more careful.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/