From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx154.postini.com [74.125.245.154]) by kanga.kvack.org (Postfix) with SMTP id 7B11D6B004A for ; Wed, 25 Apr 2012 04:21:28 -0400 (EDT) Date: Wed, 25 Apr 2012 10:21:26 +0200 From: Michal Hocko Subject: Re: [patch] mm: memcg: move pc lookup point to commit_charge() Message-ID: <20120425082125.GA11364@tiehlicka.suse.cz> References: <1335295860-28919-1-git-send-email-hannes@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1335295860-28919-1-git-send-email-hannes@cmpxchg.org> Sender: owner-linux-mm@kvack.org List-ID: To: Johannes Weiner Cc: Andrew Morton , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org On Tue 24-04-12 21:31:00, Johannes Weiner wrote: > None of the callsites actually need the page_cgroup descriptor > themselves, so just pass the page and do the look up in there. > > We already had two bugs (6568d4a 'mm: memcg: update the correct soft > limit tree during migration' and 'memcg: fix Bad page state after > replace_page_cache') where the passed page and pc were not referring > to the same page frame. Yes let's get rid of these relics once and for all. > Signed-off-by: Johannes Weiner > Acked-by: Hugh Dickins > Cc: KAMEZAWA Hiroyuki > Cc: Michal Hocko Acked-by: Michal Hocko Thanks! > --- > mm/memcontrol.c | 17 +++++------------ > 1 files changed, 5 insertions(+), 12 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 884e936..1a28dd8 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2461,10 +2461,10 @@ struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page) > static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg, > struct page *page, > unsigned int nr_pages, > - struct page_cgroup *pc, > enum charge_type ctype, > bool lrucare) > { > + struct page_cgroup *pc = lookup_page_cgroup(page); > struct zone *uninitialized_var(zone); > bool was_on_lru = false; > bool anon; > @@ -2701,7 +2701,6 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > { > struct mem_cgroup *memcg = NULL; > unsigned int nr_pages = 1; > - struct page_cgroup *pc; > bool oom = true; > int ret; > > @@ -2715,11 +2714,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > oom = false; > } > > - pc = lookup_page_cgroup(page); > ret = __mem_cgroup_try_charge(mm, gfp_mask, nr_pages, &memcg, oom); > if (ret == -ENOMEM) > return ret; > - __mem_cgroup_commit_charge(memcg, page, nr_pages, pc, ctype, false); > + __mem_cgroup_commit_charge(memcg, page, nr_pages, ctype, false); > return 0; > } > > @@ -2816,16 +2814,13 @@ static void > __mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *memcg, > enum charge_type ctype) > { > - struct page_cgroup *pc; > - > if (mem_cgroup_disabled()) > return; > if (!memcg) > return; > cgroup_exclude_rmdir(&memcg->css); > > - pc = lookup_page_cgroup(page); > - __mem_cgroup_commit_charge(memcg, page, 1, pc, ctype, true); > + __mem_cgroup_commit_charge(memcg, page, 1, ctype, true); > /* > * Now swap is on-memory. This means this page may be > * counted both as mem and swap....double count. > @@ -3254,14 +3249,13 @@ int mem_cgroup_prepare_migration(struct page *page, > * page. In the case new page is migrated but not remapped, new page's > * mapcount will be finally 0 and we call uncharge in end_migration(). > */ > - pc = lookup_page_cgroup(newpage); > if (PageAnon(page)) > ctype = MEM_CGROUP_CHARGE_TYPE_MAPPED; > else if (page_is_file_cache(page)) > ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > else > ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, ctype, false); > + __mem_cgroup_commit_charge(memcg, newpage, 1, ctype, false); > return ret; > } > > @@ -3348,8 +3342,7 @@ void mem_cgroup_replace_page_cache(struct page *oldpage, > * the newpage may be on LRU(or pagevec for LRU) already. We lock > * LRU while we overwrite pc->mem_cgroup. > */ > - pc = lookup_page_cgroup(newpage); > - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); > + __mem_cgroup_commit_charge(memcg, newpage, 1, type, true); > } > > #ifdef CONFIG_DEBUG_VM > -- > 1.7.7.6 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ > Don't email: email@kvack.org -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: email@kvack.org