On Wed 18-07-12 19:08:54, Wanpeng Li wrote:
> use exist interface mem_cgroup_css instead of &mem->css.
This interface has been added to enable mem->css outside of
mm/memcontrol.c (where we define struct mem_cgroup). There is one user
left (hwpoison_filter_task) after recent clean ups.
I think we shouldn't spread the usage inside the mm/memcontrol.c. The
compiler inlines the function for all callers added by this patch but I
wouldn't rely on it. It is also unfortunate that we cannot convert all
dereferences (e.g. const mem_cgroup).
Moreover it doesn't add any additional type safety. So I would vote for
not taking the patch but if others like it I will not block it.
> V2->V1:
> * restore the mistake modify in mem_cgroup_css
>
> Signed-off-by: Wanpeng Li
> ---
> mm/memcontrol.c | 89 --
> 1 files changed, 46 insertions(+), 43 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f72b5e5..f27084c 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -645,7 +645,7 @@ retry:
>*/
> __mem_cgroup_remove_exceeded(mz->memcg, mz, mctz);
> if (!res_counter_soft_limit_excess(&mz->memcg->res) ||
> - !css_tryget(&mz->memcg->css))
> + !css_tryget(mem_cgroup_css(mz->memcg)))
> goto retry;
> done:
> return mz;
> @@ -899,7 +899,7 @@ struct mem_cgroup *try_get_mem_cgroup_from_mm(struct
> mm_struct *mm)
> memcg = mem_cgroup_from_task(rcu_dereference(mm->owner));
> if (unlikely(!memcg))
> break;
> - } while (!css_tryget(&memcg->css));
> + } while (!css_tryget(mem_cgroup_css(memcg)));
> rcu_read_unlock();
> return memcg;
> }
> @@ -935,10 +935,10 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup
> *root,
> root = root_mem_cgroup;
>
> if (prev && !reclaim)
> - id = css_id(&prev->css);
> + id = css_id(mem_cgroup_css(prev));
>
> if (prev && prev != root)
> - css_put(&prev->css);
> + css_put(mem_cgroup_css(prev));
>
> if (!root->use_hierarchy && root != root_mem_cgroup) {
> if (prev)
> @@ -963,9 +963,10 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup
> *root,
> }
>
> rcu_read_lock();
> - css = css_get_next(&mem_cgroup_subsys, id + 1, &root->css, &id);
> + css = css_get_next(&mem_cgroup_subsys, id + 1,
> + mem_cgroup_css(root), &id);
> if (css) {
> - if (css == &root->css || css_tryget(css))
> + if (css == mem_cgroup_css(root) || css_tryget(css))
> memcg = container_of(css,
>struct mem_cgroup, css);
> } else
> @@ -997,7 +998,7 @@ void mem_cgroup_iter_break(struct mem_cgroup *root,
> if (!root)
> root = root_mem_cgroup;
> if (prev && prev != root)
> - css_put(&prev->css);
> + css_put(mem_cgroup_css(prev));
> }
>
> /*
> @@ -1150,7 +1151,8 @@ bool __mem_cgroup_same_or_subtree(const struct
> mem_cgroup *root_memcg,
> return true;
> if (!root_memcg->use_hierarchy || !memcg)
> return false;
> - return css_is_ancestor(&memcg->css, &root_memcg->css);
> + return css_is_ancestor(mem_cgroup_css(memcg),
> + &root_memcg->css);
> }
>
> static bool mem_cgroup_same_or_subtree(const struct mem_cgroup *root_memcg,
> @@ -1183,7 +1185,7 @@ int task_in_mem_cgroup(struct task_struct *task, const
> struct mem_cgroup *memcg)
> task_lock(task);
> curr = mem_cgroup_from_task(task);
> if (curr)
> - css_get(&curr->css);
> + css_get(mem_cgroup_css(curr));
> task_unlock(task);
> }
> if (!curr)
> @@ -1195,7 +1197,7 @@ int task_in_mem_cgroup(struct task_struct *task, const
> struct mem_cgroup *memcg)
>* hierarchy(even if use_hierarchy is disabled in "memcg").
>*/
> ret = mem_cgroup_same_or_subtree(memcg, curr);
> - css_put(&curr->css);
> + css_put(mem_cgroup_css(curr));
> return ret;
> }
>
> @@ -1251,7 +1253,7 @@ static unsigned long mem_cgroup_margin(struct
> mem_cgroup *memcg)
>
> int mem_cgroup_swappiness(struct mem_cgroup *memcg)
> {
> - struct cgroup *cgrp = memcg->css.cgroup;
> + struct cgroup *cgrp = mem_cgroup_css(memcg)->cgroup;
>
> /* root ? */
> if (cgrp->parent == NULL)
> @@ -1396,7 +1398,7 @@ void mem_cgroup_print_oom_info(struct mem_cgroup
> *memcg, struct task_struct *p)
>
> rcu_read_lock();
>
> - mem_cgrp = memcg->css.cgroup;
> + mem_cgrp = mem_cgroup_css(memcg)->cgroup;
> task_cgrp = task_cgroup(p, mem_cgroup_subsys_id);
>
> ret = cgroup_path(task_cgrp
