lkml.org 
[lkml]   [2019]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/5] mm: release memcg percpu data prematurely
On Thu, Mar 07, 2019 at 03:00:31PM -0800, Roman Gushchin wrote:
> To reduce the memory footprint of a dying memory cgroup, let's
> release massive percpu data (vmstats_percpu) as early as possible,
> and use atomic counterparts instead.
>
> A dying cgroup can remain in the dying state for quite a long
> time, being pinned in memory by any reference. For example,
> if a page mlocked by some other cgroup, is charged to the dying
> cgroup, it won't go away until the page will be released.
>
> A dying memory cgroup can have some memory activity (e.g. dirty
> pages can be flushed after cgroup removal), but in general it's
> not expected to be very active in comparison to living cgroups.
>
> So reducing the memory footprint by releasing percpu data
> and switching over to atomics seems to be a good trade off.
>
> Signed-off-by: Roman Gushchin <guro@fb.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

One nitpick below:

> @@ -4612,6 +4612,26 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
> return 0;
> }
>
> +static void mem_cgroup_free_percpu(struct rcu_head *rcu)
> +{
> + struct mem_cgroup *memcg = container_of(rcu, struct mem_cgroup, rcu);
> +
> + free_percpu(memcg->vmstats_percpu_offlined);
> + WARN_ON_ONCE(memcg->vmstats_percpu);
> +
> + css_put(&memcg->css);

Nitpick: I had to double take seeing a "mem_cgroup_free_*" function
that does css_put(). We use "free" terminology (mem_cgroup_css_free,
memcg_free_kmem, memcg_free_shrinker_maps, mem_cgroup_free) from the
.css_free callback, which only happens when the last reference is put.

Can we go with something less ambigous? We can add "rcu" and drop the
mem_cgroup prefix since it's narrowly scoped. "percpu_rcu_free"?

> +static void mem_cgroup_offline_percpu(struct mem_cgroup *memcg)
> +{
> + memcg->vmstats_percpu_offlined = (struct memcg_vmstats_percpu __percpu*)
> + rcu_dereference(memcg->vmstats_percpu);
> + rcu_assign_pointer(memcg->vmstats_percpu, NULL);
> +
> + css_get(&memcg->css);
> + call_rcu(&memcg->rcu, mem_cgroup_free_percpu);
> +}
> +
> static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
> {
> struct mem_cgroup *memcg = mem_cgroup_from_css(css);

\
 
 \ /
  Last update: 2019-03-11 18:26    [W:0.198 / U:0.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site