From: Chengming Zhou <chengming.zhou@linux.dev>
To: Nhat Pham <nphamcs@gmail.com>, akpm@linux-foundation.org
Cc: hannes@cmpxchg.org, yosryahmed@google.com, yosry.ahmed@linux.dev,
linux-mm@kvack.org, kernel-team@meta.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] page_io: zswap: do not crash the kernel on decompression failure
Date: Fri, 7 Mar 2025 11:06:23 +0800 [thread overview]
Message-ID: <6a7c9ef6-2511-404e-b9c2-117765c90d95@linux.dev> (raw)
In-Reply-To: <20250306205011.784787-1-nphamcs@gmail.com>
On 2025/3/7 04:50, Nhat Pham wrote:
> Currently, we crash the kernel when a decompression failure occurs in
> zswap (either because of memory corruption, or a bug in the compression
> algorithm). This is overkill. We should only SIGBUS the unfortunate
> process asking for the zswap entry on zswap load, and skip the corrupted
> entry in zswap writeback.
>
> See [1] for a recent upstream discussion about this.
>
> The zswap writeback case is relatively straightforward to fix. For the
> zswap_load() case, we change the return behavior:
>
> * Return 0 on success.
> * Return -ENOENT (with the folio locked) if zswap does not own the
> swapped out content.
> * Return -EIO if zswap owns the swapped out content, but encounters a
> decompression failure for some reasons. The folio will be unlocked,
> but not be marked up-to-date, which will eventually cause the process
> requesting the page to SIGBUS (see the handling of not-up-to-date
> folio in do_swap_page() in mm/memory.c), without crashing the kernel.
> * Return -EINVAL if we encounter a large folio, as large folio should
> not be swapped in while zswap is being used. Similar to the -EIO case,
> we also unlock the folio but do not mark it as up-to-date to SIGBUS
> the faulting process.
>
> As a side effect, we require one extra zswap tree traversal in the load
> and writeback paths. Quick benchmarking on a kernel build test shows no
> performance difference:
>
> With the new scheme:
> real: mean: 125.1s, stdev: 0.12s
> user: mean: 3265.23s, stdev: 9.62s
> sys: mean: 2156.41s, stdev: 13.98s
>
> The old scheme:
> real: mean: 125.78s, stdev: 0.45s
> user: mean: 3287.18s, stdev: 5.95s
> sys: mean: 2177.08s, stdev: 26.52s
>
> [1]: https://lore.kernel.org/all/ZsiLElTykamcYZ6J@casper.infradead.org/
>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Suggested-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Nhat Pham <nphamcs@gmail.com>
Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev>
Thanks!
next prev parent reply other threads:[~2025-03-07 3:06 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-06 20:50 Nhat Pham
2025-03-06 21:32 ` Yosry Ahmed
2025-03-06 22:08 ` Nhat Pham
2025-03-06 22:24 ` [PATCH v4] page_io: zswap: do not crash the kernel on decompression failure (fix) Nhat Pham
2025-03-07 1:35 ` [PATCH v4] page_io: zswap: do not crash the kernel on decompression failure Johannes Weiner
2025-03-07 3:06 ` Chengming Zhou [this message]
2025-05-12 19:03 ` Matthew Wilcox
2025-05-12 19:49 ` Nhat Pham
2025-05-12 20:42 ` Matthew Wilcox
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=6a7c9ef6-2511-404e-b9c2-117765c90d95@linux.dev \
--to=chengming.zhou@linux.dev \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=kernel-team@meta.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nphamcs@gmail.com \
--cc=yosry.ahmed@linux.dev \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox