| Date | Fri, 26 Sep 2008 15:17:48 +0530 | From | Balbir Singh <> | Subject | Re: [PATCH 4/12] memcg make page->mapping NULL before calling uncharge |
| |
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 <kamezawa.hiroyu@jp.fujitsu.com>
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
|