[RFC PATCH 0/1] mm/swap: keep reclaim batches contiguous on swap

[RFC PATCH 0/1] mm/swap: keep reclaim batches contiguous on swap

[Alexandre Ghiti]

7.3 regressed swap I/O: for the same swap volume, a workload that
rewrites swapped out memory sends many more requests to the device.
mysql/sysbench in a 256M memcg swapping to an NVMe partition, 7.3
compared to the previous version:

  device read requests per transaction        +59%
  device write requests per transaction       +27%
  throughput, iocost off                      no change
  throughput, iocost on                       -19%

Two commits interact:

- Commit 9a42aed48421 ("mm, swap: prefer nonfull over free clusters"),
  in 6.18, makes order-0 allocations fill nonfull clusters before
  taking a free one.

- Commit 3cedb745d354 ("mm: clarify the folio_free_swap() for
  do_swap_page()"), in 7.3, frees the swap slot on an exclusive write
  fault. 7.0 to 7.2 kept the slot, but only as a side effect of commit
  4b34f1d82c65 ("mm, swap: free the swap cache after folio is
  mapped").

The freed slots leave holes in full clusters, which then go back on
the nonfull list. Reclaim allocates slots one folio at a time and
merges the writes afterwards, so a batch of LRU-adjacent folios gets
scattered over those holes and is written with many small requests.
Readahead later finds those pages scattered as well, and needs more
read requests to bring them back.

I tried three fixes, which perform similarly:

A. Revert 3cedb745d354. This keeps the slot on write fault, but undoes
   the intent of that commit and keeps swap space pinned under dirty
   pages.

B. Below 50% swap usage, make order-0 allocations take a free cluster
   before a nonfull one. This is what the allocator already does, at
   any usage, on swap devices with discard enabled (swapon
   --discard=pages). It is about five lines, but the threshold is
   arbitrary, it stops working as swap fills up, and it applies to
   every order-0 allocation, using up the free clusters that
   nonfull-first keeps for mTHP.

C. This patch. Reclaim tells the allocator how many pages of each
   small order the batch is about to swap out, and those allocations
   then follow contiguous free slots, in a nonfull cluster if one has
   them and in a free cluster otherwise. Nothing is reserved, there
   is no threshold, and nonfull-first is kept.

Same workload as above, 7.3 with C relative to 7.3:

  device read requests per transaction        -12%
  device write requests per transaction       -36%
  throughput, iocost off                      no change
  throughput, iocost on                       +41%

I think C is the way to go. I just add the patch to this regression
notice so that we have an idea of what it takes to implement, the
implementation may not be perfect.

I would welcome other ideas.

Alexandre Ghiti (1):
  mm/swap: allocate contiguous swap slots for a reclaim batch

 mm/shmem.c    |   2 +
 mm/swap.h     |  10 +++
 mm/swapfile.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 mm/vmscan.c   |  11 ++-
 4 files changed, 229 insertions(+), 5 deletions(-)

base-commit: fcec4f4bcc1101dbd7e3aff267856b3bbfe128c6

[RFC PATCH 1/1] mm/swap: allocate contiguous swap slots for a reclaim batch

[Alexandre Ghiti]

Reclaim allocates swap slots one folio at a time and only then batches
the writes, merging folios whose slots happen to be contiguous. Order-0
allocations scan the nonfull clusters before taking a free cluster, so
once frees have left holes in otherwise full clusters, a batch of LRU-
adjacent folios is scattered over those holes and written with many
small requests. Since swap slots are freed on exclusive write faults,
this happens with any workload that rewrites swapped out memory.

Before swapping out a batch, record how many pages of each small order
need a slot, and make those allocations follow contiguous free slots,
SWAP_CLUSTER_MAX at a time: in a nonfull cluster if one has them,
otherwise in a free cluster. When neither has them, the rest of the
batch fills the holes as before. Nothing is reserved.

Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
---
 mm/shmem.c    |   2 +
 mm/swap.h     |  10 +++
 mm/swapfile.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++-
 mm/vmscan.c   |  11 ++-
 4 files changed, 229 insertions(+), 5 deletions(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index fd0953d55fd6..1415aeb72cf3 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1850,6 +1850,8 @@ int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
 		folio_test_set_dirty(folio);
 		if (split_folio_to_list(folio, folio_list))
 			goto redirty;
+		if (folio_list)
+			folio_alloc_swap_split(1 << order);
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 		if (order >= HPAGE_PMD_ORDER) {
diff --git a/mm/swap.h b/mm/swap.h
index d5bf21f517dc..c52ad5ee262b 100644
--- a/mm/swap.h
+++ b/mm/swap.h
@@ -243,6 +243,8 @@ extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
  *
  * folio_put_swap(): does the opposite thing of folio_dup_swap().
  */
+void folio_alloc_swap_prepare(struct list_head *folios);
+void folio_alloc_swap_split(unsigned int nr_pages);
 int folio_alloc_swap(struct folio *folio);
 int folio_dup_swap(struct folio *folio, struct page *page);
 void folio_put_swap(struct folio *folio, struct page *page);
@@ -363,6 +365,14 @@ static inline struct swap_info_struct *__swap_entry_to_info(swp_entry_t entry)
 	return NULL;
 }
 
+static inline void folio_alloc_swap_prepare(struct list_head *folios)
+{
+}
+
+static inline void folio_alloc_swap_split(unsigned int nr_pages)
+{
+}
+
 static inline int folio_alloc_swap(struct folio *folio)
 {
 	return -EINVAL;
diff --git a/mm/swapfile.c b/mm/swapfile.c
index c3288910b3e3..67161e184905 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -120,6 +120,8 @@ 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];
+	/* Pages of the current reclaim batch still to allocate, per order */
+	unsigned int batch_left[SWAP_NR_ORDERS];
 	local_lock_t lock;
 };
 
@@ -916,6 +918,41 @@ static bool cluster_scan_range(struct swap_info_struct *si,
 	return true;
 }
 
+/*
+ * First offset in [@start, @end), aligned to @nr_pages, followed by
+ * @nr_contig free slots. Reclaimable swap cache does not count as free.
+ */
+static unsigned long cluster_find_free_contig(struct swap_cluster_info *ci,
+					      unsigned long start,
+					      unsigned long end,
+					      unsigned int nr_pages,
+					      unsigned int nr_contig)
+{
+	unsigned long offset, contig_start = start;
+
+	if (cluster_is_empty(ci))
+		return start + nr_contig <= end ? start : SWAP_ENTRY_INVALID;
+
+	for (offset = start; offset < end; offset++) {
+		if (!swp_tb_is_null(__swap_table_get(ci, offset % SWAPFILE_CLUSTER))) {
+			contig_start = ALIGN(offset + 1, nr_pages);
+			offset = contig_start - 1;
+			continue;
+		}
+		if (offset + 1 - contig_start >= nr_contig)
+			return contig_start;
+	}
+
+	return SWAP_ENTRY_INVALID;
+}
+
+static bool cluster_slots_free(struct swap_cluster_info *ci,
+			       unsigned long offset, unsigned int nr_pages)
+{
+	return cluster_find_free_contig(ci, offset, offset + nr_pages,
+					nr_pages, nr_pages) == offset;
+}
+
 static bool __swap_cluster_alloc_entries(struct swap_info_struct *si,
 					 struct swap_cluster_info *ci,
 					 struct folio *folio,
@@ -1042,6 +1079,59 @@ static unsigned int alloc_swap_scan_list(struct swap_info_struct *si,
 	return found;
 }
 
+/* Larger folios already make large enough write requests on their own */
+#define SWAP_BATCH_MAX_ORDER	(ilog2(SWAP_CLUSTER_MAX) - 1)
+
+static inline unsigned int swap_batch_contig(unsigned int left)
+{
+	return min_t(unsigned int, left, SWAP_CLUSTER_MAX);
+}
+
+/*
+ * Like alloc_swap_scan_list(), but only allocate at the start of @nr_contig
+ * free slots. Order 0 clusters without them move to the frag list until a
+ * slot is freed, larger orders stay nonfull as the folio may still fit.
+ */
+static unsigned int alloc_swap_scan_list_contig(struct swap_info_struct *si,
+						struct list_head *list,
+						struct folio *folio,
+						unsigned int nr_contig)
+{
+	unsigned int found = SWAP_ENTRY_INVALID, order = folio_order(folio);
+	struct swap_cluster_info *ci;
+	unsigned long start, offset;
+	LIST_HEAD(skipped);
+
+	while ((ci = isolate_lock_cluster(si, list))) {
+		start = cluster_offset(si, ci);
+		offset = SWAP_ENTRY_INVALID;
+		if (SWAPFILE_CLUSTER - ci->count >= nr_contig)
+			offset = cluster_find_free_contig(ci, start,
+							  start + SWAPFILE_CLUSTER,
+							  1 << order, nr_contig);
+		if (offset == SWAP_ENTRY_INVALID) {
+			if (order)
+				move_cluster(si, ci, &skipped,
+					     CLUSTER_FLAG_NONFULL);
+			else
+				relocate_cluster(si, ci);
+			swap_cluster_unlock(ci);
+			continue;
+		}
+		found = alloc_swap_scan_cluster(si, ci, folio, offset);
+		if (found)
+			break;
+	}
+
+	if (!list_empty(&skipped)) {
+		spin_lock(&si->lock);
+		list_splice_tail(&skipped, list);
+		spin_unlock(&si->lock);
+	}
+
+	return found;
+}
+
 static void swap_reclaim_full_clusters(struct swap_info_struct *si, bool force)
 {
 	long to_scan = 1;
@@ -1109,6 +1199,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 	struct swap_cluster_info *ci;
 	unsigned int order = likely(folio) ? folio_order(folio) : 0;
 	unsigned int offset = SWAP_ENTRY_INVALID, found = SWAP_ENTRY_INVALID;
+	unsigned int left;
 
 	/*
 	 * Swapfile is not block device so unable
@@ -1129,7 +1220,14 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 		if (cluster_is_usable(ci, order)) {
 			if (cluster_is_empty(ci))
 				offset = cluster_offset(si, ci);
-			found = alloc_swap_scan_cluster(si, ci, folio, offset);
+			/* The batch's contiguous slots are used up */
+			if (likely(folio) &&
+			    this_cpu_read(percpu_swap_cluster.batch_left[order]) &&
+			    !cluster_slots_free(ci, offset, 1 << order))
+				swap_cluster_unlock(ci);
+			else
+				found = alloc_swap_scan_cluster(si, ci, folio,
+								offset);
 		} else {
 			swap_cluster_unlock(ci);
 		}
@@ -1148,6 +1246,21 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
 			goto done;
 	}
 
+	/* Keep a reclaim batch on contiguous free slots, not in small holes */
+	left = likely(folio) ?
+	       this_cpu_read(percpu_swap_cluster.batch_left[order]) : 0;
+	if (left && order < PMD_ORDER) {
+		found = alloc_swap_scan_list_contig(si, &si->nonfull_clusters[order],
+						    folio, swap_batch_contig(left));
+		if (found)
+			goto done;
+		found = alloc_swap_scan_list(si, &si->free_clusters, folio, false);
+		if (found)
+			goto done;
+		/* No contiguous slots left, allocate the rest of the batch as usual */
+		this_cpu_write(percpu_swap_cluster.batch_left[order], 0);
+	}
+
 	if (order < PMD_ORDER) {
 		found = alloc_swap_scan_list(si, &si->nonfull_clusters[order], folio, true);
 		if (found)
@@ -1390,7 +1503,15 @@ static bool swap_alloc_fast(struct folio *folio)
 	if (cluster_is_usable(ci, order)) {
 		if (cluster_is_empty(ci))
 			offset = cluster_offset(si, ci);
-		alloc_swap_scan_cluster(si, ci, folio, offset);
+		/* The batch's contiguous slots are used up */
+		if (this_cpu_read(percpu_swap_cluster.batch_left[order]) &&
+		    !cluster_slots_free(ci, offset, 1 << order)) {
+			swap_cluster_unlock(ci);
+			this_cpu_write(percpu_swap_cluster.offset[order],
+				       SWAP_ENTRY_INVALID);
+		} else {
+			alloc_swap_scan_cluster(si, ci, folio, offset);
+		}
 	} else {
 		swap_cluster_unlock(ci);
 	}
@@ -1729,6 +1850,86 @@ static int swap_dup_entries_cluster(struct swap_info_struct *si,
 	return err;
 }
 
+static void swap_alloc_prepare(unsigned int order, unsigned int nr)
+{
+	unsigned int nr_contig = swap_batch_contig(nr);
+	struct swap_cluster_info *ci;
+	struct swap_info_struct *si;
+	unsigned long offset;
+	bool keep = false;
+
+	local_lock(&percpu_swap_cluster.lock);
+	if (nr <= (1 << order)) {
+		this_cpu_write(percpu_swap_cluster.batch_left[order], 0);
+		goto out;
+	}
+	this_cpu_write(percpu_swap_cluster.batch_left[order], nr);
+
+	/* Keep the cursor if enough free slots follow it */
+	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)) {
+		ci = swap_cluster_lock(si, offset);
+		if (cluster_is_usable(ci, order)) {
+			if (cluster_is_empty(ci))
+				offset = cluster_offset(si, ci);
+			keep = offset % SWAPFILE_CLUSTER + nr_contig <=
+			       SWAPFILE_CLUSTER &&
+			       cluster_find_free_contig(ci, offset,
+							offset + nr_contig,
+							1 << order,
+							nr_contig) == offset;
+		}
+		swap_cluster_unlock(ci);
+		put_swap_device(si);
+	}
+	if (!keep)
+		this_cpu_write(percpu_swap_cluster.offset[order],
+			       SWAP_ENTRY_INVALID);
+out:
+	local_unlock(&percpu_swap_cluster.lock);
+}
+
+/**
+ * folio_alloc_swap_prepare - prepare swap allocation for a reclaim batch
+ * @folios: list of folios reclaim is about to swap out
+ *
+ * Record how many pages of each small order the batch will swap out, so
+ * that their slots are allocated contiguously. Nothing is reserved.
+ */
+void folio_alloc_swap_prepare(struct list_head *folios)
+{
+	unsigned int max_order = min(SWAP_BATCH_MAX_ORDER, SWAP_NR_ORDERS - 1);
+	unsigned int nr[SWAP_BATCH_MAX_ORDER + 1] = {};
+	struct folio *folio;
+	unsigned int order;
+
+	if (!total_swap_pages)
+		return;
+
+	list_for_each_entry(folio, folios, lru) {
+		if (!folio_test_swapbacked(folio) || folio_test_swapcache(folio))
+			continue;
+		order = folio_order(folio);
+		if (order <= max_order)
+			nr[order] += 1 << order;
+	}
+
+	for (order = 0; order <= max_order; order++)
+		swap_alloc_prepare(order, nr[order]);
+}
+
+/**
+ * folio_alloc_swap_split - account for a large folio split by reclaim
+ * @nr_pages: number of order-0 folios the split produced
+ */
+void folio_alloc_swap_split(unsigned int nr_pages)
+{
+	local_lock(&percpu_swap_cluster.lock);
+	this_cpu_add(percpu_swap_cluster.batch_left[0], nr_pages);
+	local_unlock(&percpu_swap_cluster.lock);
+}
+
 /**
  * folio_alloc_swap - allocate swap space for a folio
  * @folio: folio we want to move to swap
@@ -1771,6 +1972,12 @@ int folio_alloc_swap(struct folio *folio)
 	local_lock(&percpu_swap_cluster.lock);
 	if (!swap_alloc_fast(folio))
 		swap_alloc_slow(folio);
+	if (folio_test_swapcache(folio)) {
+		unsigned int left = this_cpu_read(percpu_swap_cluster.batch_left[order]);
+
+		this_cpu_write(percpu_swap_cluster.batch_left[order],
+			       left > size ? left - size : 0);
+	}
 	local_unlock(&percpu_swap_cluster.lock);
 
 	if (!order && unlikely(!folio_test_swapcache(folio))) {
diff --git a/mm/vmscan.c b/mm/vmscan.c
index ea2d4c8d1890..86245036872c 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1175,6 +1175,8 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
 	memset(stat, 0, sizeof(*stat));
 	cond_resched();
 	do_demote_pass = can_demote(pgdat->node_id, sc, memcg);
+	if (sc->gfp_mask & __GFP_IO)
+		folio_alloc_swap_prepare(folio_list);
 
 retry:
 	while (!list_empty(folio_list)) {
@@ -1378,9 +1380,11 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
 				 * We can free the unmapped pages without IO.
 				 */
 				if (data_race(!list_empty(&folio->_deferred_list) &&
-				    folio_test_partially_mapped(folio)) &&
-				    split_folio_to_list(folio, folio_list))
-					goto activate_locked;
+				    folio_test_partially_mapped(folio))) {
+					if (split_folio_to_list(folio, folio_list))
+						goto activate_locked;
+					folio_alloc_swap_split(nr_pages);
+				}
 			}
 			ret = folio_alloc_swap(folio);
 			if (ret) {
@@ -1393,6 +1397,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
 				/* Fallback to swap normal pages */
 				if (split_folio_to_list(folio, folio_list))
 					goto activate_locked;
+				folio_alloc_swap_split(nr_pages);
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 				if (nr_pages >= HPAGE_PMD_NR) {
 					count_memcg_folio_events(folio,
-- 
2.53.0-Meta

Re: [RFC PATCH 1/1] mm/swap: allocate contiguous swap slots for a reclaim batch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm/swap: stack use-after-return on skipped list in alloc_swap_scan_list_contig()
- [Medium] mm/swap: folio_alloc_swap_split() skips cursor check and order accounting
- [Medium] mm/swap: swap_alloc_prepare() and swap_alloc_fast() strand usable clusters on frag_clusters
--

commit 0160f7d5d1e92861c5a388fc552e5871f0f57f93
Author: Alexandre Ghiti <alex@ghiti.fr>

mm/swap: allocate contiguous swap slots for a reclaim batch

This commit records the number of pages per small order in a reclaim batch
before swapping them out and updates the swap cluster allocator to seek
contiguous runs of up to SWAP_CLUSTER_MAX free slots in nonfull or free
clusters.

> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index c3288910b3e35..67161e1849052 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c

[ ... ]

> @@ -1042,6 +1079,59 @@ static unsigned int alloc_swap_scan_list(struct swap_info_struct *si,
>  	return found;
>  }
>  
> +/* Larger folios already make large enough write requests on their own */
> +#define SWAP_BATCH_MAX_ORDER	(ilog2(SWAP_CLUSTER_MAX) - 1)
> +
> +static inline unsigned int swap_batch_contig(unsigned int left)
> +{
> +	return min_t(unsigned int, left, SWAP_CLUSTER_MAX);
> +}
> +
> +/*
> + * Like alloc_swap_scan_list(), but only allocate at the start of @nr_contig
> + * free slots. Order 0 clusters without them move to the frag list until a
> + * slot is freed, larger orders stay nonfull as the folio may still fit.
> + */
> +static unsigned int alloc_swap_scan_list_contig(struct swap_info_struct *si,
> +						struct list_head *list,
> +						struct folio *folio,
> +						unsigned int nr_contig)
> +{
> +	unsigned int found = SWAP_ENTRY_INVALID, order = folio_order(folio);
> +	struct swap_cluster_info *ci;
> +	unsigned long start, offset;
> +	LIST_HEAD(skipped);
> +
> +	while ((ci = isolate_lock_cluster(si, list))) {
> +		start = cluster_offset(si, ci);
> +		offset = SWAP_ENTRY_INVALID;
> +		if (SWAPFILE_CLUSTER - ci->count >= nr_contig)
> +			offset = cluster_find_free_contig(ci, start,
> +							  start + SWAPFILE_CLUSTER,
> +							  1 << order, nr_contig);
> +		if (offset == SWAP_ENTRY_INVALID) {
> +			if (order)
> +				move_cluster(si, ci, &skipped,
> +					     CLUSTER_FLAG_NONFULL);
> +			else
> +				relocate_cluster(si, ci);
> +			swap_cluster_unlock(ci);
> +			continue;
> +		}
> +		found = alloc_swap_scan_cluster(si, ci, folio, offset);
> +		if (found)
> +			break;
> +	}
> +
> +	if (!list_empty(&skipped)) {
> +		spin_lock(&si->lock);
> +		list_splice_tail(&skipped, list);
> +		spin_unlock(&si->lock);
> +	}
> +
> +	return found;
> +}

[Severity: High]
Can moving an order > 0 cluster onto the stack-local skipped list with
CLUSTER_FLAG_NONFULL and then dropping ci->lock cause a stack
use-after-return on skipped?

When shrink_folio_list() reclaims a batch of mTHP folios (order > 0, up to
SWAP_BATCH_MAX_ORDER), folio_alloc_swap() -> swap_alloc_slow() ->
cluster_alloc_swap_entry() -> alloc_swap_scan_list_contig() places any
cluster from si->nonfull_clusters[order] lacking nr_contig contiguous free
slots onto skipped with ci->flags set to CLUSTER_FLAG_NONFULL, and then
unlocks ci->lock.

While ci is unlocked on skipped, another CPU can lock ci by offset (for
example, when freeing its remaining entries via
__swap_cluster_free_entries() -> free_cluster(), or through a cached
percpu_swap_cluster / global_cluster cursor in swap_alloc_fast() /
cluster_alloc_swap_entry() -> alloc_swap_scan_cluster() ->
relocate_cluster()). Because ci->flags != CLUSTER_FLAG_NONE, move_cluster()
removes ci from skipped under si->lock:

mm/swapfile.c:move_cluster() {
	spin_lock(&si->lock);
	if (ci->flags == CLUSTER_FLAG_NONE)
		list_add_tail(&ci->list, list);
	else
		list_move_tail(&ci->list, list);
	spin_unlock(&si->lock);
	ci->flags = new_flags;
}

When ci is the last entry on skipped, __list_del(&skipped, &skipped) in
include/linux/list.h updates skipped.prev and skipped.next without a
memory barrier:

static inline void __list_del(struct list_head * prev, struct list_head * next)
{
	next->prev = prev;
	WRITE_ONCE(prev->next, next);
}

Because alloc_swap_scan_list_contig() tests !list_empty(&skipped)
(READ_ONCE(skipped.next) == &skipped) outside si->lock before returning,
can it observe skipped.next == &skipped, skip spin_lock(&si->lock), and
return while the other CPU is still writing skipped.prev on the popped
stack frame?

Also, while those order > 0 clusters sit on the local skipped list, they
are hidden from concurrent alloc_swap_scan_list() callers scanning
si->nonfull_clusters[order], which could cause premature -E2BIG splits.

[ ... ]

> @@ -1390,7 +1503,15 @@ static bool swap_alloc_fast(struct folio *folio)
>  	if (cluster_is_usable(ci, order)) {
>  		if (cluster_is_empty(ci))
>  			offset = cluster_offset(si, ci);
> -		alloc_swap_scan_cluster(si, ci, folio, offset);
> +		/* The batch's contiguous slots are used up */
> +		if (this_cpu_read(percpu_swap_cluster.batch_left[order]) &&
> +		    !cluster_slots_free(ci, offset, 1 << order)) {
> +			swap_cluster_unlock(ci);
> +			this_cpu_write(percpu_swap_cluster.offset[order],
> +				       SWAP_ENTRY_INVALID);
> +		} else {
> +			alloc_swap_scan_cluster(si, ci, folio, offset);
> +		}
>  	} else {
>  		swap_cluster_unlock(ci);
>  	}
> @@ -1729,6 +1850,86 @@ static int swap_dup_entries_cluster(struct swap_info_struct *si,
>  	return err;
>  }
>  
> +static void swap_alloc_prepare(unsigned int order, unsigned int nr)
> +{
> +	unsigned int nr_contig = swap_batch_contig(nr);
> +	struct swap_cluster_info *ci;
> +	struct swap_info_struct *si;
> +	unsigned long offset;
> +	bool keep = false;
> +
> +	local_lock(&percpu_swap_cluster.lock);
> +	if (nr <= (1 << order)) {
> +		this_cpu_write(percpu_swap_cluster.batch_left[order], 0);
> +		goto out;
> +	}
> +	this_cpu_write(percpu_swap_cluster.batch_left[order], nr);
> +
> +	/* Keep the cursor if enough free slots follow it */
> +	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)) {
> +		ci = swap_cluster_lock(si, offset);
> +		if (cluster_is_usable(ci, order)) {
> +			if (cluster_is_empty(ci))
> +				offset = cluster_offset(si, ci);
> +			keep = offset % SWAPFILE_CLUSTER + nr_contig <=
> +			       SWAPFILE_CLUSTER &&
> +			       cluster_find_free_contig(ci, offset,
> +							offset + nr_contig,
> +							1 << order,
> +							nr_contig) == offset;
> +		}
> +		swap_cluster_unlock(ci);
> +		put_swap_device(si);
> +	}
> +	if (!keep)
> +		this_cpu_write(percpu_swap_cluster.offset[order],
> +			       SWAP_ENTRY_INVALID);
> +out:
> +	local_unlock(&percpu_swap_cluster.lock);
> +}

[Severity: Medium]
When swap_alloc_prepare() or swap_alloc_fast() clears
percpu_swap_cluster.offset[order] to SWAP_ENTRY_INVALID without calling
alloc_swap_scan_cluster(), does that strand usable clusters on
si->frag_clusters[order]?

On the first allocation from a cluster, alloc_swap_scan_cluster() calls
relocate_cluster(), which moves any partially populated cluster
(0 < ci->count < SWAPFILE_CLUSTER) to si->frag_clusters[ci->order]:

mm/swapfile.c:relocate_cluster() {
	...
	} else if (ci->count != SWAPFILE_CLUSTER) {
		if (ci->flags != CLUSTER_FLAG_FRAG)
			move_cluster(si, ci, &si->frag_clusters[ci->order],
				     CLUSTER_FLAG_FRAG);
	}
}

