linux-kernel - Re: [PATCH -V4 04/10] memcg: Add HugeTLB extension

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]

Message-ID: <4F73AA5F.5050604@jp.fujitsu.com>
Date:	Thu, 29 Mar 2012 09:18:39 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
CC:	Michal Hocko <mhocko@...e.cz>, linux-mm@...ck.org, mgorman@...e.de,
	dhillf@...il.com, aarcange@...hat.com, akpm@...ux-foundation.org,
	hannes@...xchg.org, linux-kernel@...r.kernel.org,
	cgroups@...r.kernel.org
Subject: Re: [PATCH -V4 04/10] memcg: Add HugeTLB extension

(2012/03/29 2:37), Aneesh Kumar K.V wrote:

> Michal Hocko <mhocko@...e.cz> writes:
> 
>> On Fri 16-03-12 23:09:24, Aneesh Kumar K.V wrote:
>> [...]
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index 6728a7a..4b36c5e 100644
>>> --- a/mm/memcontrol.c
>>> +++ b/mm/memcontrol.c
>> [...]
>>> @@ -4887,6 +5013,7 @@ err_cleanup:
>>>  static struct cgroup_subsys_state * __ref
>>>  mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
>>>  {
>>> +	int idx;
>>>  	struct mem_cgroup *memcg, *parent;
>>>  	long error = -ENOMEM;
>>>  	int node;
>>> @@ -4929,9 +5056,14 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
>>>  		 * mem_cgroup(see mem_cgroup_put).
>>>  		 */
>>>  		mem_cgroup_get(parent);
>>> +		for (idx = 0; idx < HUGE_MAX_HSTATE; idx++)
>>> +			res_counter_init(&memcg->hugepage[idx],
>>> +					 &parent->hugepage[idx]);
>>
>> Hmm, I do not think we want to make groups deeper in the hierarchy
>> unlimited as we cannot reclaim. Shouldn't we copy the limit from the parent?
>> Still not ideal but slightly more expected behavior IMO.
> 
> But we should be limiting the child group based on parent's limit only
> when hierarchy is set right ?
> 
>>
>> The hierarchy setups are still interesting and the limitations should be
>> described in the documentation...
>>
> 
> It should behave similar to memcg. ie, if hierarchy is set, then we limit
> using MIN(parent's limit, child's limit). May be I am missing some of
> the details of memcg use_hierarchy config. My goal was to keep it
> similar to memcg. Can you explain why do you think the patch would
> make it any different ?
> 


Maybe this is a different story but....

Tejun(Cgroup Maintainer) asked us to remove 'use_hierarchy' settings because
most of other cgroups are hierarchical(*). I answered that improvement in res_counter 
latency is required. And now, we have some idea to improve res_counter.
(I'd like to try this after page_cgroup diet series..)

If we change and drop use_hierarchy, the usage similar to current use_hierarchy=0
will be..

	/cgroup/memory/			       = unlimited
			level1		       = unlimited
				level2	       = unlimited
					level3 = limit

To do this, after improvement of res_counter, we entry use_hierarchy into
feature-removal-list and wait for 2 versions..So, this will not affect
your developments, anyway.
 
Thanks,
-Kame

(*) AFAIK, blkio cgroup needs tons of work to be hierarchical...


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