lkml.org 
[lkml]   [2014]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch v2] mm: memcontrol: convert reclaim iterator to simple css refcounting
On Fri, Sep 19, 2014 at 05:28:43PM -0400, Johannes Weiner wrote:
> The memcg reclaim iterators use a complicated weak reference scheme to
> prevent pinning cgroups indefinitely in the absence of memory pressure.
>
> However, during the ongoing cgroup core rework, css lifetime has been
> decoupled such that a pinned css no longer interferes with removal of
> the user-visible cgroup, and all this complexity is now unnecessary.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
> mm/memcontrol.c | 201 ++++++++++----------------------------------------------
> 1 file changed, 34 insertions(+), 167 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
[...]
> -static void mem_cgroup_iter_invalidate(struct mem_cgroup *root)
> -{
> - /*
> - * When a group in the hierarchy below root is destroyed, the
> - * hierarchy iterator can no longer be trusted since it might
> - * have pointed to the destroyed group. Invalidate it.
> - */
> - atomic_inc(&root->dead_count);

After your patch is applied, mem_cgroup->dead_count is not used any
more. Please remove it.

[...]
> @@ -1300,8 +1183,11 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
> struct mem_cgroup *prev,
> struct mem_cgroup_reclaim_cookie *reclaim)
> {
> + struct mem_cgroup_per_zone *uninitialized_var(mz);
> + struct cgroup_subsys_state *css = NULL;
> + int uninitialized_var(priority);
> struct mem_cgroup *memcg = NULL;
> - struct mem_cgroup *last_visited = NULL;
> + struct mem_cgroup *pos = NULL;
>
> if (mem_cgroup_disabled())
> return NULL;
> @@ -1310,50 +1196,51 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
> root = root_mem_cgroup;
>
> if (prev && !reclaim)
> - last_visited = prev;
> + pos = prev;
>
> if (!root->use_hierarchy && root != root_mem_cgroup) {
> if (prev)
> - goto out_css_put;
> + goto out;
> return root;
> }
>
> rcu_read_lock();
> - while (!memcg) {
> - struct mem_cgroup_reclaim_iter *uninitialized_var(iter);
> - int uninitialized_var(seq);
>
> - if (reclaim) {
> - struct mem_cgroup_per_zone *mz;
> + if (reclaim) {
> + mz = mem_cgroup_zone_zoneinfo(root, reclaim->zone);
> + priority = reclaim->priority;
>
> - mz = mem_cgroup_zone_zoneinfo(root, reclaim->zone);
> - iter = &mz->reclaim_iter[reclaim->priority];
> - if (prev && reclaim->generation != iter->generation) {

Again, you are removing all generation checks, but leaving
mem_cgroup_reclaim_cookie->generation defined. Please remove it too.

BTW, don't we still need the generation check to eliminate the
possibility of a process iterating infinitely over a memory cgroup tree
in case of concurrent reclaim?

Thanks,
Vladimir


\
 
 \ /
  Last update: 2014-09-22 12:41    [W:0.065 / U:0.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site