Messages in this thread | | | Date | Tue, 10 Feb 2009 17:50:19 +0900 | From | KAMEZAWA Hiroyuki <> | Subject | Re: [RFC][PATCH] Reduce size of swap_cgroup by CSS ID v2 |
| |
On Tue, 10 Feb 2009 13:37:00 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-02-09 14:55:57]: > > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page) > > { > > - struct mem_cgroup *mem; > > + unsigned short id; > > + struct mem_cgroup *mem = NULL; > > swp_entry_t ent; > > > > if (!PageSwapCache(page)) > > return NULL; > > > > ent.val = page_private(page); > > - mem = lookup_swap_cgroup(ent); > > - if (!mem) > > - return NULL; > > + id = lookup_swap_cgroup(ent); > > + rcu_read_lock(); > > + mem = mem_cgroup_lookup(id); > > if (!css_tryget(&mem->css)) > > - return NULL; > > + mem = NULL; > > This part is a bit confusing. If the page got swapped out and the CSS > it belonged to got swapped out, we set mem to NULL. Is this so that it > can be charged to root cgroup? IIUC, this charge will go to "current" process's cgroup.
> If so, could you please add a comment indicating the same. > Ah yes, I'll add some comments.
> > + rcu_read_unlock(); > > return mem; > > } > > > > @@ -1275,12 +1296,20 @@ int mem_cgroup_cache_charge(struct page > > > > if (do_swap_account && !ret && PageSwapCache(page)) { > > swp_entry_t ent = {.val = page_private(page)}; > > + unsigned short id; > > /* avoid double counting */ > > - mem = swap_cgroup_record(ent, NULL); > > + id = swap_cgroup_record(ent, 0); > > + rcu_read_lock(); > > + mem = mem_cgroup_lookup(id); > > if (mem) { > > + /* > > + * Recorded ID can be obsolete. We avoid calling > > + * css_tryget() > > + */ > > res_counter_uncharge(&mem->memsw, PAGE_SIZE); > > mem_cgroup_put(mem); > > } > > If !mem, do we leak charge? No, it just means some other thread removed the ID before us.
> BTW, We no longer hold css references if the page is swapped out? > The situation is a bit complicated. When cgroup is obsolete but its mem_cgroup is alive (because of reference from swap), css_tryget() always fails. swap_cgroup_record() is atomic compare-and-exchange, so I think we can trust mem_cgroup_put/get refcnt management and doesn't need to rely on css's refcnt at swap management.
Thanks, -Kame
| |