Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg
@ 2019-08-12 23:37 Roman Gushchin
  2019-08-13  0:30 ` Roman Gushchin
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Roman Gushchin @ 2019-08-12 23:37 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Michal Hocko, Johannes Weiner, linux-kernel, kernel-team, Roman Gushchin

Similar to vmstats, percpu caching of local vmevents leads to an
accumulation of errors on non-leaf levels. This happens because
some leftovers may remain in percpu caches, so that they are
never propagated up by the cgroup tree and just disappear into
nonexistence with on releasing of the memory cgroup.

To fix this issue let's accumulate and propagate percpu vmevents
values before releasing the memory cgroup similar to what we're
doing with vmstats.

Since on cpu hotplug we do flush percpu vmstats anyway, we can
iterate only over online cpus.

Fixes: 42a300353577 ("mm: memcontrol: fix recursive statistics correctness & scalabilty")
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/memcontrol.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6d2427abcc0c..249187907339 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3459,6 +3459,25 @@ static void memcg_flush_percpu_vmstats(struct mem_cgroup *memcg, bool slab_only)
 	}
 }
 
+static void memcg_flush_percpu_vmevents(struct mem_cgroup *memcg)
+{
+	unsigned long events[NR_VM_EVENT_ITEMS];
+	struct mem_cgroup *mi;
+	int cpu, i;
+
+	for (i = 0; i < NR_VM_EVENT_ITEMS; i++)
+		events[i] = 0;
+
+	for_each_online_cpu(cpu)
+		for (i = 0; i < NR_VM_EVENT_ITEMS; i++)
+			events[i] += raw_cpu_read(
+				memcg->vmstats_percpu->events[i]);
+
+	for (mi = memcg; mi; mi = parent_mem_cgroup(mi))
+		for (i = 0; i < NR_VM_EVENT_ITEMS; i++)
+			atomic_long_add(events[i], &mi->vmevents[i]);
+}
+
 static void memcg_offline_kmem(struct mem_cgroup *memcg)
 {
 	struct cgroup_subsys_state *css;
@@ -4860,10 +4879,11 @@ static void __mem_cgroup_free(struct mem_cgroup *memcg)
 	int node;
 
 	/*
-	 * Flush percpu vmstats to guarantee the value correctness
+	 * Flush percpu vmstats and vmevents to guarantee the value correctness
 	 * on parent's and all ancestor levels.
 	 */
 	memcg_flush_percpu_vmstats(memcg, false);
+	memcg_flush_percpu_vmevents(memcg);
 	for_each_node(node)
 		free_mem_cgroup_per_node_info(memcg, node);
 	free_percpu(memcg->vmstats_percpu);
-- 
2.21.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg
  2019-08-12 23:37 [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg Roman Gushchin
@ 2019-08-13  0:30 ` Roman Gushchin
  2019-08-13 21:31 ` Andrew Morton
  2019-08-14 11:33 ` Michal Hocko
  2 siblings, 0 replies; 5+ messages in thread
From: Roman Gushchin @ 2019-08-13  0:30 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Michal Hocko, Johannes Weiner, linux-kernel, Kernel Team

On Mon, Aug 12, 2019 at 04:37:54PM -0700, Roman Gushchin wrote:
> Similar to vmstats, percpu caching of local vmevents leads to an
> accumulation of errors on non-leaf levels. This happens because
> some leftovers may remain in percpu caches, so that they are
> never propagated up by the cgroup tree and just disappear into
> nonexistence with on releasing of the memory cgroup.
> 
> To fix this issue let's accumulate and propagate percpu vmevents
> values before releasing the memory cgroup similar to what we're
> doing with vmstats.
> 
> Since on cpu hotplug we do flush percpu vmstats anyway, we can
> iterate only over online cpus.

Just to clarify: this patch should be placed on top of two other
patches, which I sent a bit earlier today:

1) mm: memcontrol: flush percpu slab vmstats on kmem offlining
2) mm: memcontrol: flush percpu vmstats before releasing memcg

Sorry for the inconvenience, I forgot about vmevents during
working on the final version, and remembered too late.

