All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Roman Gushchin <guro@fb.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>, Michal Hocko <mhocko@suse.com>,
	linux-mm@kvack.org, cgroups@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 1/7] mm: memcontrol: fix cpuhotplug statistics flushing
Date: Thu, 4 Feb 2021 14:29:57 -0500	[thread overview]
Message-ID: <YBxLNZJ/83P7H8+H@cmpxchg.org> (raw)
In-Reply-To: <20210203022853.GG1812008@carbon.dhcp.thefacebook.com>

On Tue, Feb 02, 2021 at 06:28:53PM -0800, Roman Gushchin wrote:
> On Tue, Feb 02, 2021 at 03:07:47PM -0800, Roman Gushchin wrote:
> > On Tue, Feb 02, 2021 at 01:47:40PM -0500, Johannes Weiner wrote:
> > > The memcg hotunplug callback erroneously flushes counts on the local
> > > CPU, not the counts of the CPU going away; those counts will be lost.
> > > 
> > > Flush the CPU that is actually going away.
> > > 
> > > Also simplify the code a bit by using mod_memcg_state() and
> > > count_memcg_events() instead of open-coding the upward flush - this is
> > > comparable to how vmstat.c handles hotunplug flushing.
> > 
> > To the whole series: it's really nice to have an accurate stats at
> > non-leaf levels. Just as an illustration: if there are 32 CPUs and
> > 1000 sub-cgroups (which is an absolutely realistic number, because
> > often there are many dying generations of each cgroup), the error
> > margin is 3.9GB. It makes all numbers pretty much random and all
> > possible tests extremely flaky.
> 
> Btw, I was just looking into kmem kselftests failures/flakiness,
> which is caused by exactly this problem: without waiting for the
> finish of dying cgroups reclaim, we can't make any reliable assumptions
> about what to expect from memcg stats.

Good point about the selftests. I gave them a shot, and indeed this
series makes test_kmem work again:

vanilla:
ok 1 test_kmem_basic
memory.current = 8810496
slab + anon + file + kernel_stack = 17074568
slab = 6101384
anon = 946176
file = 0
kernel_stack = 10027008
not ok 2 test_kmem_memcg_deletion
ok 3 test_kmem_proc_kpagecgroup
ok 4 test_kmem_kernel_stacks
ok 5 test_kmem_dead_cgroups
ok 6 test_percpu_basic

patched:
ok 1 test_kmem_basic
ok 2 test_kmem_memcg_deletion
ok 3 test_kmem_proc_kpagecgroup
ok 4 test_kmem_kernel_stacks
ok 5 test_kmem_dead_cgroups
ok 6 test_percpu_basic

It even passes with a reduced margin in the patched kernel, since the
percpu drift - which this test already tried to account for - is now
only on the page_counter side (whereas memory.stat is always precise).

I'm going to include that data in the v2 changelog, as well as a patch
to update test_kmem.c to the more stringent error tolerances.

> So looking forward to have this patchset merged!

Thanks

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
To: Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org>
Cc: Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org>,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	kernel-team-b10kYP2dOMg@public.gmane.org
Subject: Re: [PATCH 1/7] mm: memcontrol: fix cpuhotplug statistics flushing
Date: Thu, 4 Feb 2021 14:29:57 -0500	[thread overview]
Message-ID: <YBxLNZJ/83P7H8+H@cmpxchg.org> (raw)
In-Reply-To: <20210203022853.GG1812008-cx5fftMpWqeCjSd+JxjunQ2O0Ztt9esIQQ4Iyu8u01E@public.gmane.org>

On Tue, Feb 02, 2021 at 06:28:53PM -0800, Roman Gushchin wrote:
> On Tue, Feb 02, 2021 at 03:07:47PM -0800, Roman Gushchin wrote:
> > On Tue, Feb 02, 2021 at 01:47:40PM -0500, Johannes Weiner wrote:
> > > The memcg hotunplug callback erroneously flushes counts on the local
> > > CPU, not the counts of the CPU going away; those counts will be lost.
> > > 
> > > Flush the CPU that is actually going away.
> > > 
> > > Also simplify the code a bit by using mod_memcg_state() and
> > > count_memcg_events() instead of open-coding the upward flush - this is
> > > comparable to how vmstat.c handles hotunplug flushing.
> > 
> > To the whole series: it's really nice to have an accurate stats at
> > non-leaf levels. Just as an illustration: if there are 32 CPUs and
> > 1000 sub-cgroups (which is an absolutely realistic number, because
> > often there are many dying generations of each cgroup), the error
> > margin is 3.9GB. It makes all numbers pretty much random and all
> > possible tests extremely flaky.
> 
> Btw, I was just looking into kmem kselftests failures/flakiness,
> which is caused by exactly this problem: without waiting for the
> finish of dying cgroups reclaim, we can't make any reliable assumptions
> about what to expect from memcg stats.

Good point about the selftests. I gave them a shot, and indeed this
series makes test_kmem work again:

vanilla:
ok 1 test_kmem_basic
memory.current = 8810496
slab + anon + file + kernel_stack = 17074568
slab = 6101384
anon = 946176
file = 0
kernel_stack = 10027008
not ok 2 test_kmem_memcg_deletion
ok 3 test_kmem_proc_kpagecgroup
ok 4 test_kmem_kernel_stacks
ok 5 test_kmem_dead_cgroups
ok 6 test_percpu_basic

