kernel: cgroup: push rcu read locking from css_is_ancestor() to callsite

Library functions should not grab locks when the callsites can do it, even
if the lock nests like the rcu read-side lock does.

Push the rcu_read_lock() from css_is_ancestor() to its single user,
mem_cgroup_same_or_subtree() in preparation for another user that may
already hold the rcu read-side lock.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
Acked-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index ad8eae5bb801f52128ad3c0dab0cbbdebd685e96..240b02fa1303dfaef9da1d032f03da7c040e2b38 100644 (file)
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -5132,7 +5132,7 @@ EXPORT_SYMBOL_GPL(css_depth);
  * @root: the css supporsed to be an ancestor of the child.
  *
  * Returns true if "root" is an ancestor of "child" in its hierarchy. Because
- * this function reads css->id, this use rcu_dereference() and rcu_read_lock().
+ * this function reads css->id, the caller must hold rcu_read_lock().
  * But, considering usual usage, the csses should be valid objects after test.
  * Assuming that the caller will do some action to the child if this returns
  * returns true, the caller must take "child";s reference count.
@@ -5144,18 +5144,18 @@ bool css_is_ancestor(struct cgroup_subsys_state *child,
 {
        struct css_id *child_id;
        struct css_id *root_id;
-       bool ret = true;
 
-       rcu_read_lock();
        child_id  = rcu_dereference(child->id);
+       if (!child_id)
+               return false;
        root_id = rcu_dereference(root->id);
-       if (!child_id
-           || !root_id
-           || (child_id->depth < root_id->depth)
-           || (child_id->stack[root_id->depth] != root_id->id))
-               ret = false;
-       rcu_read_unlock();
-       return ret;
+       if (!root_id)
+               return false;
+       if (child_id->depth < root_id->depth)
+               return false;
+       if (child_id->stack[root_id->depth] != root_id->id)
+               return false;
+       return true;
 }
 
 void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 49d59e670e69080ed7068c70365915530361920e..954c152beb9261dfaa3114108eb0639f05d177f3 100644 (file)
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1171,12 +1171,16 @@ struct lruvec *mem_cgroup_lru_move_lists(struct zone *zone,
 static bool mem_cgroup_same_or_subtree(const struct mem_cgroup *root_memcg,
                struct mem_cgroup *memcg)
 {
-       if (root_memcg != memcg) {
-               return (root_memcg->use_hierarchy &&
-                       css_is_ancestor(&memcg->css, &root_memcg->css));
-       }
+       bool ret;
 
-       return true;
+       if (root_memcg == memcg)
+               return true;
+       if (!root_memcg->use_hierarchy)
+               return false;
+       rcu_read_lock();
+       ret = css_is_ancestor(&memcg->css, &root_memcg->css);
+       rcu_read_unlock();
+       return ret;
 }
 
 int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup *memcg)