linux-kernel - Re: [PATCH 1/7] Fix mem_cgroup_hierarchical_reclaim() to do stable hierarchy walk.

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: <20110623152111.a491e954.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Thu, 23 Jun 2011 15:21:11 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	Michal Hocko <mhocko@...e.cz>
Cc:	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"nishimura@....nes.nec.co.jp" <nishimura@....nes.nec.co.jp>,
	"bsingharora@...il.com" <bsingharora@...il.com>,
	Ying Han <yinghan@...gle.com>,
	"hannes@...xchg.org" <hannes@...xchg.org>
Subject: Re: [PATCH 1/7] Fix mem_cgroup_hierarchical_reclaim() to do stable
 hierarchy walk.

On Wed, 22 Jun 2011 20:33:31 +0200
Michal Hocko <mhocko@...e.cz> wrote:

> On Wed 22-06-11 17:15:00, Michal Hocko wrote:
> > On Thu 16-06-11 12:51:41, KAMEZAWA Hiroyuki wrote:
> > [...]
> > > @@ -1667,41 +1668,28 @@ static int mem_cgroup_hierarchical_recla
> > >  	if (!check_soft && root_mem->memsw_is_minimum)
> > >  		noswap = true;
> > >  
> > > -	while (1) {
> > > +again:
> > > +	if (!shrink) {
> > > +		visit = 0;
> > > +		for_each_mem_cgroup_tree(victim, root_mem)
> > > +			visit++;
> > > +	} else {
> > > +		/*
> > > +		 * At shrinking, we check the usage again in caller side.
> > > +		 * so, visit children one by one.
> > > +		 */
> > > +		visit = 1;
> > > +	}
> > > +	/*
> > > +	 * We are not draining per cpu cached charges during soft limit reclaim
> > > +	 * because global reclaim doesn't care about charges. It tries to free
> > > +	 * some memory and  charges will not give any.
> > > +	 */
> > > +	if (!check_soft)
> > > +		drain_all_stock_async(root_mem);
> > > +
> > > +	while (visit--) {
> > 
> > This is racy, isn't it? What prevents some groups to disapear in the
> > meantime? We would reclaim from those that are left more that we want.
> > 
> > Why cannot we simply do something like (totally untested):
> > 
> > Index: linus_tree/mm/memcontrol.c
> > ===================================================================
> > --- linus_tree.orig/mm/memcontrol.c	2011-06-22 17:11:54.000000000 +0200
> > +++ linus_tree/mm/memcontrol.c	2011-06-22 17:13:05.000000000 +0200
> > @@ -1652,7 +1652,7 @@ static int mem_cgroup_hierarchical_recla
> >  						unsigned long reclaim_options,
> >  						unsigned long *total_scanned)
> >  {
> > -	struct mem_cgroup *victim;
> > +	struct mem_cgroup *victim, *first_victim = NULL;
> >  	int ret, total = 0;
> >  	int loop = 0;
> >  	bool noswap = reclaim_options & MEM_CGROUP_RECLAIM_NOSWAP;
> > @@ -1669,6 +1669,11 @@ static int mem_cgroup_hierarchical_recla
> >  
> >  	while (1) {
> >  		victim = mem_cgroup_select_victim(root_mem);
> > +		if (!first_victim)
> > +			first_victim = victim;
> > +		else if (first_victim == victim)
> > +			break;
> 
> this will obviously need css_get and css_put to make sure that the group
> doesn't disappear in the meantime.
> 

I forgot why we didn't this. Hmm, ok, I'll use this style.

Thanks,
-Kame




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