lkml.org 
[lkml]   [2009]   [Jan]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC][PATCH 3/4] memcg: fix for mem_cgroup_hierarchical_reclaim
    On Thu, 8 Jan 2009 20:08:01 +0900 (JST)
    "KAMEZAWA Hiroyuki" <kamezawa.hiroyu@jp.fujitsu.com> wrote:

    > Daisuke Nishimura said:
    > > If root_mem has no children, last_scaned_child is set to root_mem itself.
    > > But after some children added to root_mem, mem_cgroup_get_next_node can
    > > mem_cgroup_put the root_mem although root_mem has not been mem_cgroup_get.
    > >
    > > This patch fixes this behavior by:
    > > - Set last_scanned_child to NULL if root_mem has no children or DFS search
    > > has returned to root_mem itself(root_mem is not a "child" of root_mem).
    > > Make mem_cgroup_get_first_node return root_mem in this case.
    > > There are no mem_cgroup_get/put for root_mem.
    > > - Rename mem_cgroup_get_next_node to __mem_cgroup_get_next_node, and
    > > mem_cgroup_get_first_node to mem_cgroup_get_next_node.
    > > Make mem_cgroup_hierarchical_reclaim call only new
    > > mem_cgroup_get_next_node.
    > >
    > >
    > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
    >
    > Hmm, seems necessary fix. Then, it's better to rebase my patch on to this.
    >
    > Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    >
    > Maybe simpler one can be written but my patch remove all this out later....
    >
    How about this ? (just an exmaple and not tested, sorry)



    ---
    mm/memcontrol.c | 52 ++++++++++++++++++++++------------------------------
    1 file changed, 22 insertions(+), 30 deletions(-)

    Index: mmotm-2.6.28-Jan7/mm/memcontrol.c
    ===================================================================
    --- mmotm-2.6.28-Jan7.orig/mm/memcontrol.c
    +++ mmotm-2.6.28-Jan7/mm/memcontrol.c
    @@ -621,6 +621,7 @@ static struct mem_cgroup *
    mem_cgroup_get_next_node(struct mem_cgroup *curr, struct mem_cgroup *root_mem)
    {
    struct cgroup *cgroup, *curr_cgroup, *root_cgroup;
    + struct mem_cgroup *orig = root_mem->last_scanned_child;

    curr_cgroup = curr->css.cgroup;
    root_cgroup = root_mem->css.cgroup;
    @@ -629,19 +630,15 @@ mem_cgroup_get_next_node(struct mem_cgro
    /*
    * Walk down to children
    */
    - mem_cgroup_put(curr);
    cgroup = list_entry(curr_cgroup->children.next,
    struct cgroup, sibling);
    curr = mem_cgroup_from_cont(cgroup);
    - mem_cgroup_get(curr);
    goto done;
    }

    visit_parent:
    if (curr_cgroup == root_cgroup) {
    - mem_cgroup_put(curr);
    curr = root_mem;
    - mem_cgroup_get(curr);
    goto done;
    }

    @@ -649,11 +646,9 @@ visit_parent:
    * Goto next sibling
    */
    if (curr_cgroup->sibling.next != &curr_cgroup->parent->children) {
    - mem_cgroup_put(curr);
    cgroup = list_entry(curr_cgroup->sibling.next, struct cgroup,
    sibling);
    curr = mem_cgroup_from_cont(cgroup);
    - mem_cgroup_get(curr);
    goto done;
    }

    @@ -664,7 +659,10 @@ visit_parent:
    goto visit_parent;

    done:
    + if (orig)
    + mem_cgroup_put(orig);
    root_mem->last_scanned_child = curr;
    + mem_cgroup_get(curr);
    return curr;
    }

    @@ -677,35 +675,25 @@ static struct mem_cgroup *
    mem_cgroup_get_first_node(struct mem_cgroup *root_mem)
    {
    struct cgroup *cgroup;
    - struct mem_cgroup *ret;
    - bool obsolete;
    + struct mem_cgroup *ret, *orig;

    - obsolete = mem_cgroup_is_obsolete(root_mem->last_scanned_child);
    -
    - /*
    - * Scan all children under the mem_cgroup mem
    - */
    mutex_lock(&mem_cgroup_subsys.hierarchy_mutex);
    - if (list_empty(&root_mem->css.cgroup->children)) {
    - ret = root_mem;
    - goto done;
    - }
    -
    - if (!root_mem->last_scanned_child || obsolete) {
    -
    - if (obsolete && root_mem->last_scanned_child)
    - mem_cgroup_put(root_mem->last_scanned_child);
    + orig = root_mem->last_scanned_child;

    - cgroup = list_first_entry(&root_mem->css.cgroup->children,
    - struct cgroup, sibling);
    - ret = mem_cgroup_from_cont(cgroup);
    + if (!orig) {
    + if (list_empty(&root_mem->css.cgroup->children)) {
    + ret = root_mem;
    + } else {
    + cgroup =
    + list_first_entry(&root_mem->css.cgroup->children,
    + struct cgroup, sibling);
    + ret = mem_cgroup_from_cont(cgroup);
    + }
    + root_mem->last_scanned_child = ret;
    mem_cgroup_get(ret);
    - } else
    + } else /* get_next_node will manage refcnt */
    ret = mem_cgroup_get_next_node(root_mem->last_scanned_child,
    root_mem);
    -
    -done:
    - root_mem->last_scanned_child = ret;
    mutex_unlock(&mem_cgroup_subsys.hierarchy_mutex);
    return ret;
    }
    @@ -2232,7 +2220,11 @@ static void mem_cgroup_pre_destroy(struc
    static void mem_cgroup_destroy(struct cgroup_subsys *ss,
    struct cgroup *cont)
    {
    - mem_cgroup_put(mem_cgroup_from_cont(cont));
    + struct mem_cgroup *mem = mem_cgroup_from_cont(cont);
    +
    + if (mem->last_scanned_child == mem)
    + mem_cgroup_put(mem);
    + mem_cgroup_put(mem);
    }

    static int mem_cgroup_populate(struct cgroup_subsys *ss,


    \
     
     \ /
      Last update: 2009-01-09 02:11    [W:3.821 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site