lkml.org 
[lkml]   [2011]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/4] memcg: fix rmdir, force_empty with THP
    On Tue, Jan 18, 2011 at 11:43:48AM +0900, KAMEZAWA Hiroyuki wrote:
    >
    > Now, when THP is enabled, memcg's rmdir() function is broken
    > because move_account() for THP page is not supported.
    >
    > This will cause account leak or -EBUSY issue at rmdir().
    > This patch fixes the issue by supporting move_account() THP pages.
    >
    > Changelog:
    > - style fix.
    > - add compound_lock for avoiding races.
    >
    > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    > ---
    > mm/memcontrol.c | 37 ++++++++++++++++++++++++++-----------
    > 1 file changed, 26 insertions(+), 11 deletions(-)
    >
    > Index: mmotm-0107/mm/memcontrol.c
    > ===================================================================
    > --- mmotm-0107.orig/mm/memcontrol.c
    > +++ mmotm-0107/mm/memcontrol.c

    > @@ -2267,6 +2274,8 @@ static int mem_cgroup_move_parent(struct
    > struct cgroup *cg = child->css.cgroup;
    > struct cgroup *pcg = cg->parent;
    > struct mem_cgroup *parent;
    > + int charge = PAGE_SIZE;

    No need to initialize, you assign it unconditionally below.

    It's also a bit unfortunate that the parameter/variable with this
    meaning appears under a whole bunch of different names. page_size,
    charge_size, and now charge. Could you stick with page_size?

    > @@ -2278,17 +2287,23 @@ static int mem_cgroup_move_parent(struct
    > goto out;
    > if (isolate_lru_page(page))
    > goto put;
    > + /* The page is isolated from LRU and we have no race with splitting */
    > + charge = PAGE_SIZE << compound_order(page);

    Why is LRU isolation preventing the splitting?

    I think we need the compound lock to get a stable read, like
    compound_trans_order() does.

    Hannes

    > parent = mem_cgroup_from_cont(pcg);
    > - ret = __mem_cgroup_try_charge(NULL, gfp_mask, &parent, false,
    > - PAGE_SIZE);
    > + ret = __mem_cgroup_try_charge(NULL, gfp_mask, &parent, false, charge);
    > if (ret || !parent)
    > goto put_back;
    >
    > - ret = mem_cgroup_move_account(pc, child, parent, true);
    > + if (charge > PAGE_SIZE)
    > + flags = compound_lock_irqsave(page);
    > +
    > + ret = mem_cgroup_move_account(pc, child, parent, true, charge);
    > if (ret)
    > - mem_cgroup_cancel_charge(parent, PAGE_SIZE);
    > + mem_cgroup_cancel_charge(parent, charge);
    > put_back:
    > + if (charge > PAGE_SIZE)
    > + compound_unlock_irqrestore(page, flags);
    > putback_lru_page(page);
    > put:
    > put_page(page);


    \
     
     \ /
      Last update: 2011-01-20 14:43    [W:0.022 / U:30.312 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site