Previously, the per-CPU cursor kept allocating from that cluster until the
end of the cluster was reached. With this change, if swap_alloc_prepare()
finds fewer than nr_contig free slots immediately at offset, or if
swap_alloc_fast() (or cluster_alloc_swap_entry() for rotating storage)
encounters an occupied slot at offset while batch_left[order] is non-zero,
the cursor is abandoned while ci remains on si->frag_clusters[order], even
if large contiguous free regions remain later in the same cluster.

Since si->frag_clusters[order] is only scanned after si->nonfull_clusters
and si->free_clusters are exhausted (and only one cluster at a time), could
abandoning the cursor without returning ci to si->nonfull_clusters[order]
or scanning the rest of ci cause premature cluster exhaustion and -E2BIG
mTHP swapout fallbacks?

> +
> +/**
> + * folio_alloc_swap_prepare - prepare swap allocation for a reclaim batch
> + * @folios: list of folios reclaim is about to swap out
> + *
> + * Record how many pages of each small order the batch will swap out, so
> + * that their slots are allocated contiguously. Nothing is reserved.
> + */
> +void folio_alloc_swap_prepare(struct list_head *folios)
> +{
> +	unsigned int max_order = min(SWAP_BATCH_MAX_ORDER, SWAP_NR_ORDERS - 1);
> +	unsigned int nr[SWAP_BATCH_MAX_ORDER + 1] = {};
> +	struct folio *folio;
> +	unsigned int order;
> +
> +	if (!total_swap_pages)
> +		return;
> +
> +	list_for_each_entry(folio, folios, lru) {
> +		if (!folio_test_swapbacked(folio) || folio_test_swapcache(folio))
> +			continue;
> +		order = folio_order(folio);
> +		if (order <= max_order)
> +			nr[order] += 1 << order;
> +	}
> +
> +	for (order = 0; order <= max_order; order++)
> +		swap_alloc_prepare(order, nr[order]);
> +}
> +
> +/**
> + * folio_alloc_swap_split - account for a large folio split by reclaim
> + * @nr_pages: number of order-0 folios the split produced
> + */
> +void folio_alloc_swap_split(unsigned int nr_pages)
> +{
> +	local_lock(&percpu_swap_cluster.lock);
> +	this_cpu_add(percpu_swap_cluster.batch_left[0], nr_pages);
> +	local_unlock(&percpu_swap_cluster.lock);
> +}

