Re: [PATCH v3 0/4] mm, swap: keep hibernation swap slots out of the swap cache
Andrew Morton <[email protected]>
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Wed, 12 Aug 2026 21:13:37 +0900 Youngjun Park <[email protected]> wrote: > On Tue, Aug 11, 2026 at 11:46:22AM -0700, Andrew Morton wrote: > > When fixing things, please always take care to describe the > > userspace-visible runtime effects of the bug, particularly when > > proposing a -stable backport. > > Thank you for the advice and sorry for it. > I will make sure future patches describe the userspace-visible effects clearly! It's common, trust me ;) > > For [1/4] Gemini tells me "At a high level, this bug triggers silent > > memory corruption, process crashes, or data instability across > > completely unrelated userspace applications - typically occurring after a > > system resumes from hibernation (suspend-to-disk)." Which is what I > > figured too. > > That is right, with one small correction. this can happen with > uswsusp while the hibernation image is being created, not at resume > time. Memory corruption, process crashes, or data instability across > completely unrelated userspace applications can occur there. OK. > > Do we have any reports of this? Reported-by/Closes? > > I found this while working on giving hibernation slots their own > marker in the swap table, which I had discussed with Kairui. > (https://lore.kernel.org/linux-mm/abp7aDgYLrxF3Me8@KASONG-MC4/) > As far as I know there are no reports, so there is no Reported-by/Closes to > add. OK. > > I'd like to grab [1/4] only, and defer the other three until 7.3-rc1. > > This might be mistaken, but from a quick read, it's not clear what > > benefit those three patches offer our users. > > Simply put, they are close to cleanups that remove unneeded work. > (with a some little optimization.) > > Patch 2 keeps bad slots out of the swap cache, so readahead no > longer wastes a folio on them. > > Patch 3 keeps hibernation slots out of the swap cache, with the same > effect. no wasted folio at readahead time. > > Patch 4 removes dead code. So I'll retain [1/4] with the below changelog. Please plan on sending out the other patches after 7.3-rc1. From: Youngjun Park <[email protected]> Subject: mm, swap: don't free a hibernation slot that is in the swap cache Date: Tue, 11 Aug 2026 22:22:06 +0900 A slot with a folio in the swap cache is freed when the folio leaves the cache, not when its count drops. swap_put_entries_cluster() follows that rule. swap_free_hibernation_slot() does not, it calls __swap_cluster_free_entries() whether or not a folio sits on the slot. Cluster readahead can put one there. It walks a raw page_cluster sized window of offsets around the faulting entry, and a hibernation slot passes __swap_cache_add_check() because it is not a folio and its count is not zero. Freeing the slot then clears the entry under that folio. The folio is now unreachable from the swap table, and the offset goes back to the allocator. The folio is still on the LRU though, so reclaim can pick it up later. It then takes the old offset out of folio->swap and overwrites the table entry there, which by then may belong to someone else. This bug can trigger silent memory corruption, process crashes, or data instability across completely unrelated userspace applications - typically occurring when uswsusp is preparing the hibernation image. I found this while working on giving hibernation slots their own marker in the swap table, which I had discussed with Kairui. (https://lore.kernel.org/linux-mm/abp7aDgYLrxF3Me8@KASONG-MC4/) As far as I know there are no reports, so there is no Reported-by/Closes to add. Check for a cached folio before freeing. The slot is then left in the ordinary state where only the swap cache holds it, and it is freed when the folio leaves the cache, either through the reclaim below or through normal reclaim later. Link: https://lore.kernel.org/[email protected] Fixes: 0d6af9bcf383 ("mm, swap: use the swap table to track the swap count") Signed-off-by: Youngjun Park <[email protected]> Acked-by: Kairui Song <[email protected]> Cc: Baoquan He <[email protected]> Cc: Barry Song <[email protected]> Cc: Chris Li <[email protected]> Cc: Kemeng Shi <[email protected]> Cc: Nhat Pham <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> --- mm/swapfile.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) --- a/mm/swapfile.c~mm-swap-dont-free-a-hibernation-slot-that-is-in-the-swap-cache +++ a/mm/swapfile.c @@ -2196,7 +2196,14 @@ void swap_free_hibernation_slot(swp_entr ci = swap_cluster_lock(si, offset); __swap_cluster_put_entry(ci, offset % SWAPFILE_CLUSTER); - __swap_cluster_free_entries(si, ci, offset % SWAPFILE_CLUSTER, 1); + /* + * A slot with a folio in the swap cache is freed when the folio + * leaves the cache, the same rule swap_put_entries_cluster() follows. + * Readahead can put a folio here, and freeing the slot now would + * leave that folio with no entry behind it. + */ + if (!swp_tb_is_folio(__swap_table_get(ci, offset % SWAPFILE_CLUSTER))) + __swap_cluster_free_entries(si, ci, offset % SWAPFILE_CLUSTER, 1); swap_cluster_unlock(ci); /* In theory readahead might add it to the swap cache by accident */ _