patched:
ok 1 test_kmem_basic
ok 2 test_kmem_memcg_deletion
ok 3 test_kmem_proc_kpagecgroup
ok 4 test_kmem_kernel_stacks
ok 5 test_kmem_dead_cgroups
ok 6 test_percpu_basic

It even passes with a reduced margin in the patched kernel, since the
percpu drift - which this test already tried to account for - is now
only on the page_counter side (whereas memory.stat is always precise).

I'm going to include that data in the v2 changelog, as well as a patch
to update test_kmem.c to the more stringent error tolerances.

> So looking forward to have this patchset merged!

Thanks

  reply	other threads:[~2021-02-04 19:31 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-02 18:47 [PATCH 0/7]: mm: memcontrol: switch to rstat Johannes Weiner
2021-02-02 18:47 ` Johannes Weiner
2021-02-02 18:47 ` [PATCH 1/7] mm: memcontrol: fix cpuhotplug statistics flushing Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-02 22:23   ` Shakeel Butt
2021-02-02 22:23     ` Shakeel Butt
2021-02-02 22:23     ` Shakeel Butt
2021-02-02 23:07   ` Roman Gushchin
2021-02-02 23:07     ` Roman Gushchin
2021-02-03  2:28     ` Roman Gushchin
2021-02-03  2:28       ` Roman Gushchin
2021-02-04 19:29       ` Johannes Weiner [this message]
2021-02-04 19:29         ` Johannes Weiner
2021-02-04 19:34         ` Roman Gushchin
2021-02-04 19:34           ` Roman Gushchin
2021-02-05 17:50           ` Johannes Weiner
2021-02-05 17:50             ` Johannes Weiner
2021-02-04 13:28   ` Michal Hocko
2021-02-04 13:28     ` Michal Hocko
2021-02-02 18:47 ` [PATCH 2/7] mm: memcontrol: kill mem_cgroup_nodeinfo() Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-02 22:24   ` Shakeel Butt
2021-02-02 22:24     ` Shakeel Butt
2021-02-02 22:24     ` Shakeel Butt
2021-02-02 23:13   ` Roman Gushchin
2021-02-02 23:13     ` Roman Gushchin
2021-02-04 13:29   ` Michal Hocko
2021-02-04 13:29     ` Michal Hocko
2021-02-02 18:47 ` [PATCH 3/7] mm: memcontrol: privatize memcg_page_state query functions Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-02 22:26   ` Shakeel Butt
2021-02-02 22:26     ` Shakeel Butt
2021-02-02 23:17   ` Roman Gushchin
2021-02-02 23:17     ` Roman Gushchin
2021-02-04 13:30   ` Michal Hocko
2021-02-04 13:30     ` Michal Hocko
2021-02-02 18:47 ` [PATCH 4/7] cgroup: rstat: support cgroup1 Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-03  1:16   ` Roman Gushchin
2021-02-03  1:16     ` Roman Gushchin
2021-02-04 13:39   ` Michal Hocko
2021-02-04 13:39     ` Michal Hocko
2021-02-04 16:01     ` Johannes Weiner
2021-02-04 16:01       ` Johannes Weiner
2021-02-04 16:42       ` Michal Hocko
2021-02-04 16:42         ` Michal Hocko
2021-02-02 18:47 ` [PATCH 5/7] cgroup: rstat: punt root-level optimization to individual controllers Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-02 18:47 ` [PATCH 6/7] mm: memcontrol: switch to rstat Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-03  1:47   ` Roman Gushchin
2021-02-03  1:47     ` Roman Gushchin
2021-02-04 16:26     ` Johannes Weiner
2021-02-04 16:26       ` Johannes Weiner
2021-02-04 18:45       ` Roman Gushchin
2021-02-04 18:45         ` Roman Gushchin
2021-02-04 20:05         ` Johannes Weiner
2021-02-04 20:05           ` Johannes Weiner
2021-02-04 14:19   ` Michal Hocko
2021-02-04 14:19     ` Michal Hocko
2021-02-04 16:15     ` Johannes Weiner
2021-02-04 16:44       ` Michal Hocko
2021-02-04 16:44         ` Michal Hocko
2021-02-04 20:28         ` Johannes Weiner
2021-02-04 20:28           ` Johannes Weiner
2021-02-05 15:05   ` Michal Hocko
2021-02-05 15:05     ` Michal Hocko
2021-02-05 16:34     ` Johannes Weiner
2021-02-05 16:34       ` Johannes Weiner
2021-02-08 14:07       ` Michal Hocko
2021-02-08 14:07         ` Michal Hocko
2021-02-02 18:47 ` [PATCH 7/7] mm: memcontrol: consolidate lruvec stat flushing Johannes Weiner
2021-02-03  2:25   ` Roman Gushchin
2021-02-03  2:25     ` Roman Gushchin
2021-02-04 21:44     ` Johannes Weiner
2021-02-04 21:44       ` Johannes Weiner
2021-02-04 21:47       ` Roman Gushchin
2021-02-04 21:47         ` Roman Gushchin
2021-02-05 15:17   ` Michal Hocko
2021-02-05 15:17     ` Michal Hocko
2021-02-05 17:10     ` Johannes Weiner
2021-02-05 17:10       ` 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=YBxLNZJ/83P7H8+H@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=guro@fb.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=tj@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.