mm: clarify swap_count_continued and improve readability for __swap_duplicate

When usage=1 and swapcount is very large, the situation can become quite
complex.  The first entry might succeed with swap_count_continued(), but
the second entry may require extending to an additional continued page.
Rolling back these changes can be extremely challenging.  Therefore,
anyone using usage==1 for batched __swap_duplicate() operations should
proceed with caution.

Additionally, we have moved swap_count_continued() to the second iteration
to enhance readability, as this function may modify data.

Link: https://lkml.kernel.org/r/20240802071817.47081-1-21cnbao@gmail.com
Signed-off-by: Barry Song <v-songbaohua@oppo.com>
Suggested-by: "Huang, Ying" <ying.huang@intel.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Chris Li <chrisl@kernel.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Gao Xiang <xiang@kernel.org>
Cc: Hugh Dickins <hughd@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Kairui Song <kasong@tencent.com>
Cc: Kalesh Singh <kaleshsingh@google.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Nhat Pham <nphamcs@gmail.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Yang Shi <shy828301@gmail.com>
Cc: Yosry Ahmed <yosryahmed@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 757d38a86f5680b62949b553a31bef32e265241b..0259f36de715f529e9ee980c28f97ee244d4dd41 100644 (file)
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -3386,6 +3386,7 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage, int nr)
 
        offset = swp_offset(entry);
        VM_WARN_ON(nr > SWAPFILE_CLUSTER - offset % SWAPFILE_CLUSTER);
+       VM_WARN_ON(usage == 1 && nr > 1);
        ci = lock_cluster_or_swap_info(p, offset);
 
        err = 0;
@@ -3404,27 +3405,14 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage, int nr)
                has_cache = count & SWAP_HAS_CACHE;
                count &= ~SWAP_HAS_CACHE;
 
-               if (usage == SWAP_HAS_CACHE) {
-                       /* set SWAP_HAS_CACHE if there is no cache and entry is used */
-                       if (!has_cache && count)
-                               continue;
-                       else if (has_cache)             /* someone else added cache */
+               if (!count && !has_cache) {
+                       err = -ENOENT;
+               } else if (usage == SWAP_HAS_CACHE) {
+                       if (has_cache)
                                err = -EEXIST;
-                       else                            /* no users remaining */
-                               err = -ENOENT;
-
-               } else if (count || has_cache) {
-
-                       if ((count & ~COUNT_CONTINUED) < SWAP_MAP_MAX)
-                               continue;
-                       else if ((count & ~COUNT_CONTINUED) > SWAP_MAP_MAX)
-                               err = -EINVAL;
-                       else if (swap_count_continued(p, offset + i, count))
-                               continue;
-                       else
-                               err = -ENOMEM;
-               } else
-                       err = -ENOENT;                  /* unused swap entry */
+               } else if ((count & ~COUNT_CONTINUED) > SWAP_MAP_MAX) {
+                       err = -EINVAL;
+               }
 
                if (err)
                        goto unlock_out;
@@ -3439,8 +3427,16 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage, int nr)
                        has_cache = SWAP_HAS_CACHE;
                else if ((count & ~COUNT_CONTINUED) < SWAP_MAP_MAX)
                        count += usage;
-               else
+               else if (swap_count_continued(p, offset + i, count))
                        count = COUNT_CONTINUED;
+               else {
+                       /*
+                        * Don't need to rollback changes, because if
+                        * usage == 1, there must be nr == 1.
+                        */
+                       err = -ENOMEM;
+                       goto unlock_out;
+               }
 
                WRITE_ONCE(p->swap_map[offset + i], count | has_cache);
        }