linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jörn Engel" <joern@purestorage.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: David Rientjes <rientjes@google.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Naoya Horiguchi <nao.horiguchi@gmail.com>
Subject: Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to /proc/PID/status
Date: Thu, 27 Aug 2015 10:23:51 -0700	[thread overview]
Message-ID: <20150827172351.GA29092@Sligo.logfs.org> (raw)
In-Reply-To: <20150827064817.GB14367@dhcp22.suse.cz>

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.  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.

Jörn

--
A quarrel is quickly settled when deserted by one party; there is
no battle unless there be two.
-- Seneca

  reply	other threads:[~2015-08-27 17:23 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27 23:26 hugetlb pages not accounted for in rss Mike Kravetz
2015-07-28 18:32 ` Jörn Engel
2015-07-28 21:15   ` Mike Kravetz
2015-07-28 22:15     ` David Rientjes
2015-07-28 22:26       ` Jörn Engel
2015-07-28 23:30         ` David Rientjes
2015-07-29  0:53           ` Jörn Engel
2015-07-29 19:08             ` David Rientjes
2015-07-29 23:20               ` Mike Kravetz
2015-07-30 21:34                 ` Jörn Engel
2015-07-31 21:09                   ` David Rientjes
2015-08-04  2:55                 ` Naoya Horiguchi
2015-08-04  5:13                   ` [PATCH] smaps: fill missing fields for vma(VM_HUGETLB) Naoya Horiguchi
2015-08-04 18:21                     ` Jörn Engel
2015-08-06  2:18                       ` David Rientjes
2015-08-06  7:44                         ` Naoya Horiguchi
2015-08-07  7:24                           ` [PATCH v2 0/2] hugetlb: display per-process/per-vma usage Naoya Horiguchi
2015-08-07  7:24                             ` [PATCH v2 2/2] mm: hugetlb: add VmHugetlbRSS: field in /proc/pid/status Naoya Horiguchi
2015-08-07 22:55                               ` Andrew Morton
2015-08-10  0:47                                 ` Naoya Horiguchi
2015-08-10  0:47                                   ` [PATCH v3 1/3] smaps: fill missing fields for vma(VM_HUGETLB) Naoya Horiguchi
2015-08-10  0:47                                   ` [PATCH v3 3/3] Documentation/filesystems/proc.txt: document hugetlb RSS Naoya Horiguchi
2015-08-11  0:44                                     ` David Rientjes
2015-08-12  0:03                                       ` Naoya Horiguchi
2015-08-12  7:45                                         ` [PATCH v4 1/2] mm: hugetlb: proc: add HugetlbPages field to /proc/PID/smaps Naoya Horiguchi
2015-08-12  7:45                                           ` [PATCH v4 2/2] mm: hugetlb: proc: add HugetlbPages field to /proc/PID/status Naoya Horiguchi
2015-08-12 20:30                                             ` David Rientjes
2015-08-13  0:45                                               ` Naoya Horiguchi
2015-08-13 21:14                                                 ` Jörn Engel
2015-08-13 21:13                                               ` Jörn Engel
2015-08-17 21:28                                               ` David Rientjes
2015-08-12 20:25                                           ` [PATCH v4 1/2] mm: hugetlb: proc: add HugetlbPages field to /proc/PID/smaps David Rientjes
2015-08-13 21:14                                             ` Jörn Engel
2015-08-20  8:26                                         ` [PATCH v5 0/2] hugetlb: display per-process/per-vma usage Naoya Horiguchi
2015-08-20  8:26                                           ` [PATCH v5 1/2] mm: hugetlb: proc: add HugetlbPages field to /proc/PID/smaps Naoya Horiguchi
2015-08-20 10:49                                             ` Michal Hocko
2015-08-20 23:20                                               ` Naoya Horiguchi
2015-08-21  6:33                                                 ` Michal Hocko
2015-09-07  1:29                                             ` Pádraig Brady
2015-09-07  2:23                                               ` Naoya Horiguchi
2015-09-07  6:46                                                 ` Naoya Horiguchi
2015-09-07  9:52                                                   ` Pádraig Brady
2015-09-07 10:52                                                     ` Pádraig Brady
2015-09-17  9:39                                                       ` Naoya Horiguchi
2015-09-09 15:12                                             ` Vlastimil Babka
2015-09-09 22:14                                               ` David Rientjes
2015-08-20  8:26                                           ` [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to /proc/PID/status Naoya Horiguchi
2015-08-20 11:00                                             ` Michal Hocko
2015-08-20 19:49                                               ` David Rientjes
2015-08-21  6:32                                                 ` Michal Hocko
2015-08-21 16:38                                                   ` Jörn Engel
2015-08-20 23:34                                               ` Naoya Horiguchi
2015-08-21  6:53                                                 ` Michal Hocko
2015-08-21 16:30                                                   ` Jörn Engel
2015-08-24  8:51                                                     ` Michal Hocko
2015-08-25 23:23                                                       ` David Rientjes
2015-08-26  6:38                                                         ` Michal Hocko
2015-08-26 22:02                                                           ` David Rientjes
2015-08-27  6:48                                                             ` Michal Hocko
2015-08-27 17:23                                                               ` Jörn Engel [this message]
2015-08-27 20:44                                                                 ` David Rientjes
2015-08-31  9:12                                                                 ` Michal Hocko
2015-09-16  0:21                                         ` [PATCH v1] mm: migrate: hugetlb: putback destination hugepage to active list Naoya Horiguchi
2015-09-16  2:53                                           ` Naoya Horiguchi
2015-08-10  0:47                                   ` [PATCH v3 2/3] mm: hugetlb: add VmHugetlbRSS: field in /proc/pid/status Naoya Horiguchi
2015-08-10  1:16                                     ` Naoya Horiguchi
2015-08-07  7:24                             ` [PATCH v2 1/2] smaps: fill missing fields for vma(VM_HUGETLB) Naoya Horiguchi
2015-08-07 22:50                               ` Andrew Morton
2015-08-11  0:37                               ` David Rientjes
2015-08-11 23:32                                 ` Naoya Horiguchi
2015-08-11 23:48                                   ` David Rientjes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150827172351.GA29092@Sligo.logfs.org \
    --to=joern@purestorage.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mike.kravetz@oracle.com \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=nao.horiguchi@gmail.com \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).