The patch titled memcg: improve performance in moving charge has been added to the -mm tree. Its filename is memcg-improve-performance-in-moving-charge.patch Before you just go and hit "reply", please: a) Consider who else should be cc'ed b) Prefer to cc a suitable mailing list as well c) Ideally: find the original patch on the mailing list and do a reply-to-all to that, adding suitable additional cc's *** Remember to use Documentation/SubmitChecklist when testing your code *** See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find out what to do about this The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/ ------------------------------------------------------ Subject: memcg: improve performance in moving charge From: Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx> Try to reduce overheads in moving charge by: - Instead of calling res_counter_uncharge() against the old cgroup in __mem_cgroup_move_account() everytime, call res_counter_uncharge() at the end of task migration once. - removed css_get(&to->css) from __mem_cgroup_move_account() because callers should have already called css_get(). And removed css_put(&to->css) too, which was called by callers of move_account on success of move_account. - Instead of calling __mem_cgroup_try_charge(), i.e. res_counter_charge(), repeatedly, call res_counter_charge(PAGE_SIZE * count) in can_attach() if possible. - Instead of calling css_get()/css_put() repeatedly, make use of coalesce __css_get()/__css_put() if possible. These changes reduces the overhead from 1.7sec to 0.6sec to move charges of 1G anonymous memory in my test environment. Signed-off-by: Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx> Cc: Balbir Singh <balbir@xxxxxxxxxxxxxxxxxx> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> Cc: Li Zefan <lizf@xxxxxxxxxxxxxx> Cc: Paul Menage <menage@xxxxxxxxxx> Cc: Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- mm/memcontrol.c | 152 +++++++++++++++++++++++++++++----------------- 1 file changed, 98 insertions(+), 54 deletions(-) diff -puN mm/memcontrol.c~memcg-improve-performance-in-moving-charge mm/memcontrol.c --- a/mm/memcontrol.c~memcg-improve-performance-in-moving-charge +++ a/mm/memcontrol.c @@ -253,6 +253,7 @@ static struct move_charge_struct { struct mem_cgroup *from; struct mem_cgroup *to; unsigned long precharge; + unsigned long moved_charge; } mc; /* @@ -1536,14 +1537,23 @@ nomem: * This function is for that and do uncharge, put css's refcnt. * gotten by try_charge(). */ -static void mem_cgroup_cancel_charge(struct mem_cgroup *mem) +static void __mem_cgroup_cancel_charge(struct mem_cgroup *mem, + unsigned long count) { if (!mem_cgroup_is_root(mem)) { - res_counter_uncharge(&mem->res, PAGE_SIZE); + res_counter_uncharge(&mem->res, PAGE_SIZE * count); if (do_swap_account) - res_counter_uncharge(&mem->memsw, PAGE_SIZE); + res_counter_uncharge(&mem->memsw, PAGE_SIZE * count); + VM_BUG_ON(test_bit(CSS_ROOT, &mem->css.flags)); + WARN_ON_ONCE(count > INT_MAX); + __css_put(&mem->css, (int)count); } - css_put(&mem->css); + /* we don't need css_put for root */ +} + +static void mem_cgroup_cancel_charge(struct mem_cgroup *mem) +{ + __mem_cgroup_cancel_charge(mem, 1); } /* @@ -1646,17 +1656,20 @@ static void __mem_cgroup_commit_charge(s * @pc: page_cgroup of the page. * @from: mem_cgroup which the page is moved from. * @to: mem_cgroup which the page is moved to. @from != @to. + * @uncharge: whether we should call uncharge and css_put against @from. * * The caller must confirm following. * - page is not on LRU (isolate_page() is useful.) * - the pc is locked, used, and ->mem_cgroup points to @from. * - * This function does "uncharge" from old cgroup but doesn't do "charge" to - * new cgroup. It should be done by a caller. + * This function doesn't do "charge" nor css_get to new cgroup. It should be + * done by a caller(__mem_cgroup_try_charge would be usefull). If @uncharge is + * true, this function does "uncharge" from old cgroup, but it doesn't if + * @uncharge is false, so a caller should do "uncharge". */ static void __mem_cgroup_move_account(struct page_cgroup *pc, - struct mem_cgroup *from, struct mem_cgroup *to) + struct mem_cgroup *from, struct mem_cgroup *to, bool uncharge) { struct page *page; int cpu; @@ -1669,10 +1682,6 @@ static void __mem_cgroup_move_account(st VM_BUG_ON(!PageCgroupUsed(pc)); VM_BUG_ON(pc->mem_cgroup != from); - if (!mem_cgroup_is_root(from)) - res_counter_uncharge(&from->res, PAGE_SIZE); - mem_cgroup_charge_statistics(from, pc, false); - page = pc->page; if (page_mapped(page) && !PageAnon(page)) { cpu = smp_processor_id(); @@ -1688,12 +1697,12 @@ static void __mem_cgroup_move_account(st __mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_FILE_MAPPED, 1); } + mem_cgroup_charge_statistics(from, pc, false); + if (uncharge) + /* This is not "cancel", but cancel_charge does all we need. */ + mem_cgroup_cancel_charge(from); - if (do_swap_account && !mem_cgroup_is_root(from)) - res_counter_uncharge(&from->memsw, PAGE_SIZE); - css_put(&from->css); - - css_get(&to->css); + /* caller should have done css_get */ pc->mem_cgroup = to; mem_cgroup_charge_statistics(to, pc, true); /* @@ -1710,12 +1719,12 @@ static void __mem_cgroup_move_account(st * __mem_cgroup_move_account() */ static int mem_cgroup_move_account(struct page_cgroup *pc, - struct mem_cgroup *from, struct mem_cgroup *to) + struct mem_cgroup *from, struct mem_cgroup *to, bool uncharge) { int ret = -EINVAL; lock_page_cgroup(pc); if (PageCgroupUsed(pc) && pc->mem_cgroup == from) { - __mem_cgroup_move_account(pc, from, to); + __mem_cgroup_move_account(pc, from, to, uncharge); ret = 0; } unlock_page_cgroup(pc); @@ -1751,11 +1760,9 @@ static int mem_cgroup_move_parent(struct if (ret || !parent) goto put_back; - ret = mem_cgroup_move_account(pc, child, parent); - if (!ret) - css_put(&parent->css); /* drop extra refcnt by try_charge() */ - else - mem_cgroup_cancel_charge(parent); /* does css_put */ + ret = mem_cgroup_move_account(pc, child, parent, true); + if (ret) + mem_cgroup_cancel_charge(parent); put_back: putback_lru_page(page); put: @@ -3441,16 +3448,58 @@ static int mem_cgroup_populate(struct cg } /* Handlers for move charge at task migration. */ -static int mem_cgroup_do_precharge(void) +#define PRECHARGE_COUNT_AT_ONCE 256 +static int mem_cgroup_do_precharge(unsigned long count) { - int ret = -ENOMEM; + int ret = 0; + int batch_count = PRECHARGE_COUNT_AT_ONCE; struct mem_cgroup *mem = mc.to; - ret = __mem_cgroup_try_charge(NULL, GFP_KERNEL, &mem, false, NULL); - if (ret || !mem) - return -ENOMEM; - - mc.precharge++; + if (mem_cgroup_is_root(mem)) { + mc.precharge += count; + /* we don't need css_get for root */ + return ret; + } + /* try to charge at once */ + if (count > 1) { + struct res_counter *dummy; + /* + * "mem" cannot be under rmdir() because we've already checked + * by cgroup_lock_live_cgroup() that it is not removed and we + * are still under the same cgroup_mutex. So we can postpone + * css_get(). + */ + if (res_counter_charge(&mem->res, PAGE_SIZE * count, &dummy)) + goto one_by_one; + if (do_swap_account && res_counter_charge(&mem->memsw, + PAGE_SIZE * count, &dummy)) { + res_counter_uncharge(&mem->res, PAGE_SIZE * count); + goto one_by_one; + } + mc.precharge += count; + VM_BUG_ON(test_bit(CSS_ROOT, &mem->css.flags)); + WARN_ON_ONCE(count > INT_MAX); + __css_get(&mem->css, (int)count); + return ret; + } +one_by_one: + /* fall back to one by one charge */ + while (count--) { + if (signal_pending(current)) { + ret = -EINTR; + break; + } + if (!batch_count--) { + batch_count = PRECHARGE_COUNT_AT_ONCE; + cond_resched(); + } + ret = __mem_cgroup_try_charge(NULL, GFP_KERNEL, &mem, + false, NULL); + if (ret || !mem) + /* mem_cgroup_clear_mc() will do uncharge later */ + return -ENOMEM; + mc.precharge++; + } return ret; } @@ -3573,34 +3622,25 @@ static unsigned long mem_cgroup_count_pr return precharge; } -#define PRECHARGE_AT_ONCE 256 static int mem_cgroup_precharge_mc(struct mm_struct *mm) { - int ret = 0; - int count = PRECHARGE_AT_ONCE; - unsigned long precharge = mem_cgroup_count_precharge(mm); - - while (!ret && precharge--) { - if (signal_pending(current)) { - ret = -EINTR; - break; - } - if (!count--) { - count = PRECHARGE_AT_ONCE; - cond_resched(); - } - ret = mem_cgroup_do_precharge(); - } - - return ret; + return mem_cgroup_do_precharge(mem_cgroup_count_precharge(mm)); } static void mem_cgroup_clear_mc(void) { /* we must uncharge all the leftover precharges from mc.to */ - while (mc.precharge) { - mem_cgroup_cancel_charge(mc.to); - mc.precharge--; + if (mc.precharge) { + __mem_cgroup_cancel_charge(mc.to, mc.precharge); + mc.precharge = 0; + } + /* + * we didn't uncharge from mc.from at mem_cgroup_move_account(), so + * we must uncharge here. + */ + if (mc.moved_charge) { + __mem_cgroup_cancel_charge(mc.from, mc.moved_charge); + mc.moved_charge = 0; } mc.from = NULL; mc.to = NULL; @@ -3628,9 +3668,11 @@ static int mem_cgroup_can_attach(struct VM_BUG_ON(mc.from); VM_BUG_ON(mc.to); VM_BUG_ON(mc.precharge); + VM_BUG_ON(mc.moved_charge); mc.from = from; mc.to = mem; mc.precharge = 0; + mc.moved_charge = 0; ret = mem_cgroup_precharge_mc(mm); if (ret) @@ -3677,9 +3719,11 @@ retry: if (isolate_lru_page(page)) goto put; pc = lookup_page_cgroup(page); - if (!mem_cgroup_move_account(pc, mc.from, mc.to)) { - css_put(&mc.to->css); + if (!mem_cgroup_move_account(pc, + mc.from, mc.to, false)) { mc.precharge--; + /* we uncharge from mc.from later. */ + mc.moved_charge++; } putback_lru_page(page); put: /* is_target_pte_for_mc() gets the page */ @@ -3699,7 +3743,7 @@ put: /* is_target_pte_for_mc() gets th * charges to mc.to if we have failed in charge once in attach() * phase. */ - ret = mem_cgroup_do_precharge(); + ret = mem_cgroup_do_precharge(1); if (!ret) goto retry; } _ Patches currently in -mm which might be from nishimura@xxxxxxxxxxxxxxxxx are cgroup-introduce-cancel_attach.patch cgroup-introduce-coalesce-css_get-and-css_put.patch memcg-add-interface-to-move-charge-at-task-migration.patch memcg-move-charges-of-anonymous-page.patch memcg-move-charges-of-anonymous-page-cleanup.patch memcg-improve-performance-in-moving-charge.patch memcg-avoid-oom-during-moving-charge.patch memcg-move-charges-of-anonymous-swap.patch memcg-improve-performance-in-moving-swap-charge.patch -- To unsubscribe from this list: send the line "unsubscribe mm-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html