Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1216119

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

Show all headers | View raw

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-20 10:50 +0200
  Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Michal Hocko <mhocko@kernel.org> - 2015-08-20 13:10 +0200
    Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status David Rientjes <rientjes@google.com> - 2015-08-20 22:00 +0200
      Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Michal Hocko <mhocko@kernel.org> - 2015-08-21 08:40 +0200
        Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Jörn Engel <joern@purestorage.com> - 2015-08-21 18:40 +0200
    Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-21 01:40 +0200
      Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Michal Hocko <mhocko@kernel.org> - 2015-08-21 09:00 +0200
        Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Jörn Engel <joern@purestorage.com> - 2015-08-21 18:40 +0200
          Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Michal Hocko <mhocko@kernel.org> - 2015-08-24 11:00 +0200
            Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status David Rientjes <rientjes@google.com> - 2015-08-26 01:30 +0200
              Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Michal Hocko <mhocko@kernel.org> - 2015-08-26 08:40 +0200
                Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status David Rientjes <rientjes@google.com> - 2015-08-27 00:10 +0200
                Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Michal Hocko <mhocko@kernel.org> - 2015-08-27 08:50 +0200
                Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Jörn Engel <joern@purestorage.com> - 2015-08-27 19:30 +0200
                Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status David Rientjes <rientjes@google.com> - 2015-08-27 22:50 +0200
                Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to  /proc/PID/status Michal Hocko <mhocko@kernel.org> - 2015-08-31 11:20 +0200

csiph-web