lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 27 Aug 2015 08:48:18 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Jörn Engel <joern@...estorage.com>,
	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mike Kravetz <mike.kravetz@...cle.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Naoya Horiguchi <nao.horiguchi@...il.com>
Subject: Re: [PATCH v5 2/2] mm: hugetlb: proc: add HugetlbPages field to
 /proc/PID/status

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

Powered by blists - more mailing lists