linux-kernel - Re: [RFC][PATCH] memcg remove css

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, 10 Jun 2010 11:34:24 +0900
From:	Daisuke Nishimura <nishimura@....nes.nec.co.jp>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc:	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"balbir@...ux.vnet.ibm.com" <balbir@...ux.vnet.ibm.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Daisuke Nishimura <nishimura@....nes.nec.co.jp>
Subject: Re: [RFC][PATCH] memcg remove css_get/put per pages v2

I can't find any trivial bugs from my review at the moment.
I'll do some tests.

Some minor commens.

On Wed, 9 Jun 2010 15:59:40 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com> wrote:
> Still RFC, added lkml to CC: list.
> ==
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
> 
> Now, memory cgroup increments css(cgroup subsys state)'s reference
> count per a charged page. And the reference count is kept until
> the page is uncharged. But this has 2 bad effect. 
> 
>  1. Because css_get/put calls atoimic_inc()/dec, heavy call of them
>     on large smp will not scale well.
>  2. Because css's refcnt cannot be in a state as "ready-to-release",
>     cgroup's notify_on_release handler can't work with memcg.
> 
> This is a trial to remove css's refcnt per a page. Even if we remove
> refcnt, pre_destroy() does enough synchronization.
> 
> After this patch, it seems css_get() is still called in try_charge().
> But the logic is.
> 
>   1. task_lock(mm->owner)
There is no task_lock() in this version :)

(snip)
> @@ -4219,7 +4252,6 @@ static int mem_cgroup_do_precharge(unsig
>  		mc.precharge += count;
>  		VM_BUG_ON(test_bit(CSS_ROOT, &mem->css.flags));
>  		WARN_ON_ONCE(count > INT_MAX);
> -		__css_get(&mem->css, (int)count);
>  		return ret;
>  	}
>  one_by_one:
You can remove VM_BUG_ON() and WARN_ON_ONCE() here, too.

> @@ -4469,8 +4501,6 @@ static void mem_cgroup_clear_mc(void)
>  			 */
>  			res_counter_uncharge(&mc.to->res,
>  						PAGE_SIZE * mc.moved_swap);
> -			VM_BUG_ON(test_bit(CSS_ROOT, &mc.to->css.flags));
> -			__css_put(&mc.to->css, mc.moved_swap);
>  		}
>  		/* we've already done mem_cgroup_get(mc.to) */
>  
> 
And, you can remove "WARN_ON_ONCE(mc.moved_swap > INT_MAX)" at the beginning
of this block, too.


Thanks,
Daisuke Nishimura.
--
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/