[PATCH RFC 00/13] mm/swap: introduce priority queue to remove global cluster cache and plist

[PATCH RFC 00/13] mm/swap: introduce priority queue to remove global cluster cache and plist

[Kairui Song via B4 Relay]

Hi all,

This series introduces a new way to fairly distribute the IO to multiple
swap devices, completely gets rid of the mutable plist and global lock,
hence removes the global cluster cache and allows allocation and
rotation to happen more freely. This is especially helpful for swap
tiering or layered swap. Sending as RFC for review first as tiering
should land first, this can rebase and join later as an optimization,
and there are a few items that can still be improved based on this
series. I specifically kept the tiering and layered ideas in mind to
make the new design friendly for new ideas like these and tested a
few different appraoches locally.

As a dependency, this also cleanup and rework the locking pattern for swap
device management. All swap devices and the newly introduced percpu reader
priority queue are now guarded by a percpu rwsem for better scalability
and a more future-proof design.

Previously, each swap device had its own percpu cluster cache. To improve
performance, commit 1b7e90020eb7 ("mm, swap: use percpu cluster as
allocation fast path") moved the cluster cache to a global scope and let
every device share the same global cache, so we don't need to always
touch the plist. Fast swap allocation will just use each CPU's cluster
cache. Device rotation is still always handled by the plist when one
cluster is drained in the cluster cache, which bundled cluster allocation
with rotation. It works, but this whole design caused several
long-standing issues:

1) The global percpu cluster cache sitting above device selection
   fundamentally conflicts with concepts like swap tiering and layered
   swap allocation. We need to bypass certain swap devices, so we have
   to bypass the cache as well because the cache doesn't have stable
   ownership. That would result in very poor performance. And there
   are other problems like priority inversion and device rotation.
   These are described in more detail in the swap tier series [1].

2) The plist itself requires plist_requeue() to rotate devices for
   round-robin, which needs swap_avail_lock held. Allocators must
   cycle: lock -> pick -> unlock -> allocate -> lock -> rotate. All
   CPUs contend on the same global list head, and the design is
   neither flexible nor robust.

3) Rotation is considered the slow path. The fast / slow path design to
   avoid touching the plist has caused workarounds like commit
   9fb749cd15078 ("mm, swap: do not perform synchronous discard during
   allocation"), we can't stably grab an actual device before touching
   the plist, so the design is suboptimal and has many constraints.

4) We don't have a clear rule of how rotation works. The round-robin
   rotation is strongly bundled to the cluster allocation algorithm,
   and cluster drain is unpredictable. We will have a roughly 2M
   rotation block size but that varies from time to time.

This series gets rid of all these issues step by step:

Patches 1-4: Cleanups and infrastructure
Patches 5-7: Lock and accounting consolidation
Patch 8: Revert the global percpu cluster by YoungJun
Patch 9: This is the core change: percpu reader swap queue
Patch 10: Remove the old swap_avail_head plist (no longer needed)
Patch 11: Drop the global sync discard workaround
Patches 12-13: Drop plist entirely

The design is described in Patch 9. In general, devices of the same
priority are arranged in the same static ring. The ring itself is mostly
read only, but each CPU's reader of the ring rotates, with a counter
built into each reader. The counter is adjustable and controls the
rotation pace. The priority ring design should fit well with the current
tiering design.

The performance is looking on par with the old global cluster cache
design, while it enables more potential features like weighted
interleave or adjustable shard value for device rotation of same
priority (by simply adjusting the counter value, SWAP_ROUND_ROBIN_QUOTA).

Testing with 8 ZRAM devices, running a typical build linux kernel test,
with make -j96 in a 2G memcg (Using 8 ZRAM, might not be a very common
setup but there are such configs to avoid lock contention inside each
ZRAM device, and this also simulates having multiple high speed block
devices as SWAP), measure average system time for 12 testruns:

Before:          6633.09s
After patch  8: 12294.74s (Move the global cluster cache back to device scope)
After patch 13:  6551.27s (Introduce the percpu queue to avoid device contention)

And doing the workload in a 3G memcg, avg sys time of 12 testrun:

Before:          3312.63s
After patch  8:  6450.35s
After patch 13:  3311.38s

Using 12 brd, testing with: usemem --init-time -O -y -x -n 32 1536M
Before:         4347.58 MB/s
After patch  8: 2773.39 MB/s
After patch 13: 4345.04 MB/s

Patch 8 removed the global percpu cluster cache, so we are touching the
plist before accessing the cache, and it's very clear that the performance
is really bad. Then with the new percpu reader queue design, we got rid
of the plist, performance is on-par or better, while there is no more
slow / fast path design, and cache stays inside each device. We can
always select a device based on priority before accessing the cache.

And the shard size is adjustable through SWAP_ROUND_ROBIN_QUOTA which is
hardcoded for now to 2M, changing that effect the performance in
different ways too.

And the distributions are fair among devices during the test (similiar
with brd):

NAME       TYPE      SIZE   USED PRIO
/dev/zram0 partition   8G 692.4M   -1
/dev/zram1 partition   8G 693.2M   -1
/dev/zram2 partition   8G 694.9M   -1
/dev/zram3 partition   8G 694.1M   -1
/dev/zram4 partition   8G 695.5M   -1
/dev/zram5 partition   8G 695.1M   -1
/dev/zram6 partition   8G 694.3M   -1
/dev/zram7 partition   8G 693.6M   -1

This work was discussed several times on the mailing list [2] and
rebased and reworked to leave room for future tiering, optimization,
e.g. by assigning separate iterators per priority ring.

One patch from YoungJun is included as well [3].

Link: https://lore.kernel.org/linux-mm/20260713025644.170839-1-youngjun.park@lge.com/ [1]
Link: https://lore.kernel.org/linux-mm/CAMgjq7BhOn48xEyC=2j837R7qddfjeBVHMiRqdx8no4ZEBpBLg@mail.gmail.com/ [2]
Link: https://lore.kernel.org/linux-mm/20260126065242.1221862-5-youngjun.park@lge.com/ [3]

Signed-off-by: Kairui Song <kasong@tencent.com>
---
Kairui Song (12):
      mm/swap: remove unused parameter for reading swap header
      mm/swap: slightly cleanup the code for hibernation error handling
      mm/swap: cleanup and document swap device availability flag usage
      mm/swap: introduce swap device iteration helper
      mm/swap: change the swapon lock into a percpu rwsem
      mm/swap: remove swapon mutex and update proc reader
      mm/swap: consolidate swap inuse accounting helpers
      mm/swap: add priority queue for swap device allocation
      mm/swap: remove available list
      mm/swap: perform sync discard on single device more proactively
      mm/swap: drop swap active plist
      lib/plist.c: remove requeue function

Youngjun Park (1):
      mm/swap: change back to use each swap device's percpu cluster

 include/linux/plist.h |    2 -
 include/linux/swap.h  |   42 +-
 lib/plist.c           |   64 ---
 mm/swap.h             |   12 +-
 mm/swapfile.c         | 1165 +++++++++++++++++++++++++++++--------------------
 5 files changed, 718 insertions(+), 567 deletions(-)
---
base-commit: bdc38bfc1262e3d1432afadd2aa2ffd83d139dbb
change-id: 20260529-swap-pcp-priq-68d3d925578c

Best regards,
--  
Kairui Song <kasong@tencent.com>

[PATCH RFC 01/13] mm/swap: remove unused parameter for reading swap header

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

No feature change, just a minor cleanup.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/swapfile.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 775c385490a5..27b4d11125d6 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -3453,9 +3453,8 @@ __weak unsigned long arch_max_swapfile_size(void)
 	return generic_max_swapfile_size();
 }
 
-static unsigned long read_swap_header(struct swap_info_struct *si,
-					union swap_header *swap_header,
-					struct inode *inode)
+static unsigned long read_swap_header(union swap_header *swap_header,
+				      struct inode *inode)
 {
 	int i;
 	unsigned long maxpages;
@@ -3679,7 +3678,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	}
 	swap_header = kmap_local_folio(folio, 0);
 
-	maxpages = read_swap_header(si, swap_header, inode);
+	maxpages = read_swap_header(swap_header, inode);
 	if (unlikely(!maxpages)) {
 		error = -EINVAL;
 		goto bad_swap_unlock_inode;

-- 
2.55.0

[PATCH RFC 02/13] mm/swap: slightly cleanup the code for hibernation error handling

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

Restructure the error paths in pin_hibernation_swap_type() using a
goto out pattern to avoid repeated spin_unlock() calls and simplify
the return flow.  Also simplify unpin_hibernation_swap_type() by
making the si NULL check inline, and clean up find_first_swap() to
avoid an early return inside the loop.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/swapfile.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 27b4d11125d6..30ee75bdec40 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2247,21 +2247,18 @@ static int __find_hibernation_swap_type(dev_t device, sector_t offset)
  */
 int pin_hibernation_swap_type(dev_t device, sector_t offset)
 {
-	int type;
+	int ret;
 	struct swap_info_struct *si;
 
 	spin_lock(&swap_lock);
+	ret = __find_hibernation_swap_type(device, offset);
+	if (ret < 0)
+		goto out;
 
-	type = __find_hibernation_swap_type(device, offset);
-	if (type < 0) {
-		spin_unlock(&swap_lock);
-		return type;
-	}
-
-	si = swap_type_to_info(type);
+	si = swap_type_to_info(ret);
 	if (WARN_ON_ONCE(!si)) {
-		spin_unlock(&swap_lock);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out;
 	}
 
 	/*
@@ -2270,14 +2267,15 @@ int pin_hibernation_swap_type(dev_t device, sector_t offset)
 	 * the same session.
 	 */
 	if (WARN_ON_ONCE(si->flags & SWP_HIBERNATION)) {
-		spin_unlock(&swap_lock);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto out;
 	}
 
 	si->flags |= SWP_HIBERNATION;
 
+out:
 	spin_unlock(&swap_lock);
-	return type;
+	return ret;
 }
 
 /**
@@ -2296,11 +2294,8 @@ void unpin_hibernation_swap_type(int type)
 
 	spin_lock(&swap_lock);
 	si = swap_type_to_info(type);
-	if (!si) {
-		spin_unlock(&swap_lock);
-		return;
-	}
-	si->flags &= ~SWP_HIBERNATION;
+	if (si)
+		si->flags &= ~SWP_HIBERNATION;
 	spin_unlock(&swap_lock);
 }
 
@@ -2335,7 +2330,7 @@ int find_hibernation_swap_type(dev_t device, sector_t offset)
 
 int find_first_swap(dev_t *device)
 {
-	int type;
+	int type, ret = -ENODEV;
 
 	spin_lock(&swap_lock);
 	for (type = 0; type < nr_swapfiles; type++) {
@@ -2344,11 +2339,11 @@ int find_first_swap(dev_t *device)
 		if (!(sis->flags & SWP_WRITEOK))
 			continue;
 		*device = sis->bdev->bd_dev;
-		spin_unlock(&swap_lock);
-		return type;
+		ret = type;
+		break;
 	}
 	spin_unlock(&swap_lock);
-	return -ENODEV;
+	return ret;
 }
 
 /*

-- 
2.55.0

[PATCH RFC 03/13] mm/swap: cleanup and document swap device availability flag usage

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

Rework swap device flag and metedata handling and swapon/swapoff to
be cleaner and better documented, in preparation for locking cleanup.

Consolidate existing routines and introduce swap_device_enable and
swap_device_disable as the clean boundary of exposing or isolating
a swap device. swap_device_enable sets the proper flags and exposes
the device as an allocation candidate, while swap_device_disable
clears related flags and ensures no more allocations will happen.

And add comment blocks documenting the lifetime and locking rules for
swap device flags and their locking conventions.

No feature change, only code rearrangement and documentation.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 include/linux/swap.h |  27 +++++---
 mm/swapfile.c        | 189 +++++++++++++++++++++++----------------------------
 2 files changed, 104 insertions(+), 112 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 696ed01709c2..a8ce05024cfe 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -194,6 +194,22 @@ struct swap_extent {
 	((offsetof(union swap_header, magic.magic) - \
 	  offsetof(union swap_header, info.badpages)) / sizeof(int))
 
+/*
+ * Swap device flags, except the ones documented below, all are immutable
+ * after exposed by swap_device_enable, and until the device is freed again
+ * (SWP_USED unset). The exceptions:
+ * - SWP_USED: Protected by swap_lock. Indicates the device is inuse. Once
+ *   set, won't be cleared unless all reference to this device is freed and
+ *   swapoff finished.
+ * - SWP_WRITEOK: Protected by both swap_lock and swap_avail_lock, clearing
+ *   this flag also waits for all current cluster lock users to exit so
+ *   checking this flag while holding any of these locks ensures the device
+ *   is safe to use at the moment. Note: clearing this flag doesn't affect
+ *   pending IO or async requests, it only prevents further entry allocation
+ *   or new async request (e.g. discard) from initiating.
+ * - SWP_HIBERNATION: Protected by swap_lock. Indicates if the device
+ *   is pinned for hibernation.
+ */
 enum {
 	SWP_USED	= (1 << 0),	/* is slot in swap_info[] used? */
 	SWP_WRITEOK	= (1 << 1),	/* ok to write to this swap?	*/
@@ -263,14 +279,9 @@ struct swap_info_struct {
 	struct file *swap_file;		/* seldom referenced */
 	struct completion comp;		/* seldom referenced */
 	spinlock_t lock;		/*
-					 * protect map scan related fields like
-					 * inuse_pages and all cluster lists.
-					 * Other fields are only changed
-					 * at swapon/swapoff, so are protected
-					 * by swap_lock. changing flags need
-					 * hold this lock and swap_lock. If
-					 * both locks need hold, hold swap_lock
-					 * first.
+					 * Protect cluster lists. Other fields
+					 * are only changed at swapon/swapoff,
+					 * so are protected by swap_lock.
 					 */
 	struct work_struct discard_work; /* discard worker */
 	struct work_struct reclaim_work; /* reclaim worker */
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 30ee75bdec40..a4701692d330 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1193,28 +1193,20 @@ static void del_from_avail_list(struct swap_info_struct *si, bool swapoff)
 
 	spin_lock(&swap_avail_lock);
 
-	if (swapoff) {
-		/*
-		 * Forcefully remove it. Clear the SWP_WRITEOK flags for
-		 * swapoff here so it's synchronized by both si->lock and
-		 * swap_avail_lock, to ensure the result can be seen by
-		 * add_to_avail_list.
-		 */
-		lockdep_assert_held(&si->lock);
-		si->flags &= ~SWP_WRITEOK;
-		atomic_long_or(SWAP_USAGE_OFFLIST_BIT, &si->inuse_pages);
-	} else {
-		/*
-		 * If not called by swapoff, take it off-list only if it's
-		 * full and SWAP_USAGE_OFFLIST_BIT is not set (strictly
-		 * si->inuse_pages == pages), any concurrent slot freeing,
-		 * or device already removed from plist by someone else
-		 * will make this return false.
-		 */
+	/*
+	 * Force remove it only for swapoff. Else, take it off-list only if
+	 * it's full and SWAP_USAGE_OFFLIST_BIT is not set (strictly
+	 * si->inuse_pages == pages), so concurrent slot freeing, or
+	 * concurrent list removal will make the cmpxchg fail and skip
+	 * the removal.
+	 */
+	if (!swapoff) {
 		pages = si->pages;
 		if (!atomic_long_try_cmpxchg(&si->inuse_pages, &pages,
 					     pages | SWAP_USAGE_OFFLIST_BIT))
 			goto skip;
+	} else {
+		atomic_long_or(SWAP_USAGE_OFFLIST_BIT, &si->inuse_pages);
 	}
 
 	plist_del(&si->avail_list, &swap_avail_head);
@@ -1224,21 +1216,21 @@ static void del_from_avail_list(struct swap_info_struct *si, bool swapoff)
 }
 
 /* SWAP_USAGE_OFFLIST_BIT can only be cleared by this helper. */
-static void add_to_avail_list(struct swap_info_struct *si, bool swapon)
+static void add_to_avail_list(struct swap_info_struct *si)
 {
 	long val;
 	unsigned long pages;
 
 	spin_lock(&swap_avail_lock);
 
-	/* Corresponding to SWP_WRITEOK clearing in del_from_avail_list */
-	if (swapon) {
-		lockdep_assert_held(&si->lock);
-		si->flags |= SWP_WRITEOK;
-	} else {
-		if (!(READ_ONCE(si->flags) & SWP_WRITEOK))
-			goto skip;
-	}
+	/*
+	 * Add the device to the avail list if SWP_WRITEOK is set and
+	 * SWAP_USAGE_OFFLIST_BIT is still set. Swapoff clears
+	 * SWP_WRITEOK first, so the device won't be re-added after
+	 * swapoff starts unless swap_device_enable resurrects it.
+	 */
+	if (!(si->flags & SWP_WRITEOK))
+		goto skip;
 
 	if (!(atomic_long_read(&si->inuse_pages) & SWAP_USAGE_OFFLIST_BIT))
 		goto skip;
@@ -1294,7 +1286,7 @@ static void swap_usage_sub(struct swap_info_struct *si, unsigned int nr_entries)
 	 * add it to the plist.
 	 */
 	if (unlikely(val & SWAP_USAGE_OFFLIST_BIT))
-		add_to_avail_list(si, false);
+		add_to_avail_list(si);
 }
 
 static void swap_range_alloc(struct swap_info_struct *si,
@@ -1348,7 +1340,7 @@ static bool get_swap_device_info(struct swap_info_struct *si)
 	 * up to dated.
 	 *
 	 * Paired with the spin_unlock() after setup_swap_info() in
-	 * enable_swap_info(), and smp_wmb() in swapoff.
+	 * swap_device_enable(), and smp_wmb() in swapoff.
 	 */
 	smp_rmb();
 	return true;
@@ -2963,58 +2955,71 @@ static int setup_swap_extents(struct swap_info_struct *sis,
 	return generic_swapfile_activate(sis, swap_file, span);
 }
 
-static void _enable_swap_info(struct swap_info_struct *si)
-{
-	atomic_long_add(si->pages, &nr_swap_pages);
-	total_swap_pages += si->pages;
-
-	assert_spin_locked(&swap_lock);
-
-	plist_add(&si->list, &swap_active_head);
-
-	/* Add back to available list */
-	add_to_avail_list(si, true);
-}
-
 /*
- * Called after the swap device is ready, resurrect its percpu ref, it's now
- * safe to reference it. Add it to the list to expose it to the allocator.
+ * Called after the swap device is ready to be used. Marking it writable and
+ * exposing it to the allocator. Resurrect its percpu ref if it was dead before.
  */
-static void enable_swap_info(struct swap_info_struct *si)
+static void swap_device_enable(struct swap_info_struct *si)
 {
-	percpu_ref_resurrect(&si->users);
 	spin_lock(&swap_lock);
-	spin_lock(&si->lock);
-	_enable_swap_info(si);
-	spin_unlock(&si->lock);
-	spin_unlock(&swap_lock);
-}
 
-static void reinsert_swap_info(struct swap_info_struct *si)
-{
-	spin_lock(&swap_lock);
-	spin_lock(&si->lock);
-	_enable_swap_info(si);
-	spin_unlock(&si->lock);
+	spin_lock(&swap_avail_lock);
+	si->flags |= SWP_WRITEOK;
+	spin_unlock(&swap_avail_lock);
+
+	atomic_long_add(si->pages, &nr_swap_pages);
+	total_swap_pages += si->pages;
+	plist_add(&si->list, &swap_active_head);
 	spin_unlock(&swap_lock);
+
+	add_to_avail_list(si);
 }
 
-/*
- * Called after clearing SWP_WRITEOK, ensures cluster_alloc_range
- * see the updated flags, so there will be no more allocations.
- */
-static void wait_for_allocation(struct swap_info_struct *si)
+static int swap_device_disable(struct swap_info_struct *si)
 {
 	unsigned long offset;
 	unsigned long end = ALIGN(si->max, SWAPFILE_CLUSTER);
 	struct swap_cluster_info *ci;
 
-	BUG_ON(si->flags & SWP_WRITEOK);
+	/*
+	 * If SWP_WRITEOK is not set: another process already disabling it.
+	 * If SWP_HIBERNATION is set: the device is pinned for hibernation.
+	 */
+	spin_lock(&swap_lock);
+	if (!(si->flags & SWP_WRITEOK) ||
+	    si->flags & SWP_HIBERNATION) {
+		spin_unlock(&swap_lock);
+		return -EBUSY;
+	}
 
+	if (security_vm_enough_memory_mm(current->mm, si->pages)) {
+		spin_unlock(&swap_lock);
+		return -ENOMEM;
+	}
+	vm_unacct_memory(si->pages);
+
+	spin_lock(&swap_avail_lock);
+	si->flags &= ~SWP_WRITEOK;
+	spin_unlock(&swap_avail_lock);
+
+	plist_del(&si->list, &swap_active_head);
+	total_swap_pages -= si->pages;
+	atomic_long_sub(si->pages, &nr_swap_pages);
+	spin_unlock(&swap_lock);
+
+	del_from_avail_list(si, true);
+
+	/*
+	 * Swap allocator doesn't touch si lock, so looping through all
+	 * ci locks ensures __swap_cluster_alloc_entries sees the
+	 * updated flags, and no more allocations will occur.
+	 */
 	for (offset = 0; offset < end; offset += SWAPFILE_CLUSTER) {
 		ci = swap_cluster_lock(si, offset);
 		swap_cluster_unlock(ci);
 	}
+
+	return 0;
 }
 
 static void free_swap_cluster_info(struct swap_cluster_info *cluster_info,
@@ -3059,7 +3064,6 @@ static void flush_percpu_swap_cluster(struct swap_info_struct *si)
 	}
 }
 
-
 SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 {
 	struct swap_info_struct *p = NULL;
@@ -3083,42 +3087,21 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	mapping = victim->f_mapping;
 	spin_lock(&swap_lock);
 	plist_for_each_entry(p, &swap_active_head, list) {
-		if (p->flags & SWP_WRITEOK) {
-			if (p->swap_file->f_mapping == mapping) {
-				found = 1;
-				break;
-			}
+		if (p->flags & SWP_WRITEOK &&
+		    p->swap_file->f_mapping == mapping) {
+			found = 1;
+			break;
 		}
 	}
-	if (!found) {
-		err = -EINVAL;
-		spin_unlock(&swap_lock);
-		goto out_dput;
-	}
-
-	/* Refuse swapoff while the device is pinned for hibernation */
-	if (p->flags & SWP_HIBERNATION) {
-		err = -EBUSY;
-		spin_unlock(&swap_lock);
-		goto out_dput;
-	}
-
-	if (!security_vm_enough_memory_mm(current->mm, p->pages))
-		vm_unacct_memory(p->pages);
-	else {
-		err = -ENOMEM;
-		spin_unlock(&swap_lock);
-		goto out_dput;
-	}
-	spin_lock(&p->lock);
-	del_from_avail_list(p, true);
-	plist_del(&p->list, &swap_active_head);
-	atomic_long_sub(p->pages, &nr_swap_pages);
-	total_swap_pages -= p->pages;
-	spin_unlock(&p->lock);
 	spin_unlock(&swap_lock);
+	filp_close(victim, NULL);
+
+	if (!found)
+		return -EINVAL;
 
-	wait_for_allocation(p);
+	err = swap_device_disable(p);
+	if (err)
+		return err;
 
 	set_current_oom_origin();
 	err = try_to_unuse(p->type);
@@ -3126,8 +3109,8 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 
 	if (err) {
 		/* re-insert swap space back into swap_list */
-		reinsert_swap_info(p);
-		goto out_dput;
+		swap_device_enable(p);
+		return err;
 	}
 
 	/*
@@ -3187,13 +3170,10 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	p->flags = 0;
 	spin_unlock(&swap_lock);
 
-	err = 0;
 	atomic_inc(&proc_poll_event);
 	wake_up_interruptible(&proc_poll_wait);
 
-out_dput:
-	filp_close(victim, NULL);
-	return err;
+	return 0;
 }
 
 #ifdef CONFIG_PROC_FS
@@ -3615,7 +3595,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	/*
 	 * Allocate or reuse existing !SWP_USED swap_info. The returned
 	 * si will stay in a dying status, so nothing will access its content
-	 * until enable_swap_info resurrects its percpu ref and expose it.
+	 * until swap_device_enable resurrects its percpu ref and expose it.
 	 */
 	si = alloc_swap_info();
 	if (IS_ERR(si))
@@ -3773,7 +3753,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	si->swap_file = swap_file;
 
 	/* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
-	enable_swap_info(si);
+	percpu_ref_resurrect(&si->users);
+	swap_device_enable(si);
 
 	pr_info("Adding %uk swap on %s.  Priority:%d extents:%d across:%lluk %s%s%s%s\n",
 		K(si->pages), name->name, si->prio, nr_extents,

-- 
2.55.0

[PATCH RFC 04/13] mm/swap: introduce swap device iteration helper

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

There are many users that access the swap info array just to iterate
through the swap devices to find usable ones. Introduce a generic helper
for this to prepare for dropping the lock convention.

Also slightly adjust __swap_type_to_info to allow access of swapped off
devices by moving the sanity check into its only current caller, and this
way makes more sense too: the only caller is __swap_entry_to_info, where
we should never see an entry referring to a dead device.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/swap.h     | 12 +++++-----
 mm/swapfile.c | 77 ++++++++++++++++++++++++++++++++++++-----------------------
 2 files changed, 53 insertions(+), 36 deletions(-)

diff --git a/mm/swap.h b/mm/swap.h
index b51ad3071a73..08a782f3d5ac 100644
--- a/mm/swap.h
+++ b/mm/swap.h
@@ -91,16 +91,16 @@ static inline unsigned int swp_cluster_offset(swp_entry_t entry)
  */
 static inline struct swap_info_struct *__swap_type_to_info(int type)
 {
-	struct swap_info_struct *si;
-
-	si = READ_ONCE(swap_info[type]); /* rcu_dereference() */
-	VM_WARN_ON_ONCE(percpu_ref_is_zero(&si->users)); /* race with swapoff */
-	return si;
+	return READ_ONCE(swap_info[type]); /* rcu_dereference() */
 }
 
 static inline struct swap_info_struct *__swap_entry_to_info(swp_entry_t entry)
 {
-	return __swap_type_to_info(swp_type(entry));
+	struct swap_info_struct *si;
+
+	si = __swap_type_to_info(swp_type(entry)); /* rcu_dereference() */
+	VM_WARN_ON_ONCE(percpu_ref_is_zero(&si->users)); /* race with swapoff */
+	return si;
 }
 
 static inline struct swap_cluster_info *__swap_offset_to_cluster(
diff --git a/mm/swapfile.c b/mm/swapfile.c
index a4701692d330..8faa4e7c32bf 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -106,6 +106,34 @@ static DEFINE_SPINLOCK(swap_avail_lock);
 
 struct swap_info_struct *swap_info[MAX_SWAPFILES];
 
+static inline struct swap_info_struct *__swap_iter(int *i, unsigned long flag)
+{
+	lockdep_assert_held(&swap_lock);
+	while (*i < nr_swapfiles) {
+		struct swap_info_struct *si = __swap_type_to_info(*i);
+
+		VM_WARN_ON(!si);
+		(*i)++;
+		if (flag && !((si->flags & flag) == flag))
+			continue;
+		return si;
+	}
+	return NULL;
+}
+
+#define __for_each_swap(si, flag)			\
+	for (int __i = 0; ((si) = __swap_iter(&__i, flag));)
+
+/*
+ * for_each_swap - iterate through all allocated and inuse swap devices
+ * @si: the iterator
+ *
+ * Context: The caller must hold swap_lock. The lock may be dropped during
+ * the loop body but must be re-acquired before the next iteration.
+ */
+#define for_each_swap(si) __for_each_swap(si, SWP_USED)
+#define for_each_avail_swap(si) __for_each_swap(si, SWP_USED | SWP_WRITEOK)
+
 static struct kmem_cache *swap_table_cachep;
 
 /* Protects si->swap_file for /proc/swaps usage */
@@ -2197,26 +2225,18 @@ void swap_free_hibernation_slot(swp_entry_t entry)
 
 static int __find_hibernation_swap_type(dev_t device, sector_t offset)
 {
-	int type;
-
-	lockdep_assert_held(&swap_lock);
+	struct swap_info_struct *si;
 
 	if (!device)
 		return -EINVAL;
 
-	for (type = 0; type < nr_swapfiles; type++) {
-		struct swap_info_struct *sis = swap_info[type];
-
-		if (!(sis->flags & SWP_WRITEOK))
-			continue;
-
-		if (device == sis->bdev->bd_dev) {
-			struct swap_extent *se = first_se(sis);
-
-			if (se->start_block == offset)
-				return type;
+	for_each_avail_swap(si) {
+		if (device == si->bdev->bd_dev) {
+			if (first_se(si)->start_block == offset)
+				return si->type;
 		}
 	}
+
 	return -ENODEV;
 }
 
@@ -2322,16 +2342,13 @@ int find_hibernation_swap_type(dev_t device, sector_t offset)
 
 int find_first_swap(dev_t *device)
 {
-	int type, ret = -ENODEV;
+	int ret = -ENODEV;
+	struct swap_info_struct *si;
 
 	spin_lock(&swap_lock);
-	for (type = 0; type < nr_swapfiles; type++) {
-		struct swap_info_struct *sis = swap_info[type];
-
-		if (!(sis->flags & SWP_WRITEOK))
-			continue;
-		*device = sis->bdev->bd_dev;
-		ret = type;
+	for_each_avail_swap(si) {
+		*device = si->bdev->bd_dev;
+		ret = si->type;
 		break;
 	}
 	spin_unlock(&swap_lock);
@@ -2817,12 +2834,14 @@ static int try_to_unuse(unsigned int type)
  */
 static void drain_mmlist(void)
 {
+	struct swap_info_struct *si;
 	struct list_head *p, *next;
-	unsigned int type;
 
-	for (type = 0; type < nr_swapfiles; type++)
-		if (swap_usage_in_pages(swap_info[type]))
+	for_each_swap(si) {
+		if (swap_usage_in_pages(si))
 			return;
+	}
+
 	spin_lock(&mmlist_lock);
 	list_for_each_safe(p, next, &init_mm.mmlist)
 		list_del_init(p);
@@ -3802,14 +3821,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 
 void si_swapinfo(struct sysinfo *val)
 {
-	unsigned int type;
+	struct swap_info_struct *si;
 	unsigned long nr_to_be_unused = 0;
 
 	spin_lock(&swap_lock);
-	for (type = 0; type < nr_swapfiles; type++) {
-		struct swap_info_struct *si = swap_info[type];
-
-		if ((si->flags & SWP_USED) && !(si->flags & SWP_WRITEOK))
+	for_each_swap(si) {
+		if (!(si->flags & SWP_WRITEOK))
 			nr_to_be_unused += swap_usage_in_pages(si);
 	}
 	val->freeswap = atomic_long_read(&nr_swap_pages) + nr_to_be_unused;

-- 
2.55.0

[PATCH RFC 05/13] mm/swap: change the swapon lock into a percpu rwsem

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

Reading swap metadata is a common hot path, while swapon and swapoff
are costly and very rare.  Convert the existing swap_lock spinlock
into a percpu rwsem so that readers can run concurrently without
contention.  This is a first step toward cleaning up the swap lock
model and we might already see a performance gain for existing
readers.

Also update the comments in swap_info_struct to reflect the lock
name change.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 include/linux/swap.h |  10 ++---
 mm/swapfile.c        | 121 +++++++++++++++++++++++++++------------------------
 2 files changed, 69 insertions(+), 62 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index a8ce05024cfe..d0c4e0ab5806 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -197,17 +197,17 @@ struct swap_extent {
 /*
  * Swap device flags, except the ones documented below, all are immutable
  * after exposed by swap_device_enable, and until the device is freed again
- * (SWP_USED unset). The exceptions:
- * - SWP_USED: Protected by swap_lock. Indicates the device is inuse. Once
+ * (SWP_USED unset). The exceptions are all protected by swapon_rwsem:
+ * - SWP_USED: Protected by swapon_rwsem. Indicates the device is inuse. Once
  *   set, won't be cleared unless all reference to this device is freed and
  *   swapoff finished.
- * - SWP_WRITEOK: Protected by both swap_lock and swap_avail_lock, clearing
+ * - SWP_WRITEOK: Protected by both swapon_rwsem and swap_avail_lock, clearing
  *   this flag also waits for all current cluster lock users to exit so
  *   checking this flag while holding any of these locks ensures the device
  *   is safe to use at the moment. Note: clearing this flag doesn't affect
  *   pending IO or async requests, it only prevents further entry allocation
  *   or new async request (e.g. discard) from initiating.
- * - SWP_HIBERNATION: Protected by swap_lock. Indicates if the device
+ * - SWP_HIBERNATION: Protected by swapon_rwsem. Indicates if the device
  *   is pinned for hibernation.
  */
 enum {
@@ -281,7 +281,7 @@ struct swap_info_struct {
 	spinlock_t lock;		/*
 					 * Protect cluster lists. Other fields
 					 * are only changed at swapon/swapoff,
-					 * so are protected by swap_lock.
+					 * so are protected by swapon_rwsem.
 					 */
 	struct work_struct discard_work; /* discard worker */
 	struct work_struct reclaim_work; /* reclaim worker */
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 8faa4e7c32bf..d75fad161ba5 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -57,23 +57,29 @@ static void move_cluster(struct swap_info_struct *si,
 			 enum swap_cluster_flags new_flags);
 
 /*
- * Protects the swap_info array, and the SWP_USED flag. swap_info contains
- * lazily allocated & freed swap device info struts, and SWP_USED indicates
- * which device is used, ~SWP_USED devices and can be reused.
- *
- * Also protects swap_active_head total_swap_pages, and the SWP_WRITEOK flag.
+ * Serializes swapon/swapoff (writers) and protects the swap_info
+ * array, nr_swapfiles, total_swap_pages, and part of swap device
+ * info content (see comment of swap_info_struct).  Readers
+ * (allocation, /proc/swaps, etc.) take percpu_down_read() which
+ * is cheap as the hot path.
+ */
+DEFINE_STATIC_PERCPU_RWSEM(swapon_rwsem);
+/*
+ * swap_info contains lazily allocated swap device info structs, and
+ * SWP_USED indicates which device is used, ~SWP_USED devices can be
+ * reused. Protected by swapon_rwsem, but reading could be lockless.
  */
-static DEFINE_SPINLOCK(swap_lock);
+struct swap_info_struct *swap_info[MAX_SWAPFILES];
 static unsigned int nr_swapfiles;
-atomic_long_t nr_swap_pages;
+long total_swap_pages;
+
 /*
  * Some modules use swappable objects and may try to swap them out under
  * memory pressure (via the shrinker). Before doing so, they may wish to
  * check to see if any swap space is available.
  */
+atomic_long_t nr_swap_pages;
 EXPORT_SYMBOL_GPL(nr_swap_pages);
-/* protected with swap_lock. reading in vm_swap_full() doesn't need lock */
-long total_swap_pages;
 #define DEF_SWAP_PRIO  -1
 unsigned long swapfile_maximum_size;
 #ifdef CONFIG_MIGRATION
@@ -85,7 +91,7 @@ static const char Bad_offset[] = "Bad swap offset entry ";
 
 /*
  * all active swap_info_structs
- * protected with swap_lock, and ordered by priority.
+ * protected with swapon_rwsem, and ordered by priority.
  */
 static PLIST_HEAD(swap_active_head);
 
@@ -95,20 +101,18 @@ static PLIST_HEAD(swap_active_head);
  * This is used by folio_alloc_swap() instead of swap_active_head
  * because swap_active_head includes all swap_info_structs,
  * but folio_alloc_swap() doesn't need to look at full ones.
- * This uses its own lock instead of swap_lock because when a
+ * This uses its own lock instead of swapon_rwsem because when a
  * swap_info_struct changes between not-full/full, it needs to
  * add/remove itself to/from this list, but the swap_info_struct->lock
- * is held and the locking order requires swap_lock to be taken
+ * is held and the locking order requires swapon_rwsem to be taken
  * before any swap_info_struct->lock.
  */
 static PLIST_HEAD(swap_avail_head);
 static DEFINE_SPINLOCK(swap_avail_lock);
 
-struct swap_info_struct *swap_info[MAX_SWAPFILES];
-
 static inline struct swap_info_struct *__swap_iter(int *i, unsigned long flag)
 {
-	lockdep_assert_held(&swap_lock);
+	lockdep_assert_held(&swapon_rwsem);
 	while (*i < nr_swapfiles) {
 		struct swap_info_struct *si = __swap_type_to_info(*i);
 
@@ -128,7 +132,7 @@ static inline struct swap_info_struct *__swap_iter(int *i, unsigned long flag)
  * for_each_swap - iterate through all allocated and inuse swap devices
  * @si: the iterator
  *
- * Context: The caller must hold swap_lock. The lock may be dropped during
+ * Context: The caller must hold swapon_rwsem. The lock may be dropped during
  * the loop body but must be re-acquired before the next iteration.
  */
 #define for_each_swap(si) __for_each_swap(si, SWP_USED)
@@ -1365,10 +1369,10 @@ static bool get_swap_device_info(struct swap_info_struct *si)
 	/*
 	 * Guarantee the si->users are checked before accessing other
 	 * fields of swap_info_struct, and si->flags (SWP_WRITEOK) is
-	 * up to dated.
+	 * up to date.
 	 *
-	 * Paired with the spin_unlock() after setup_swap_info() in
-	 * swap_device_enable(), and smp_wmb() in swapoff.
+	 * Paired with percpu_up_write() in swap_device_enable(), and
+	 * smp_wmb() after clearing SWP_WRITEOK in swapoff.
 	 */
 	smp_rmb();
 	return true;
@@ -1453,10 +1457,10 @@ static bool swap_sync_discard(void)
 	bool ret = false;
 	struct swap_info_struct *si, *next;
 
-	spin_lock(&swap_lock);
+	percpu_down_read(&swapon_rwsem);
 start_over:
 	plist_for_each_entry_safe(si, next, &swap_active_head, list) {
-		spin_unlock(&swap_lock);
+		percpu_up_read(&swapon_rwsem);
 		if (get_swap_device_info(si)) {
 			if (si->flags & SWP_PAGE_DISCARD)
 				ret = swap_do_scheduled_discard(si);
@@ -1465,11 +1469,11 @@ static bool swap_sync_discard(void)
 		if (ret)
 			return true;
 
-		spin_lock(&swap_lock);
+		percpu_down_read(&swapon_rwsem);
 		if (plist_node_empty(&next->list))
 			goto start_over;
 	}
-	spin_unlock(&swap_lock);
+	percpu_up_read(&swapon_rwsem);
 
 	return false;
 }
@@ -2262,7 +2266,7 @@ int pin_hibernation_swap_type(dev_t device, sector_t offset)
 	int ret;
 	struct swap_info_struct *si;
 
-	spin_lock(&swap_lock);
+	percpu_down_write(&swapon_rwsem);
 	ret = __find_hibernation_swap_type(device, offset);
 	if (ret < 0)
 		goto out;
@@ -2286,7 +2290,7 @@ int pin_hibernation_swap_type(dev_t device, sector_t offset)
 	si->flags |= SWP_HIBERNATION;
 
 out:
-	spin_unlock(&swap_lock);
+	percpu_up_write(&swapon_rwsem);
 	return ret;
 }
 
@@ -2304,11 +2308,11 @@ void unpin_hibernation_swap_type(int type)
 {
 	struct swap_info_struct *si;
 
-	spin_lock(&swap_lock);
+	percpu_down_write(&swapon_rwsem);
 	si = swap_type_to_info(type);
 	if (si)
 		si->flags &= ~SWP_HIBERNATION;
-	spin_unlock(&swap_lock);
+	percpu_up_write(&swapon_rwsem);
 }
 
 /**
@@ -2333,9 +2337,9 @@ int find_hibernation_swap_type(dev_t device, sector_t offset)
 {
 	int type;
 
-	spin_lock(&swap_lock);
+	percpu_down_read(&swapon_rwsem);
 	type = __find_hibernation_swap_type(device, offset);
-	spin_unlock(&swap_lock);
+	percpu_up_read(&swapon_rwsem);
 
 	return type;
 }
@@ -2345,13 +2349,13 @@ int find_first_swap(dev_t *device)
 	int ret = -ENODEV;
 	struct swap_info_struct *si;
 
-	spin_lock(&swap_lock);
+	percpu_down_read(&swapon_rwsem);
 	for_each_avail_swap(si) {
 		*device = si->bdev->bd_dev;
 		ret = si->type;
 		break;
 	}
-	spin_unlock(&swap_lock);
+	percpu_up_read(&swapon_rwsem);
 	return ret;
 }
 
@@ -2380,7 +2384,7 @@ unsigned int count_swap_pages(int type, int free)
 {
 	unsigned int n = 0;
 
-	spin_lock(&swap_lock);
+	percpu_down_read(&swapon_rwsem);
 	if ((unsigned int)type < nr_swapfiles) {
 		struct swap_info_struct *sis = swap_info[type];
 
@@ -2392,7 +2396,7 @@ unsigned int count_swap_pages(int type, int free)
 		}
 		spin_unlock(&sis->lock);
 	}
-	spin_unlock(&swap_lock);
+	percpu_up_read(&swapon_rwsem);
 	return n;
 }
 #endif /* CONFIG_HIBERNATION */
@@ -2704,10 +2708,10 @@ static unsigned int find_next_to_unuse(struct swap_info_struct *si,
 	unsigned long swp_tb;
 
 	/*
-	 * No need for swap_lock here: we're just looking
+	 * No need for swapon_rwsem here: we're just looking
 	 * for whether an entry is in use, not modifying it; false
 	 * hits are okay, and sys_swapoff() has already prevented new
-	 * allocations from this area (while holding swap_lock).
+	 * allocations from this area (while holding swapon_rwsem).
 	 */
 	for (i = prev + 1; i < si->max; i++) {
 		swp_tb = swap_table_get(__swap_offset_to_cluster(si, i),
@@ -2828,8 +2832,8 @@ static int try_to_unuse(unsigned int type)
 
 /*
  * After a successful try_to_unuse, if no swap is now in use, we know
- * we can empty the mmlist.  swap_lock must be held on entry and exit.
- * Note that mmlist_lock nests inside swap_lock, and an mm must be
+ * we can empty the mmlist. swapon_rwsem must be held on entry and exit.
+ * Note that mmlist_lock nests inside swapon_rwsem, and an mm must be
  * added to the mmlist just after page_duplicate - before would be racy.
  */
 static void drain_mmlist(void)
@@ -2980,8 +2984,7 @@ static int setup_swap_extents(struct swap_info_struct *sis,
  */
 static void swap_device_enable(struct swap_info_struct *si)
 {
-	spin_lock(&swap_lock);
-
+	percpu_down_write(&swapon_rwsem);
 	spin_lock(&swap_avail_lock);
 	si->flags |= SWP_WRITEOK;
 	spin_unlock(&swap_avail_lock);
@@ -2989,7 +2992,7 @@ static void swap_device_enable(struct swap_info_struct *si)
 	atomic_long_add(si->pages, &nr_swap_pages);
 	total_swap_pages += si->pages;
 	plist_add(&si->list, &swap_active_head);
-	spin_unlock(&swap_lock);
+	percpu_up_write(&swapon_rwsem);
 
 	add_to_avail_list(si);
 }
@@ -3004,15 +3007,15 @@ static int swap_device_disable(struct swap_info_struct *si)
 	 * If SWP_WRITEOK is not set: another process already disabling it.
 	 * If SWP_HIBERNATION is set: the device is pinned for hibernation.
 	 */
-	spin_lock(&swap_lock);
+	percpu_down_write(&swapon_rwsem);
 	if (!(si->flags & SWP_WRITEOK) ||
 	    si->flags & SWP_HIBERNATION) {
-		spin_unlock(&swap_lock);
+		percpu_up_write(&swapon_rwsem);
 		return -EBUSY;
 	}
 
 	if (security_vm_enough_memory_mm(current->mm, si->pages)) {
-		spin_unlock(&swap_lock);
+		percpu_up_write(&swapon_rwsem);
 		return -ENOMEM;
 	}
 	vm_unacct_memory(si->pages);
@@ -3024,7 +3027,7 @@ static int swap_device_disable(struct swap_info_struct *si)
 	plist_del(&si->list, &swap_active_head);
 	total_swap_pages -= si->pages;
 	atomic_long_sub(si->pages, &nr_swap_pages);
-	spin_unlock(&swap_lock);
+	percpu_up_write(&swapon_rwsem);
 
 	del_from_avail_list(si, true);
 
@@ -3104,7 +3107,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 		return PTR_ERR(victim);
 
 	mapping = victim->f_mapping;
-	spin_lock(&swap_lock);
+	percpu_down_read(&swapon_rwsem);
 	plist_for_each_entry(p, &swap_active_head, list) {
 		if (p->flags & SWP_WRITEOK &&
 		    p->swap_file->f_mapping == mapping) {
@@ -3112,7 +3115,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 			break;
 		}
 	}
-	spin_unlock(&swap_lock);
+	percpu_up_read(&swapon_rwsem);
 	filp_close(victim, NULL);
 
 	if (!found)
@@ -3154,7 +3157,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 		atomic_dec(&nr_rotate_swap);
 
 	mutex_lock(&swapon_mutex);
-	spin_lock(&swap_lock);
+	percpu_down_write(&swapon_rwsem);
 	spin_lock(&p->lock);
 	drain_mmlist();
 
@@ -3165,7 +3168,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	p->max = 0;
 	p->cluster_info = NULL;
 	spin_unlock(&p->lock);
-	spin_unlock(&swap_lock);
+	percpu_up_write(&swapon_rwsem);
 	arch_swap_invalidate_area(p->type);
 	zswap_swapoff(p->type);
 	mutex_unlock(&swapon_mutex);
@@ -3184,10 +3187,14 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	 * Clear the SWP_USED flag after all resources are freed so that swapon
 	 * can reuse this swap_info in alloc_swap_info() safely.  It is ok to
 	 * not hold p->lock after we cleared its SWP_WRITEOK.
+	 *
+	 * The write lock ensures the flag clear is visible to lockless
+	 * readers of swap_type_to_info() before alloc_swap_info() reuses
+	 * this slot.
 	 */
-	spin_lock(&swap_lock);
+	percpu_down_write(&swapon_rwsem);
 	p->flags = 0;
-	spin_unlock(&swap_lock);
+	percpu_up_write(&swapon_rwsem);
 
 	atomic_inc(&proc_poll_event);
 	wake_up_interruptible(&proc_poll_wait);
@@ -3347,13 +3354,13 @@ static struct swap_info_struct *alloc_swap_info(void)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	spin_lock(&swap_lock);
+	percpu_down_write(&swapon_rwsem);
 	for (type = 0; type < nr_swapfiles; type++) {
 		if (!(swap_info[type]->flags & SWP_USED))
 			break;
 	}
 	if (type >= MAX_SWAPFILES) {
-		spin_unlock(&swap_lock);
+		percpu_up_write(&swapon_rwsem);
 		percpu_ref_exit(&p->users);
 		kvfree(p);
 		return ERR_PTR(-EPERM);
@@ -3378,7 +3385,7 @@ static struct swap_info_struct *alloc_swap_info(void)
 	plist_node_init(&p->list, 0);
 	plist_node_init(&p->avail_list, 0);
 	p->flags = SWP_USED;
-	spin_unlock(&swap_lock);
+	percpu_up_write(&swapon_rwsem);
 	if (defer) {
 		percpu_ref_exit(&defer->users);
 		kvfree(defer);
@@ -3804,9 +3811,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	 * Clear the SWP_USED flag after all resources are freed so
 	 * alloc_swap_info can reuse this si safely.
 	 */
-	spin_lock(&swap_lock);
+	percpu_down_write(&swapon_rwsem);
 	si->flags = 0;
-	spin_unlock(&swap_lock);
+	percpu_up_write(&swapon_rwsem);
 	if (inced_nr_rotate_swap)
 		atomic_dec(&nr_rotate_swap);
 	if (swap_file)
@@ -3824,14 +3831,14 @@ void si_swapinfo(struct sysinfo *val)
 	struct swap_info_struct *si;
 	unsigned long nr_to_be_unused = 0;
 
-	spin_lock(&swap_lock);
+	percpu_down_read(&swapon_rwsem);
 	for_each_swap(si) {
 		if (!(si->flags & SWP_WRITEOK))
 			nr_to_be_unused += swap_usage_in_pages(si);
 	}
 	val->freeswap = atomic_long_read(&nr_swap_pages) + nr_to_be_unused;
 	val->totalswap = total_swap_pages + nr_to_be_unused;
-	spin_unlock(&swap_lock);
+	percpu_up_read(&swapon_rwsem);
 }
 
 /*

-- 
2.55.0

[PATCH RFC 06/13] mm/swap: remove swapon mutex and update proc reader

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

Now that swapon_rwsem protects all swapon/swapoff-sensitive data,
the old swapon_mutex is redundant.  Remove it and convert the
/proc/swaps seq_file reader to use the rwsem instead.  Also convert
the swap_start iterator to use for_each_swap() for consistency.

Also adapt swap_next, it needs an explicit SWP_USED check. swap_file is
not reliably protected by swapon_rwsem, so checking SWP_USED is more
accurate and stable for filtering in-use devices.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/swapfile.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index d75fad161ba5..8b073569d1d9 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -140,9 +140,6 @@ static inline struct swap_info_struct *__swap_iter(int *i, unsigned long flag)
 
 static struct kmem_cache *swap_table_cachep;
 
-/* Protects si->swap_file for /proc/swaps usage */
-static DEFINE_MUTEX(swapon_mutex);
-
 static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
 /* Activity counter to indicate that a swapon or swapoff has occurred */
 static atomic_t proc_poll_event = ATOMIC_INIT(0);
@@ -3156,7 +3153,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	if (!(p->flags & SWP_SOLIDSTATE))
 		atomic_dec(&nr_rotate_swap);
 
-	mutex_lock(&swapon_mutex);
 	percpu_down_write(&swapon_rwsem);
 	spin_lock(&p->lock);
 	drain_mmlist();
@@ -3171,7 +3167,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	percpu_up_write(&swapon_rwsem);
 	arch_swap_invalidate_area(p->type);
 	zswap_swapoff(p->type);
-	mutex_unlock(&swapon_mutex);
 	kfree(p->global_cluster);
 	p->global_cluster = NULL;
 	free_swap_cluster_info(cluster_info, maxpages);
@@ -3217,20 +3212,18 @@ static __poll_t swaps_poll(struct file *file, poll_table *wait)
 	return EPOLLIN | EPOLLRDNORM;
 }
 
-/* iterator */
 static void *swap_start(struct seq_file *swap, loff_t *pos)
 {
 	struct swap_info_struct *si;
-	int type;
 	loff_t l = *pos;
 
-	mutex_lock(&swapon_mutex);
+	percpu_down_read(&swapon_rwsem);
 
 	if (!l)
 		return SEQ_START_TOKEN;
 
-	for (type = 0; (si = swap_type_to_info(type)); type++) {
-		if (!(si->swap_file))
+	for_each_swap(si) {
+		if (!si->swap_file)
 			continue;
 		if (!--l)
 			return si;
@@ -3251,7 +3244,7 @@ static void *swap_next(struct seq_file *swap, void *v, loff_t *pos)
 
 	++(*pos);
 	for (; (si = swap_type_to_info(type)); type++) {
-		if (!(si->swap_file))
+		if (!(si->flags & SWP_USED) || !(si->swap_file))
 			continue;
 		return si;
 	}
@@ -3261,7 +3254,7 @@ static void *swap_next(struct seq_file *swap, void *v, loff_t *pos)
 
 static void swap_stop(struct seq_file *swap, void *v)
 {
-	mutex_unlock(&swapon_mutex);
+	percpu_up_read(&swapon_rwsem);
 }
 
 static int swap_show(struct seq_file *swap, void *v)
@@ -3764,7 +3757,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		goto free_swap_zswap;
 	}
 
-	mutex_lock(&swapon_mutex);
 	prio = DEF_SWAP_PRIO;
 	if (swap_flags & SWAP_FLAG_PREFER)
 		prio = swap_flags & SWAP_FLAG_PRIO_MASK;
@@ -3790,7 +3782,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		(si->flags & SWP_AREA_DISCARD) ? "s" : "",
 		(si->flags & SWP_PAGE_DISCARD) ? "c" : "");
 
-	mutex_unlock(&swapon_mutex);
 	atomic_inc(&proc_poll_event);
 	wake_up_interruptible(&proc_poll_wait);
 

-- 
2.55.0

[PATCH RFC 07/13] mm/swap: consolidate swap inuse accounting helpers

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

Inline the swap_usage_add/sub helpers into their only callers and
rename the callers to better reflect what they actually do: track
per-device inuse page counts.

No functional change.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/swapfile.c | 64 +++++++++++++++++++++++++----------------------------------
 1 file changed, 27 insertions(+), 37 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 8b073569d1d9..df9855401a2e 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -49,8 +49,8 @@
 #include "internal.h"
 #include "swap.h"
 
-static void swap_range_alloc(struct swap_info_struct *si,
-			     unsigned int nr_entries);
+static void swap_device_inuse_add(struct swap_info_struct *si,
+				  unsigned int nr_entries);
 static bool folio_swapcache_freeable(struct folio *folio);
 static void move_cluster(struct swap_info_struct *si,
 			 struct swap_cluster_info *ci, struct list_head *list,
@@ -984,7 +984,7 @@ static bool __swap_cluster_alloc_entries(struct swap_info_struct *si,
 	if (cluster_is_empty(ci))
 		ci->order = order;
 	ci->count += nr_pages;
-	swap_range_alloc(si, nr_pages);
+	swap_device_inuse_add(si, nr_pages);
 
 	return true;
 }
@@ -1286,53 +1286,36 @@ static void add_to_avail_list(struct swap_info_struct *si)
 }
 
 /*
- * swap_usage_add / swap_usage_sub of each slot are serialized by ci->lock
- * within each cluster, so the total contribution to the global counter should
- * always be positive and cannot exceed the total number of usable slots.
+ * swap_device_inuse_add / swap_device_inuse_sub are called within cluster
+ * update critical sections and serialized by ci->lock within each cluster,
+ * so the total contribution to the global counter should always be positive
+ * and cannot exceed the total number of usable slots.
  */
-static bool swap_usage_add(struct swap_info_struct *si, unsigned int nr_entries)
+static void swap_device_inuse_add(struct swap_info_struct *si,
+				  unsigned int nr_entries)
 {
-	long val = atomic_long_add_return_relaxed(nr_entries, &si->inuse_pages);
+	long inuse_pages;
 
 	/*
 	 * If device is full, and SWAP_USAGE_OFFLIST_BIT is not set,
 	 * remove it from the plist.
 	 */
-	if (unlikely(val == si->pages)) {
-		del_from_avail_list(si, false);
-		return true;
-	}
-
-	return false;
-}
-
-static void swap_usage_sub(struct swap_info_struct *si, unsigned int nr_entries)
-{
-	long val = atomic_long_sub_return_relaxed(nr_entries, &si->inuse_pages);
-
-	/*
-	 * If device is not full, and SWAP_USAGE_OFFLIST_BIT is set,
-	 * add it to the plist.
-	 */
-	if (unlikely(val & SWAP_USAGE_OFFLIST_BIT))
-		add_to_avail_list(si);
-}
-
-static void swap_range_alloc(struct swap_info_struct *si,
-			     unsigned int nr_entries)
-{
-	if (swap_usage_add(si, nr_entries)) {
+	inuse_pages = atomic_long_add_return_relaxed(nr_entries, &si->inuse_pages);
+	if (unlikely(inuse_pages == si->pages)) {
 		if (vm_swap_full())
 			schedule_work(&si->reclaim_work);
+		del_from_avail_list(si, false);
 	}
+
 	atomic_long_sub(nr_entries, &nr_swap_pages);
 }
 
-static void swap_range_free(struct swap_info_struct *si, unsigned long offset,
-			    unsigned int nr_entries)
+static void swap_device_inuse_sub(struct swap_info_struct *si, unsigned long offset,
+				  unsigned int nr_entries)
 {
 	unsigned long end = offset + nr_entries - 1;
 	void (*swap_slot_free_notify)(struct block_device *, unsigned long);
+	long inuse_pages;
 	unsigned int i;
 
 	for (i = 0; i < nr_entries; i++)
@@ -1356,7 +1339,14 @@ static void swap_range_free(struct swap_info_struct *si, unsigned long offset,
 	 */
 	smp_wmb();
 	atomic_long_add(nr_entries, &nr_swap_pages);
-	swap_usage_sub(si, nr_entries);
+
+	/*
+	 * If device is not full, and SWAP_USAGE_OFFLIST_BIT is set,
+	 * add it back to the plist.
+	 */
+	inuse_pages = atomic_long_sub_return_relaxed(nr_entries, &si->inuse_pages);
+	if (unlikely(inuse_pages & SWAP_USAGE_OFFLIST_BIT))
+		add_to_avail_list(si);
 }
 
 static bool get_swap_device_info(struct swap_info_struct *si)
@@ -1969,7 +1959,7 @@ void __swap_cluster_free_entries(struct swap_info_struct *si,
 	if (batch_id)
 		mem_cgroup_uncharge_swap(batch_id, ci_off - batch_off);
 
-	swap_range_free(si, ci_head + ci_start, nr_pages);
+	swap_device_inuse_sub(si, ci_head + ci_start, nr_pages);
 	swap_cluster_assert_empty(ci, ci_start, nr_pages, false);
 
 	if (!ci->count)
@@ -2821,7 +2811,7 @@ static int try_to_unuse(unsigned int type)
 success:
 	/*
 	 * Make sure that further cleanups after try_to_unuse() returns happen
-	 * after swap_range_free() reduces si->inuse_pages to 0.
+	 * after swap_device_inuse_sub() reduces si->inuse_pages to 0.
 	 */
 	smp_mb();
 	return 0;

-- 
2.55.0

[PATCH RFC 08/13] mm/swap: change back to use each swap device's percpu cluster

[Kairui Song via B4 Relay]

From: Youngjun Park <youngjun.park@lge.com>

This reverts commit 1b7e90020eb7 ("mm, swap: use percpu cluster as
allocation fast path").

The global percpu cluster causes several issues:

1) It prevents efficient swap allocation for users that do not want
   the default swap device policy.  Because the cluster cache sits
   above device selection, all allocation users must stick with the
   default allocation policy.  This fundamentally conflicts with
   incoming ideas like swap tiering.

2) It can cause priority inversion.  Consider two memcgs: memcg1 can
   access devices A and B, where A has higher priority; memcg2 can
   only access B.  Memcg2 can write the global percpu cluster with
   device B, then memcg1 takes B in the fast path even though
   higher-priority device A is not exhausted.

3) The fast-path / slow-path design bundled with the global per-CPU
   cache is problematic: only the fast path uses the global cache,
   and the slow path is forced to rotate the allocation list.  This
   complicates consumers like discard.

Revert to per-device percpu clusters first.  A later patch will
introduce new infrastructure for a high-performance global
allocation path.

Suggested-by: Kairui Song <kasong@tencent.com>
Co-developed-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Youngjun Park <youngjun.park@lge.com>
Signed-off-by: Kairui Song <kasong@tencent.com>
---
 include/linux/swap.h |  13 +++-
 mm/swapfile.c        | 184 +++++++++++++++++----------------------------------
 2 files changed, 72 insertions(+), 125 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index d0c4e0ab5806..6733a8979aa2 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -246,10 +246,17 @@ enum {
 #endif
 
 /*
- * We keep using same cluster for rotational device so IO will be sequential.
- * The purpose is to optimize SWAP throughput on these device.
+ * We assign a cluster to each CPU, so each CPU can allocate swap entry from
+ * its own cluster and swapout sequentially. The purpose is to optimize swapout
+ * throughput.
  */
+struct percpu_cluster {
+	local_lock_t lock; /* Protect the percpu_cluster above */
+	unsigned int next[SWAP_NR_ORDERS]; /* Likely next allocation offset */
+};
+
 struct swap_sequential_cluster {
+	spinlock_t lock; /* Serialize usage of global cluster */
 	unsigned int next[SWAP_NR_ORDERS]; /* Likely next allocation offset */
 };
 
@@ -272,8 +279,8 @@ struct swap_info_struct {
 					/* list of cluster that are fragmented or contented */
 	unsigned int pages;		/* total of usable pages of swap */
 	atomic_long_t inuse_pages;	/* number of those currently in use */
+	struct percpu_cluster	__percpu *percpu_cluster; /* per cpu's swap location */
 	struct swap_sequential_cluster *global_cluster; /* Use one global cluster for rotating device */
-	spinlock_t global_cluster_lock;	/* Serialize usage of global cluster */
 	struct rb_root swap_extent_root;/* root of the swap extent rbtree */
 	struct block_device *bdev;	/* swap device or bdev of swap file */
 	struct file *swap_file;		/* seldom referenced */
diff --git a/mm/swapfile.c b/mm/swapfile.c
index df9855401a2e..b2adf36a0884 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -146,18 +146,6 @@ static atomic_t proc_poll_event = ATOMIC_INIT(0);
 
 atomic_t nr_rotate_swap = ATOMIC_INIT(0);
 
-struct percpu_swap_cluster {
-	struct swap_info_struct *si[SWAP_NR_ORDERS];
-	unsigned long offset[SWAP_NR_ORDERS];
-	local_lock_t lock;
-};
-
-static DEFINE_PER_CPU(struct percpu_swap_cluster, percpu_swap_cluster) = {
-	.si = { NULL },
-	.offset = { SWAP_ENTRY_INVALID },
-	.lock = INIT_LOCAL_LOCK(),
-};
-
 /* May return NULL on invalid type, caller must check for NULL return */
 static struct swap_info_struct *swap_type_to_info(int type)
 {
@@ -562,9 +550,10 @@ swap_cluster_populate(struct swap_info_struct *si,
 	 * Only cluster isolation from the allocator does table allocation.
 	 * Swap allocator uses percpu clusters and holds the local lock.
 	 */
-	lockdep_assert_held(&this_cpu_ptr(&percpu_swap_cluster)->lock);
-	if (!(si->flags & SWP_SOLIDSTATE))
-		lockdep_assert_held(&si->global_cluster_lock);
+	if (si->flags & SWP_SOLIDSTATE)
+		lockdep_assert_held(this_cpu_ptr(&si->percpu_cluster->lock));
+	else
+		lockdep_assert_held(&si->global_cluster->lock);
 	lockdep_assert_held(&ci->lock);
 
 	if (!swap_cluster_alloc_table(ci, __GFP_HIGH | __GFP_NOMEMALLOC |
@@ -577,9 +566,10 @@ swap_cluster_populate(struct swap_info_struct *si,
 	 * the potential recursive allocation is limited.
 	 */
 	spin_unlock(&ci->lock);
-	if (!(si->flags & SWP_SOLIDSTATE))
-		spin_unlock(&si->global_cluster_lock);
-	local_unlock(&percpu_swap_cluster.lock);
+	if (si->flags & SWP_SOLIDSTATE)
+		local_unlock(&si->percpu_cluster->lock);
+	else
+		spin_unlock(&si->global_cluster->lock);
 
 	ret = swap_cluster_alloc_table(ci, __GFP_HIGH | __GFP_NOMEMALLOC |
 					   GFP_KERNEL);
@@ -592,9 +582,10 @@ swap_cluster_populate(struct swap_info_struct *si,
 	 * could happen with ignoring the percpu cluster is fragmentation,
 	 * which is acceptable since this fallback and race is rare.
 	 */
-	local_lock(&percpu_swap_cluster.lock);
-	if (!(si->flags & SWP_SOLIDSTATE))
-		spin_lock(&si->global_cluster_lock);
+	if (si->flags & SWP_SOLIDSTATE)
+		local_lock(&si->percpu_cluster->lock);
+	else
+		spin_lock(&si->global_cluster->lock);
 	spin_lock(&ci->lock);
 
 	if (ret) {
@@ -700,7 +691,7 @@ static bool swap_do_scheduled_discard(struct swap_info_struct *si)
 		ci = list_first_entry(&si->discard_clusters, struct swap_cluster_info, list);
 		/*
 		 * Delete the cluster from list to prepare for discard, but keep
-		 * the CLUSTER_FLAG_DISCARD flag, percpu_swap_cluster could be
+		 * the CLUSTER_FLAG_DISCARD flag, there could be percpu_cluster
 		 * pointing to it, or ran into by relocate_cluster.
 		 */
 		list_del(&ci->list);
@@ -1032,12 +1023,10 @@ static unsigned int alloc_swap_scan_cluster(struct swap_info_struct *si,
 out:
 	relocate_cluster(si, ci);
 	swap_cluster_unlock(ci);
-	if (si->flags & SWP_SOLIDSTATE) {
-		this_cpu_write(percpu_swap_cluster.offset[order], next);
-		this_cpu_write(percpu_swap_cluster.si[order], si);
-	} else {
+	if (si->flags & SWP_SOLIDSTATE)
+		this_cpu_write(si->percpu_cluster->next[order], next);
+	else
 		si->global_cluster->next[order] = next;
-	}
 	return found;
 }
 
@@ -1132,13 +1121,17 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 	if (order && !(si->flags & SWP_BLKDEV))
 		return 0;
 
-	if (!(si->flags & SWP_SOLIDSTATE)) {
+	if (si->flags & SWP_SOLIDSTATE) {
+		/* Fast path using per CPU cluster */
+		local_lock(&si->percpu_cluster->lock);
+		offset = __this_cpu_read(si->percpu_cluster->next[order]);
+	} else {
 		/* Serialize HDD SWAP allocation for each device. */
-		spin_lock(&si->global_cluster_lock);
+		spin_lock(&si->global_cluster->lock);
 		offset = si->global_cluster->next[order];
-		if (offset == SWAP_ENTRY_INVALID)
-			goto new_cluster;
+	}
 
+	if (offset != SWAP_ENTRY_INVALID) {
 		ci = swap_cluster_lock(si, offset);
 		/* Cluster could have been used by another order */
 		if (cluster_is_usable(ci, order)) {
@@ -1152,7 +1145,6 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 			goto done;
 	}
 
-new_cluster:
 	/*
 	 * If the device need discard, prefer new cluster over nonfull
 	 * to spread out the writes.
@@ -1209,8 +1201,10 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 			goto done;
 	}
 done:
-	if (!(si->flags & SWP_SOLIDSTATE))
-		spin_unlock(&si->global_cluster_lock);
+	if (si->flags & SWP_SOLIDSTATE)
+		local_unlock(&si->percpu_cluster->lock);
+	else
+		spin_unlock(&si->global_cluster->lock);
 
 	return found;
 }
@@ -1365,41 +1359,8 @@ static bool get_swap_device_info(struct swap_info_struct *si)
 	return true;
 }
 
-/*
- * Fast path try to get swap entries with specified order from current
- * CPU's swap entry pool (a cluster).
- */
-static bool swap_alloc_fast(struct folio *folio)
-{
-	unsigned int order = folio_order(folio);
-	struct swap_cluster_info *ci;
-	struct swap_info_struct *si;
-	unsigned int offset;
-
-	/*
-	 * Once allocated, swap_info_struct will never be completely freed,
-	 * so checking it's liveness by get_swap_device_info is enough.
-	 */
-	si = this_cpu_read(percpu_swap_cluster.si[order]);
-	offset = this_cpu_read(percpu_swap_cluster.offset[order]);
-	if (!si || !offset || !get_swap_device_info(si))
-		return false;
-
-	ci = swap_cluster_lock(si, offset);
-	if (cluster_is_usable(ci, order)) {
-		if (cluster_is_empty(ci))
-			offset = cluster_offset(si, ci);
-		alloc_swap_scan_cluster(si, ci, folio, offset);
-	} else {
-		swap_cluster_unlock(ci);
-	}
-
-	put_swap_device(si);
-	return folio_test_swapcache(folio);
-}
-
 /* Rotate the device and switch to a new cluster */
-static void swap_alloc_slow(struct folio *folio)
+static void swap_alloc_entry(struct folio *folio)
 {
 	struct swap_info_struct *si, *next;
 
@@ -1769,10 +1730,7 @@ int folio_alloc_swap(struct folio *folio)
 	}
 
 again:
-	local_lock(&percpu_swap_cluster.lock);
-	if (!swap_alloc_fast(folio))
-		swap_alloc_slow(folio);
-	local_unlock(&percpu_swap_cluster.lock);
+	swap_alloc_entry(folio);
 
 	if (!order && unlikely(!folio_test_swapcache(folio))) {
 		if (swap_sync_discard())
@@ -2161,31 +2119,14 @@ void swap_put_entries_direct(swp_entry_t entry, int nr)
  */
 swp_entry_t swap_alloc_hibernation_slot(int type)
 {
-	struct swap_info_struct *pcp_si, *si = swap_type_to_info(type);
-	unsigned long pcp_offset, offset = SWAP_ENTRY_INVALID;
-	struct swap_cluster_info *ci;
+	struct swap_info_struct *si = swap_type_to_info(type);
+	unsigned long offset = SWAP_ENTRY_INVALID;
 	swp_entry_t entry = {0};
 
 	if (!si)
 		goto fail;
 
-	/*
-	 * Try the local cluster first if it matches the device. If
-	 * not, try grab a new cluster and override local cluster.
-	 */
-	local_lock(&percpu_swap_cluster.lock);
-	pcp_si = this_cpu_read(percpu_swap_cluster.si[0]);
-	pcp_offset = this_cpu_read(percpu_swap_cluster.offset[0]);
-	if (pcp_si == si && pcp_offset) {
-		ci = swap_cluster_lock(si, pcp_offset);
-		if (cluster_is_usable(ci, 0))
-			offset = alloc_swap_scan_cluster(si, ci, NULL, pcp_offset);
-		else
-			swap_cluster_unlock(ci);
-	}
-	if (!offset)
-		offset = cluster_alloc_swap_entry(si, NULL);
-	local_unlock(&percpu_swap_cluster.lock);
+	offset = cluster_alloc_swap_entry(si, NULL);
 	if (offset)
 		entry = swp_entry(si->type, offset);
 
@@ -3052,27 +2993,6 @@ static void free_swap_cluster_info(struct swap_cluster_info *cluster_info,
 	kvfree(cluster_info);
 }
 
-/*
- * Called after swap device's reference count is dead, so
- * neither scan nor allocation will use it.
- */
-static void flush_percpu_swap_cluster(struct swap_info_struct *si)
-{
-	int cpu, i;
-	struct swap_info_struct **pcp_si;
-
-	for_each_possible_cpu(cpu) {
-		pcp_si = per_cpu_ptr(percpu_swap_cluster.si, cpu);
-		/*
-		 * Invalidate the percpu swap cluster cache, si->users
-		 * is dead, so no new user will point to it, just flush
-		 * any existing user.
-		 */
-		for (i = 0; i < SWAP_NR_ORDERS; i++)
-			cmpxchg(&pcp_si[i], si, NULL);
-	}
-}
-
 SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 {
 	struct swap_info_struct *p = NULL;
@@ -3136,7 +3056,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 
 	flush_work(&p->discard_work);
 	flush_work(&p->reclaim_work);
-	flush_percpu_swap_cluster(p);
 
 	destroy_swap_extents(p, p->swap_file);
 
@@ -3157,6 +3076,8 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	percpu_up_write(&swapon_rwsem);
 	arch_swap_invalidate_area(p->type);
 	zswap_swapoff(p->type);
+	free_percpu(p->percpu_cluster);
+	p->percpu_cluster = NULL;
 	kfree(p->global_cluster);
 	p->global_cluster = NULL;
 	free_swap_cluster_info(cluster_info, maxpages);
@@ -3505,7 +3426,7 @@ static int setup_swap_clusters_info(struct swap_info_struct *si,
 {
 	unsigned long nr_clusters = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
 	struct swap_cluster_info *cluster_info;
-	int err = -ENOMEM;
+	int cpu, err = -ENOMEM;
 	unsigned long i;
 
 	cluster_info = kvzalloc_objs(*cluster_info, nr_clusters);
@@ -3515,13 +3436,26 @@ static int setup_swap_clusters_info(struct swap_info_struct *si,
 	for (i = 0; i < nr_clusters; i++)
 		spin_lock_init(&cluster_info[i].lock);
 
-	if (!(si->flags & SWP_SOLIDSTATE)) {
+	if (si->flags & SWP_SOLIDSTATE) {
+		si->percpu_cluster = alloc_percpu(struct percpu_cluster);
+		if (!si->percpu_cluster)
+			goto err;
+
+		for_each_possible_cpu(cpu) {
+			struct percpu_cluster *cluster;
+
+			cluster = per_cpu_ptr(si->percpu_cluster, cpu);
+			for (i = 0; i < SWAP_NR_ORDERS; i++)
+				cluster->next[i] = SWAP_ENTRY_INVALID;
+			local_lock_init(&cluster->lock);
+		}
+	} else {
 		si->global_cluster = kmalloc_obj(*si->global_cluster);
 		if (!si->global_cluster)
 			goto err;
 		for (i = 0; i < SWAP_NR_ORDERS; i++)
 			si->global_cluster->next[i] = SWAP_ENTRY_INVALID;
-		spin_lock_init(&si->global_cluster_lock);
+		spin_lock_init(&si->global_cluster->lock);
 	}
 
 	/*
@@ -3683,11 +3617,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 
 	maxpages = si->max;
 
-	/* Set up the swap cluster info */
-	error = setup_swap_clusters_info(si, swap_header, maxpages);
-	if (error)
-		goto bad_swap_unlock_inode;
-
 	if (si->bdev && bdev_stable_writes(si->bdev))
 		si->flags |= SWP_STABLE_WRITES;
 
@@ -3701,6 +3630,15 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		inced_nr_rotate_swap = true;
 	}
 
+	/*
+	 * Set up the swap cluster info. This must run after SWP_SOLIDSTATE
+	 * is determined above, as it decides whether to allocate the per-CPU
+	 * cluster (solid state) or the global cluster (rotational).
+	 */
+	error = setup_swap_clusters_info(si, swap_header, maxpages);
+	if (error)
+		goto bad_swap_unlock_inode;
+
 	if ((swap_flags & SWAP_FLAG_DISCARD) &&
 	    si->bdev && bdev_max_discard_sectors(si->bdev)) {
 		/*
@@ -3782,6 +3720,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 bad_swap_unlock_inode:
 	inode_unlock(inode);
 bad_swap:
+	free_percpu(si->percpu_cluster);
+	si->percpu_cluster = NULL;
 	kfree(si->global_cluster);
 	si->global_cluster = NULL;
 	inode = NULL;

-- 
2.55.0

[PATCH RFC 09/13] mm/swap: add priority queue for swap device allocation

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

The swap allocator uses a plist (swap_avail_head) ordered by
priority to pick the next device.  To spread IO across devices at
the same priority it rotates the current device to the tail via
plist_requeue(), which requires holding swap_avail_lock.  It then
drops the lock for the allocation attempt and reacquires to check
whether the next device is still on the list.  This per-allocation
lock cycling and the single global rotation point mean all CPUs
contend for the same list head.

Add a custom priority queue that groups devices into rings, ordered
by priority to form a static array as the queue.  Devices in the
same priority ring also form a static array.

To eliminate CPU contention on a global lock, swapon_rwsem is reused
to protect the queue.  The queue stays read-only, only swapon and
swapoff can adjust the length of the queue or rings in it.  The
writer lock is costly but swapon and swapoff are rare so this is
acceptable.

Since the queue is read-only, rotation is done from the reader side.
Each CPU maintains its own iterator using a counter that counts down
to advance to the next device in the same ring, so allocation is
round-robin while retaining locality for clustering.  After a failed
attempt the iterator also advances so the caller does not retry the
same full or fragmented device.

If a whole ring is attempted and allocation could not be made, the
iterator falls through to the next ring.  This almost never happens
because we always want to allocate from the device with the highest
priority, unless that device is full.

To speed up iteration when there are multiple full devices, a device
pointer stored in the ring can be marked as unavailable so the
iterator skips past it.  Fully used devices can mark their pointer
as unavailable without taking the rwsem lock.  The marking is done using
a spin lock to sync with potential writer because percpu rwsem is too
heavy for writers, and full device transitions are more frequent and
performance-sensitive compared to swapon and swapoff.

With this patch, swap allocation should be faster and cleaner.  The
percpu rwsem has very low overhead for the allocator, and it can now
hold swapon_rwsem for the entire allocation loop, no longer needing to
keep dropping and reacquiring a lock to rotate list entries.

The old swap_avail_head plist is still maintained in parallel for
now, updated alongside the queue in del_from_avail_list() and
add_to_avail_list().  It will be removed once the remaining consumer
is converted in a follow-up patch.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/swapfile.c | 455 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 398 insertions(+), 57 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index b2adf36a0884..f6dd85968928 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -55,6 +55,7 @@ static bool folio_swapcache_freeable(struct folio *folio);
 static void move_cluster(struct swap_info_struct *si,
 			 struct swap_cluster_info *ci, struct list_head *list,
 			 enum swap_cluster_flags new_flags);
+static bool get_swap_device_info(struct swap_info_struct *si);
 
 /*
  * Serializes swapon/swapoff (writers) and protects the swap_info
@@ -160,18 +161,350 @@ static struct swap_info_struct *swap_entry_to_info(swp_entry_t entry)
 	return swap_type_to_info(swp_type(entry));
 }
 
+/*
+ * All available swap_info_structs are grouped by priority rings, the rings
+ * are ordered in a queue by priority (lower prio value = higher priority).
+ * The allocator iterates and rotates devices within each priority ring.
+ * When all devices in a ring are iterated, it goes to the next lower
+ * priority ring.
+ */
+struct swap_prio_ring {
+	int prio;
+	unsigned int size;
+	struct swap_info_struct *dev[] __counted_by(size);
+};
+
+/*
+ * The ring is protected by swapon_rwsem so updating it is costly. To make
+ * the allocator and other users skip full devices faster, the lowest bit of
+ * a device pointer is used to mark it disabled (temporarily unavailable).
+ * This relies on struct swap_info_struct being sufficiently
+ * aligned (guaranteed by kmalloc).
+ */
+#define SWAP_DEVICE_MASKED_SHIFT	0
+#define SWAP_DEVICE_MASKED_BIT		BIT(SWAP_DEVICE_MASKED_SHIFT)
+
+/*
+ * Serializes queue content mutations and keeps SWAP_USAGE_OFFLIST_BIT
+ * consistent with the masked state of each device pointer.
+ */
+static DEFINE_SPINLOCK(swap_queue_update_lock);
+
+/*
+ * Swap queue is protected by both swap_queue_update_lock and swapon_rwsem.
+ * Only swapon/swapoff will take the write lock, and modify the queue length
+ * or any ring's length. swap_queue_update_lock protects the content so
+ * devices can be masked easily without taking the writelock, which is heavy.
+ */
+static struct swap_prio_ring **swap_queue;
+static unsigned int swap_queue_len;
+
+/*
+ * Each CPU has its read iterator, so the queue itself will remain read
+ * only and the CPU side reader rotates by iterating the devices
+ * periodically using the counter.
+ */
+#define SWAP_ROUND_ROBIN_QUOTA SWAPFILE_CLUSTER
+struct swap_ring_iterator {
+	int offset;
+	long rr_counter;
+};
+
+struct swap_queue_reader {
+	local_lock_t lock;
+	struct swap_ring_iterator ri[];
+};
+
+static __percpu struct swap_queue_reader *swap_queue_readers;
+
+static inline bool swap_device_masked(struct swap_info_struct *si)
+{
+	return (unsigned long)si & SWAP_DEVICE_MASKED_BIT;
+}
+
+static inline struct swap_info_struct *swap_device_unmask_ptr(struct swap_info_struct *si)
+{
+	return (struct swap_info_struct *)((unsigned long)si & ~SWAP_DEVICE_MASKED_BIT);
+}
+
+static struct swap_queue_reader __percpu *swap_queue_prealloc_readers(int nr_rings, gfp_t gfp)
+{
+	struct swap_queue_reader __percpu *readers;
+
+	if (!nr_rings)
+		return NULL;
+
+	readers = __alloc_percpu_gfp(struct_size(readers, ri, nr_rings),
+				     __alignof__(*readers), gfp);
+	return readers;
+}
+
+static void swap_queue_install_readers(struct swap_queue_reader __percpu *readers)
+{
+	int ring_idx, cpu;
+	struct swap_prio_ring *ring;
+
+	free_percpu(swap_queue_readers);
+	swap_queue_readers = readers;
+	if (!readers)
+		return;
+
+	/* Distribute each CPU's swap IO fairly across devices. */
+	for_each_possible_cpu(cpu) {
+		local_lock_init(&per_cpu_ptr(readers, cpu)->lock);
+
+		for (ring_idx = 0; ring_idx < swap_queue_len; ring_idx++) {
+			ring = swap_queue[ring_idx];
+			per_cpu_ptr(readers, cpu)->ri[ring_idx].offset =
+				    cpu % ring->size;
+		}
+	}
+}
+
+static struct swap_info_struct *swap_queue_get_device(long nr_alloc, int nr_iter)
+{
+	bool rotate = false;
+	struct swap_info_struct *si;
+	struct swap_ring_iterator *ri;
+	struct swap_prio_ring *ring;
+	unsigned int queue_idx;
+
+	if (!swap_queue_len)
+		return ERR_PTR(-ENOENT);
+
+	queue_idx = 0;
+	while (nr_iter >= swap_queue[queue_idx]->size) {
+		nr_iter -= swap_queue[queue_idx]->size;
+		if (++queue_idx >= swap_queue_len)
+			return ERR_PTR(-ENOENT);
+	}
+
+	ring = swap_queue[queue_idx];
+	local_lock(&swap_queue_readers->lock);
+	ri = this_cpu_ptr(&swap_queue_readers->ri[queue_idx]);
+	/* Rotate while iterating the ring, just not on the first try */
+	if (nr_iter)
+		rotate = true;
+	else if (ri->rr_counter < nr_alloc)
+		rotate = true;
+	else if (ri->offset >= ring->size)
+		rotate = true;
+	if (rotate) {
+		ri->offset++;
+		ri->offset %= ring->size;
+		ri->rr_counter = SWAP_ROUND_ROBIN_QUOTA;
+	}
+	ri->rr_counter -= nr_alloc;
+	si = READ_ONCE(ring->dev[ri->offset]);
+	local_unlock(&swap_queue_readers->lock);
+
+	if (swap_device_masked(si))
+		return ERR_PTR(-EBUSY);
+
+	si = swap_device_unmask_ptr(si);
+	return si;
+}
+
+static bool swap_queue_find(struct swap_info_struct *si,
+			    unsigned int *ring_idx, unsigned int *dev_idx)
+{
+	unsigned int i, j;
+	struct swap_prio_ring *ring;
+
+	lockdep_assert(lockdep_is_held(&swapon_rwsem) ||
+		       lockdep_is_held(&swap_queue_update_lock));
+
+	for (i = 0; i < swap_queue_len; i++) {
+		ring = swap_queue[i];
+		if (ring->prio != si->prio)
+			continue;
+		for (j = 0; j < ring->size; j++) {
+			if (swap_device_unmask_ptr(READ_ONCE(ring->dev[j])) != si)
+				continue;
+			*ring_idx = i;
+			*dev_idx = j;
+			return true;
+		}
+	}
+	return false;
+}
+
+static void swap_queue_mask(struct swap_info_struct *si)
+{
+	unsigned int ring_idx, dev_idx;
+
+	lockdep_assert_held(&swap_queue_update_lock);
+	if (swap_queue_find(si, &ring_idx, &dev_idx))
+		__set_bit(SWAP_DEVICE_MASKED_SHIFT,
+			  (unsigned long *)&swap_queue[ring_idx]->dev[dev_idx]);
+}
+
+static void swap_queue_unmask(struct swap_info_struct *si)
+{
+	unsigned int ring_idx, dev_idx;
+
+	lockdep_assert_held(&swap_queue_update_lock);
+	if (swap_queue_find(si, &ring_idx, &dev_idx))
+		__clear_bit(SWAP_DEVICE_MASKED_SHIFT,
+			    (unsigned long *)&swap_queue[ring_idx]->dev[dev_idx]);
+}
+
+static int swap_queue_add(struct swap_info_struct *si)
+{
+	struct swap_prio_ring **new_queue = NULL, **old_queue = NULL;
+	struct swap_queue_reader __percpu *new_readers = NULL;
+	struct swap_prio_ring *ring, *new_ring = NULL, *old_ring = NULL;
+	int prio = si->prio;
+	int i, pos, err = -ENOMEM;
+	gfp_t gfp;
+
+	/* Swap not usable here because this is swap, just reclaim cache. */
+	gfp = GFP_NOIO | __GFP_HIGH;
+	lockdep_assert_held_write(&swapon_rwsem);
+
+	for (pos = 0; pos < swap_queue_len; pos++) {
+		if (swap_queue[pos]->prio == prio)
+			goto add_to_ring;
+		if (swap_queue[pos]->prio < prio)
+			break;
+	}
+
+	/* No ring at this priority: insert a new one at pos. */
+	new_readers = swap_queue_prealloc_readers(swap_queue_len + 1, gfp);
+	if (!new_readers)
+		goto failed;
+	new_queue = kmalloc_array(swap_queue_len + 1, sizeof(*swap_queue), gfp);
+	if (!new_queue)
+		goto failed;
+	new_ring = kmalloc(struct_size(new_ring, dev, 1), gfp);
+	if (!new_ring)
+		goto failed;
+	if (!get_swap_device_info(si))
+		goto failed;
+
+	new_ring->prio = prio;
+	new_ring->size = 1;
+	new_ring->dev[0] = si;
+	for (i = 0; i < pos; i++)
+		new_queue[i] = swap_queue[i];
+	new_queue[pos] = new_ring;
+	for (i = pos; i < swap_queue_len; i++)
+		new_queue[i + 1] = swap_queue[i];
+
+	spin_lock(&swap_queue_update_lock);
+	old_queue = swap_queue;
+	swap_queue = new_queue;
+	swap_queue_len++;
+	spin_unlock(&swap_queue_update_lock);
+	kfree(old_queue);
+
+	swap_queue_install_readers(new_readers);
+	return 0;
+
+add_to_ring:
+	ring = swap_queue[pos];
+	new_ring = kmalloc(struct_size(ring, dev, ring->size + 1), gfp);
+	if (!new_ring)
+		goto failed;
+	if (!get_swap_device_info(si))
+		goto failed;
+	spin_lock(&swap_queue_update_lock);
+	memcpy(new_ring, ring, struct_size(ring, dev, ring->size));
+	new_ring->size++;
+	new_ring->dev[new_ring->size - 1] = si;
+	old_ring = swap_queue[pos];
+	swap_queue[pos] = new_ring;
+	spin_unlock(&swap_queue_update_lock);
+	kfree(old_ring);
+	return 0;
+
+failed:
+	free_percpu(new_readers);
+	kfree(new_queue);
+	kfree(new_ring);
+	return err;
+}
+
+static void swap_queue_del(struct swap_info_struct *si)
+{
+	gfp_t gfp;
+	unsigned int ring_idx, dev_idx;
+	struct swap_queue_reader __percpu *new_readers = NULL;
+	struct swap_prio_ring *ring, *new_ring = NULL, *old_ring = NULL;
+	struct swap_prio_ring **new_queue = NULL, **old_queue = NULL;
+
+	lockdep_assert_held_write(&swapon_rwsem);
+	if (!swap_queue_find(si, &ring_idx, &dev_idx)) {
+		WARN_ON(1);
+		return;
+	}
+
+	/*
+	 * To shrink memory usage, pre-allocate new smaller data before
+	 * locking. Failure is fine, swapoff will release them anyway.
+	 */
+	gfp = GFP_NOIO | __GFP_HIGH;
+	ring = swap_queue[ring_idx];
+	if (ring->size > 1)
+		new_ring = kmalloc(struct_size(ring, dev, ring->size - 1), gfp);
+	if (ring->size == 1 && swap_queue_len > 1) {
+		new_readers = swap_queue_prealloc_readers(
+					swap_queue_len - 1, gfp);
+		new_queue = kmalloc(sizeof(*swap_queue) *
+				    (swap_queue_len - 1), gfp);
+	}
+
+	spin_lock(&swap_queue_update_lock);
+	if (ring->size > 1) {
+		/* Shift trailing devices left to fill the gap. */
+		while (++dev_idx < ring->size)
+			ring->dev[dev_idx - 1] =
+				ring->dev[dev_idx];
+		ring->size--;
+		if (new_ring) {
+			memcpy(new_ring, ring,
+			       struct_size(ring, dev, ring->size));
+			old_ring = ring;
+			swap_queue[ring_idx] = new_ring;
+		}
+	} else {
+		/* Last device in this ring: remove the ring. */
+		old_ring = ring;
+		swap_queue_len--;
+		while (++ring_idx <= swap_queue_len)
+			swap_queue[ring_idx - 1] =
+				swap_queue[ring_idx];
+		if (new_queue) {
+			memcpy(new_queue, swap_queue,
+			       sizeof(*swap_queue) * swap_queue_len);
+			old_queue = swap_queue;
+			swap_queue = new_queue;
+		} else if (!swap_queue_len) {
+			old_queue = swap_queue;
+			swap_queue = NULL;
+		}
+		if (new_readers || !swap_queue_len)
+			swap_queue_install_readers(new_readers);
+	}
+	spin_unlock(&swap_queue_update_lock);
+
+	kfree(old_ring);
+	kfree(old_queue);
+	put_swap_device(si);
+}
+
 /*
  * Use the second highest bit of inuse_pages counter as the indicator
- * if one swap device is on the available plist, so the atomic can
+ * if one swap device is unavailable for allocation, so the atomic can
  * still be updated arithmetically while having special data embedded.
  *
  * inuse_pages counter is the only thing indicating if a device should
- * be on avail_lists or not (except swapon / swapoff). By embedding the
- * off-list bit in the atomic counter, updates no longer need any lock
- * to check the list status.
+ * be in the available queue or not (except swapon / swapoff). By
+ * embedding the off-list bit in the atomic counter, updates no longer
+ * need any lock to check the list status.
  *
- * This bit will be set if the device is not on the plist and not
- * usable, will be cleared if the device is on the plist.
+ * This bit will be set if the device is not in the available queue
+ * and not usable, will be cleared if the device is in the queue.
  */
 #define SWAP_USAGE_OFFLIST_BIT (1UL << (BITS_PER_TYPE(atomic_t) - 2))
 #define SWAP_USAGE_COUNTER_MASK (~SWAP_USAGE_OFFLIST_BIT)
@@ -1215,6 +1548,7 @@ static void del_from_avail_list(struct swap_info_struct *si, bool swapoff)
 	unsigned long pages;
 
 	spin_lock(&swap_avail_lock);
+	spin_lock(&swap_queue_update_lock);
 
 	/*
 	 * Force remove it only for swapoff. Else, take it off-list only if
@@ -1232,9 +1566,10 @@ static void del_from_avail_list(struct swap_info_struct *si, bool swapoff)
 		atomic_long_or(SWAP_USAGE_OFFLIST_BIT, &si->inuse_pages);
 	}
 
+	swap_queue_mask(si);
 	plist_del(&si->avail_list, &swap_avail_head);
-
 skip:
+	spin_unlock(&swap_queue_update_lock);
 	spin_unlock(&swap_avail_lock);
 }
 
@@ -1245,12 +1580,12 @@ static void add_to_avail_list(struct swap_info_struct *si)
 	unsigned long pages;
 
 	spin_lock(&swap_avail_lock);
+	spin_lock(&swap_queue_update_lock);
 
 	/*
-	 * Add the device to the avail list if SWP_WRITEOK is set and
-	 * SWAP_USAGE_OFFLIST_BIT is still set. Swapoff clears
-	 * SWP_WRITEOK first, so the device won't be re-added after
-	 * swapoff starts unless swap_device_enable resurrects it.
+	 * Mark the device as avail if SWP_WRITEOK is set. Swapoff clears
+	 * SWP_WRITEOK first, so check that first so the device won't be
+	 * re-added after swapoff started.
 	 */
 	if (!(si->flags & SWP_WRITEOK))
 		goto skip;
@@ -1261,9 +1596,10 @@ static void add_to_avail_list(struct swap_info_struct *si)
 	val = atomic_long_fetch_and_relaxed(~SWAP_USAGE_OFFLIST_BIT, &si->inuse_pages);
 
 	/*
-	 * When device is full and device is on the plist, only one updater will
-	 * see (inuse_pages == si->pages) and will call del_from_avail_list. If
-	 * that updater happen to be here, just skip adding.
+	 * When device is full and marked as available, one reader will see
+	 * (inuse_pages == si->pages) and should mark it as unavailable and
+	 * set SWAP_USAGE_OFFLIST_BIT. If that updater happens to be here, just
+	 * skip the rest.
 	 */
 	pages = si->pages;
 	if (val == pages) {
@@ -1273,10 +1609,10 @@ static void add_to_avail_list(struct swap_info_struct *si)
 			goto skip;
 	}
 
+	swap_queue_unmask(si);
 	plist_add(&si->avail_list, &swap_avail_head);
-
 skip:
-	spin_unlock(&swap_avail_lock);
+	spin_unlock(&swap_queue_update_lock);
 }
 
 /*
@@ -1292,7 +1628,7 @@ static void swap_device_inuse_add(struct swap_info_struct *si,
 
 	/*
 	 * If device is full, and SWAP_USAGE_OFFLIST_BIT is not set,
-	 * remove it from the plist.
+	 * mark it unavailable.
 	 */
 	inuse_pages = atomic_long_add_return_relaxed(nr_entries, &si->inuse_pages);
 	if (unlikely(inuse_pages == si->pages)) {
@@ -1336,7 +1672,7 @@ static void swap_device_inuse_sub(struct swap_info_struct *si, unsigned long off
 
 	/*
 	 * If device is not full, and SWAP_USAGE_OFFLIST_BIT is set,
-	 * add it back to the plist.
+	 * add it back to the available queue.
 	 */
 	inuse_pages = atomic_long_sub_return_relaxed(nr_entries, &si->inuse_pages);
 	if (unlikely(inuse_pages & SWAP_USAGE_OFFLIST_BIT))
@@ -1359,41 +1695,40 @@ static bool get_swap_device_info(struct swap_info_struct *si)
 	return true;
 }
 
-/* Rotate the device and switch to a new cluster */
-static void swap_alloc_entry(struct folio *folio)
+static int swap_alloc_entry(struct folio *folio)
 {
-	struct swap_info_struct *si, *next;
+	long nr_pages = folio_nr_pages(folio);
+	struct swap_info_struct *si;
+	int nr_iter, ret;
 
-	spin_lock(&swap_avail_lock);
-start_over:
-	plist_for_each_entry_safe(si, next, &swap_avail_head, avail_list) {
-		/* Rotate the device and switch to a new cluster */
-		plist_requeue(&si->avail_list, &swap_avail_head);
-		spin_unlock(&swap_avail_lock);
-		if (get_swap_device_info(si)) {
-			cluster_alloc_swap_entry(si, folio);
-			put_swap_device(si);
-			if (folio_test_swapcache(folio))
-				return;
-			if (folio_test_large(folio))
-				return;
+	percpu_down_read(&swapon_rwsem);
+	for (nr_iter = 0;; nr_iter++) {
+		si = swap_queue_get_device(nr_pages, nr_iter);
+		if (IS_ERR(si)) {
+			ret = PTR_ERR(si);
+			if (ret == -EBUSY)
+				continue;
+			break;
+		}
+		cluster_alloc_swap_entry(si, folio);
+
+		if (folio_test_swapcache(folio)) {
+			ret = 0;
+			break;
 		}
 
-		spin_lock(&swap_avail_lock);
 		/*
-		 * if we got here, it's likely that si was almost full before,
-		 * multiple callers probably all tried to get a page from the
-		 * same si and it filled up before we could get one; or, the si
-		 * filled up between us dropping swap_avail_lock.
-		 * Since we dropped the swap_avail_lock, the swap_avail_list
-		 * may have been modified; so if next is still in the
-		 * swap_avail_head list then try it, otherwise start over if we
-		 * have not gotten any slots.
+		 * For large allocation, return error directly to inform the
+		 * caller to split it instead of fallback to other devices.
 		 */
-		if (plist_node_empty(&next->avail_list))
-			goto start_over;
+		if (folio_test_large(folio)) {
+			ret = -E2BIG;
+			break;
+		}
 	}
-	spin_unlock(&swap_avail_lock);
+
+	percpu_up_read(&swapon_rwsem);
+	return ret;
 }
 
 /*
@@ -1707,6 +2042,7 @@ int folio_alloc_swap(struct folio *folio)
 {
 	unsigned int order = folio_order(folio);
 	unsigned int size = 1 << order;
+	int ret;
 
 	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
 	VM_BUG_ON_FOLIO(!folio_test_uptodate(folio), folio);
@@ -1730,7 +2066,7 @@ int folio_alloc_swap(struct folio *folio)
 	}
 
 again:
-	swap_alloc_entry(folio);
+	ret = swap_alloc_entry(folio);
 
 	if (!order && unlikely(!folio_test_swapcache(folio))) {
 		if (swap_sync_discard())
@@ -1742,7 +2078,7 @@ int folio_alloc_swap(struct folio *folio)
 		swap_cache_del_folio(folio);
 
 	if (unlikely(!folio_test_swapcache(folio)))
-		return -ENOMEM;
+		return ret ? ret : -ENOMEM;
 
 	return 0;
 }
@@ -2913,10 +3249,9 @@ static int setup_swap_extents(struct swap_info_struct *sis,
 static void swap_device_enable(struct swap_info_struct *si)
 {
 	percpu_down_write(&swapon_rwsem);
-	spin_lock(&swap_avail_lock);
+	spin_lock(&swap_queue_update_lock);
 	si->flags |= SWP_WRITEOK;
-	spin_unlock(&swap_avail_lock);
-
+	spin_unlock(&swap_queue_update_lock);
 	atomic_long_add(si->pages, &nr_swap_pages);
 	total_swap_pages += si->pages;
 	plist_add(&si->list, &swap_active_head);
@@ -2948,16 +3283,14 @@ static int swap_device_disable(struct swap_info_struct *si)
 	}
 	vm_unacct_memory(si->pages);
 
-	spin_lock(&swap_avail_lock);
+	spin_lock(&swap_queue_update_lock);
 	si->flags &= ~SWP_WRITEOK;
-	spin_unlock(&swap_avail_lock);
-
+	spin_unlock(&swap_queue_update_lock);
 	plist_del(&si->list, &swap_active_head);
 	total_swap_pages -= si->pages;
 	atomic_long_sub(si->pages, &nr_swap_pages);
-	percpu_up_write(&swapon_rwsem);
-
 	del_from_avail_list(si, true);
+	percpu_up_write(&swapon_rwsem);
 
 	/*
 	 * Swap allocator doesn't touch si lock, so looping through all
@@ -3042,6 +3375,9 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 		return err;
 	}
 
+	percpu_down_write(&swapon_rwsem);
+	swap_queue_del(p);
+
 	/*
 	 * Wait for swap operations protected by get/put_swap_device()
 	 * to complete.  Because of synchronize_rcu() here, all swap
@@ -3062,7 +3398,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	if (!(p->flags & SWP_SOLIDSTATE))
 		atomic_dec(&nr_rotate_swap);
 
-	percpu_down_write(&swapon_rwsem);
 	spin_lock(&p->lock);
 	drain_mmlist();
 
@@ -3700,6 +4035,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 
 	/* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
 	percpu_ref_resurrect(&si->users);
+	percpu_down_write(&swapon_rwsem);
+	error = swap_queue_add(si);
+	percpu_up_write(&swapon_rwsem);
+	if (error)
+		goto free_swap_zswap;
+
 	swap_device_enable(si);
 
 	pr_info("Adding %uk swap on %s.  Priority:%d extents:%d across:%lluk %s%s%s%s\n",

-- 
2.55.0

[PATCH RFC 10/13] mm/swap: remove available list

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

Pure cleanup, no functional change.

After the priority queue replaced the allocator's device selection,
swap_avail_head and swap_avail_lock are useless now.  Remove them.

__folio_throttle_swaprate() now iterates swap_active_head under
swapon_rwsem instead, the SWP_WRITEOK filtering makes it equivalent to
before.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 include/linux/swap.h |  1 -
 mm/swapfile.c        | 30 ++++--------------------------
 2 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 6733a8979aa2..189c9898641b 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -293,7 +293,6 @@ struct swap_info_struct {
 	struct work_struct discard_work; /* discard worker */
 	struct work_struct reclaim_work; /* reclaim worker */
 	struct list_head discard_clusters; /* discard clusters list */
-	struct plist_node avail_list;   /* entry in swap_avail_head */
 };
 
 static inline swp_entry_t page_swap_entry(struct page *page)
diff --git a/mm/swapfile.c b/mm/swapfile.c
index f6dd85968928..01240c4c9db3 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -96,21 +96,6 @@ static const char Bad_offset[] = "Bad swap offset entry ";
  */
 static PLIST_HEAD(swap_active_head);
 
-/*
- * all available (active, not full) swap_info_structs
- * protected with swap_avail_lock, ordered by priority.
- * This is used by folio_alloc_swap() instead of swap_active_head
- * because swap_active_head includes all swap_info_structs,
- * but folio_alloc_swap() doesn't need to look at full ones.
- * This uses its own lock instead of swapon_rwsem because when a
- * swap_info_struct changes between not-full/full, it needs to
- * add/remove itself to/from this list, but the swap_info_struct->lock
- * is held and the locking order requires swapon_rwsem to be taken
- * before any swap_info_struct->lock.
- */
-static PLIST_HEAD(swap_avail_head);
-static DEFINE_SPINLOCK(swap_avail_lock);
-
 static inline struct swap_info_struct *__swap_iter(int *i, unsigned long flag)
 {
 	lockdep_assert_held(&swapon_rwsem);
@@ -1547,7 +1532,6 @@ static void del_from_avail_list(struct swap_info_struct *si, bool swapoff)
 {
 	unsigned long pages;
 
-	spin_lock(&swap_avail_lock);
 	spin_lock(&swap_queue_update_lock);
 
 	/*
@@ -1567,10 +1551,8 @@ static void del_from_avail_list(struct swap_info_struct *si, bool swapoff)
 	}
 
 	swap_queue_mask(si);
-	plist_del(&si->avail_list, &swap_avail_head);
 skip:
 	spin_unlock(&swap_queue_update_lock);
-	spin_unlock(&swap_avail_lock);
 }
 
 /* SWAP_USAGE_OFFLIST_BIT can only be cleared by this helper. */
@@ -1579,7 +1561,6 @@ static void add_to_avail_list(struct swap_info_struct *si)
 	long val;
 	unsigned long pages;
 
-	spin_lock(&swap_avail_lock);
 	spin_lock(&swap_queue_update_lock);
 
 	/*
@@ -1610,7 +1591,6 @@ static void add_to_avail_list(struct swap_info_struct *si)
 	}
 
 	swap_queue_unmask(si);
-	plist_add(&si->avail_list, &swap_avail_head);
 skip:
 	spin_unlock(&swap_queue_update_lock);
 }
@@ -3622,7 +3602,6 @@ static struct swap_info_struct *alloc_swap_info(void)
 	}
 	p->swap_extent_root = RB_ROOT;
 	plist_node_init(&p->list, 0);
-	plist_node_init(&p->avail_list, 0);
 	p->flags = SWP_USED;
 	percpu_up_write(&swapon_rwsem);
 	if (defer) {
@@ -4030,7 +4009,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	 */
 	si->prio = prio;
 	si->list.prio = -si->prio;
-	si->avail_list.prio = -si->prio;
 	si->swap_file = swap_file;
 
 	/* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
@@ -4163,14 +4141,14 @@ void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
 	if (current->throttle_disk)
 		return;
 
-	spin_lock(&swap_avail_lock);
-	plist_for_each_entry(si, &swap_avail_head, avail_list) {
-		if (si->bdev) {
+	percpu_down_read(&swapon_rwsem);
+	plist_for_each_entry(si, &swap_active_head, list) {
+		if ((si->flags & SWP_WRITEOK) && si->bdev) {
 			blkcg_schedule_throttle(si->bdev->bd_disk, true);
 			break;
 		}
 	}
-	spin_unlock(&swap_avail_lock);
+	percpu_up_read(&swapon_rwsem);
 }
 #endif
 

-- 
2.55.0

[PATCH RFC 11/13] mm/swap: perform sync discard on single device more proactively

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

Previous commit 9fb749cd15078 ("mm, swap: do not perform synchronous
discard during allocation") added a workaround-style global sync discard
when all devices are drained to prevent OOM. It wasn't an optimal
solution and in the discussion [1], it's preferred to discard more
proactively, and in the scope of a single device instead of globally.
That wasn't achievable due to the limitations of the previous swap
allocation design, or would have ended up too complex.

Now with the new workflow and swap cluster cache moved inside the device
scope, we can easily achieve that. So drop the old workaround and
implement a more proactive and simpler solution. Doing discard when the
free list is drained will prevent fragmentation better and avoid a
global discard scan.

Link: https://lore.kernel.org/all/CAMgjq7CsYhEjvtN85XGkrONYAJxve7gG593TFeOGV-oax++kWA@mail.gmail.com/ [1]
Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/swapfile.c | 88 ++++++++++++++++++++++-------------------------------------
 1 file changed, 33 insertions(+), 55 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 01240c4c9db3..74a50c39da60 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -498,6 +498,26 @@ static long swap_usage_in_pages(struct swap_info_struct *si)
 	return atomic_long_read(&si->inuse_pages) & SWAP_USAGE_COUNTER_MASK;
 }
 
+/*
+ * Serialize the allocation on single CPU or globally to avoid
+ * fragmentation and make the workflow easier to follow.
+ */
+static void swap_alloc_lock_device(struct swap_info_struct *si)
+{
+	if (si->flags & SWP_SOLIDSTATE)
+		local_lock(&si->percpu_cluster->lock);
+	else
+		spin_lock(&si->global_cluster->lock);
+}
+
+static void swap_alloc_unlock_device(struct swap_info_struct *si)
+{
+	if (si->flags & SWP_SOLIDSTATE)
+		local_unlock(&si->percpu_cluster->lock);
+	else
+		spin_unlock(&si->global_cluster->lock);
+}
+
 /* Reclaim the swap entry anyway if possible */
 #define TTRS_ANYWAY		0x1
 /*
@@ -884,10 +904,7 @@ swap_cluster_populate(struct swap_info_struct *si,
 	 * the potential recursive allocation is limited.
 	 */
 	spin_unlock(&ci->lock);
-	if (si->flags & SWP_SOLIDSTATE)
-		local_unlock(&si->percpu_cluster->lock);
-	else
-		spin_unlock(&si->global_cluster->lock);
+	swap_alloc_unlock_device(si);
 
 	ret = swap_cluster_alloc_table(ci, __GFP_HIGH | __GFP_NOMEMALLOC |
 					   GFP_KERNEL);
@@ -900,10 +917,7 @@ swap_cluster_populate(struct swap_info_struct *si,
 	 * could happen with ignoring the percpu cluster is fragmentation,
 	 * which is acceptable since this fallback and race is rare.
 	 */
-	if (si->flags & SWP_SOLIDSTATE)
-		local_lock(&si->percpu_cluster->lock);
-	else
-		spin_lock(&si->global_cluster->lock);
+	swap_alloc_lock_device(si);
 	spin_lock(&ci->lock);
 
 	if (ret) {
@@ -1439,15 +1453,12 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 	if (order && !(si->flags & SWP_BLKDEV))
 		return 0;
 
-	if (si->flags & SWP_SOLIDSTATE) {
-		/* Fast path using per CPU cluster */
-		local_lock(&si->percpu_cluster->lock);
+restart:
+	swap_alloc_lock_device(si);
+	if (si->flags & SWP_SOLIDSTATE)
 		offset = __this_cpu_read(si->percpu_cluster->next[order]);
-	} else {
-		/* Serialize HDD SWAP allocation for each device. */
-		spin_lock(&si->global_cluster->lock);
+	else
 		offset = si->global_cluster->next[order];
-	}
 
 	if (offset != SWAP_ENTRY_INVALID) {
 		ci = swap_cluster_lock(si, offset);
@@ -1471,6 +1482,12 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 		found = alloc_swap_scan_list(si, &si->free_clusters, folio, false);
 		if (found)
 			goto done;
+
+		if (!list_empty(&si->discard_clusters)) {
+			swap_alloc_unlock_device(si);
+			swap_do_scheduled_discard(si);
+			goto restart;
+		}
 	}
 
 	if (order < PMD_ORDER) {
@@ -1519,10 +1536,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 			goto done;
 	}
 done:
-	if (si->flags & SWP_SOLIDSTATE)
-		local_unlock(&si->percpu_cluster->lock);
-	else
-		spin_unlock(&si->global_cluster->lock);
+	swap_alloc_unlock_device(si);
 
 	return found;
 }
@@ -1711,36 +1725,6 @@ static int swap_alloc_entry(struct folio *folio)
 	return ret;
 }
 
-/*
- * Discard pending clusters in a synchronized way when under high pressure.
- * Return: true if any cluster is discarded.
- */
-static bool swap_sync_discard(void)
-{
-	bool ret = false;
-	struct swap_info_struct *si, *next;
-
-	percpu_down_read(&swapon_rwsem);
-start_over:
-	plist_for_each_entry_safe(si, next, &swap_active_head, list) {
-		percpu_up_read(&swapon_rwsem);
-		if (get_swap_device_info(si)) {
-			if (si->flags & SWP_PAGE_DISCARD)
-				ret = swap_do_scheduled_discard(si);
-			put_swap_device(si);
-		}
-		if (ret)
-			return true;
-
-		percpu_down_read(&swapon_rwsem);
-		if (plist_node_empty(&next->list))
-			goto start_over;
-	}
-	percpu_up_read(&swapon_rwsem);
-
-	return false;
-}
-
 static int swap_extend_table_alloc(struct swap_info_struct *si,
 				   struct swap_cluster_info *ci,
 				   unsigned int ci_off, gfp_t gfp)
@@ -2045,14 +2029,8 @@ int folio_alloc_swap(struct folio *folio)
 		}
 	}
 
-again:
 	ret = swap_alloc_entry(folio);
 
-	if (!order && unlikely(!folio_test_swapcache(folio))) {
-		if (swap_sync_discard())
-			goto again;
-	}
-
 	/* Need to call this even if allocation failed, for MEMCG_SWAP_FAIL. */
 	if (unlikely(mem_cgroup_try_charge_swap(folio)))
 		swap_cache_del_folio(folio);

-- 
2.55.0

[PATCH RFC 12/13] mm/swap: drop swap active plist

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

The swap_active_head plist tracked all writable swap devices, ordered
by priority.  Now that every consumer that needed priority ordering
has moved to the priority queue, the remaining users only need to
find any writable device.

Remove swap_active_head, the associated plist_node from
swap_info_struct, and migrate the remaining plist operations.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 include/linux/swap.h |  1 -
 mm/swapfile.c        | 26 +++++---------------------
 2 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 189c9898641b..04767540972b 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -267,7 +267,6 @@ struct swap_info_struct {
 	struct percpu_ref users;	/* indicate and keep swap device valid. */
 	unsigned long	flags;		/* SWP_USED etc: see above */
 	signed short	prio;		/* swap priority of this type */
-	struct plist_node list;		/* entry in swap_active_head */
 	signed char	type;		/* strange name for an index */
 	unsigned int	max;		/* size of this swap device */
 	struct swap_cluster_info *cluster_info; /* cluster info. Only for SSD */
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 74a50c39da60..f93863309a5a 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -41,7 +41,6 @@
 #include <linux/completion.h>
 #include <linux/suspend.h>
 #include <linux/zswap.h>
-#include <linux/plist.h>
 
 #include <asm/tlbflush.h>
 #include <linux/leafops.h>
@@ -90,12 +89,6 @@ bool swap_migration_ad_supported;
 static const char Bad_file[] = "Bad swap file entry ";
 static const char Bad_offset[] = "Bad swap offset entry ";
 
-/*
- * all active swap_info_structs
- * protected with swapon_rwsem, and ordered by priority.
- */
-static PLIST_HEAD(swap_active_head);
-
 static inline struct swap_info_struct *__swap_iter(int *i, unsigned long flag)
 {
 	lockdep_assert_held(&swapon_rwsem);
@@ -3212,7 +3205,6 @@ static void swap_device_enable(struct swap_info_struct *si)
 	spin_unlock(&swap_queue_update_lock);
 	atomic_long_add(si->pages, &nr_swap_pages);
 	total_swap_pages += si->pages;
-	plist_add(&si->list, &swap_active_head);
 	percpu_up_write(&swapon_rwsem);
 
 	add_to_avail_list(si);
@@ -3244,7 +3236,6 @@ static int swap_device_disable(struct swap_info_struct *si)
 	spin_lock(&swap_queue_update_lock);
 	si->flags &= ~SWP_WRITEOK;
 	spin_unlock(&swap_queue_update_lock);
-	plist_del(&si->list, &swap_active_head);
 	total_swap_pages -= si->pages;
 	atomic_long_sub(si->pages, &nr_swap_pages);
 	del_from_avail_list(si, true);
@@ -3306,9 +3297,8 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 
 	mapping = victim->f_mapping;
 	percpu_down_read(&swapon_rwsem);
-	plist_for_each_entry(p, &swap_active_head, list) {
-		if (p->flags & SWP_WRITEOK &&
-		    p->swap_file->f_mapping == mapping) {
+	for_each_avail_swap(p) {
+		if (p->swap_file->f_mapping == mapping) {
 			found = 1;
 			break;
 		}
@@ -3579,7 +3569,6 @@ static struct swap_info_struct *alloc_swap_info(void)
 		 */
 	}
 	p->swap_extent_root = RB_ROOT;
-	plist_node_init(&p->list, 0);
 	p->flags = SWP_USED;
 	percpu_up_write(&swapon_rwsem);
 	if (defer) {
@@ -3981,12 +3970,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	if (swap_flags & SWAP_FLAG_PREFER)
 		prio = swap_flags & SWAP_FLAG_PRIO_MASK;
 
-	/*
-	 * The plist prio is negated because plist ordering is
-	 * low-to-high, while swap ordering is high-to-low
-	 */
 	si->prio = prio;
-	si->list.prio = -si->prio;
 	si->swap_file = swap_file;
 
 	/* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
@@ -4096,7 +4080,7 @@ int swap_dup_entry_direct(swp_entry_t entry)
 #if defined(CONFIG_MEMCG) && defined(CONFIG_BLK_CGROUP)
 static bool __has_usable_swap(void)
 {
-	return !plist_head_empty(&swap_active_head);
+	return READ_ONCE(total_swap_pages) > 0;
 }
 
 void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
@@ -4120,8 +4104,8 @@ void __folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
 		return;
 
 	percpu_down_read(&swapon_rwsem);
-	plist_for_each_entry(si, &swap_active_head, list) {
-		if ((si->flags & SWP_WRITEOK) && si->bdev) {
+	for_each_avail_swap(si) {
+		if (si->bdev) {
 			blkcg_schedule_throttle(si->bdev->bd_disk, true);
 			break;
 		}

-- 
2.55.0

[PATCH RFC 13/13] lib/plist.c: remove requeue function

[Kairui Song via B4 Relay]

From: Kairui Song <kasong@tencent.com>

Now the last user of plist requeue is gone, this function can be
removed.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 include/linux/plist.h |  2 --
 lib/plist.c           | 64 ---------------------------------------------------
 2 files changed, 66 deletions(-)

diff --git a/include/linux/plist.h b/include/linux/plist.h
index 16cf4355b5c1..73d359c1a053 100644
--- a/include/linux/plist.h
+++ b/include/linux/plist.h
@@ -132,8 +132,6 @@ static inline void plist_node_init(struct plist_node *node, int prio)
 extern void plist_add(struct plist_node *node, struct plist_head *head);
 extern void plist_del(struct plist_node *node, struct plist_head *head);
 
-extern void plist_requeue(struct plist_node *node, struct plist_head *head);
-
 /**
  * plist_for_each - iterate over the plist
  * @pos:	the type * to use as a loop counter
diff --git a/lib/plist.c b/lib/plist.c
index a5bef38add43..b05ae7ffea87 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -142,58 +142,6 @@ void plist_del(struct plist_node *node, struct plist_head *head)
 	plist_check_head(head);
 }
 
-/**
- * plist_requeue - Requeue @node at end of same-prio entries.
- *
- * This is essentially an optimized plist_del() followed by
- * plist_add().  It moves an entry already in the plist to
- * after any other same-priority entries.
- *
- * @node:	&struct plist_node pointer - entry to be moved
- * @head:	&struct plist_head pointer - list head
- */
-void plist_requeue(struct plist_node *node, struct plist_head *head)
-{
-	struct plist_node *iter;
-	struct list_head *node_next = &head->node_list;
-
-	plist_check_head(head);
-	BUG_ON(plist_head_empty(head));
-	BUG_ON(plist_node_empty(node));
-
-	if (node == plist_last(head))
-		return;
-
-	iter = plist_next(node);
-
-	if (node->prio != iter->prio)
-		return;
-
-	plist_del(node, head);
-
-	/*
-	 * After plist_del(), iter is the replacement of the node.  If the node
-	 * was on prio_list, take shortcut to find node_next instead of looping.
-	 */
-	if (!list_empty(&iter->prio_list)) {
-		iter = list_entry(iter->prio_list.next, struct plist_node,
-				  prio_list);
-		node_next = &iter->node_list;
-		goto queue;
-	}
-
-	plist_for_each_continue(iter, head) {
-		if (node->prio != iter->prio) {
-			node_next = &iter->node_list;
-			break;
-		}
-	}
-queue:
-	list_add_tail(&node->node_list, node_next);
-
-	plist_check_head(head);
-}
-
 #ifdef CONFIG_DEBUG_PLIST
 #include <linux/sched.h>
 #include <linux/sched/clock.h>
@@ -231,14 +179,6 @@ static void __init plist_test_check(int nr_expect)
 	BUG_ON(prio_pos->prio_list.next != &first->prio_list);
 }
 
-static void __init plist_test_requeue(struct plist_node *node)
-{
-	plist_requeue(node, &test_head);
-
-	if (node != plist_last(&test_head))
-		BUG_ON(node->prio == plist_next(node)->prio);
-}
-
 static int  __init plist_test(void)
 {
 	int nr_expect = 0, i, loop;
@@ -262,10 +202,6 @@ static int  __init plist_test(void)
 			nr_expect--;
 		}
 		plist_test_check(nr_expect);
-		if (!plist_node_empty(test_node + i)) {
-			plist_test_requeue(test_node + i);
-			plist_test_check(nr_expect);
-		}
 	}
 
 	for (i = 0; i < ARRAY_SIZE(test_node); i++) {

-- 
2.55.0

Re: [PATCH RFC 09/13] mm/swap: add priority queue for swap device allocation

[Youngjun Park]

On Tue, Jul 14, 2026 at 01:25:47AM +0800, Kairui Song via B4 Relay wrote:
> From: Kairui Song <kasong@tencent.com>

Hi Kairui,

Thanks for the patch :)
I like the idea. A few questions and comments on the core implementation below.

I'll share my higher-level thoughts (e.g. on a tunable interface for the round-robin policy) 
separately in a follow-up.

> +/*
> + * All available swap_info_structs are grouped by priority rings, the rings
> + * are ordered in a queue by priority (lower prio value = higher priority).

looks like comment updated needed :)

[...]
> +
> +/*
> + * The ring is protected by swapon_rwsem so updating it is costly. To make
> + * the allocator and other users skip full devices faster, the lowest bit of
> + * a device pointer is used to mark it disabled (temporarily unavailable).
> + * This relies on struct swap_info_struct being sufficiently
> + * aligned (guaranteed by kmalloc).
> + */
> +#define SWAP_DEVICE_MASKED_SHIFT	0
> +#define SWAP_DEVICE_MASKED_BIT		BIT(SWAP_DEVICE_MASKED_SHIFT)

Unlike the old plist, full devices stay in the ring, so every
allocation keeps walking past them. When the higher priority
devices are all full, each swap out pays one masked pointer check
per full device before reaching a usable one. That is also the
steady state for swap tiering, so I want to be sure it stays cheap.
It looks like a deliberate trade-off of the static ring design.
What do you think?

[...]

> +static struct swap_info_struct *swap_queue_get_device(long nr_alloc, int nr_iter)
> +{
> +	bool rotate = false;
> +	struct swap_info_struct *si;
> +	struct swap_ring_iterator *ri;
> +	struct swap_prio_ring *ring;
> +	unsigned int queue_idx;
> +
> +	if (!swap_queue_len)
> +		return ERR_PTR(-ENOENT);
> +
> +	queue_idx = 0;
> +	while (nr_iter >= swap_queue[queue_idx]->size) {
> +		nr_iter -= swap_queue[queue_idx]->size;
> +		if (++queue_idx >= swap_queue_len)
> +			return ERR_PTR(-ENOENT);
> +	}
> +
> +	ring = swap_queue[queue_idx];
> +	local_lock(&swap_queue_readers->lock);
> +	ri = this_cpu_ptr(&swap_queue_readers->ri[queue_idx]);
> +	/* Rotate while iterating the ring, just not on the first try */
> +	if (nr_iter)
> +		rotate = true;
> +	else if (ri->rr_counter < nr_alloc)
> +		rotate = true;
> +	else if (ri->offset >= ring->size)
> +		rotate = true;
> +	if (rotate) {
> +		ri->offset++;
> +		ri->offset %= ring->size;
> +		ri->rr_counter = SWAP_ROUND_ROBIN_QUOTA;
> +	}
> +	ri->rr_counter -= nr_alloc;
> +	si = READ_ONCE(ring->dev[ri->offset]);
> +	local_unlock(&swap_queue_readers->lock);
> +
> +	if (swap_device_masked(si))
> +		return ERR_PTR(-EBUSY);
> +
> +	si = swap_device_unmask_ptr(si);
> +	return si;
> +}

Tasks interleaving on the same CPU can rotate the ring more than
once for a single failure, since the lock is dropped between two
get_device() calls of one allocation. A retry walk can then visit
one device twice and skip another. It is transient and looks
harmless, and every fix I could think of costs more than it is
worth.... 

Unless you have a better idea, how about noting it in a
comment?

> -/* Rotate the device and switch to a new cluster */
> -static void swap_alloc_entry(struct folio *folio)
> +static int swap_alloc_entry(struct folio *folio)
>  {
> -	struct swap_info_struct *si, *next;
> +	long nr_pages = folio_nr_pages(folio);
> +	struct swap_info_struct *si;
> +	int nr_iter, ret;
>
> -	spin_lock(&swap_avail_lock);
> -start_over:
> -	plist_for_each_entry_safe(si, next, &swap_avail_head, avail_list) {
> -		/* Rotate the device and switch to a new cluster */
> -		plist_requeue(&si->avail_list, &swap_avail_head);
> -		spin_unlock(&swap_avail_lock);
> -		if (get_swap_device_info(si)) {
> -			cluster_alloc_swap_entry(si, folio);
> -			put_swap_device(si);
> -			if (folio_test_swapcache(folio))
> -				return;
> -			if (folio_test_large(folio))
> -				return;
> +	percpu_down_read(&swapon_rwsem);

The local lock is only held inside swap_queue_get_device() now, so
the task can migrate between device selection and
cluster_alloc_swap_entry(). Then rr_counter is charged on one CPU
while the pages land in another CPU's cluster, so the pacing of
one cluster per visit does not hold anymore.

Since you want the loop to stay sleepable, how about
migrate_disable() around the loop?

> +	for (nr_iter = 0;; nr_iter++) {
> +		si = swap_queue_get_device(nr_pages, nr_iter);

When a ring has a single device, which is probably the most common
setup, the iterator does nothing useful. Would a fast path that
skips the local_lock and just reads ring->dev[0] for size == 1
rings sequentially be worth it? swapon_rwsem keeps the queue stable, so choosing
between the two paths should be race free, I think.

> +		if (IS_ERR(si)) {
> +			ret = PTR_ERR(si);
> +			if (ret == -EBUSY)
> +				continue;
> +			break;
> +		}
> +		cluster_alloc_swap_entry(si, folio);
> +
> +		if (folio_test_swapcache(folio)) {
> +			ret = 0;
> +			break;
>		}

[...]

>	del_from_avail_list(si, true);
> +	percpu_up_write(&swapon_rwsem);
>
>	/*
>	 * Swap allocator doesn't touch si lock, so looping through all
> @@ -3042,6 +3375,9 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>		return err;
>	}
>
> +	percpu_down_write(&swapon_rwsem);
> +	swap_queue_del(p);
> +
>	/*
>	 * Wait for swap operations protected by get/put_swap_device()
>	 * to complete.  Because of synchronize_rcu() here, all swap
> @@ -3062,7 +3398,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>	if (!(p->flags & SWP_SOLIDSTATE))
>		atomic_dec(&nr_rotate_swap);
>
> -	percpu_down_write(&swapon_rwsem);
>	spin_lock(&p->lock);
>	drain_mmlist();

synchronize_rcu() can take a while and it runs here with the write
lock held, together with wait_for_completion() and flush_work(), so
a single swapoff can stall all swap allocation during a little bit long time.

Only swap_queue_del() and the final teardown seem to need the write
lock. If my assumption is right, how about dropping it after
swap_queue_del() and re-taking it for the teardown? 

> @@ -3700,6 +4035,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>
>	/* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
>	percpu_ref_resurrect(&si->users);
> +	percpu_down_write(&swapon_rwsem);
> +	error = swap_queue_add(si);
> +	percpu_up_write(&swapon_rwsem);
> +	if (error)
> +		goto free_swap_zswap;
> +
>	swap_device_enable(si);

Thanks!
Youngjun Park

Re: [PATCH RFC 09/13] mm/swap: add priority queue for swap device allocation

[Jihan LIN]

Hi Kairui,

Thanks for this series! I noticed a few issues in this patch:

[...]

>+static void swap_queue_install_readers(struct swap_queue_reader __percpu *readers)
>+{
>+	int ring_idx, cpu;
>+	struct swap_prio_ring *ring;
>+
>+	free_percpu(swap_queue_readers);
>+	swap_queue_readers = readers;
>+	if (!readers)
>+		return;
>+
>+	/* Distribute each CPU's swap IO fairly across devices. */
>+	for_each_possible_cpu(cpu) {
>+		local_lock_init(&per_cpu_ptr(readers, cpu)->lock);
>+
>+		for (ring_idx = 0; ring_idx < swap_queue_len; ring_idx++) {
>+			ring = swap_queue[ring_idx];
>+			per_cpu_ptr(readers, cpu)->ri[ring_idx].offset =
>+				    cpu % ring->size;
>+		}
>+	}
>+}
>+static struct swap_info_struct *swap_queue_get_device(long nr_alloc, int nr_iter)
>+{
>+	bool rotate = false;
>+	struct swap_info_struct *si;
>+	struct swap_ring_iterator *ri;
>+	struct swap_prio_ring *ring;
>+	unsigned int queue_idx;
>+
>+	if (!swap_queue_len)
>+		return ERR_PTR(-ENOENT);
>+
>+	queue_idx = 0;
>+	while (nr_iter >= swap_queue[queue_idx]->size) {
>+		nr_iter -= swap_queue[queue_idx]->size;
>+		if (++queue_idx >= swap_queue_len)
>+			return ERR_PTR(-ENOENT);
>+	}
>+
>+	ring = swap_queue[queue_idx];
>+	local_lock(&swap_queue_readers->lock);
>+	ri = this_cpu_ptr(&swap_queue_readers->ri[queue_idx]);
>+	/* Rotate while iterating the ring, just not on the first try */
>+	if (nr_iter)
>+		rotate = true;
>+	else if (ri->rr_counter < nr_alloc)
>+		rotate = true;
>+	else if (ri->offset >= ring->size)
>+		rotate = true;
>+	if (rotate) {
>+		ri->offset++;
>+		ri->offset %= ring->size;
>+		ri->rr_counter = SWAP_ROUND_ROBIN_QUOTA;
>+	}
>+	ri->rr_counter -= nr_alloc;

ri->rr_counter is left at 0 so the first allocation on each CPU hits
ri->rr_counter < nr_alloc and rotates, shifting offset away from
intended cpu % ring->size start. Initializing rr_counter to
SWAP_ROUND_ROBIN_QUOTA would avoid this, or is this initial rotation
perhaps intentional?

[...]

> @@ -1245,12 +1580,12 @@ static void add_to_avail_list(struct swap_info_struct *si)
> 	unsigned long pages;
> 
> 	spin_lock(&swap_avail_lock);
>+	spin_lock(&swap_queue_update_lock);
> 
> 	/*
>-	 * Add the device to the avail list if SWP_WRITEOK is set and
>-	 * SWAP_USAGE_OFFLIST_BIT is still set. Swapoff clears
>-	 * SWP_WRITEOK first, so the device won't be re-added after
>-	 * swapoff starts unless swap_device_enable resurrects it.
>+	 * Mark the device as avail if SWP_WRITEOK is set. Swapoff clears
>+	 * SWP_WRITEOK first, so check that first so the device won't be
>+	 * re-added after swapoff started.
> 	 */
> 	if (!(si->flags & SWP_WRITEOK))
> 		goto skip;
>@@ -1261,9 +1596,10 @@ static void add_to_avail_list(struct swap_info_struct *si)
> 	val = atomic_long_fetch_and_relaxed(~SWAP_USAGE_OFFLIST_BIT, &si->inuse_pages);
> 
> 	/*
>-	 * When device is full and device is on the plist, only one updater will
>-	 * see (inuse_pages == si->pages) and will call del_from_avail_list. If
>-	 * that updater happen to be here, just skip adding.
>+	 * When device is full and marked as available, one reader will see
>+	 * (inuse_pages == si->pages) and should mark it as unavailable and
>+	 * set SWAP_USAGE_OFFLIST_BIT. If that updater happens to be here, just
>+	 * skip the rest.
> 	 */
> 	pages = si->pages;
> 	if (val == pages) {
>@@ -1273,10 +1609,10 @@ static void add_to_avail_list(struct swap_info_struct *si)
> 			goto skip;
> 	}
> 
>+	swap_queue_unmask(si);
> 	plist_add(&si->avail_list, &swap_avail_head);
>-
> skip:
>-	spin_unlock(&swap_avail_lock);
>+	spin_unlock(&swap_queue_update_lock);
> }
swap_avail_lock is acquired but never released when add_to_avail_list()
returns.

[...]

>@@ -3700,6 +4035,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>
> 	/* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
> 	percpu_ref_resurrect(&si->users);
>+	percpu_down_write(&swapon_rwsem);
>+	error = swap_queue_add(si);
>+	percpu_up_write(&swapon_rwsem);
>+	if (error)
>+		goto free_swap_zswap;
>+
>	swap_device_enable(si);
The swapfile is inserted into the ring unmasked before SWP_WRITEOK was set in
swap_device_enable(). Allocators can pick it and get -EBUSY from
get_swap_device_info(). How about adding masked and then letting
swap_device_enable() unmask the new swapfile?

---
Thanks!
Jihan LIN

Re: [PATCH RFC 09/13] mm/swap: add priority queue for swap device allocation

[Kairui Song]

On Wed, Jul 15, 2026 at 2:10 AM Youngjun Park <youngjun.park@lge.com> wrote:
>
> On Tue, Jul 14, 2026 at 01:25:47AM +0800, Kairui Song via B4 Relay wrote:
> > From: Kairui Song <kasong@tencent.com>
>
> Hi Kairui,
>
> Thanks for the patch :)
> I like the idea. A few questions and comments on the core implementation below.
>
> I'll share my higher-level thoughts (e.g. on a tunable interface for the round-robin policy)
> separately in a follow-up.

Thanks for the review!

> > +/*
> > + * All available swap_info_structs are grouped by priority rings, the rings
> > + * are ordered in a queue by priority (lower prio value = higher priority).
>
> looks like comment updated needed :)

Nice catch :), I got confused by the priority defination again, will fix it.

> [...]
> > +
> > +/*
> > + * The ring is protected by swapon_rwsem so updating it is costly. To make
> > + * the allocator and other users skip full devices faster, the lowest bit of
> > + * a device pointer is used to mark it disabled (temporarily unavailable).
> > + * This relies on struct swap_info_struct being sufficiently
> > + * aligned (guaranteed by kmalloc).
> > + */
> > +#define SWAP_DEVICE_MASKED_SHIFT     0
> > +#define SWAP_DEVICE_MASKED_BIT               BIT(SWAP_DEVICE_MASKED_SHIFT)
>
> Unlike the old plist, full devices stay in the ring, so every
> allocation keeps walking past them. When the higher priority
> devices are all full, each swap out pays one masked pointer check
> per full device before reaching a usable one. That is also the
> steady state for swap tiering, so I want to be sure it stays cheap.
> It looks like a deliberate trade-off of the static ring design.
> What do you think?

Right, this should be super cheap as modern processors are really good
at walking arrays. We have at most 32 swap devices in most cases, only
4 cache lines if they are in the same top ring and I think the CPU
will only spend a dozen cycles on them. And rings are mostly
read-only; they will stay cached for hot access.

If they fallback to secondary rings... I think these devices are
mostly performance insensitive anyway, and CPUs are still pretty good
at reading the readonly arrays.

I did plan to add a ring->avail to skip completely empty rings, or use
cmpxchg & a seq style lock (don't shrink the ring, just shift the
content on full device removal) to update the ring content, to archive
0 overhead if we got more full devices, it works but that's ends up
super complex with no measurable gain.

Maybe we can implement these if this percpu reader queue can be used
as a generic infrastructure for other components, or if we grow to
support hundreds of swaps someday, not really a issue for now I think.

>
> [...]
>
> > +static struct swap_info_struct *swap_queue_get_device(long nr_alloc, int nr_iter)
> > +{
> > +     bool rotate = false;
> > +     struct swap_info_struct *si;
> > +     struct swap_ring_iterator *ri;
> > +     struct swap_prio_ring *ring;
> > +     unsigned int queue_idx;
> > +
> > +     if (!swap_queue_len)
> > +             return ERR_PTR(-ENOENT);
> > +
> > +     queue_idx = 0;
> > +     while (nr_iter >= swap_queue[queue_idx]->size) {
> > +             nr_iter -= swap_queue[queue_idx]->size;
> > +             if (++queue_idx >= swap_queue_len)
> > +                     return ERR_PTR(-ENOENT);
> > +     }
> > +
> > +     ring = swap_queue[queue_idx];
> > +     local_lock(&swap_queue_readers->lock);
> > +     ri = this_cpu_ptr(&swap_queue_readers->ri[queue_idx]);
> > +     /* Rotate while iterating the ring, just not on the first try */
> > +     if (nr_iter)
> > +             rotate = true;
> > +     else if (ri->rr_counter < nr_alloc)
> > +             rotate = true;
> > +     else if (ri->offset >= ring->size)
> > +             rotate = true;
> > +     if (rotate) {
> > +             ri->offset++;
> > +             ri->offset %= ring->size;
> > +             ri->rr_counter = SWAP_ROUND_ROBIN_QUOTA;
> > +     }
> > +     ri->rr_counter -= nr_alloc;
> > +     si = READ_ONCE(ring->dev[ri->offset]);
> > +     local_unlock(&swap_queue_readers->lock);
> > +
> > +     if (swap_device_masked(si))
> > +             return ERR_PTR(-EBUSY);
> > +
> > +     si = swap_device_unmask_ptr(si);
> > +     return si;
> > +}
>
> Tasks interleaving on the same CPU can rotate the ring more than
> once for a single failure, since the lock is dropped between two
> get_device() calls of one allocation. A retry walk can then visit
> one device twice and skip another. It is transient and looks
> harmless, and every fix I could think of costs more than it is
> worth....

You mean dropping the local lock? Rotating it a bit more might be
harmless I think, if that happens randomly, then the rotation is still
fair.

I did think about keep holding the local lock during the whole
process, but making this unsleepable is a trade-off and reduce future
flexibility... the local lock of the cluster below is already causing
some trouble for swap table allocation so I hoped we could avoid that.
So far, stress test didn't show any fragmentation issue.

>
> Unless you have a better idea, how about noting it in a
> comment?

Yeah, we can definitely doc that more cleanly.

> > -/* Rotate the device and switch to a new cluster */
> > -static void swap_alloc_entry(struct folio *folio)
> > +static int swap_alloc_entry(struct folio *folio)
> >  {
> > -     struct swap_info_struct *si, *next;
> > +     long nr_pages = folio_nr_pages(folio);
> > +     struct swap_info_struct *si;
> > +     int nr_iter, ret;
> >
> > -     spin_lock(&swap_avail_lock);
> > -start_over:
> > -     plist_for_each_entry_safe(si, next, &swap_avail_head, avail_list) {
> > -             /* Rotate the device and switch to a new cluster */
> > -             plist_requeue(&si->avail_list, &swap_avail_head);
> > -             spin_unlock(&swap_avail_lock);
> > -             if (get_swap_device_info(si)) {
> > -                     cluster_alloc_swap_entry(si, folio);
> > -                     put_swap_device(si);
> > -                     if (folio_test_swapcache(folio))
> > -                             return;
> > -                     if (folio_test_large(folio))
> > -                             return;
> > +     percpu_down_read(&swapon_rwsem);
>
> The local lock is only held inside swap_queue_get_device() now, so
> the task can migrate between device selection and
> cluster_alloc_swap_entry(). Then rr_counter is charged on one CPU
> while the pages land in another CPU's cluster, so the pacing of
> one cluster per visit does not hold anymore.
>
> Since you want the loop to stay sleepable, how about
> migrate_disable() around the loop?

That sounds like a good idea to avoid a potential race. I think these
races are harmless though.

>
> > +     for (nr_iter = 0;; nr_iter++) {
> > +             si = swap_queue_get_device(nr_pages, nr_iter);
>
> When a ring has a single device, which is probably the most common
> setup, the iterator does nothing useful. Would a fast path that
> skips the local_lock and just reads ring->dev[0] for size == 1
> rings sequentially be worth it? swapon_rwsem keeps the queue stable, so choosing
> between the two paths should be race free, I think.

Ah, that's a good idea indeed.

> >       del_from_avail_list(si, true);
> > +     percpu_up_write(&swapon_rwsem);
> >
> >       /*
> >        * Swap allocator doesn't touch si lock, so looping through all
> > @@ -3042,6 +3375,9 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
> >               return err;
> >       }
> >
> > +     percpu_down_write(&swapon_rwsem);
> > +     swap_queue_del(p);
> > +
> >       /*
> >        * Wait for swap operations protected by get/put_swap_device()
> >        * to complete.  Because of synchronize_rcu() here, all swap
> > @@ -3062,7 +3398,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
> >       if (!(p->flags & SWP_SOLIDSTATE))
> >               atomic_dec(&nr_rotate_swap);
> >
> > -     percpu_down_write(&swapon_rwsem);
> >       spin_lock(&p->lock);
> >       drain_mmlist();
>
> synchronize_rcu() can take a while and it runs here with the write
> lock held, together with wait_for_completion() and flush_work(), so
> a single swapoff can stall all swap allocation during a little bit long time.
>
> Only swap_queue_del() and the final teardown seem to need the write
> lock. If my assumption is right, how about dropping it after
> swap_queue_del() and re-taking it for the teardown?

Good observation! I noticed we could tidy the locking scope, I skipped
that optimization for the RFC :P. Swapoff is very performance
insensitive IMO, it will put a huge pressure to the system and IO, so
an RCU sync / flush hardly matters. The design is heavily optimized
toward readers (ordinary swapin/swapout). But a tidier implementation
and optimizations are definitely great to have.

Thanks again for the detailed review! I think this can be rebased on
top of your tiering work, there are some conflicts but nothing
fundamental, and I think the ring design actually fits well. I've been
experiement this for a while so I think better send it out to plan out
the next steps, and this series can be used to optimize the tiering
alloc as follow up.

Re: [PATCH RFC 09/13] mm/swap: add priority queue for swap device allocation

[Kairui Song]

On Wed, Jul 15, 2026 at 11:58 PM Jihan LIN <linjh22s@gmail.com> wrote:
>
> Hi Kairui,
>
> Thanks for this series! I noticed a few issues in this patch:

Thanks for the review!

> >+static struct swap_info_struct *swap_queue_get_device(long nr_alloc, int nr_iter)
> >+{
> >+      bool rotate = false;
> >+      struct swap_info_struct *si;
> >+      struct swap_ring_iterator *ri;
> >+      struct swap_prio_ring *ring;
> >+      unsigned int queue_idx;
> >+
> >+      if (!swap_queue_len)
> >+              return ERR_PTR(-ENOENT);
> >+
> >+      queue_idx = 0;
> >+      while (nr_iter >= swap_queue[queue_idx]->size) {
> >+              nr_iter -= swap_queue[queue_idx]->size;
> >+              if (++queue_idx >= swap_queue_len)
> >+                      return ERR_PTR(-ENOENT);
> >+      }
> >+
> >+      ring = swap_queue[queue_idx];
> >+      local_lock(&swap_queue_readers->lock);
> >+      ri = this_cpu_ptr(&swap_queue_readers->ri[queue_idx]);
> >+      /* Rotate while iterating the ring, just not on the first try */
> >+      if (nr_iter)
> >+              rotate = true;
> >+      else if (ri->rr_counter < nr_alloc)
> >+              rotate = true;
> >+      else if (ri->offset >= ring->size)
> >+              rotate = true;
> >+      if (rotate) {
> >+              ri->offset++;
> >+              ri->offset %= ring->size;
> >+              ri->rr_counter = SWAP_ROUND_ROBIN_QUOTA;
> >+      }
> >+      ri->rr_counter -= nr_alloc;
>
> ri->rr_counter is left at 0 so the first allocation on each CPU hits
> ri->rr_counter < nr_alloc and rotates, shifting offset away from
> intended cpu % ring->size start. Initializing rr_counter to
> SWAP_ROUND_ROBIN_QUOTA would avoid this, or is this initial rotation
> perhaps intentional?

Yeah, that's true, it hardly matters as we will do million of
allocation and rotates a lot randomly, so the first allocation is
really trivial. But initlalize it as SWAP_ROUND_ROBIN_QUOTA definately
make it more consistent. Agree on that.

> [...]
>
> > @@ -1245,12 +1580,12 @@ static void add_to_avail_list(struct swap_info_struct *si)
> >       unsigned long pages;
> >
> >       spin_lock(&swap_avail_lock);
> >+      spin_lock(&swap_queue_update_lock);
> >
> >       /*
> >-       * Add the device to the avail list if SWP_WRITEOK is set and
> >-       * SWAP_USAGE_OFFLIST_BIT is still set. Swapoff clears
> >-       * SWP_WRITEOK first, so the device won't be re-added after
> >-       * swapoff starts unless swap_device_enable resurrects it.
> >+       * Mark the device as avail if SWP_WRITEOK is set. Swapoff clears
> >+       * SWP_WRITEOK first, so check that first so the device won't be
> >+       * re-added after swapoff started.
> >        */
> >       if (!(si->flags & SWP_WRITEOK))
> >               goto skip;
> >@@ -1261,9 +1596,10 @@ static void add_to_avail_list(struct swap_info_struct *si)
> >       val = atomic_long_fetch_and_relaxed(~SWAP_USAGE_OFFLIST_BIT, &si->inuse_pages);
> >
> >       /*
> >-       * When device is full and device is on the plist, only one updater will
> >-       * see (inuse_pages == si->pages) and will call del_from_avail_list. If
> >-       * that updater happen to be here, just skip adding.
> >+       * When device is full and marked as available, one reader will see
> >+       * (inuse_pages == si->pages) and should mark it as unavailable and
> >+       * set SWAP_USAGE_OFFLIST_BIT. If that updater happens to be here, just
> >+       * skip the rest.
> >        */
> >       pages = si->pages;
> >       if (val == pages) {
> >@@ -1273,10 +1609,10 @@ static void add_to_avail_list(struct swap_info_struct *si)
> >                       goto skip;
> >       }
> >
> >+      swap_queue_unmask(si);
> >       plist_add(&si->avail_list, &swap_avail_head);
> >-
> > skip:
> >-      spin_unlock(&swap_avail_lock);
> >+      spin_unlock(&swap_queue_update_lock);
> > }
> swap_avail_lock is acquired but never released when add_to_avail_list()
> returns.

Ah, that really a silly error of mine, in the following commit
swap_avail_lock is completely removed, so I didn't notice that during
review or test. Need to fix this intermeidate commit. Thanks!

>
> [...]
>
> >@@ -3700,6 +4035,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
> >
> >       /* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
> >       percpu_ref_resurrect(&si->users);
> >+      percpu_down_write(&swapon_rwsem);
> >+      error = swap_queue_add(si);
> >+      percpu_up_write(&swapon_rwsem);
> >+      if (error)
> >+              goto free_swap_zswap;
> >+
> >       swap_device_enable(si);
> The swapfile is inserted into the ring unmasked before SWP_WRITEOK was set in
> swap_device_enable(). Allocators can pick it and get -EBUSY from
> get_swap_device_info(). How about adding masked and then letting
> swap_device_enable() unmask the new swapfile?

Yeah, that's a good idea. I considered it but thought it wasn't
necessary for the RFC, so I skipped that part. We can definitely do
that, it will also help for some other synchronization issues that are
still present.

Thanks again!