From: Michal Hocko <mhocko@suse.cz>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: linux-mm@kvack.org, Vladimir Davydov <vdavydov@parallels.com>,
Greg Thelen <gthelen@google.com>, Tejun Heo <tj@kernel.org>,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 2/3] mm: memcontrol: remove obsolete kmemcg pinning tricks
Date: Wed, 8 Oct 2014 15:32:53 +0200 [thread overview]
Message-ID: <20141008133253.GD4592@dhcp22.suse.cz> (raw)
In-Reply-To: <1411243235-24680-3-git-send-email-hannes@cmpxchg.org>
On Sat 20-09-14 16:00:34, Johannes Weiner wrote:
> As charges now pin the css explicitely, there is no more need for
> kmemcg to acquire a proxy reference for outstanding pages during
> offlining, or maintain state to identify such "dead" groups.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
More than happy to get rid of this trickery.
Acked-by: Michal Hocko <mhocko@suse.cz>
> ---
> mm/memcontrol.c | 74 +--------------------------------------------------------
> 1 file changed, 1 insertion(+), 73 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index b832c87ec43b..019a44ac25d6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -471,7 +471,6 @@ struct mem_cgroup {
> /* internal only representation about the status of kmem accounting. */
> enum {
> KMEM_ACCOUNTED_ACTIVE, /* accounted by this cgroup itself */
> - KMEM_ACCOUNTED_DEAD, /* dead memcg with pending kmem charges */
> };
>
> #ifdef CONFIG_MEMCG_KMEM
> @@ -485,22 +484,6 @@ static bool memcg_kmem_is_active(struct mem_cgroup *memcg)
> return test_bit(KMEM_ACCOUNTED_ACTIVE, &memcg->kmem_account_flags);
> }
>
> -static void memcg_kmem_mark_dead(struct mem_cgroup *memcg)
> -{
> - /*
> - * Our caller must use css_get() first, because memcg_uncharge_kmem()
> - * will call css_put() if it sees the memcg is dead.
> - */
> - smp_wmb();
> - if (test_bit(KMEM_ACCOUNTED_ACTIVE, &memcg->kmem_account_flags))
> - set_bit(KMEM_ACCOUNTED_DEAD, &memcg->kmem_account_flags);
> -}
> -
> -static bool memcg_kmem_test_and_clear_dead(struct mem_cgroup *memcg)
> -{
> - return test_and_clear_bit(KMEM_ACCOUNTED_DEAD,
> - &memcg->kmem_account_flags);
> -}
> #endif
>
> /* Stuffs for move charges at task migration. */
> @@ -2807,22 +2790,7 @@ static void memcg_uncharge_kmem(struct mem_cgroup *memcg,
> if (do_swap_account)
> page_counter_uncharge(&memcg->memsw, nr_pages);
>
> - /* Not down to 0 */
> - if (page_counter_uncharge(&memcg->kmem, nr_pages)) {
> - css_put_many(&memcg->css, nr_pages);
> - return;
> - }
> -
> - /*
> - * Releases a reference taken in kmem_cgroup_css_offline in case
> - * this last uncharge is racing with the offlining code or it is
> - * outliving the memcg existence.
> - *
> - * The memory barrier imposed by test&clear is paired with the
> - * explicit one in memcg_kmem_mark_dead().
> - */
> - if (memcg_kmem_test_and_clear_dead(memcg))
> - css_put(&memcg->css);
> + page_counter_uncharge(&memcg->kmem, nr_pages);
>
> css_put_many(&memcg->css, nr_pages);
> }
> @@ -4805,40 +4773,6 @@ static void memcg_destroy_kmem(struct mem_cgroup *memcg)
> {
> mem_cgroup_sockets_destroy(memcg);
> }
> -
> -static void kmem_cgroup_css_offline(struct mem_cgroup *memcg)
> -{
> - if (!memcg_kmem_is_active(memcg))
> - return;
> -
> - /*
> - * kmem charges can outlive the cgroup. In the case of slab
> - * pages, for instance, a page contain objects from various
> - * processes. As we prevent from taking a reference for every
> - * such allocation we have to be careful when doing uncharge
> - * (see memcg_uncharge_kmem) and here during offlining.
> - *
> - * The idea is that that only the _last_ uncharge which sees
> - * the dead memcg will drop the last reference. An additional
> - * reference is taken here before the group is marked dead
> - * which is then paired with css_put during uncharge resp. here.
> - *
> - * Although this might sound strange as this path is called from
> - * css_offline() when the referencemight have dropped down to 0 and
> - * shouldn't be incremented anymore (css_tryget_online() would
> - * fail) we do not have other options because of the kmem
> - * allocations lifetime.
> - */
> - css_get(&memcg->css);
> -
> - memcg_kmem_mark_dead(memcg);
> -
> - if (atomic_long_read(&memcg->kmem.count))
> - return;
> -
> - if (memcg_kmem_test_and_clear_dead(memcg))
> - css_put(&memcg->css);
> -}
> #else
> static int memcg_init_kmem(struct mem_cgroup *memcg, struct cgroup_subsys *ss)
> {
> @@ -4848,10 +4782,6 @@ static int memcg_init_kmem(struct mem_cgroup *memcg, struct cgroup_subsys *ss)
> static void memcg_destroy_kmem(struct mem_cgroup *memcg)
> {
> }
> -
> -static void kmem_cgroup_css_offline(struct mem_cgroup *memcg)
> -{
> -}
> #endif
>
> /*
> @@ -5443,8 +5373,6 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
> }
> spin_unlock(&memcg->event_list_lock);
>
> - kmem_cgroup_css_offline(memcg);
> -
> /*
> * This requires that offlining is serialized. Right now that is
> * guaranteed because css_killed_work_fn() holds the cgroup_mutex.
> --
> 2.1.0
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2014-10-08 13:32 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-20 20:00 [patch 0/3] mm: memcontrol: eliminate charge reparenting Johannes Weiner
2014-09-20 20:00 ` [patch 1/3] mm: memcontrol: take a css reference for each charged page Johannes Weiner
2014-09-22 8:24 ` Vladimir Davydov
2014-10-08 13:27 ` Michal Hocko
2014-10-08 13:29 ` Michal Hocko
2014-09-20 20:00 ` [patch 2/3] mm: memcontrol: remove obsolete kmemcg pinning tricks Johannes Weiner
2014-09-22 8:25 ` Vladimir Davydov
2014-10-08 13:32 ` Michal Hocko [this message]
2014-09-20 20:00 ` [patch 3/3] mm: memcontrol: continue cache reclaim from offlined groups Johannes Weiner
2014-09-22 8:32 ` Vladimir Davydov
2014-10-08 14:03 ` Michal Hocko
2014-09-21 15:50 ` [patch 0/3] mm: memcontrol: eliminate charge reparenting Vladimir Davydov
2014-10-08 12:48 ` Michal Hocko
2014-10-08 14:17 ` Johannes Weiner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141008133253.GD4592@dhcp22.suse.cz \
--to=mhocko@suse.cz \
--cc=cgroups@vger.kernel.org \
--cc=gthelen@google.com \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=tj@kernel.org \
--cc=vdavydov@parallels.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).