| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87vckh8uhx.fsf@skywalker.in.ibm.com>
Date: Mon, 30 Apr 2012 14:30:10 +0530
From: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
Linux Kernel <linux-kernel@...r.kernel.org>
Cc: "linux-mm\@kvack.org" <linux-mm@...ck.org>,
"cgroups\@vger.kernel.org" <cgroups@...r.kernel.org>,
Michal Hocko <mhocko@...e.cz>,
Johannes Weiner <hannes@...xchg.org>,
Frederic Weisbecker <fweisbec@...il.com>,
Glauber Costa <glommer@...allels.com>,
Tejun Heo <tj@...nel.org>, Han Ying <yinghan@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
kamezawa.hiroyuki@...il.com
Subject: Re: [RFC][PATCH 4/7 v2] memcg: use res_counter_uncharge_until in move_parent
KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com> writes:
> By using res_counter_uncharge_until(), we can avoid
> unnecessary charging.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>
> ---
> mm/memcontrol.c | 63 ++++++++++++++++++++++++++++++++++++------------------
> 1 files changed, 42 insertions(+), 21 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 613bb15..ed53d64 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2420,6 +2420,24 @@ static void __mem_cgroup_cancel_charge(struct mem_cgroup *memcg,
> }
>
> /*
> + * Cancel chages in this cgroup....doesn't propagates to parent cgroup.
> + * This is useful when moving usage to parent cgroup.
> + */
> +static void __mem_cgroup_cancel_local_charge(struct mem_cgroup *memcg,
> + unsigned int nr_pages)
> +{
> + if (!mem_cgroup_is_root(memcg)) {
> + unsigned long bytes = nr_pages * PAGE_SIZE;
> +
> + res_counter_uncharge_until(&memcg->res,
> + memcg->res.parent, bytes);
> + if (do_swap_account)
> + res_counter_uncharge_until(&memcg->memsw,
> + memcg->memsw.parent, bytes);
> + }
> +}
> +
> +/*
> * A helper function to get mem_cgroup from ID. must be called under
> * rcu_read_lock(). The caller must check css_is_removed() or some if
> * it's concern. (dropping refcnt from swap can be called against removed
> @@ -2677,16 +2695,28 @@ static int mem_cgroup_move_parent(struct page *page,
> nr_pages = hpage_nr_pages(page);
>
> parent = mem_cgroup_from_cont(pcg);
> - ret = __mem_cgroup_try_charge(NULL, gfp_mask, nr_pages, &parent, false);
> - if (ret)
> - goto put_back;
> + if (!parent->use_hierarchy) {
> + ret = __mem_cgroup_try_charge(NULL,
> + gfp_mask, nr_pages, &parent, false);
> + if (ret)
> + goto put_back;
> + }
>
> if (nr_pages > 1)
> flags = compound_lock_irqsave(page);
>
> - ret = mem_cgroup_move_account(page, nr_pages, pc, child, parent, true);
> - if (ret)
> - __mem_cgroup_cancel_charge(parent, nr_pages);
> + if (parent->use_hierarchy) {
> + ret = mem_cgroup_move_account(page, nr_pages,
> + pc, child, parent, false);
> + if (!ret)
> + __mem_cgroup_cancel_local_charge(child, nr_pages);
> + } else {
> + ret = mem_cgroup_move_account(page, nr_pages,
> + pc, child, parent, true);
> +
> + if (ret)
> + __mem_cgroup_cancel_charge(parent, nr_pages);
> + }
>
May be a comment around this ? I had to look closer to find why
there is a if (!ret) and if (ret) difference.
> if (nr_pages > 1)
> compound_unlock_irqrestore(page, flags);
> @@ -3295,6 +3325,7 @@ int mem_cgroup_move_hugetlb_parent(int idx, struct cgroup *cgroup,
> struct cgroup *pcgrp = cgroup->parent;
> struct mem_cgroup *parent = mem_cgroup_from_cont(pcgrp);
> struct mem_cgroup *memcg = mem_cgroup_from_cont(cgroup);
> + struct res_counter *counter;
>
> if (!get_page_unless_zero(page))
> goto out;
> @@ -3305,28 +3336,18 @@ int mem_cgroup_move_hugetlb_parent(int idx, struct cgroup *cgroup,
> goto err_out;
>
> csize = PAGE_SIZE << compound_order(page);
> - /*
> - * If we have use_hierarchy set we can never fail here. So instead of
> - * using res_counter_uncharge use the open-coded variant which just
> - * uncharge the child res_counter. The parent will retain the charge.
> - */
> - if (parent->use_hierarchy) {
> - unsigned long flags;
> - struct res_counter *counter;
> -
> - counter = &memcg->hugepage[idx];
> - spin_lock_irqsave(&counter->lock, flags);
> - res_counter_uncharge_locked(counter, csize);
> - spin_unlock_irqrestore(&counter->lock, flags);
> - } else {
> + /* If parent->use_hierarchy == 0, we need to charge parent */
> + if (!parent->use_hierarchy) {
> ret = res_counter_charge(&parent->hugepage[idx],
> csize, &fail_res);
> if (ret) {
> ret = -EBUSY;
> goto err_out;
> }
> - res_counter_uncharge(&memcg->hugepage[idx], csize);
> }
> + counter = &memcg->hugepage[idx];
> + res_counter_uncharge_until(counter, counter->parent, csize);
> +
> pc->mem_cgroup = parent;
> err_out:
> unlock_page_cgroup(pc);
-aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists