From: Johannes Weiner <hannes@cmpxchg.org>
To: gregkh@linuxfoundation.org
Cc: akpm@linux-foundation.org, chengming.zhou@linux.dev,
christian@heusel.eu, ddstreet@ieee.org, nphamcs@gmail.com,
rjones@redhat.com, sjenning@redhat.com, stable@vger.kernel.org,
vitaly.wool@konsulko.com, yosryahmed@google.com
Subject: [PATCH 6.8.y] mm: zswap: fix shrinker NULL crash with cgroup_disable=memory
Date: Mon, 29 Apr 2024 09:02:16 -0400 [thread overview]
Message-ID: <20240429130216.GB1155473@cmpxchg.org> (raw)
In-Reply-To: <2024042923-monday-hamlet-26ca@gregkh>
Christian reports a NULL deref in zswap that he bisected down to the zswap
shrinker. The issue also cropped up in the bug trackers of libguestfs [1]
and the Red Hat bugzilla [2].
The problem is that when memcg is disabled with the boot time flag, the
zswap shrinker might get called with sc->memcg == NULL. This is okay in
many places, like the lruvec operations. But it crashes in
memcg_page_state() - which is only used due to the non-node accounting of
cgroup's the zswap memory to begin with.
Nhat spotted that the memcg can be NULL in the memcg-disabled case, and I
was then able to reproduce the crash locally as well.
[1] https://github.com/libguestfs/libguestfs/issues/139
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2275252
Link: https://lkml.kernel.org/r/20240418124043.GC1055428@cmpxchg.org
Link: https://lkml.kernel.org/r/20240417143324.GA1055428@cmpxchg.org
Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Reported-by: Christian Heusel <christian@heusel.eu>
Debugged-by: Nhat Pham <nphamcs@gmail.com>
Suggested-by: Nhat Pham <nphamcs@gmail.com>
Tested-by: Christian Heusel <christian@heusel.eu>
Acked-by: Yosry Ahmed <yosryahmed@google.com>
Cc: Chengming Zhou <chengming.zhou@linux.dev>
Cc: Dan Streetman <ddstreet@ieee.org>
Cc: Richard W.M. Jones <rjones@redhat.com>
Cc: Seth Jennings <sjenning@redhat.com>
Cc: Vitaly Wool <vitaly.wool@konsulko.com>
Cc: <stable@vger.kernel.org> [v6.8]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 682886ec69d22363819a83ddddd5d66cb5c791e1)
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
mm/zswap.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
Two minor conflicts in the else branch:
- zswap_pool_total_size was get_zswap_pool_size() in 6.8
- zswap_nr_stored was pool->nr_stored in 6.8
diff --git a/mm/zswap.c b/mm/zswap.c
index e9c04387195f..69766f2c5a6c 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -653,15 +653,22 @@ static unsigned long zswap_shrinker_count(struct shrinker *shrinker,
if (!gfp_has_io_fs(sc->gfp_mask))
return 0;
-#ifdef CONFIG_MEMCG_KMEM
- mem_cgroup_flush_stats(memcg);
- nr_backing = memcg_page_state(memcg, MEMCG_ZSWAP_B) >> PAGE_SHIFT;
- nr_stored = memcg_page_state(memcg, MEMCG_ZSWAPPED);
-#else
- /* use pool stats instead of memcg stats */
- nr_backing = get_zswap_pool_size(pool) >> PAGE_SHIFT;
- nr_stored = atomic_read(&pool->nr_stored);
-#endif
+ /*
+ * For memcg, use the cgroup-wide ZSWAP stats since we don't
+ * have them per-node and thus per-lruvec. Careful if memcg is
+ * runtime-disabled: we can get sc->memcg == NULL, which is ok
+ * for the lruvec, but not for memcg_page_state().
+ *
+ * Without memcg, use the zswap pool-wide metrics.
+ */
+ if (!mem_cgroup_disabled()) {
+ mem_cgroup_flush_stats(memcg);
+ nr_backing = memcg_page_state(memcg, MEMCG_ZSWAP_B) >> PAGE_SHIFT;
+ nr_stored = memcg_page_state(memcg, MEMCG_ZSWAPPED);
+ } else {
+ nr_backing = get_zswap_pool_size(pool) >> PAGE_SHIFT;
+ nr_stored = atomic_read(&pool->nr_stored);
+ }
if (!nr_stored)
return 0;
--
2.44.0
next prev parent reply other threads:[~2024-04-29 13:02 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 11:25 FAILED: patch "[PATCH] mm: zswap: fix shrinker NULL crash with cgroup_disable=memory" failed to apply to 6.8-stable tree gregkh
2024-04-29 13:02 ` Johannes Weiner [this message]
2024-04-29 14:18 ` [PATCH 6.8.y] mm: zswap: fix shrinker NULL crash with cgroup_disable=memory Greg KH
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=20240429130216.GB1155473@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=akpm@linux-foundation.org \
--cc=chengming.zhou@linux.dev \
--cc=christian@heusel.eu \
--cc=ddstreet@ieee.org \
--cc=gregkh@linuxfoundation.org \
--cc=nphamcs@gmail.com \
--cc=rjones@redhat.com \
--cc=sjenning@redhat.com \
--cc=stable@vger.kernel.org \
--cc=vitaly.wool@konsulko.com \
--cc=yosryahmed@google.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 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.