mm, swap: always lock and check the swap cache folio before use

Swap cache lookup only increases the reference count of the returned
folio.  That's not enough to ensure a folio is stable in the swap cache,
so the folio could be removed from the swap cache at any time.  The caller
should always lock and check the folio before using it.

We have just documented this in kerneldoc, now introduce a helper for swap
cache folio verification with proper sanity checks.

Also, sanitize a few current users to use this convention and the new
helper for easier debugging.  They were not having observable problems
yet, only trivial issues like wasted CPU cycles on swapoff or reclaiming.
They would fail in some other way, but it is still better to always follow
this convention to make things robust and make later commits easier to do.

Link: https://lkml.kernel.org/r/20250910160833.3464-6-ryncsn@gmail.com
Signed-off-by: Kairui Song <kasong@tencent.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Barry Song <baohua@kernel.org>
Cc: Chris Li <chrisl@kernel.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: "Huang, Ying" <ying.huang@linux.alibaba.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Kemeng Shi <shikemeng@huaweicloud.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Nhat Pham <nphamcs@gmail.com>
Cc: Yosry Ahmed <yosryahmed@google.com>
Cc: Zi Yan <ziy@nvidia.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/mm/memory.c b/mm/memory.c
index 94a5928e8acea5e387af0679ccf874fce9b5a9df..5808c4ef21b330a0a05231d8a536846107f248e5 100644 (file)
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4748,8 +4748,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
                 * swapcache, we need to check that the page's swap has not
                 * changed.
                 */
-               if (unlikely(!folio_test_swapcache(folio) ||
-                            page_swap_entry(page).val != entry.val))
+               if (unlikely(!folio_matches_swap_entry(folio, entry)))
                        goto out_page;
 
                if (unlikely(PageHWPoison(page))) {

diff --git a/mm/swap.h b/mm/swap.h
index efb6d7ff9f30eaadc624b0def9909455436b54db..7d868f8de696b6f6ab25dc1cf81b725ee970955a 100644 (file)
--- a/mm/swap.h
+++ b/mm/swap.h
@@ -52,6 +52,28 @@ static inline pgoff_t swap_cache_index(swp_entry_t entry)
        return swp_offset(entry) & SWAP_ADDRESS_SPACE_MASK;
 }
 
+/**
+ * folio_matches_swap_entry - Check if a folio matches a given swap entry.
+ * @folio: The folio.
+ * @entry: The swap entry to check against.
+ *
+ * Context: The caller should have the folio locked to ensure it's stable
+ * and nothing will move it in or out of the swap cache.
+ * Return: true or false.
+ */
+static inline bool folio_matches_swap_entry(const struct folio *folio,
+                                           swp_entry_t entry)
+{
+       swp_entry_t folio_entry = folio->swap;
+       long nr_pages = folio_nr_pages(folio);
+
+       VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
+       if (!folio_test_swapcache(folio))
+               return false;
+       VM_WARN_ON_ONCE_FOLIO(!IS_ALIGNED(folio_entry.val, nr_pages), folio);
+       return folio_entry.val == round_down(entry.val, nr_pages);
+}
+
 void show_swap_cache_info(void);
 void *get_shadow_from_swap_cache(swp_entry_t entry);
 int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
@@ -144,6 +166,11 @@ static inline pgoff_t swap_cache_index(swp_entry_t entry)
        return 0;
 }
 
+static inline bool folio_matches_swap_entry(const struct folio *folio, swp_entry_t entry)
+{
+       return false;
+}
+
 static inline void show_swap_cache_info(void)
 {
 }

diff --git a/mm/swap_state.c b/mm/swap_state.c
index 68ec531d0f2b4d1531a14e627069c207b0cab38b..9225d6b695ade9826dc94a43c6931dc5ca61839b 100644 (file)
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -79,7 +79,7 @@ void show_swap_cache_info(void)
  * with reference count or locks.
  * Return: Returns the found folio on success, NULL otherwise. The caller
  * must lock and check if the folio still matches the swap entry before
- * use.
+ * use (e.g. with folio_matches_swap_entry).
  */
 struct folio *swap_cache_get_folio(swp_entry_t entry)
 {
@@ -346,7 +346,10 @@ struct folio *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
        for (;;) {
                int err;
 
-               /* Check the swap cache in case the folio is already there */
+               /*
+                * Check the swap cache first, if a cached folio is found,
+                * return it unlocked. The caller will lock and check it.
+                */
                folio = swap_cache_get_folio(entry);
                if (folio)
                        goto got_folio;

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4baebd8b48f4c281ea239d8ea03b68885f36b632..f1a4d381d719587365ee82b5ea8217f0bde0332c 100644 (file)
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -240,14 +240,12 @@ again:
         * Offset could point to the middle of a large folio, or folio
         * may no longer point to the expected offset before it's locked.
         */
-       if (offset < swp_offset(folio->swap) ||
-           offset >= swp_offset(folio->swap) + nr_pages) {
+       if (!folio_matches_swap_entry(folio, entry)) {
                folio_unlock(folio);
                folio_put(folio);
                goto again;
        }
        offset = swp_offset(folio->swap);
-
        need_reclaim = ((flags & TTRS_ANYWAY) ||
                        ((flags & TTRS_UNMAPPED) && !folio_mapped(folio)) ||
                        ((flags & TTRS_FULL) && mem_cgroup_swap_full(folio)));
@@ -2004,6 +2002,13 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
        bool hwpoisoned = false;
        int ret = 1;
 
+       /*
+        * If the folio is removed from swap cache by others, continue to
+        * unuse other PTEs. try_to_unuse may try again if we missed this one.
+        */
+       if (!folio_matches_swap_entry(folio, entry))
+               return 0;
+
        swapcache = folio;
        folio = ksm_might_need_to_copy(folio, vma, addr);
        if (unlikely(!folio))