On Tue, Feb 25, 2025 at 2:03 AM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> From: Kairui Song <kasong@xxxxxxxxxxx>
>
> Current allocation workflow first traverses the plist with a global lock
> held, after choosing a device, it uses the percpu cluster on that swap
> device. This commit moves the percpu cluster variable out of being tied
> to individual swap devices, making it a global percpu variable, and will
> be used directly for allocation as a fast path.
>
> The global percpu cluster variable will never point to a HDD device, and
> allocations on a HDD device are still globally serialized.
>
> This improves the allocator performance and prepares for removal of the
> slot cache in later commits. There shouldn't be much observable behavior
> change, except one thing: this changes how swap device allocation
> rotation works.
>
> Currently, each allocation will rotate the plist, and because of the
> existence of slot cache (one order 0 allocation usually returns 64
> entries), swap devices of the same priority are rotated for every 64
> order 0 entries consumed. High order allocations are different, they
> will bypass the slot cache, and so swap device is rotated for every
> 16K, 32K, or up to 2M allocation.
>
> The rotation rule was never clearly defined or documented, it was changed
> several times without mentioning.
>
> After this commit, and once slot cache is gone in later commits, swap
> device rotation will happen for every consumed cluster. Ideally non-HDD
> devices will be rotated if 2M space has been consumed for each order.
> Fragmented clusters will rotate the device faster, which seems OK.
> HDD devices is rotated for every allocation regardless of the allocation
> order, which should be OK too and trivial.
>
> This commit also slightly changes allocation behaviour for slot cache.
> The new added cluster allocation fast path may allocate entries from
> different device to the slot cache, this is not observable from user
> space, only impact performance very slightly, and slot cache will be
> just gone in next commit, so this can be ignored.
>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---
> include/linux/swap.h | 11 ++--
> mm/swapfile.c | 136 +++++++++++++++++++++++++++++--------------
> 2 files changed, 95 insertions(+), 52 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 2fe91c293636..374bffc87427 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -284,12 +284,10 @@ enum swap_cluster_flags {
> #endif
>
> /*
> - * 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.
> + * We keep using same cluster for rotational device so IO will be sequential.
> + * The purpose is to optimize SWAP throughput on these device.
> */
> -struct percpu_cluster {
> - local_lock_t lock; /* Protect the percpu_cluster above */
> +struct swap_sequential_cluster {
> unsigned int next[SWAP_NR_ORDERS]; /* Likely next allocation offset */
> };
>
> @@ -315,8 +313,7 @@ struct swap_info_struct {
> atomic_long_t frag_cluster_nr[SWAP_NR_ORDERS];
> 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 percpu_cluster *global_cluster; /* Use one global cluster for rotating device */
> + 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 */
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index db836670c334..7caaaea95408 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -116,6 +116,18 @@ 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(),
> +};
> +
> static struct swap_info_struct *swap_type_to_swap_info(int type)
> {
> if (type >= MAX_SWAPFILES)
> @@ -539,7 +551,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, there could be percpu_cluster
> + * the CLUSTER_FLAG_DISCARD flag, percpu_swap_cluster could be
> * pointing to it, or ran into by relocate_cluster.
> */
> list_del(&ci->list);
> @@ -805,10 +817,12 @@ static unsigned int alloc_swap_scan_cluster(struct swap_info_struct *si,
> out:
> relocate_cluster(si, ci);
> unlock_cluster(ci);
> - if (si->flags & SWP_SOLIDSTATE)
> - __this_cpu_write(si->percpu_cluster->next[order], next);
> - else
> + if (si->flags & SWP_SOLIDSTATE) {
> + __this_cpu_write(percpu_swap_cluster.si[order], si);
> + __this_cpu_write(percpu_swap_cluster.offset[order], next);
> + } else {
> si->global_cluster->next[order] = next;
> + }
> return found;
> }
>
> @@ -862,9 +876,8 @@ static void swap_reclaim_work(struct work_struct *work)
> }
>
> /*
> - * Try to get swap entries with specified order from current cpu's swap entry
> - * pool (a cluster). This might involve allocating a new cluster for current CPU
> - * too.
> + * Try to allocate swap entries with specified order and try set a new
> + * cluster for current CPU too.
> */
> static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si, int order,
> unsigned char usage)
> @@ -872,18 +885,12 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si, int o
> struct swap_cluster_info *ci;
> unsigned int offset, found = 0;
>
> - 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 {
> + if (!(si->flags & SWP_SOLIDSTATE)) {
> /* Serialize HDD SWAP allocation for each device. */
> spin_lock(&si->global_cluster_lock);
> offset = si->global_cluster->next[order];
> - }
> -
> - if (offset) {
> ci = lock_cluster(si, offset);
> +
> /* Cluster could have been used by another order */
> if (cluster_is_usable(ci, order)) {
> if (cluster_is_empty(ci))
> @@ -973,9 +980,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si, int o
> }
> }
> done:
> - if (si->flags & SWP_SOLIDSTATE)
> - local_unlock(&si->percpu_cluster->lock);
> - else
> + if (!(si->flags & SWP_SOLIDSTATE))
> spin_unlock(&si->global_cluster_lock);
> return found;
> }
> @@ -1196,6 +1201,49 @@ 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 int swap_alloc_fast(swp_entry_t entries[],
> + unsigned char usage,
> + int order, int n_goal)
> +{
> + struct swap_cluster_info *ci;
> + struct swap_info_struct *si;
> + unsigned int offset, found;
> + int n_ret = 0;
> +
> + n_goal = min(n_goal, SWAP_BATCH);
> +
> + /*
> + * 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 0;
Found one issue with this intermediate patch, "si" will be reused upon
swapoff & swapon again. So after the reuse, get_swap_device_info()
returns true and `offset` is not 0, but `offset` is invalid, it could
point to a value larger than si->max if the previous device is larger.
To fix it, need to squash this onto it:
(also include a code fix for cluster_is_usable check, this code error
is fixed by 7/7 but shouldn't be here in the first place)
(replacing __this_cpu_xx with this_cpu_xx because the swapoff flush
will access other CPU's variable now)
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 7caaaea95408..68b40e74be93 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -818,8 +818,8 @@ static unsigned int alloc_swap_scan_cluster(struct
swap_info_struct *si,
relocate_cluster(si, ci);
unlock_cluster(ci);
if (si->flags & SWP_SOLIDSTATE) {
- __this_cpu_write(percpu_swap_cluster.si[order], si);
- __this_cpu_write(percpu_swap_cluster.offset[order], next);
+ this_cpu_write(percpu_swap_cluster.si[order], si);
+ this_cpu_write(percpu_swap_cluster.offset[order], next);
} else {
si->global_cluster->next[order] = next;
}
@@ -1220,15 +1220,17 @@ static int swap_alloc_fast(swp_entry_t entries[],
* 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]);
+ 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 0;
while (offset) {
ci = lock_cluster(si, offset);
- if (!cluster_is_usable(ci, order))
+ if (!cluster_is_usable(ci, order)) {
+ unlock_cluster(ci);
break;
+ }
if (cluster_is_empty(ci))
offset = cluster_offset(si, ci);
found = alloc_swap_scan_cluster(si, ci, offset, order, usage);
@@ -1237,7 +1239,7 @@ static int swap_alloc_fast(swp_entry_t entries[],
entries[n_ret++] = swp_entry(si->type, found);
if (n_ret == n_goal)
break;
- offset = __this_cpu_read(percpu_swap_cluster.offset[order]);
+ offset = this_cpu_read(percpu_swap_cluster.offset[order]);
}
put_swap_device(si);
@@ -2660,6 +2662,27 @@ static void wait_for_allocation(struct
swap_info_struct *si)
}
}
+/*
+ * Called after swap devices reference count is dead, so
+ * neither scan or allocation will go into it.
+ */
+static void flush_percpu_swap_cluster(struct swap_info_struct *si)
+{
+ int cpu;
+ 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, si->users
+ * is dead, so no new users will point to it, flush any
+ * existing cache is enough.
+ */
+ cmpxchg(pcp_si, si, NULL);
+ }
+}
+
+
SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
{
struct swap_info_struct *p = NULL;
@@ -2761,6 +2784,7 @@ 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);
if (p->flags & SWP_CONTINUED)
---
There will be minor conflict in the next commit after this squash, I
will send a V3 shortly to resolve this if there is no objection on
this.