[Severity: Medium]
Should folio_alloc_swap_split() also validate percpu_swap_cluster.offset[0]
for contiguous free slots and decrement batch_left[order] for the split
folio's original order?

When folio_alloc_swap_prepare() sees 0 or 1 order-0 folios in the batch,
swap_alloc_prepare(0, nr[0]) sets batch_left[0] to 0 and leaves the
existing percpu_swap_cluster.offset[0] cursor untouched. If a large folio
is subsequently split in shrink_folio_list() or shmem_writeout(),
folio_alloc_swap_split() adds nr_pages to batch_left[0] without checking
whether percpu_swap_cluster.offset[0] has swap_batch_contig(batch_left[0])
contiguous free slots following it.

Because swap_alloc_fast() only checks cluster_slots_free(ci, offset, 1) for
order-0 allocations, if percpu_swap_cluster.offset[0] points into a small
free hole, the first subpages of the split folio will be allocated into
that hole until an occupied slot is reached.

In addition, if the split folio had order <= max_order,
folio_alloc_swap_prepare() already added 1 << order to batch_left[order].
When that folio is split before folio_alloc_swap() (such as a partially
mapped folio in shrink_folio_list() or a folio in shmem_writeout()) or
after folio_alloc_swap() fails without clearing batch_left[order],
batch_left[order] is not decremented, leaving it inflated for subsequent
folios of that order.

титься

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022145615.2184343-1-alex@ghiti.fr?part=1

Re: [RFC PATCH 1/1] mm/swap: allocate contiguous swap slots for a reclaim batch

[Nhat Pham]

On Thu, Oct 22, 2026 at 7:57 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> Reclaim allocates swap slots one folio at a time and only then batches
> the writes, merging folios whose slots happen to be contiguous. Order-0
> allocations scan the nonfull clusters before taking a free cluster, so
> once frees have left holes in otherwise full clusters, a batch of LRU-
> adjacent folios is scattered over those holes and written with many
> small requests. Since swap slots are freed on exclusive write faults,
> this happens with any workload that rewrites swapped out memory.
>
> Before swapping out a batch, record how many pages of each small order
> need a slot, and make those allocations follow contiguous free slots,
> SWAP_CLUSTER_MAX at a time: in a nonfull cluster if one has them,
> otherwise in a free cluster. When neither has them, the rest of the
> batch fills the holes as before. Nothing is reserved.
>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>

Thanks for looking into this! There are still workloads that benefit
more from disk swapping than compression swapping... We should make
the swap subsystem work for them.

I haven't read the code too closely, as it's an RFC meant to show the
general idea. So I'll just make some comments and ask some questions
about the idea:

>  static bool __swap_cluster_alloc_entries(struct swap_info_struct *si,
>                                          struct swap_cluster_info *ci,
>                                          struct folio *folio,
> @@ -1042,6 +1079,59 @@ static unsigned int alloc_swap_scan_list(struct swap_info_struct *si,
>         return found;
>  }
>
> +/* Larger folios already make large enough write requests on their own */
> +#define SWAP_BATCH_MAX_ORDER   (ilog2(SWAP_CLUSTER_MAX) - 1)

Ah so we don't invoke this heuristics for folio larger than certain
orders? Makes sense.

> +
> +static inline unsigned int swap_batch_contig(unsigned int left)
> +{
> +       return min_t(unsigned int, left, SWAP_CLUSTER_MAX);
> +}
> +
> +/*
> + * Like alloc_swap_scan_list(), but only allocate at the start of @nr_contig
> + * free slots. Order 0 clusters without them move to the frag list until a
> + * slot is freed, larger orders stay nonfull as the folio may still fit.
> + */
> +static unsigned int alloc_swap_scan_list_contig(struct swap_info_struct *si,
> +                                               struct list_head *list,
> +                                               struct folio *folio,
> +                                               unsigned int nr_contig)
> +{
> +       unsigned int found = SWAP_ENTRY_INVALID, order = folio_order(folio);
> +       struct swap_cluster_info *ci;
> +       unsigned long start, offset;
> +       LIST_HEAD(skipped);
> +
> +       while ((ci = isolate_lock_cluster(si, list))) {
> +               start = cluster_offset(si, ci);
> +               offset = SWAP_ENTRY_INVALID;
> +               if (SWAPFILE_CLUSTER - ci->count >= nr_contig)
> +                       offset = cluster_find_free_contig(ci, start,
> +                                                         start + SWAPFILE_CLUSTER,
> +                                                         1 << order, nr_contig);
> +               if (offset == SWAP_ENTRY_INVALID) {
> +                       if (order)
> +                               move_cluster(si, ci, &skipped,
> +                                            CLUSTER_FLAG_NONFULL);
> +                       else
> +                               relocate_cluster(si, ci);
> +                       swap_cluster_unlock(ci);
> +                       continue;
> +               }
> +               found = alloc_swap_scan_cluster(si, ci, folio, offset);
> +               if (found)
> +                       break;
> +       }
> +
> +       if (!list_empty(&skipped)) {
> +               spin_lock(&si->lock);
> +               list_splice_tail(&skipped, list);
> +               spin_unlock(&si->lock);
> +       }
> +
> +       return found;
> +}
> +
>  static void swap_reclaim_full_clusters(struct swap_info_struct *si, bool force)
>  {
>         long to_scan = 1;
> @@ -1109,6 +1199,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
>         struct swap_cluster_info *ci;
>         unsigned int order = likely(folio) ? folio_order(folio) : 0;
>         unsigned int offset = SWAP_ENTRY_INVALID, found = SWAP_ENTRY_INVALID;
> +       unsigned int left;
>
>         /*
>          * Swapfile is not block device so unable
> @@ -1129,7 +1220,14 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
>                 if (cluster_is_usable(ci, order)) {
>                         if (cluster_is_empty(ci))
>                                 offset = cluster_offset(si, ci);
> -                       found = alloc_swap_scan_cluster(si, ci, folio, offset);
> +                       /* The batch's contiguous slots are used up */
> +                       if (likely(folio) &&
> +                           this_cpu_read(percpu_swap_cluster.batch_left[order]) &&
> +                           !cluster_slots_free(ci, offset, 1 << order))
> +                               swap_cluster_unlock(ci);
> +                       else
> +                               found = alloc_swap_scan_cluster(si, ci, folio,
> +                                                               offset);
>                 } else {
>                         swap_cluster_unlock(ci);
>                 }
> @@ -1148,6 +1246,21 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
>                         goto done;
>         }
>
> +       /* Keep a reclaim batch on contiguous free slots, not in small holes */
> +       left = likely(folio) ?
> +              this_cpu_read(percpu_swap_cluster.batch_left[order]) : 0;
> +       if (left && order < PMD_ORDER) {
> +               found = alloc_swap_scan_list_contig(si, &si->nonfull_clusters[order],
> +                                                   folio, swap_batch_contig(left));
> +               if (found)
> +                       goto done;
> +               found = alloc_swap_scan_list(si, &si->free_clusters, folio, false);
> +               if (found)
> +                       goto done;
> +               /* No contiguous slots left, allocate the rest of the batch as usual */
> +               this_cpu_write(percpu_swap_cluster.batch_left[order], 0);
> +       }
> +
>         if (order < PMD_ORDER) {
>                 found = alloc_swap_scan_list(si, &si->nonfull_clusters[order], folio, true);

So we prefer non-full clusters with contiguous slots, over free
clusters, over other nonfull clusters.

I think this part makes sense to me FWIW. We do skip fast path and go
for slow paths more often, so do make sure we're not paying with too
much lock contention, especially as the swapfile fils or get more
fragmented (which this patch series does nudge us towards). But FWIW,
we pretty much go back to the old behavior the moment we fail to find
contiguity for the first time in the batch, so I think we'll be
fine...?

>                 if (found)
> @@ -1390,7 +1503,15 @@ static bool swap_alloc_fast(struct folio *folio)
>         if (cluster_is_usable(ci, order)) {
>                 if (cluster_is_empty(ci))
>                         offset = cluster_offset(si, ci);
> -               alloc_swap_scan_cluster(si, ci, folio, offset);
> +               /* The batch's contiguous slots are used up */
> +               if (this_cpu_read(percpu_swap_cluster.batch_left[order]) &&
> +                   !cluster_slots_free(ci, offset, 1 << order)) {
> +                       swap_cluster_unlock(ci);
> +                       this_cpu_write(percpu_swap_cluster.offset[order],
> +                                      SWAP_ENTRY_INVALID);
> +               } else {
> +                       alloc_swap_scan_cluster(si, ci, folio, offset);
> +               }
>         } else {
>                 swap_cluster_unlock(ci);
>         }
> @@ -1729,6 +1850,86 @@ static int swap_dup_entries_cluster(struct swap_info_struct *si,
>         return err;
>  }
>
> +static void swap_alloc_prepare(unsigned int order, unsigned int nr)
> +{
> +       unsigned int nr_contig = swap_batch_contig(nr);
> +       struct swap_cluster_info *ci;
> +       struct swap_info_struct *si;
> +       unsigned long offset;
> +       bool keep = false;
> +
> +       local_lock(&percpu_swap_cluster.lock);
> +       if (nr <= (1 << order)) {
> +               this_cpu_write(percpu_swap_cluster.batch_left[order], 0);
> +               goto out;
> +       }
> +       this_cpu_write(percpu_swap_cluster.batch_left[order], nr);
> +
> +       /* Keep the cursor if enough free slots follow it */
> +       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)) {
> +               ci = swap_cluster_lock(si, offset);
> +               if (cluster_is_usable(ci, order)) {
> +                       if (cluster_is_empty(ci))
> +                               offset = cluster_offset(si, ci);
> +                       keep = offset % SWAPFILE_CLUSTER + nr_contig <=
> +                              SWAPFILE_CLUSTER &&
> +                              cluster_find_free_contig(ci, offset,
> +                                                       offset + nr_contig,
> +                                                       1 << order,
> +                                                       nr_contig) == offset;
> +               }
> +               swap_cluster_unlock(ci);
> +               put_swap_device(si);
> +       }
> +       if (!keep)
> +               this_cpu_write(percpu_swap_cluster.offset[order],
> +                              SWAP_ENTRY_INVALID);
> +out:
> +       local_unlock(&percpu_swap_cluster.lock);
> +}
> +
> +/**
> + * folio_alloc_swap_prepare - prepare swap allocation for a reclaim batch
> + * @folios: list of folios reclaim is about to swap out
> + *
> + * Record how many pages of each small order the batch will swap out, so
> + * that their slots are allocated contiguously. Nothing is reserved.
> + */
> +void folio_alloc_swap_prepare(struct list_head *folios)
> +{
> +       unsigned int max_order = min(SWAP_BATCH_MAX_ORDER, SWAP_NR_ORDERS - 1);
> +       unsigned int nr[SWAP_BATCH_MAX_ORDER + 1] = {};
> +       struct folio *folio;
> +       unsigned int order;
> +
> +       if (!total_swap_pages)
> +               return;
> +
> +       list_for_each_entry(folio, folios, lru) {
> +               if (!folio_test_swapbacked(folio) || folio_test_swapcache(folio))
> +                       continue;
> +               order = folio_order(folio);
> +               if (order <= max_order)
> +                       nr[order] += 1 << order;
> +       }
> +
> +       for (order = 0; order <= max_order; order++)
> +               swap_alloc_prepare(order, nr[order]);
> +}
> +
> +/**
> + * folio_alloc_swap_split - account for a large folio split by reclaim
> + * @nr_pages: number of order-0 folios the split produced
> + */
> +void folio_alloc_swap_split(unsigned int nr_pages)
> +{
> +       local_lock(&percpu_swap_cluster.lock);
> +       this_cpu_add(percpu_swap_cluster.batch_left[0], nr_pages);
> +       local_unlock(&percpu_swap_cluster.lock);
> +}
> +
>  /**
>   * folio_alloc_swap - allocate swap space for a folio
>   * @folio: folio we want to move to swap
> @@ -1771,6 +1972,12 @@ int folio_alloc_swap(struct folio *folio)
>         local_lock(&percpu_swap_cluster.lock);
>         if (!swap_alloc_fast(folio))
>                 swap_alloc_slow(folio);
> +       if (folio_test_swapcache(folio)) {
> +               unsigned int left = this_cpu_read(percpu_swap_cluster.batch_left[order]);
> +
> +               this_cpu_write(percpu_swap_cluster.batch_left[order],
> +                              left > size ? left - size : 0);
> +       }
>         local_unlock(&percpu_swap_cluster.lock);
>
>         if (!order && unlikely(!folio_test_swapcache(folio))) {
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index ea2d4c8d1890..86245036872c 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1175,6 +1175,8 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
>         memset(stat, 0, sizeof(*stat));
>         cond_resched();
>         do_demote_pass = can_demote(pgdat->node_id, sc, memcg);
> +       if (sc->gfp_mask & __GFP_IO)
> +               folio_alloc_swap_prepare(folio_list);
>

Some general questions:

1. How many pages in folio_list typically do get reclaimed? There can
be quite a few with references, which we rotate/promote, when we scan,
right? Would constantly overshooting the estimate create problem down
the line?

(This is one of the few places where indirection (vswap) would have
helped - we can collect a batch of pages after unmapping, then pass
them to the phys swap allocator (after checking zswappability, etc.).
Maybe we can solve it better after it's landed, and accept an
imperfect solution for now?)

2. Should we skip this heuristics for cgroups that enables zswap (and
maybe zram)? Contiguity matters less there... unless it help at
writeback time? Hard to say without experimentation - maybe you can do
the same check you did for your zswap writeback batching patch series?

Re: [RFC PATCH 0/1] mm/swap: keep reclaim batches contiguous on swap

[Kairui Song]

On Thu, Oct 22, 2026 at 11:00 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> 7.3 regressed swap I/O: for the same swap volume, a workload that
> rewrites swapped out memory sends many more requests to the device.
> mysql/sysbench in a 256M memcg swapping to an NVMe partition, 7.3
> compared to the previous version:
>
>   device read requests per transaction        +59%
>   device write requests per transaction       +27%
>   throughput, iocost off                      no change
>   throughput, iocost on                       -19%
>
> Two commits interact:
>
> - Commit 9a42aed48421 ("mm, swap: prefer nonfull over free clusters"),
>   in 6.18, makes order-0 allocations fill nonfull clusters before
>   taking a free one.
>
> - Commit 3cedb745d354 ("mm: clarify the folio_free_swap() for
>   do_swap_page()"), in 7.3, frees the swap slot on an exclusive write
>   fault. 7.0 to 7.2 kept the slot, but only as a side effect of commit
>   4b34f1d82c65 ("mm, swap: free the swap cache after folio is
>   mapped").
>
> The freed slots leave holes in full clusters, which then go back on
> the nonfull list. Reclaim allocates slots one folio at a time and
> merges the writes afterwards, so a batch of LRU-adjacent folios gets
> scattered over those holes and is written with many small requests.
> Readahead later finds those pages scattered as well, and needs more
> read requests to bring them back.
>
> I tried three fixes, which perform similarly:
>
> A. Revert 3cedb745d354. This keeps the slot on write fault, but undoes
>    the intent of that commit and keeps swap space pinned under dirty
>    pages.
>
> B. Below 50% swap usage, make order-0 allocations take a free cluster
>    before a nonfull one. This is what the allocator already does, at
>    any usage, on swap devices with discard enabled (swapon
>    --discard=pages). It is about five lines, but the threshold is
>    arbitrary, it stops working as swap fills up, and it applies to
>    every order-0 allocation, using up the free clusters that
>    nonfull-first keeps for mTHP.

Hi Alexandre,

That's a interesting problem. I think the real problem is with commit
9a42aed48421 ("mm, swap: prefer nonfull over free clusters"), right?

It's more like a balance between fragmentation and contiguity of small
order allocation issue.

Perhaps we should instead always scan the full clusters and revert
that one commit? That commit was only following the idea of keeping
the allocation more compacted for that series, it looked good at that
time. Since fragmentation issues can be solved in other ways,
contiguity seems to matter more for large physical swap devices.

Re: [RFC PATCH 0/1] mm/swap: keep reclaim batches contiguous on swap

[Alexandre Ghiti]

Hi Kairui,

On Thu, Oct 22, 2026 at 5:27 PM Kairui Song <ryncsn@gmail.com> wrote:
>
> >
> On Thu, Oct 22, 2026 at 11:00 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> >
> > 7.3 regressed swap I/O: for the same swap volume, a workload that
> > rewrites swapped out memory sends many more requests to the device.
> > mysql/sysbench in a 256M memcg swapping to an NVMe partition, 7.3
> > compared to the previous version:
> >
> >   device read requests per transaction        +59%
> >   device write requests per transaction       +27%
> >   throughput, iocost off                      no change
> >   throughput, iocost on                       -19%
> >
> > Two commits interact:
> >
> > - Commit 9a42aed48421 ("mm, swap: prefer nonfull over free clusters"),
> >   in 6.18, makes order-0 allocations fill nonfull clusters before
> >   taking a free one.
> >
> > - Commit 3cedb745d354 ("mm: clarify the folio_free_swap() for
> >   do_swap_page()"), in 7.3, frees the swap slot on an exclusive write
> >   fault. 7.0 to 7.2 kept the slot, but only as a side effect of commit
> >   4b34f1d82c65 ("mm, swap: free the swap cache after folio is
> >   mapped").
> >
> > The freed slots leave holes in full clusters, which then go back on
> > the nonfull list. Reclaim allocates slots one folio at a time and
> > merges the writes afterwards, so a batch of LRU-adjacent folios gets
> > scattered over those holes and is written with many small requests.
> > Readahead later finds those pages scattered as well, and needs more
> > read requests to bring them back.
> >
> > I tried three fixes, which perform similarly:
> >
> > A. Revert 3cedb745d354. This keeps the slot on write fault, but undoes
> >    the intent of that commit and keeps swap space pinned under dirty
> >    pages.
> >
> > B. Below 50% swap usage, make order-0 allocations take a free cluster
> >    before a nonfull one. This is what the allocator already does, at
> >    any usage, on swap devices with discard enabled (swapon
> >    --discard=pages). It is about five lines, but the threshold is
> >    arbitrary, it stops working as swap fills up, and it applies to
> >    every order-0 allocation, using up the free clusters that
> >    nonfull-first keeps for mTHP.
>
> Hi Alexandre,
>
> That's a interesting problem. I think the real problem is with commit
> 9a42aed48421 ("mm, swap: prefer nonfull over free clusters"), right?
>
> It's more like a balance between fragmentation and contiguity of small
> order allocation issue.
>
> Perhaps we should instead always scan the full clusters and revert
> that one commit? That commit was only following the idea of keeping
> the allocation more compacted for that series, it looked good at that
> time. Since fragmentation issues can be solved in other ways,
> contiguity seems to matter more for large physical swap devices.
>

Oh, okay. I thought reverting it wasn't an option given the good
numbers the patch provides.

What are the other fragmentation solutions you are referring to?

Thanks,

Alex

Re: [RFC PATCH 0/1] mm/swap: keep reclaim batches contiguous on swap

[Kairui Song]

On Fri, Oct 23, 2026 at 12:17 AM Alexandre Ghiti <alexghiti@meta.com> wrote:
>
> Hi Kairui,
>
> On Thu, Oct 22, 2026 at 5:27 PM Kairui Song <ryncsn@gmail.com> wrote:
> >
> > >
> > On Thu, Oct 22, 2026 at 11:00 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> > >
> > > 7.3 regressed swap I/O: for the same swap volume, a workload that
> > > rewrites swapped out memory sends many more requests to the device.
> > > mysql/sysbench in a 256M memcg swapping to an NVMe partition, 7.3
> > > compared to the previous version:
> > >
> > >   device read requests per transaction        +59%
> > >   device write requests per transaction       +27%
> > >   throughput, iocost off                      no change
> > >   throughput, iocost on                       -19%
> > >
> > > Two commits interact:
> > >
> > > - Commit 9a42aed48421 ("mm, swap: prefer nonfull over free clusters"),
> > >   in 6.18, makes order-0 allocations fill nonfull clusters before
> > >   taking a free one.
> > >
> > > - Commit 3cedb745d354 ("mm: clarify the folio_free_swap() for
> > >   do_swap_page()"), in 7.3, frees the swap slot on an exclusive write
> > >   fault. 7.0 to 7.2 kept the slot, but only as a side effect of commit
> > >   4b34f1d82c65 ("mm, swap: free the swap cache after folio is
> > >   mapped").
> > >
> > > The freed slots leave holes in full clusters, which then go back on
> > > the nonfull list. Reclaim allocates slots one folio at a time and
> > > merges the writes afterwards, so a batch of LRU-adjacent folios gets
> > > scattered over those holes and is written with many small requests.
> > > Readahead later finds those pages scattered as well, and needs more
> > > read requests to bring them back.
> > >
> > > I tried three fixes, which perform similarly:
> > >
> > > A. Revert 3cedb745d354. This keeps the slot on write fault, but undoes
> > >    the intent of that commit and keeps swap space pinned under dirty
> > >    pages.
> > >
> > > B. Below 50% swap usage, make order-0 allocations take a free cluster
> > >    before a nonfull one. This is what the allocator already does, at
> > >    any usage, on swap devices with discard enabled (swapon
> > >    --discard=pages). It is about five lines, but the threshold is
> > >    arbitrary, it stops working as swap fills up, and it applies to
> > >    every order-0 allocation, using up the free clusters that
> > >    nonfull-first keeps for mTHP.
> >
> > Hi Alexandre,
> >
> > That's a interesting problem. I think the real problem is with commit
> > 9a42aed48421 ("mm, swap: prefer nonfull over free clusters"), right?
> >
> > It's more like a balance between fragmentation and contiguity of small
> > order allocation issue.
> >
> > Perhaps we should instead always scan the full clusters and revert
> > that one commit? That commit was only following the idea of keeping
> > the allocation more compacted for that series, it looked good at that
> > time. Since fragmentation issues can be solved in other ways,
> > contiguity seems to matter more for large physical swap devices.
> >
>

Hi,

> Oh, okay. I thought reverting it wasn't an option given the good
> numbers the patch provides.

That patch is 1/3 of a series, that series yields really nice
results, but that one patch plays only a small part in them. Or
perhaps we can limit that to SYNC devices? I hope there are simpler
ways to avoid the problem you encountered. I think the ~4% reduction
of 64k mTHP failure is kind of trivial compared to the problem you
encountered, and 2M swapout is still not easily doable, as the
main blocker is from the page allocator.

>
> What are the other fragmentation solutions you are referring to?

Right, nothing has landed in upstream for that yet, but with larger
swap space the fragmentation issue will be largely solved. And before
that we might be able to sort the clusters by the continues size of
slots on it instead just blindly move then back to nonfull on any
freeing.

Re: [RFC PATCH 0/1] mm/swap: keep reclaim batches contiguous on swap

[Nhat Pham]

On Thu, Oct 22, 2026 at 8:27 AM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Thu, Oct 22, 2026 at 11:00 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> >
> > 7.3 regressed swap I/O: for the same swap volume, a workload that
> > rewrites swapped out memory sends many more requests to the device.
> > mysql/sysbench in a 256M memcg swapping to an NVMe partition, 7.3
> > compared to the previous version:
> >
> >   device read requests per transaction        +59%
> >   device write requests per transaction       +27%
> >   throughput, iocost off                      no change
> >   throughput, iocost on                       -19%
> >
> > Two commits interact:
> >
> > - Commit 9a42aed48421 ("mm, swap: prefer nonfull over free clusters"),
> >   in 6.18, makes order-0 allocations fill nonfull clusters before
> >   taking a free one.
> >
> > - Commit 3cedb745d354 ("mm: clarify the folio_free_swap() for
> >   do_swap_page()"), in 7.3, frees the swap slot on an exclusive write
> >   fault. 7.0 to 7.2 kept the slot, but only as a side effect of commit
> >   4b34f1d82c65 ("mm, swap: free the swap cache after folio is
> >   mapped").
> >
> > The freed slots leave holes in full clusters, which then go back on
> > the nonfull list. Reclaim allocates slots one folio at a time and
> > merges the writes afterwards, so a batch of LRU-adjacent folios gets
> > scattered over those holes and is written with many small requests.
> > Readahead later finds those pages scattered as well, and needs more
> > read requests to bring them back.
> >
> > I tried three fixes, which perform similarly:
> >
> > A. Revert 3cedb745d354. This keeps the slot on write fault, but undoes
> >    the intent of that commit and keeps swap space pinned under dirty
> >    pages.
> >
> > B. Below 50% swap usage, make order-0 allocations take a free cluster
> >    before a nonfull one. This is what the allocator already does, at
> >    any usage, on swap devices with discard enabled (swapon
> >    --discard=pages). It is about five lines, but the threshold is
> >    arbitrary, it stops working as swap fills up, and it applies to
> >    every order-0 allocation, using up the free clusters that
> >    nonfull-first keeps for mTHP.
>
> Hi Alexandre,
>
> That's a interesting problem. I think the real problem is with commit
> 9a42aed48421 ("mm, swap: prefer nonfull over free clusters"), right?
>
> It's more like a balance between fragmentation and contiguity of small
> order allocation issue.
>
> Perhaps we should instead always scan the full clusters and revert

You mean scanning the free clusters?

> that one commit? That commit was only following the idea of keeping
> the allocation more compacted for that series, it looked good at that
> time. Since fragmentation issues can be solved in other ways,
> contiguity seems to matter more for large physical swap devices.

Hmm what other way are you thinking of? Physical swap space compaction
requires vswap, which has not landed. And we'd just be kicking the
problem to virtual swap space. Sure, we won't be failing allocation,
but that would still increase the memory overhead, scanning overhead,
etc.

I encountered this myself. Remember when I wrote my own swap allocator
for vswap? In one of the version, I went for free clusters first
instead of partial clusters. It fragmented the swap space, so I had to
copy your homework and went for partially used clusters first...

Anyway, maybe I'm wrong - some numbers would be nice here.