Thanks!


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg
  2019-08-12 23:37 [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg Roman Gushchin
  2019-08-13  0:30 ` Roman Gushchin
@ 2019-08-13 21:31 ` Andrew Morton
  2019-08-13 21:47   ` Roman Gushchin
  2019-08-14 11:33 ` Michal Hocko
  2 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2019-08-13 21:31 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: linux-mm, Michal Hocko, Johannes Weiner, linux-kernel, kernel-team

On Mon, 12 Aug 2019 16:37:54 -0700 Roman Gushchin <guro@fb.com> wrote:

> Similar to vmstats, percpu caching of local vmevents leads to an
> accumulation of errors on non-leaf levels. This happens because
> some leftovers may remain in percpu caches, so that they are
> never propagated up by the cgroup tree and just disappear into
> nonexistence with on releasing of the memory cgroup.
> 
> To fix this issue let's accumulate and propagate percpu vmevents
> values before releasing the memory cgroup similar to what we're
> doing with vmstats.
> 
> Since on cpu hotplug we do flush percpu vmstats anyway, we can
> iterate only over online cpus.
> 
> Fixes: 42a300353577 ("mm: memcontrol: fix recursive statistics correctness & scalabilty")

No cc:stable?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg
  2019-08-13 21:31 ` Andrew Morton
@ 2019-08-13 21:47   ` Roman Gushchin
  0 siblings, 0 replies; 5+ messages in thread
From: Roman Gushchin @ 2019-08-13 21:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, Michal Hocko, Johannes Weiner, linux-kernel,
	Kernel Team, stable

On Tue, Aug 13, 2019 at 02:31:17PM -0700, Andrew Morton wrote:
> On Mon, 12 Aug 2019 16:37:54 -0700 Roman Gushchin <guro@fb.com> wrote:
> 
> > Similar to vmstats, percpu caching of local vmevents leads to an
> > accumulation of errors on non-leaf levels. This happens because
> > some leftovers may remain in percpu caches, so that they are
> > never propagated up by the cgroup tree and just disappear into
> > nonexistence with on releasing of the memory cgroup.
> > 
> > To fix this issue let's accumulate and propagate percpu vmevents
> > values before releasing the memory cgroup similar to what we're
> > doing with vmstats.
> > 
> > Since on cpu hotplug we do flush percpu vmstats anyway, we can
> > iterate only over online cpus.
> > 
> > Fixes: 42a300353577 ("mm: memcontrol: fix recursive statistics correctness & scalabilty")
> 
> No cc:stable?
> 

Here too, cc:stable is definitely missing. Adding now. Thanks!


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg
  2019-08-12 23:37 [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg Roman Gushchin
  2019-08-13  0:30 ` Roman Gushchin
  2019-08-13 21:31 ` Andrew Morton
@ 2019-08-14 11:33 ` Michal Hocko
  2 siblings, 0 replies; 5+ messages in thread
From: Michal Hocko @ 2019-08-14 11:33 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, Johannes Weiner, linux-kernel, kernel-team

On Mon 12-08-19 16:37:54, Roman Gushchin wrote:
> Similar to vmstats, percpu caching of local vmevents leads to an
> accumulation of errors on non-leaf levels. This happens because
> some leftovers may remain in percpu caches, so that they are
> never propagated up by the cgroup tree and just disappear into
> nonexistence with on releasing of the memory cgroup.
> 
> To fix this issue let's accumulate and propagate percpu vmevents
> values before releasing the memory cgroup similar to what we're
> doing with vmstats.
> 
> Since on cpu hotplug we do flush percpu vmstats anyway, we can
> iterate only over online cpus.
> 
> Fixes: 42a300353577 ("mm: memcontrol: fix recursive statistics correctness & scalabilty")
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/memcontrol.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6d2427abcc0c..249187907339 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3459,6 +3459,25 @@ static void memcg_flush_percpu_vmstats(struct mem_cgroup *memcg, bool slab_only)
>  	}
>  }
>  
> +static void memcg_flush_percpu_vmevents(struct mem_cgroup *memcg)
> +{
> +	unsigned long events[NR_VM_EVENT_ITEMS];
> +	struct mem_cgroup *mi;
> +	int cpu, i;
> +
> +	for (i = 0; i < NR_VM_EVENT_ITEMS; i++)
> +		events[i] = 0;
> +
> +	for_each_online_cpu(cpu)
> +		for (i = 0; i < NR_VM_EVENT_ITEMS; i++)
> +			events[i] += raw_cpu_read(
> +				memcg->vmstats_percpu->events[i]);
> +
> +	for (mi = memcg; mi; mi = parent_mem_cgroup(mi))
> +		for (i = 0; i < NR_VM_EVENT_ITEMS; i++)
> +			atomic_long_add(events[i], &mi->vmevents[i]);
> +}
> +
>  static void memcg_offline_kmem(struct mem_cgroup *memcg)
>  {
>  	struct cgroup_subsys_state *css;
> @@ -4860,10 +4879,11 @@ static void __mem_cgroup_free(struct mem_cgroup *memcg)
>  	int node;
>  
>  	/*
> -	 * Flush percpu vmstats to guarantee the value correctness
> +	 * Flush percpu vmstats and vmevents to guarantee the value correctness
>  	 * on parent's and all ancestor levels.
>  	 */
>  	memcg_flush_percpu_vmstats(memcg, false);
> +	memcg_flush_percpu_vmevents(memcg);
>  	for_each_node(node)
>  		free_mem_cgroup_per_node_info(memcg, node);
>  	free_percpu(memcg->vmstats_percpu);
> -- 
> 2.21.0

-- 
Michal Hocko
SUSE Labs


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-12 23:37 [PATCH] mm: memcontrol: flush percpu vmevents before releasing memcg Roman Gushchin
2019-08-13  0:30 ` Roman Gushchin
2019-08-13 21:31 ` Andrew Morton
2019-08-13 21:47   ` Roman Gushchin
2019-08-14 11:33 ` Michal Hocko

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org linux-mm@archiver.kernel.org
	public-inbox-index linux-mm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox