From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> To: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Cc: linux-mm@kvack.org, "balbir@linux.vnet.ibm.com" <balbir@linux.vnet.ibm.com>, gthelen@google.com, m-ikeda@ds.jp.nec.com, "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, kamezawa.hiroyuki@gmail.com Subject: Re: [PATCH 2/5] memcg: use array and ID for quick look up Date: Tue, 24 Aug 2010 08:51:11 +0900 [thread overview] Message-ID: <20100824085111.6acf8881.kamezawa.hiroyu@jp.fujitsu.com> (raw) In-Reply-To: <20100823123533.b75b99c5.nishimura@mxp.nes.nec.co.jp> On Mon, 23 Aug 2010 12:35:33 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > Hi, > > > +/* 0 is unused */ > > +static atomic_t mem_cgroup_num; > > +#define NR_MEMCG_GROUPS (CONFIG_MEM_CGROUP_MAX_GROUPS + 1) > > +static struct mem_cgroup *mem_cgroups[NR_MEMCG_GROUPS] __read_mostly; > > + > > +/* Must be called under rcu_read_lock */ > > +static struct mem_cgroup *id_to_memcg(unsigned short id) > > +{ > > + struct mem_cgroup *ret; > > + /* see mem_cgroup_free() */ > > + ret = rcu_dereference_check(mem_cgroups[id], rch_read_lock_held()); > > + if (likely(ret && ret->valid)) > > + return ret; > > + return NULL; > > +} > > + > I prefer "mem" to "ret". > Hmm, ok. > > @@ -2231,7 +2244,7 @@ __mem_cgroup_commit_charge_swapin(struct > > > > id = swap_cgroup_record(ent, 0); > > rcu_read_lock(); > > - memcg = mem_cgroup_lookup(id); > > + memcg = id_to_memcg(id); > > if (memcg) { > > /* > > * This recorded memcg can be obsolete one. So, avoid > > @@ -2240,9 +2253,10 @@ __mem_cgroup_commit_charge_swapin(struct > > if (!mem_cgroup_is_root(memcg)) > > res_counter_uncharge(&memcg->memsw, PAGE_SIZE); > > mem_cgroup_swap_statistics(memcg, false); > > + rcu_read_unlock(); > > mem_cgroup_put(memcg); > > - } > > - rcu_read_unlock(); > > + } else > > + rcu_read_unlock(); > > } > > /* > > * At swapin, we may charge account against cgroup which has no tasks. > > @@ -2495,7 +2509,7 @@ void mem_cgroup_uncharge_swap(swp_entry_ > > > > id = swap_cgroup_record(ent, 0); > > rcu_read_lock(); > > - memcg = mem_cgroup_lookup(id); > > + memcg = id_to_memcg(id); > > if (memcg) { > > /* > > * We uncharge this because swap is freed. > > @@ -2504,9 +2518,10 @@ void mem_cgroup_uncharge_swap(swp_entry_ > > if (!mem_cgroup_is_root(memcg)) > > res_counter_uncharge(&memcg->memsw, PAGE_SIZE); > > mem_cgroup_swap_statistics(memcg, false); > > + rcu_read_unlock(); > > mem_cgroup_put(memcg); > > - } > > - rcu_read_unlock(); > > + } else > > + rcu_read_unlock(); > > } > > > > /** > Could you explain why we need rcu_read_unlock() before mem_cgroup_put() ? > I suspect that it's because mem_cgroup_put() can free the memcg, but do we > need mem->valid then ? > mem_cgroup_put() may call synchronize_rcu(). So, we have to unlock before it. Thanks, -Kame
WARNING: multiple messages have this Message-ID (diff)
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> To: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Cc: linux-mm@kvack.org, "balbir@linux.vnet.ibm.com" <balbir@linux.vnet.ibm.com>, gthelen@google.com, m-ikeda@ds.jp.nec.com, "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, kamezawa.hiroyuki@gmail.com Subject: Re: [PATCH 2/5] memcg: use array and ID for quick look up Date: Tue, 24 Aug 2010 08:51:11 +0900 [thread overview] Message-ID: <20100824085111.6acf8881.kamezawa.hiroyu@jp.fujitsu.com> (raw) In-Reply-To: <20100823123533.b75b99c5.nishimura@mxp.nes.nec.co.jp> On Mon, 23 Aug 2010 12:35:33 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > Hi, > > > +/* 0 is unused */ > > +static atomic_t mem_cgroup_num; > > +#define NR_MEMCG_GROUPS (CONFIG_MEM_CGROUP_MAX_GROUPS + 1) > > +static struct mem_cgroup *mem_cgroups[NR_MEMCG_GROUPS] __read_mostly; > > + > > +/* Must be called under rcu_read_lock */ > > +static struct mem_cgroup *id_to_memcg(unsigned short id) > > +{ > > + struct mem_cgroup *ret; > > + /* see mem_cgroup_free() */ > > + ret = rcu_dereference_check(mem_cgroups[id], rch_read_lock_held()); > > + if (likely(ret && ret->valid)) > > + return ret; > > + return NULL; > > +} > > + > I prefer "mem" to "ret". > Hmm, ok. > > @@ -2231,7 +2244,7 @@ __mem_cgroup_commit_charge_swapin(struct > > > > id = swap_cgroup_record(ent, 0); > > rcu_read_lock(); > > - memcg = mem_cgroup_lookup(id); > > + memcg = id_to_memcg(id); > > if (memcg) { > > /* > > * This recorded memcg can be obsolete one. So, avoid > > @@ -2240,9 +2253,10 @@ __mem_cgroup_commit_charge_swapin(struct > > if (!mem_cgroup_is_root(memcg)) > > res_counter_uncharge(&memcg->memsw, PAGE_SIZE); > > mem_cgroup_swap_statistics(memcg, false); > > + rcu_read_unlock(); > > mem_cgroup_put(memcg); > > - } > > - rcu_read_unlock(); > > + } else > > + rcu_read_unlock(); > > } > > /* > > * At swapin, we may charge account against cgroup which has no tasks. > > @@ -2495,7 +2509,7 @@ void mem_cgroup_uncharge_swap(swp_entry_ > > > > id = swap_cgroup_record(ent, 0); > > rcu_read_lock(); > > - memcg = mem_cgroup_lookup(id); > > + memcg = id_to_memcg(id); > > if (memcg) { > > /* > > * We uncharge this because swap is freed. > > @@ -2504,9 +2518,10 @@ void mem_cgroup_uncharge_swap(swp_entry_ > > if (!mem_cgroup_is_root(memcg)) > > res_counter_uncharge(&memcg->memsw, PAGE_SIZE); > > mem_cgroup_swap_statistics(memcg, false); > > + rcu_read_unlock(); > > mem_cgroup_put(memcg); > > - } > > - rcu_read_unlock(); > > + } else > > + rcu_read_unlock(); > > } > > > > /** > Could you explain why we need rcu_read_unlock() before mem_cgroup_put() ? > I suspect that it's because mem_cgroup_put() can free the memcg, but do we > need mem->valid then ? > mem_cgroup_put() may call synchronize_rcu(). So, we have to unlock before it. Thanks, -Kame -- 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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2010-08-23 23:56 UTC|newest] Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top 2010-08-20 9:55 [PATCH] memcg: towards I/O aware memcg v5 KAMEZAWA Hiroyuki 2010-08-20 9:55 ` KAMEZAWA Hiroyuki 2010-08-20 9:58 ` [PATCH 1/5] cgroup: ID notification call back KAMEZAWA Hiroyuki 2010-08-20 9:58 ` KAMEZAWA Hiroyuki 2010-08-24 7:19 ` Greg Thelen 2010-08-24 7:19 ` Greg Thelen 2010-08-24 7:18 ` KAMEZAWA Hiroyuki 2010-08-24 7:18 ` KAMEZAWA Hiroyuki 2010-08-24 9:04 ` Li Zefan 2010-08-24 9:04 ` Li Zefan 2010-08-24 23:58 ` KAMEZAWA Hiroyuki 2010-08-24 23:58 ` KAMEZAWA Hiroyuki 2010-08-25 0:11 ` Paul Menage 2010-08-25 0:11 ` Paul Menage 2010-08-25 0:17 ` KAMEZAWA Hiroyuki 2010-08-25 0:17 ` KAMEZAWA Hiroyuki 2010-08-25 0:25 ` Paul Menage 2010-08-25 0:25 ` Paul Menage 2010-08-25 0:09 ` Paul Menage 2010-08-25 0:09 ` Paul Menage 2010-08-25 0:20 ` KAMEZAWA Hiroyuki 2010-08-25 0:20 ` KAMEZAWA Hiroyuki 2010-08-25 0:34 ` Paul Menage 2010-08-25 0:34 ` Paul Menage 2010-08-25 0:37 ` KAMEZAWA Hiroyuki 2010-08-25 0:37 ` KAMEZAWA Hiroyuki 2010-08-25 0:46 ` Paul Menage 2010-08-25 0:46 ` Paul Menage 2010-08-25 1:03 ` KAMEZAWA Hiroyuki 2010-08-25 1:03 ` KAMEZAWA Hiroyuki 2010-08-25 1:35 ` Paul Menage 2010-08-25 1:35 ` Paul Menage 2010-08-25 1:42 ` KAMEZAWA Hiroyuki 2010-08-25 1:42 ` KAMEZAWA Hiroyuki 2010-08-25 1:52 ` Paul Menage 2010-08-25 1:52 ` Paul Menage 2010-08-25 2:29 ` KAMEZAWA Hiroyuki 2010-08-25 2:29 ` KAMEZAWA Hiroyuki 2010-08-20 9:59 ` [PATCH 2/5] memcg: use array and ID for quick look up KAMEZAWA Hiroyuki 2010-08-20 9:59 ` KAMEZAWA Hiroyuki 2010-08-23 3:35 ` Daisuke Nishimura 2010-08-23 3:35 ` Daisuke Nishimura 2010-08-23 23:51 ` KAMEZAWA Hiroyuki [this message] 2010-08-23 23:51 ` KAMEZAWA Hiroyuki 2010-08-24 0:19 ` Daisuke Nishimura 2010-08-24 0:19 ` Daisuke Nishimura 2010-08-24 7:44 ` Greg Thelen 2010-08-24 7:44 ` Greg Thelen 2010-08-24 7:42 ` KAMEZAWA Hiroyuki 2010-08-24 7:42 ` KAMEZAWA Hiroyuki 2010-08-20 10:01 ` [PATCH] memcg: use ID in page_cgroup KAMEZAWA Hiroyuki 2010-08-20 10:01 ` KAMEZAWA Hiroyuki 2010-08-20 10:05 ` KAMEZAWA Hiroyuki 2010-08-23 5:32 ` Daisuke Nishimura 2010-08-23 5:32 ` Daisuke Nishimura 2010-08-23 23:52 ` KAMEZAWA Hiroyuki 2010-08-23 23:52 ` KAMEZAWA Hiroyuki 2010-08-24 1:14 ` Daisuke Nishimura 2010-08-24 1:14 ` Daisuke Nishimura 2010-08-24 1:54 ` KAMEZAWA Hiroyuki 2010-08-24 1:54 ` KAMEZAWA Hiroyuki 2010-08-24 4:04 ` Daisuke Nishimura 2010-08-24 4:04 ` Daisuke Nishimura 2010-08-24 6:05 ` KAMEZAWA Hiroyuki 2010-08-24 6:05 ` KAMEZAWA Hiroyuki 2010-08-24 7:47 ` Greg Thelen 2010-08-24 7:47 ` Greg Thelen 2010-08-24 7:51 ` KAMEZAWA Hiroyuki 2010-08-24 7:51 ` KAMEZAWA Hiroyuki 2010-08-24 8:35 ` Greg Thelen 2010-08-24 8:35 ` Greg Thelen 2010-08-24 8:38 ` KAMEZAWA Hiroyuki 2010-08-24 8:38 ` KAMEZAWA Hiroyuki 2010-08-20 10:02 ` [PATCH 4/5] memcg: lockless update of file_mapped KAMEZAWA Hiroyuki 2010-08-20 10:02 ` KAMEZAWA Hiroyuki 2010-08-23 8:50 ` Daisuke Nishimura 2010-08-23 8:50 ` Daisuke Nishimura 2010-08-23 23:49 ` KAMEZAWA Hiroyuki 2010-08-23 23:49 ` KAMEZAWA Hiroyuki 2010-08-24 0:19 ` Daisuke Nishimura 2010-08-24 0:19 ` Daisuke Nishimura 2010-08-20 10:03 ` [PATCH 5/5] memcg: generic file accounting update function KAMEZAWA Hiroyuki 2010-08-20 10:03 ` KAMEZAWA Hiroyuki 2010-08-24 7:46 ` [PATCH] memcg: towards I/O aware memcg v5 Balbir Singh 2010-08-24 7:46 ` Balbir Singh 2010-08-24 7:59 ` KAMEZAWA Hiroyuki 2010-08-24 7:59 ` KAMEZAWA Hiroyuki
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20100824085111.6acf8881.kamezawa.hiroyu@jp.fujitsu.com \ --to=kamezawa.hiroyu@jp.fujitsu.com \ --cc=akpm@linux-foundation.org \ --cc=balbir@linux.vnet.ibm.com \ --cc=gthelen@google.com \ --cc=kamezawa.hiroyuki@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=m-ikeda@ds.jp.nec.com \ --cc=nishimura@mxp.nes.nec.co.jp \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.