From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from d28relay02.in.ibm.com (d28relay02.in.ibm.com [9.184.220.59]) by e28esmtp02.in.ibm.com (8.13.1/8.13.1) with ESMTP id m8Q9lnKl024797 for ; Fri, 26 Sep 2008 15:17:49 +0530 Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay02.in.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id m8Q9lnqY1806350 for ; Fri, 26 Sep 2008 15:17:49 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.13.1/8.13.3) with ESMTP id m8Q9lmt9005351 for ; Fri, 26 Sep 2008 19:47:49 +1000 Message-ID: <48DCAFC4.40009@linux.vnet.ibm.com> Date: Fri, 26 Sep 2008 15:17:48 +0530 From: Balbir Singh Reply-To: balbir@linux.vnet.ibm.com MIME-Version: 1.0 Subject: Re: [PATCH 4/12] memcg make page->mapping NULL before calling uncharge References: <20080925151124.25898d22.kamezawa.hiroyu@jp.fujitsu.com> <20080925151639.5e2ddea4.kamezawa.hiroyu@jp.fujitsu.com> In-Reply-To: <20080925151639.5e2ddea4.kamezawa.hiroyu@jp.fujitsu.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org Return-Path: To: KAMEZAWA Hiroyuki Cc: "linux-mm@kvack.org" , "nishimura@mxp.nes.nec.co.jp" , "xemul@openvz.org" , Andrew Morton , LKML , Dave Hansen , ryov@valinux.co.jp List-ID: KAMEZAWA Hiroyuki wrote: > This patch tries to make page->mapping to be NULL before > mem_cgroup_uncharge_cache_page() is called. > > "page->mapping == NULL" is a good check for "whether the page is still > radix-tree or not". > This patch also adds BUG_ON() to mem_cgroup_uncharge_cache_page(); > > > Signed-off-by: KAMEZAWA Hiroyuki Looks good, small nit-pick below > #endif > ClearPagePrivate(page); > set_page_private(page, 0); > - page->mapping = NULL; > + /* page->mapping contains a flag for PageAnon() */ > + if (PageAnon(page)) { > + /* This page is uncharged at try_to_unmap(). */ > + page->mapping = NULL; > + } else { > + /* Obsolete file cache should be uncharged */ > + page->mapping = NULL; > + mem_cgroup_uncharge_cache_page(page); > + } > Isn't it better and correct coding style to do /* * Uncharge obsolete file cache */ if (!PageAnon(page)) mem_cgroup_uncharge_cache_page(page); /* else - uncharged at try_to_unmap() */ page->mapping = NULL; -- Balbir -- 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/ . Don't email: email@kvack.org