summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>2010-05-11 14:06:58 -0700
committerLinus Torvalds <torvalds@linux-foundation.org>2010-05-11 17:33:42 -0700
commit7f0f15464185a92f9d8791ad231bcd7bf6df54e4 (patch)
treea1d4f4c39632659497963b0ccf62828237478d72
parent11cad320a4f4bc53d3585c85600c782faa12b99e (diff)
downloadop-kernel-dev-7f0f15464185a92f9d8791ad231bcd7bf6df54e4.zip
op-kernel-dev-7f0f15464185a92f9d8791ad231bcd7bf6df54e4.tar.gz
memcg: fix css_id() RCU locking for real
Commit ad4ba375373937817404fd92239ef4cadbded23b ("memcg: css_id() must be called under rcu_read_lock()") modifies memcontol.c for fixing RCU check message. But Andrew Morton pointed out that the fix doesn't seems sane and it was just for hidining lockdep messages. This is a patch for do proper things. Checking again, all places, accessing without rcu_read_lock, that commit fixies was intentional.... all callers of css_id() has reference count on it. So, it's not necessary to be under rcu_read_lock(). Considering again, we can use rcu_dereference_check for css_id(). We know css->id is valid if css->refcnt > 0. (css->id never changes and freed after css->refcnt going to be 0.) This patch makes use of rcu_dereference_check() in css_id/depth and remove unnecessary rcu-read-lock added by the commit. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Cc: Balbir Singh <balbir@linux.vnet.ibm.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--kernel/cgroup.c15
-rw-r--r--mm/memcontrol.c19
2 files changed, 18 insertions, 16 deletions
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 3a53c77..6db8b7f 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4435,7 +4435,15 @@ __setup("cgroup_disable=", cgroup_disable);
*/
unsigned short css_id(struct cgroup_subsys_state *css)
{
- struct css_id *cssid = rcu_dereference(css->id);
+ struct css_id *cssid;
+
+ /*
+ * This css_id() can return correct value when somone has refcnt
+ * on this or this is under rcu_read_lock(). Once css->id is allocated,
+ * it's unchanged until freed.
+ */
+ cssid = rcu_dereference_check(css->id,
+ rcu_read_lock_held() || atomic_read(&css->refcnt));
if (cssid)
return cssid->id;
@@ -4445,7 +4453,10 @@ EXPORT_SYMBOL_GPL(css_id);
unsigned short css_depth(struct cgroup_subsys_state *css)
{
- struct css_id *cssid = rcu_dereference(css->id);
+ struct css_id *cssid;
+
+ cssid = rcu_dereference_check(css->id,
+ rcu_read_lock_held() || atomic_read(&css->refcnt));
if (cssid)
return cssid->depth;
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0f711c2..595d03f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2314,9 +2314,7 @@ mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout)
/* record memcg information */
if (do_swap_account && swapout && memcg) {
- rcu_read_lock();
swap_cgroup_record(ent, css_id(&memcg->css));
- rcu_read_unlock();
mem_cgroup_get(memcg);
}
if (swapout && memcg)
@@ -2373,10 +2371,8 @@ static int mem_cgroup_move_swap_account(swp_entry_t entry,
{
unsigned short old_id, new_id;
- rcu_read_lock();
old_id = css_id(&from->css);
new_id = css_id(&to->css);
- rcu_read_unlock();
if (swap_cgroup_cmpxchg(entry, old_id, new_id) == old_id) {
mem_cgroup_swap_statistics(from, false);
@@ -4044,16 +4040,11 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
put_page(page);
}
/* throught */
- if (ent.val && do_swap_account && !ret) {
- unsigned short id;
- rcu_read_lock();
- id = css_id(&mc.from->css);
- rcu_read_unlock();
- if (id == lookup_swap_cgroup(ent)) {
- ret = MC_TARGET_SWAP;
- if (target)
- target->ent = ent;
- }
+ if (ent.val && do_swap_account && !ret &&
+ css_id(&mc.from->css) == lookup_swap_cgroup(ent)) {
+ ret = MC_TARGET_SWAP;
+ if (target)
+ target->ent = ent;
}
return ret;
}
OpenPOWER on IntegriCloud