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

[Michal Hocko]

On Thu 27-08-15 10:23:51, Jörn Engel wrote:
> On Thu, Aug 27, 2015 at 08:48:18AM +0200, Michal Hocko wrote:
> > 
> > > 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.
> 
> I don't care much which way we decide.  But I find your reasoning a bit
> worrying.  If someone asks for a by-size breakup of hugepages in a few
> years, you might have existing binaries that depend on the _absence_ of
> those extra characters on the line.
> 
> Compare:
>   HugetlbPages:      18432 kB
>   HugetlbPages:    1069056 kB (1*1048576kB 10*2048kB)
> 
> Once someone has written a script that greps for 'HugetlbPages:.*kB$',
> you have lost the option of adding anything else to the line. 

If you think that an explicit note in the documentation is
not sufficient then I believe we can still handle it backward
compatible. Like separate entries for each existing hugetlb page:
HugetlbPages:	     1069056 kB
Hugetlb2MPages:	     20480 kB
Hugetlb1GPages:	     1048576 kB

or something similar. I would even argue this would be slightly easier
to parse. So it is not like we would be locked into anything.

> You have
> created yet another ABI compatibility headache today in order to save
> 112 lines of code.
> 
> That may be a worthwhile tradeoff, I don't know.  But at least I realize
> there is a cost, while you seem to ignore that component.  There is
> value in not painting yourself into a corner.

My primary point was that we are adding a code for a feature nobody
actually asked for just because somebody might ask for it in future.
-- 
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/