netdev - Re: [PATCH v8 1/9] Basic kernel memory functionality for the Memory Controller

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: <20111209102113.cdb85da8.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Fri, 9 Dec 2011 10:21:13 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	Glauber Costa <glommer@...allels.com>
Cc:	linux-kernel@...r.kernel.org, lizf@...fujitsu.com,
	ebiederm@...ssion.com, davem@...emloft.net, gthelen@...gle.com,
	netdev@...r.kernel.org, linux-mm@...ck.org, kirill@...temov.name,
	avagin@...allels.com, devel@...nvz.org, eric.dumazet@...il.com,
	cgroups@...r.kernel.org, hannes@...xchg.org, mhocko@...e.cz,
	Paul Menage <paul@...lmenage.org>
Subject: Re: [PATCH v8 1/9] Basic kernel memory functionality for the Memory
 Controller

On Mon,  5 Dec 2011 19:34:55 -0200
Glauber Costa <glommer@...allels.com> wrote:

> This patch lays down the foundation for the kernel memory component
> of the Memory Controller.
> 
> As of today, I am only laying down the following files:
> 
>  * memory.independent_kmem_limit
>  * memory.kmem.limit_in_bytes (currently ignored)
>  * memory.kmem.usage_in_bytes (always zero)
> 
> Signed-off-by: Glauber Costa <glommer@...allels.com>
> Reviewed-by: Kirill A. Shutemov <kirill@...temov.name>
> CC: Paul Menage <paul@...lmenage.org>
> CC: Greg Thelen <gthelen@...gle.com>

As I wrote, please CC Johannes and  Michal Hocko for memcg related parts.

A few questions.
==
> +	val = !!val;
> +
> +	if (parent && parent->use_hierarchy &&
> +	   (val != parent->kmem_independent_accounting))
> +		return -EINVAL;
==
Hm, why you check val != parent->kmem_independent_accounting ?

	if (parent && parent->use_hierarchy)
		return -EINVAL;
?

BTW, you didn't check this cgroup has children or not.
I think

	if (this_cgroup->use_hierarchy &&
             !list_empty(this_cgroup->childlen))
		return -EINVAL;


==
> +	/*
> +	 * TODO: We need to handle the case in which we are doing
> +	 * independent kmem accounting as authorized by our parent,
> +	 * but then our parent changes its parameter.
> +	 */
> +	cgroup_lock();
> +	memcg->kmem_independent_accounting = val;
> +	cgroup_unlock();

Do we need cgroup_lock() here ?

Thanks,
-Kame

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html