From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753814Ab2LKP5b (ORCPT ); Tue, 11 Dec 2012 10:57:31 -0500 Received: from mail-ob0-f174.google.com ([209.85.214.174]:39448 "EHLO mail-ob0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752816Ab2LKP5a (ORCPT ); Tue, 11 Dec 2012 10:57:30 -0500 Date: Tue, 11 Dec 2012 16:57:24 +0100 From: Michal Hocko To: Ying Han Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, KAMEZAWA Hiroyuki , Johannes Weiner , Tejun Heo , Glauber Costa , Li Zefan Subject: Re: [patch v2 4/6] memcg: simplify mem_cgroup_iter Message-ID: <20121211155724.GD1612@dhcp22.suse.cz> References: <1353955671-14385-1-git-send-email-mhocko@suse.cz> <1353955671-14385-5-git-send-email-mhocko@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun 09-12-12 09:01:48, Ying Han wrote: > On Mon, Nov 26, 2012 at 10:47 AM, Michal Hocko wrote: > > Current implementation of mem_cgroup_iter has to consider both css and > > memcg to find out whether no group has been found (css==NULL - aka the > > loop is completed) and that no memcg is associated with the found node > > (!memcg - aka css_tryget failed because the group is no longer alive). > > This leads to awkward tweaks like tests for css && !memcg to skip the > > current node. > > > > It will be much easier if we got rid off css variable altogether and > > only rely on memcg. In order to do that the iteration part has to skip > > dead nodes. This sounds natural to me and as a nice side effect we will > > get a simple invariant that memcg is always alive when non-NULL and all > > nodes have been visited otherwise. > > > > We could get rid of the surrounding while loop but keep it in for now to > > make review easier. It will go away in the following patch. > > > > Signed-off-by: Michal Hocko > > --- > > mm/memcontrol.c | 56 +++++++++++++++++++++++++++---------------------------- > > 1 file changed, 27 insertions(+), 29 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 6bcc97b..d1bc0e8 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -1086,7 +1086,6 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root, > > rcu_read_lock(); ^^^^^^^^ here > > while (!memcg) { > > struct mem_cgroup_reclaim_iter *uninitialized_var(iter); > > - struct cgroup_subsys_state *css = NULL; > > > > if (reclaim) { > > int nid = zone_to_nid(reclaim->zone); [...] > > +skip_node: > > + next_cgroup = cgroup_next_descendant_pre( > > + prev_cgroup, root->css.cgroup); > > > > - /* > > - * Even if we found a group we have to make sure it is alive. > > - * css && !memcg means that the groups should be skipped and > > - * we should continue the tree walk. > > - * last_visited css is safe to use because it is protected by > > - * css_get and the tree walk is rcu safe. > > - */ > > - if (css == &root->css || (css && css_tryget(css))) > > - memcg = mem_cgroup_from_css(css); > > + /* > > + * Even if we found a group we have to make sure it is > > + * alive. css && !memcg means that the groups should be > > + * skipped and we should continue the tree walk. > > + * last_visited css is safe to use because it is > > + * protected by css_get and the tree walk is rcu safe. > > + */ > > + if (next_cgroup) { > > + struct mem_cgroup *mem = mem_cgroup_from_cont( > > + next_cgroup); > > + if (css_tryget(&mem->css)) > > + memcg = mem; > > + else { > > + prev_cgroup = next_cgroup; > > I might be missing something here, but the comment says the > last_visited is safe to use but not the next_cgroup. What is > preventing it to be > removed ? rcu_read_lock. cgroup cannot disappear inside rcu. > > --Ying [...] -- Michal Hocko SUSE Labs