* [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
@ 2021-01-04 5:03 Hugh Dickins
2021-01-04 8:11 ` Alex Shi
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Hugh Dickins @ 2021-01-04 5:03 UTC (permalink / raw)
To: Andrew Morton
Cc: Hui Su, Alex Shi, Lorenzo Stoakes, Michal Hocko, Johannes Weiner,
Shakeel Butt, Roman Gushchin, linux-kernel, linux-mm
Boot a CONFIG_MEMCG=y kernel with "cgroup_disabled=memory" and you are
met by a series of warnings from the VM_WARN_ON_ONCE_PAGE(!memcg, page)
recently added to the inline mem_cgroup_page_lruvec().
An earlier attempt to place that warning, in mem_cgroup_lruvec(), had
been careful to do so after weeding out the mem_cgroup_disabled() case;
but was itself invalid because of the mem_cgroup_lruvec(NULL, pgdat) in
clear_pgdat_congested() and age_active_anon().
Warning in mem_cgroup_page_lruvec() was once useful in detecting a KSM
charge bug, so may be worth keeping: but skip if mem_cgroup_disabled().
Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
Signed-off-by: Hugh Dickins <hughd@google.com>
---
include/linux/memcontrol.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- 5.11-rc2/include/linux/memcontrol.h 2020-12-27 20:39:36.751923135 -0800
+++ linux/include/linux/memcontrol.h 2021-01-03 19:38:24.822978559 -0800
@@ -665,7 +665,7 @@ static inline struct lruvec *mem_cgroup_
{
struct mem_cgroup *memcg = page_memcg(page);
- VM_WARN_ON_ONCE_PAGE(!memcg, page);
+ VM_WARN_ON_ONCE_PAGE(!memcg && !mem_cgroup_disabled(), page);
return mem_cgroup_lruvec(memcg, pgdat);
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
2021-01-04 5:03 [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec() Hugh Dickins
@ 2021-01-04 8:11 ` Alex Shi
2021-01-06 6:49 ` Baoquan He
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Alex Shi @ 2021-01-04 8:11 UTC (permalink / raw)
To: Hugh Dickins, Andrew Morton
Cc: Hui Su, Lorenzo Stoakes, Michal Hocko, Johannes Weiner,
Shakeel Butt, Roman Gushchin, linux-kernel, linux-mm
Reviewed-by: Alex Shi <alex.shi@linux.alibaba.com>
在 2021/1/4 下午1:03, Hugh Dickins 写道:
> Boot a CONFIG_MEMCG=y kernel with "cgroup_disabled=memory" and you are
> met by a series of warnings from the VM_WARN_ON_ONCE_PAGE(!memcg, page)
> recently added to the inline mem_cgroup_page_lruvec().
>
> An earlier attempt to place that warning, in mem_cgroup_lruvec(), had
> been careful to do so after weeding out the mem_cgroup_disabled() case;
> but was itself invalid because of the mem_cgroup_lruvec(NULL, pgdat) in
> clear_pgdat_congested() and age_active_anon().
>
> Warning in mem_cgroup_page_lruvec() was once useful in detecting a KSM
> charge bug, so may be worth keeping: but skip if mem_cgroup_disabled().
>
> Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
> Signed-off-by: Hugh Dickins <hughd@google.com>
> ---
>
> include/linux/memcontrol.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- 5.11-rc2/include/linux/memcontrol.h 2020-12-27 20:39:36.751923135 -0800
> +++ linux/include/linux/memcontrol.h 2021-01-03 19:38:24.822978559 -0800
> @@ -665,7 +665,7 @@ static inline struct lruvec *mem_cgroup_
> {
> struct mem_cgroup *memcg = page_memcg(page);
>
> - VM_WARN_ON_ONCE_PAGE(!memcg, page);
> + VM_WARN_ON_ONCE_PAGE(!memcg && !mem_cgroup_disabled(), page);
> return mem_cgroup_lruvec(memcg, pgdat);
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
2021-01-04 5:03 [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec() Hugh Dickins
2021-01-04 8:11 ` Alex Shi
@ 2021-01-06 6:49 ` Baoquan He
2021-01-06 19:33 ` Andrew Morton
2021-01-06 19:35 ` Andrew Morton
2021-01-06 16:19 ` Chris Down
2021-01-07 18:04 ` Vlastimil Babka
3 siblings, 2 replies; 10+ messages in thread
From: Baoquan He @ 2021-01-06 6:49 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andrew Morton, Hui Su, Alex Shi, Lorenzo Stoakes, Michal Hocko,
Johannes Weiner, Shakeel Butt, Roman Gushchin, linux-kernel,
linux-mm
On 01/03/21 at 09:03pm, Hugh Dickins wrote:
> Boot a CONFIG_MEMCG=y kernel with "cgroup_disabled=memory" and you are
> met by a series of warnings from the VM_WARN_ON_ONCE_PAGE(!memcg, page)
> recently added to the inline mem_cgroup_page_lruvec().
>
> An earlier attempt to place that warning, in mem_cgroup_lruvec(), had
> been careful to do so after weeding out the mem_cgroup_disabled() case;
> but was itself invalid because of the mem_cgroup_lruvec(NULL, pgdat) in
> clear_pgdat_congested() and age_active_anon().
>
> Warning in mem_cgroup_page_lruvec() was once useful in detecting a KSM
> charge bug, so may be worth keeping: but skip if mem_cgroup_disabled().
>
> Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
> Signed-off-by: Hugh Dickins <hughd@google.com>
> ---
>
> include/linux/memcontrol.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- 5.11-rc2/include/linux/memcontrol.h 2020-12-27 20:39:36.751923135 -0800
> +++ linux/include/linux/memcontrol.h 2021-01-03 19:38:24.822978559 -0800
> @@ -665,7 +665,7 @@ static inline struct lruvec *mem_cgroup_
> {
> struct mem_cgroup *memcg = page_memcg(page);
>
> - VM_WARN_ON_ONCE_PAGE(!memcg, page);
> + VM_WARN_ON_ONCE_PAGE(!memcg && !mem_cgroup_disabled(), page);
> return mem_cgroup_lruvec(memcg, pgdat);
Thanks for fixing this. We also encountered this issue in kdump kernel
with the mainline 5.10 kernel since 'cgroup_disable=memory' is added.
Reviewed-by: Baoquan He <bhe@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
2021-01-04 5:03 [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec() Hugh Dickins
2021-01-04 8:11 ` Alex Shi
2021-01-06 6:49 ` Baoquan He
@ 2021-01-06 16:19 ` Chris Down
2021-01-07 18:04 ` Vlastimil Babka
3 siblings, 0 replies; 10+ messages in thread
From: Chris Down @ 2021-01-06 16:19 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andrew Morton, Hui Su, Alex Shi, Lorenzo Stoakes, Michal Hocko,
Johannes Weiner, Shakeel Butt, Roman Gushchin, linux-kernel,
linux-mm
Hugh Dickins writes:
>Boot a CONFIG_MEMCG=y kernel with "cgroup_disabled=memory" and you are
>met by a series of warnings from the VM_WARN_ON_ONCE_PAGE(!memcg, page)
>recently added to the inline mem_cgroup_page_lruvec().
>
>An earlier attempt to place that warning, in mem_cgroup_lruvec(), had
>been careful to do so after weeding out the mem_cgroup_disabled() case;
>but was itself invalid because of the mem_cgroup_lruvec(NULL, pgdat) in
>clear_pgdat_congested() and age_active_anon().
>
>Warning in mem_cgroup_page_lruvec() was once useful in detecting a KSM
>charge bug, so may be worth keeping: but skip if mem_cgroup_disabled().
>
>Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
>Signed-off-by: Hugh Dickins <hughd@google.com>
Thanks.
Acked-by: Chris Down <chris@chrisdown.name>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
2021-01-06 6:49 ` Baoquan He
@ 2021-01-06 19:33 ` Andrew Morton
2021-01-06 19:35 ` Andrew Morton
1 sibling, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2021-01-06 19:33 UTC (permalink / raw)
To: Baoquan He
Cc: Hugh Dickins, Hui Su, Alex Shi, Lorenzo Stoakes, Michal Hocko,
Johannes Weiner, Shakeel Butt, Roman Gushchin, linux-kernel,
linux-mm
On Wed, 6 Jan 2021 14:49:35 +0800 Baoquan He <bhe@redhat.com> wrote:
> > --- 5.11-rc2/include/linux/memcontrol.h 2020-12-27 20:39:36.751923135 -0800
> > +++ linux/include/linux/memcontrol.h 2021-01-03 19:38:24.822978559 -0800
> > @@ -665,7 +665,7 @@ static inline struct lruvec *mem_cgroup_
> > {
> > struct mem_cgroup *memcg = page_memcg(page);
> >
> > - VM_WARN_ON_ONCE_PAGE(!memcg, page);
> > + VM_WARN_ON_ONCE_PAGE(!memcg && !mem_cgroup_disabled(), page);
> > return mem_cgroup_lruvec(memcg, pgdat);
>
> Thanks for fixing this. We also encountered this issue in kdump kernel
> with the mainline 5.10 kernel since 'cgroup_disable=memory' is added.
Thanks - I added the cc:stable.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
2021-01-06 6:49 ` Baoquan He
2021-01-06 19:33 ` Andrew Morton
@ 2021-01-06 19:35 ` Andrew Morton
2021-01-07 2:53 ` Baoquan He
1 sibling, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2021-01-06 19:35 UTC (permalink / raw)
To: Baoquan He
Cc: Hugh Dickins, Hui Su, Alex Shi, Lorenzo Stoakes, Michal Hocko,
Johannes Weiner, Shakeel Butt, Roman Gushchin, linux-kernel,
linux-mm
On Wed, 6 Jan 2021 14:49:35 +0800 Baoquan He <bhe@redhat.com> wrote:
> > Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
>
> ...
>
> Thanks for fixing this. We also encountered this issue in kdump kernel
> with the mainline 5.10 kernel since 'cgroup_disable=memory' is added.
Wait. 9a1ac2288cf1 isn't present in 5.10?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
2021-01-06 19:35 ` Andrew Morton
@ 2021-01-07 2:53 ` Baoquan He
0 siblings, 0 replies; 10+ messages in thread
From: Baoquan He @ 2021-01-07 2:53 UTC (permalink / raw)
To: Andrew Morton
Cc: Hugh Dickins, Hui Su, Alex Shi, Lorenzo Stoakes, Michal Hocko,
Johannes Weiner, Shakeel Butt, Roman Gushchin, linux-kernel,
linux-mm
On 01/06/21 at 11:35am, Andrew Morton wrote:
> On Wed, 6 Jan 2021 14:49:35 +0800 Baoquan He <bhe@redhat.com> wrote:
>
> > > Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
> >
> > ...
> >
> > Thanks for fixing this. We also encountered this issue in kdump kernel
> > with the mainline 5.10 kernel since 'cgroup_disable=memory' is added.
>
> Wait. 9a1ac2288cf1 isn't present in 5.10?
>
Yes, just checked, commit 9a1ac2288cf1 was merged in 5.11-rc1, not in
5.10.0. Seems Redhat CKI doesn't treat the kernel release correctly, it
calls all kernel 5.11-rcx as 5.10.0. Sorry for the confusion, I will
send mail to them to change this.
I got the failure report from redhat's CKI test, the kernel repo is as
below, but the subject of failure report and 'uname -r' told it's
5.10.0.
Kernel repo: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Commit: 58cf05f597b0 - Merge tag 'sound-fix-5.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
2021-01-04 5:03 [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec() Hugh Dickins
` (2 preceding siblings ...)
2021-01-06 16:19 ` Chris Down
@ 2021-01-07 18:04 ` Vlastimil Babka
2021-01-09 2:23 ` Hugh Dickins
3 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2021-01-07 18:04 UTC (permalink / raw)
To: Hugh Dickins, Andrew Morton
Cc: Hui Su, Alex Shi, Lorenzo Stoakes, Michal Hocko, Johannes Weiner,
Shakeel Butt, Roman Gushchin, linux-kernel, linux-mm
On 1/4/21 6:03 AM, Hugh Dickins wrote:
> Boot a CONFIG_MEMCG=y kernel with "cgroup_disabled=memory" and you are
> met by a series of warnings from the VM_WARN_ON_ONCE_PAGE(!memcg, page)
> recently added to the inline mem_cgroup_page_lruvec().
>
> An earlier attempt to place that warning, in mem_cgroup_lruvec(), had
> been careful to do so after weeding out the mem_cgroup_disabled() case;
> but was itself invalid because of the mem_cgroup_lruvec(NULL, pgdat) in
> clear_pgdat_congested() and age_active_anon().
>
> Warning in mem_cgroup_page_lruvec() was once useful in detecting a KSM
> charge bug, so may be worth keeping: but skip if mem_cgroup_disabled().
>
> Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
> Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>
> include/linux/memcontrol.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- 5.11-rc2/include/linux/memcontrol.h 2020-12-27 20:39:36.751923135 -0800
> +++ linux/include/linux/memcontrol.h 2021-01-03 19:38:24.822978559 -0800
> @@ -665,7 +665,7 @@ static inline struct lruvec *mem_cgroup_
> {
> struct mem_cgroup *memcg = page_memcg(page);
>
> - VM_WARN_ON_ONCE_PAGE(!memcg, page);
> + VM_WARN_ON_ONCE_PAGE(!memcg && !mem_cgroup_disabled(), page);
Nit: I would reverse the order of conditions as mem_cgroup_disabled() is either
"return true" or a static key. Not that it matters too much on DEBUG_VM configs...
> return mem_cgroup_lruvec(memcg, pgdat);
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
2021-01-07 18:04 ` Vlastimil Babka
@ 2021-01-09 2:23 ` Hugh Dickins
0 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2021-01-09 2:23 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Hugh Dickins, Andrew Morton, Hui Su, Alex Shi, Lorenzo Stoakes,
Michal Hocko, Johannes Weiner, Shakeel Butt, Roman Gushchin,
Baoquan He, Chris Down, linux-kernel, linux-mm
On Thu, 7 Jan 2021, Vlastimil Babka wrote:
> On 1/4/21 6:03 AM, Hugh Dickins wrote:
> > Boot a CONFIG_MEMCG=y kernel with "cgroup_disabled=memory" and you are
> > met by a series of warnings from the VM_WARN_ON_ONCE_PAGE(!memcg, page)
> > recently added to the inline mem_cgroup_page_lruvec().
> >
> > An earlier attempt to place that warning, in mem_cgroup_lruvec(), had
> > been careful to do so after weeding out the mem_cgroup_disabled() case;
> > but was itself invalid because of the mem_cgroup_lruvec(NULL, pgdat) in
> > clear_pgdat_congested() and age_active_anon().
> >
> > Warning in mem_cgroup_page_lruvec() was once useful in detecting a KSM
> > charge bug, so may be worth keeping: but skip if mem_cgroup_disabled().
> >
> > Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
> > Signed-off-by: Hugh Dickins <hughd@google.com>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
Thanks.
>
> > ---
> >
> > include/linux/memcontrol.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- 5.11-rc2/include/linux/memcontrol.h 2020-12-27 20:39:36.751923135 -0800
> > +++ linux/include/linux/memcontrol.h 2021-01-03 19:38:24.822978559 -0800
> > @@ -665,7 +665,7 @@ static inline struct lruvec *mem_cgroup_
> > {
> > struct mem_cgroup *memcg = page_memcg(page);
> >
> > - VM_WARN_ON_ONCE_PAGE(!memcg, page);
> > + VM_WARN_ON_ONCE_PAGE(!memcg && !mem_cgroup_disabled(), page);
>
> Nit: I would reverse the order of conditions as mem_cgroup_disabled() is either
> "return true" or a static key. Not that it matters too much on DEBUG_VM configs...
tl;dr I'm going to leave the patch as is.
You are certainly right that I was forgetting the static-key-ness of
mem_cgroup_disabled() when I put the tests that way round: I was thinking
of the already-in-a-register-ness of "memcg"; but had also not realized
that page_memcg() just did an "&", so condition bits nicely set already.
And I think you are right in principle, that the tests should be better
the way you suggest, when static key is in use - in the (unusual)
mem_cgroup_disabled() case, though not in the usual enabled case.
I refuse to confess how many hours I've spent poring over "objdump -ld"s
of lock_page_lruvec_irqsave(), and comparing with how it is patched when
the kernel is booted with "cgroup_disable=memory".
But I have seen builds where my way round worked out better than yours,
for both the enabled and disabled cases (SUSE gcc 9.3.1 was good, in
the config I was trying on it); and builds where disabled was treated
rather poorly my way (with external call to mem_cgroup_disabled() from
lock_page_lruvec() and lock_page_lruvec_irqsave(), but inlined into
lock_page_lruvec_irq() - go figure! - with SUSE gcc 10.2.1).
I suspect a lot depends on what inlining is done, and on that prior
page_memcg() doing its "&", and the second mem_cgroup_disabled() which
follows immediately in mem_cgroup_lruvec(): different compilers will
make different choices, favouring one or the other ordering.
I've grown rather tired of it all (and discovered on the way that
static keys depend on CONFIG_JUMP_LABEL=y, which I didn't have in
a config I've carried forward through "make oldconfig"s for years -
thanks); but not found a decisive reason to change the patch.
Hugh
>
> > return mem_cgroup_lruvec(memcg, pgdat);
> > }
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec()
[not found] <alpine.LSU.2.11.2101032107230.1158@eggly.anvils>
@ 2021-01-04 18:09 ` Roman Gushchin
0 siblings, 0 replies; 10+ messages in thread
From: Roman Gushchin @ 2021-01-04 18:09 UTC (permalink / raw)
To: Hugh Dickins
Cc: Hui Su, Alex Shi, Lorenzo Stoakes, Michal Hocko, Johannes Weiner,
Shakeel Butt, Roman Gushchin, linux-kernel, linux-mm
> Boot a CONFIG_MEMCG=y kernel with "cgroup_disabled=memory" and you are
> met by a series of warnings from the VM_WARN_ON_ONCE_PAGE(!memcg, page)
> recently added to the inline mem_cgroup_page_lruvec().
>
> An earlier attempt to place that warning, in mem_cgroup_lruvec(), had
> been careful to do so after weeding out the mem_cgroup_disabled() case;
> but was itself invalid because of the mem_cgroup_lruvec(NULL, pgdat) in
> clear_pgdat_congested() and age_active_anon().
>
> Warning in mem_cgroup_page_lruvec() was once useful in detecting a KSM
> charge bug, so may be worth keeping: but skip if mem_cgroup_disabled().
>
> Fixes: 9a1ac2288cf1 ("mm/memcontrol:rewrite mem_cgroup_page_lruvec()")
> Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Roman Gushchin <guro@fb.com>
Thanks!
> ---
>
> include/linux/memcontrol.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- 5.11-rc2/include/linux/memcontrol.h 2020-12-27 20:39:36.751923135 -0800
> +++ linux/include/linux/memcontrol.h 2021-01-03 19:38:24.822978559 -0800
> @@ -665,7 +665,7 @@ static inline struct lruvec *mem_cgroup_
> {
> struct mem_cgroup *memcg = page_memcg(page);
>
> - VM_WARN_ON_ONCE_PAGE(!memcg, page);
> + VM_WARN_ON_ONCE_PAGE(!memcg && !mem_cgroup_disabled(), page);
> return mem_cgroup_lruvec(memcg, pgdat);
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-09 2:24 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04 5:03 [PATCH] mm/memcontrol: fix warning in mem_cgroup_page_lruvec() Hugh Dickins
2021-01-04 8:11 ` Alex Shi
2021-01-06 6:49 ` Baoquan He
2021-01-06 19:33 ` Andrew Morton
2021-01-06 19:35 ` Andrew Morton
2021-01-07 2:53 ` Baoquan He
2021-01-06 16:19 ` Chris Down
2021-01-07 18:04 ` Vlastimil Babka
2021-01-09 2:23 ` Hugh Dickins
[not found] <alpine.LSU.2.11.2101032107230.1158@eggly.anvils>
2021-01-04 18:09 ` Roman Gushchin
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).