lkml.org 
[lkml]   [2012]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch v2 3/6] memcg: rework mem_cgroup_iter to use cgroup iterators
    On Sun 09-12-12 08:59:54, Ying Han wrote:
    > On Mon, Nov 26, 2012 at 10:47 AM, Michal Hocko <mhocko@suse.cz> wrote:
    [...]
    > > + /*
    > > + * Even if we found a group we have to make sure it is alive.
    > > + * css && !memcg means that the groups should be skipped and
    > > + * we should continue the tree walk.
    > > + * last_visited css is safe to use because it is protected by
    > > + * css_get and the tree walk is rcu safe.
    > > + */
    > > + if (css == &root->css || (css && css_tryget(css)))
    > > + memcg = mem_cgroup_from_css(css);
    > >
    > > if (reclaim) {
    > > - iter->position = id;
    > > + struct mem_cgroup *curr = memcg;
    > > +
    > > + if (last_visited)
    > > + css_put(&last_visited->css);
    > > +
    > > + if (css && !memcg)
    > > + curr = mem_cgroup_from_css(css);
    >
    > In this case, the css_tryget() failed which implies the css is on the
    > way to be removed. (refcnt ==0) If so, why it is safe to call
    > css_get() directly on it below? It seems not preventing the css to be
    > removed by doing so.

    Well, I do not remember exactly but I guess the code is meant to say
    that we need to store a half-dead memcg because the loop has to be
    retried. As we are under RCU hood it is just half dead.
    Now that you brought this up I think this is not safe as well because
    another thread could have seen the cached value while we tried to retry
    and his RCU is not protecting the group anymore. The follow up patch
    fixes this by retrying within the loop. I will bring that part into
    this patch already and then leave only css clean up in the other patch.

    Thanks for spotting this Ying!

    >
    > > + /* make sure that the cached memcg is not removed */
    > > + if (curr)
    > > + css_get(&curr->css);
    >
    > --Ying
    --
    Michal Hocko
    SUSE Labs


    \
     
     \ /
      Last update: 2012-12-11 17:21    [W:2.978 / U:0.268 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site