The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan
@ 2026-08-06 19:32 Youngjun Park
  2026-08-06 19:32 ` [PATCH v3 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info Youngjun Park
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Youngjun Park @ 2026-08-06 19:32 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	Barry Song, Youngjun Park, her0gyugyu, linux-mm, linux-kernel

find_next_to_unuse() walks a swap device one offset at a time.  Slot
state now lives in a per cluster swap table, so patch 2 dismisses an
empty cluster with one counter read instead of SWAPFILE_CLUSTER table
reads.

Patch 1 is an unrelated one line comment fix noticed on the way.

A debug test confirmed the skip path runs, and swapoff completed
under load with no DEBUG_VM or lockdep splats.

Changes in v3:
- 2/2: clamp the scan end with min_t() so it stops at si->max, rather
  than running into the masked tail of the last cluster, which drops the
  need to explain why walking that tail was safe (Barry)
- 2/2: compute ci_off only where it is used
- 1/2, 2/2: pick up Barry's Reviewed-by
- Rebased on mm-new
- Link to v2: https://lore.kernel.org/r/20260805141146.127776-1-youngjun.park@lge.com

Changes in v2:
- 1/2: reword the comment to "array, one entry per cluster", dropping
  the redundant "on every device" (Barry)
- 1/2: pick up Kairui's Acked-by
- 2/2: drop the min(), the swap table is always SWAPFILE_CLUSTER entries
  and swapon() masks the tail past si->max as bad (Kairui)
- 2/2: mark the unlocked ci->count read with READ_ONCE() for KCSAN
  instead of cluster_is_empty(), whose other callers hold ci->lock
  (Kairui)
- 2/2: expand the commit message to cover both of the above
- Rebased on mm-new
- Link to v1: https://lore.kernel.org/r/20260728155907.391820-1-youngjun.park@lge.com

Youngjun Park (2):
  mm/swap: fix stale comment on swap_info_struct::cluster_info
  mm/swap: scan by cluster in find_next_to_unuse()

 include/linux/swap.h |  2 +-
 mm/swapfile.c        | 43 ++++++++++++++++++++++++++++++-------------
 2 files changed, 31 insertions(+), 14 deletions(-)


base-commit: 1fb556c523f6c18b43b1f52fb366f61c9963ce06
-- 
2.48.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info
  2026-08-06 19:32 [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan Youngjun Park
@ 2026-08-06 19:32 ` Youngjun Park
  2026-08-07  9:49   ` Baoquan He
  2026-08-06 19:32 ` [PATCH v3 2/2] mm/swap: scan by cluster in find_next_to_unuse() Youngjun Park
  2026-08-06 20:06 ` [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan Andrew Morton
  2 siblings, 1 reply; 12+ messages in thread
From: Youngjun Park @ 2026-08-06 19:32 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	Barry Song, Youngjun Park, her0gyugyu, linux-mm, linux-kernel

setup_swap_clusters_info() allocates cluster_info for every swap area,
not only for SSDs.

Signed-off-by: Youngjun Park <youngjun.park@lge.com>
Acked-by: Kairui Song <kasong@tencent.com>
Reviewed-by: Barry Song <baohua@kernel.org>
---
 include/linux/swap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 2cb1d29307c5..2b14e2e9673b 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -246,7 +246,7 @@ struct swap_info_struct {
 	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 */
+	struct swap_cluster_info *cluster_info; /* array, one entry per cluster */
 	struct list_head free_clusters; /* free clusters list */
 	struct list_head full_clusters; /* full clusters list */
 	struct list_head nonfull_clusters[SWAP_NR_ORDERS];
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/2] mm/swap: scan by cluster in find_next_to_unuse()
  2026-08-06 19:32 [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan Youngjun Park
  2026-08-06 19:32 ` [PATCH v3 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info Youngjun Park
@ 2026-08-06 19:32 ` Youngjun Park
  2026-08-07  7:33   ` Kairui Song
  2026-08-07  9:52   ` Baoquan He
  2026-08-06 20:06 ` [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan Andrew Morton
  2 siblings, 2 replies; 12+ messages in thread
From: Youngjun Park @ 2026-08-06 19:32 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	Barry Song, Youngjun Park, her0gyugyu, linux-mm, linux-kernel

find_next_to_unuse() walks every offset from 0 to si->max, and swapoff
restarts that walk on each retry, so the cost scales with the size of
the device rather than with the few slots the shmem and mmlist passes
could not free.  It has caused stalls before.

The flat walk predates the swap table.  Slot state now lives in a per
cluster table, and wait_for_allocation() stops all allocation before
try_to_unuse() runs, so a cluster that holds no slot in use stays that
way.  Skip such a cluster instead of reading all of its entries.

Commit dc644a073769 ("mm: add three more cond_resched() in swapoff")
answered those stalls with a cond_resched() every 256 offsets.  A walk
bounded by one cluster no longer needs that counter.  The loop now runs
at most SWAPFILE_CLUSTER times before it returns or reschedules, the
same bound swap_reclaim_full_clusters() already scans between
cond_resched() calls.

The scan end is clamped to si->max, so the walk stops there rather than
running into the masked tail of the last cluster.

ci->count is read without ci->lock, so READ_ONCE() marks the read for
KCSAN.  Allocation is already stopped, so the count can only drop, and a
slot stops being counted only after its folio has left the swap cache.
An empty cluster therefore holds nothing for try_to_unuse() to act on.

Signed-off-by: Youngjun Park <youngjun.park@lge.com>
Reviewed-by: Barry Song <baohua@kernel.org>
---
 mm/swapfile.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index dea2d3b36e06..0d24efd32eb0 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -370,8 +370,6 @@ static void discard_swap_cluster(struct swap_info_struct *si,
 	}
 }
 
-#define LATENCY_LIMIT		256
-
 static inline bool cluster_is_empty(struct swap_cluster_info *info)
 {
 	return info->count == 0;
@@ -2763,7 +2761,9 @@ static int unuse_mm(struct mm_struct *mm, unsigned int type)
 static unsigned int find_next_to_unuse(struct swap_info_struct *si,
 					unsigned int prev)
 {
-	unsigned int i;
+	struct swap_cluster_info *ci;
+	unsigned long i, end;
+	unsigned int ci_off;
 	unsigned long swp_tb;
 
 	/*
@@ -2772,19 +2772,36 @@ static unsigned int find_next_to_unuse(struct swap_info_struct *si,
 	 * hits are okay, and sys_swapoff() has already prevented new
 	 * allocations from this area (while holding swap_lock).
 	 */
-	for (i = prev + 1; i < si->max; i++) {
-		swp_tb = swap_table_get(__swap_offset_to_cluster(si, i),
-					i % SWAPFILE_CLUSTER);
-		if (!swp_tb_is_null(swp_tb) && !swp_tb_is_bad(swp_tb))
-			break;
-		if ((i % LATENCY_LIMIT) == 0)
+	i = prev + 1;
+	while (i < si->max) {
+		ci = __swap_offset_to_cluster(si, i);
+		end = min_t(unsigned long,
+			    ALIGN_DOWN(i, SWAPFILE_CLUSTER) + SWAPFILE_CLUSTER,
+			    si->max);
+
+		/*
+		 * An empty cluster has no slot in use, so skip it whole.
+		 * A slot is uncounted only after its folio left the swap
+		 * cache, so there is nothing here for try_to_unuse() to act on.
+		 * Count only drops here, so a READ_ONCE() without ci->lock is
+		 * enough, unlike in every other cluster_is_empty() caller.
+		 */
+		if (!READ_ONCE(ci->count)) {
+			i = end;
 			cond_resched();
-	}
+			continue;
+		}
 
-	if (i == si->max)
-		i = 0;
+		ci_off = i % SWAPFILE_CLUSTER;
+		for (; i < end; ci_off++, i++) {
+			swp_tb = swap_table_get(ci, ci_off);
+			if (!swp_tb_is_null(swp_tb) && !swp_tb_is_bad(swp_tb))
+				return i;
+		}
+		cond_resched();
+	}
 
-	return i;
+	return 0;
 }
 
 static int try_to_unuse(unsigned int type)
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan
  2026-08-06 19:32 [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan Youngjun Park
  2026-08-06 19:32 ` [PATCH v3 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info Youngjun Park
  2026-08-06 19:32 ` [PATCH v3 2/2] mm/swap: scan by cluster in find_next_to_unuse() Youngjun Park
@ 2026-08-06 20:06 ` Andrew Morton
  2026-08-07  6:41   ` Youngjun Park
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2026-08-06 20:06 UTC (permalink / raw)
  To: Youngjun Park
  Cc: Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On Fri,  7 Aug 2026 04:32:26 +0900 Youngjun Park <youngjun.park@lge.com> wrote:

> find_next_to_unuse() walks a swap device one offset at a time.  Slot
> state now lives in a per cluster swap table, so patch 2 dismisses an
> empty cluster with one counter read instead of SWAPFILE_CLUSTER table
> reads.

Thanks.

Can you help us understand how significant this change is for users? 
If "not very" then I'd prefer to defer consideraton of the series until
after 7.3-rc1.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan
  2026-08-06 20:06 ` [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan Andrew Morton
@ 2026-08-07  6:41   ` Youngjun Park
  2026-08-07  6:59     ` Youngjun Park
  2026-08-07  8:42     ` Baoquan He
  0 siblings, 2 replies; 12+ messages in thread
From: Youngjun Park @ 2026-08-07  6:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On Thu, Aug 06, 2026 at 01:06:55PM -0700, Andrew Morton wrote:
> On Fri,  7 Aug 2026 04:32:26 +0900 Youngjun Park <youngjun.park@lge.com> wrote:
> 
> > find_next_to_unuse() walks a swap device one offset at a time.  Slot
> > state now lives in a per cluster swap table, so patch 2 dismisses an
> > empty cluster with one counter read instead of SWAPFILE_CLUSTER table
> > reads.
> 
> Thanks.
> 
> Can you help us understand how significant this change is for users? 
> If "not very" then I'd prefer to defer consideraton of the series until
> after 7.3-rc1.

Hello Andrew

"Not very" in the common case, though there is a case where the win is clear.
No bug and no user report.

For now I would rather defer to after 7.3-rc1.

And for your reference, here is the details.

Every swapoff does a little less work now, because the scan steps over an
unused area one cluster at a time.
But IMHO most of the swapoff time goes to unuse_mm() and to reading the pages back in.

The gain shows on a large swap device that is almost empty, when the last
pages still in use are near the end of it.  The scan has to walk up to
them, and today it looks at every slot on the way.  Now the empty clusters
in between are skipped in one step.

I have no measured times yet, since that case has to be set up on purpose.
What I did is the arithmetic for the case that skips best,
For example 1T of swap with 256M slots with SWAPFILE_CLUSTER = 512
and everything free but the far end:

  - today:          256M table reads
  - with the skip:  512K counter reads

That should be around half a second of scan saved.

What I have checked is that empty clusters are skipped as intended, so the
scan does less work.  That work is a small part of swapoff, so depending on
the situation it may be too small to see in clock time.

Thanks,
Youngjun

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan
  2026-08-07  6:41   ` Youngjun Park
@ 2026-08-07  6:59     ` Youngjun Park
  2026-08-07  8:42     ` Baoquan He
  1 sibling, 0 replies; 12+ messages in thread
From: Youngjun Park @ 2026-08-07  6:59 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On Fri, Aug 07, 2026 at 03:41:22PM +0900, Youngjun Park wrote:
> On Thu, Aug 06, 2026 at 01:06:55PM -0700, Andrew Morton wrote:
> > On Fri,  7 Aug 2026 04:32:26 +0900 Youngjun Park <youngjun.park@lge.com> wrote:
> > 
> > > find_next_to_unuse() walks a swap device one offset at a time.  Slot
> > > state now lives in a per cluster swap table, so patch 2 dismisses an
> > > empty cluster with one counter read instead of SWAPFILE_CLUSTER table
> > > reads.
> > 
> > Thanks.
> > 
> > Can you help us understand how significant this change is for users? 
> > If "not very" then I'd prefer to defer consideraton of the series until
> > after 7.3-rc1.
> 
> Hello Andrew
> 
> "Not very" in the common case, though there is a case where the win is clear.
> No bug and no user report.

Something I forgot to mention,

This only affects swapoff.
and few users run swapoff often, so the impact is limited either way.

> For now I would rather defer to after 7.3-rc1.
> 
> And for your reference, here is the details.
> 
> Every swapoff does a little less work now, because the scan steps over an
> unused area one cluster at a time.
> But IMHO most of the swapoff time goes to unuse_mm() and to reading the pages back in.
> 
> The gain shows on a large swap device that is almost empty, when the last
> pages still in use are near the end of it.  The scan has to walk up to
> them, and today it looks at every slot on the way.  Now the empty clusters
> in between are skipped in one step.
> 
> I have no measured times yet, since that case has to be set up on purpose.
> What I did is the arithmetic for the case that skips best,
> For example 1T of swap with 256M slots with SWAPFILE_CLUSTER = 512
> and everything free but the far end:
> 
>   - today:          256M table reads
>   - with the skip:  512K counter reads
> 
> That should be around half a second of scan saved.

+ benefit.

Reclaim can put a cached folio back into a page table with the slot it
already had, so try_to_unuse() retries and the scan starts over.  The
saving then applies once per pass.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/2] mm/swap: scan by cluster in find_next_to_unuse()
  2026-08-06 19:32 ` [PATCH v3 2/2] mm/swap: scan by cluster in find_next_to_unuse() Youngjun Park
@ 2026-08-07  7:33   ` Kairui Song
  2026-08-07  9:52   ` Baoquan He
  1 sibling, 0 replies; 12+ messages in thread
From: Kairui Song @ 2026-08-07  7:33 UTC (permalink / raw)
  To: Youngjun Park
  Cc: Andrew Morton, Chris Li, Kemeng Shi, Nhat Pham, Baoquan He,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On Fri, Aug 7, 2026 at 3:47 AM Youngjun Park <youngjun.park@lge.com> wrote:
>
> find_next_to_unuse() walks every offset from 0 to si->max, and swapoff
> restarts that walk on each retry, so the cost scales with the size of
> the device rather than with the few slots the shmem and mmlist passes
> could not free.  It has caused stalls before.
>
> The flat walk predates the swap table.  Slot state now lives in a per
> cluster table, and wait_for_allocation() stops all allocation before
> try_to_unuse() runs, so a cluster that holds no slot in use stays that
> way.  Skip such a cluster instead of reading all of its entries.
>
> Commit dc644a073769 ("mm: add three more cond_resched() in swapoff")
> answered those stalls with a cond_resched() every 256 offsets.  A walk
> bounded by one cluster no longer needs that counter.  The loop now runs
> at most SWAPFILE_CLUSTER times before it returns or reschedules, the
> same bound swap_reclaim_full_clusters() already scans between
> cond_resched() calls.
>
> The scan end is clamped to si->max, so the walk stops there rather than
> running into the masked tail of the last cluster.
>
> ci->count is read without ci->lock, so READ_ONCE() marks the read for
> KCSAN.  Allocation is already stopped, so the count can only drop, and a
> slot stops being counted only after its folio has left the swap cache.
> An empty cluster therefore holds nothing for try_to_unuse() to act on.
>
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> Reviewed-by: Barry Song <baohua@kernel.org>
> ---
>  mm/swapfile.c | 43 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 30 insertions(+), 13 deletions(-)
>

Looks good.

Acked-by: Kairui Song <kasong@tencent.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan
  2026-08-07  6:41   ` Youngjun Park
  2026-08-07  6:59     ` Youngjun Park
@ 2026-08-07  8:42     ` Baoquan He
  2026-08-07  9:59       ` Youngjun Park
  2026-08-07 21:42       ` Andrew Morton
  1 sibling, 2 replies; 12+ messages in thread
From: Baoquan He @ 2026-08-07  8:42 UTC (permalink / raw)
  To: Youngjun Park
  Cc: Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On 08/07/26 at 03:41pm, Youngjun Park wrote:
> On Thu, Aug 06, 2026 at 01:06:55PM -0700, Andrew Morton wrote:
> > On Fri,  7 Aug 2026 04:32:26 +0900 Youngjun Park <youngjun.park@lge.com> wrote:
> > 
> > > find_next_to_unuse() walks a swap device one offset at a time.  Slot
> > > state now lives in a per cluster swap table, so patch 2 dismisses an
> > > empty cluster with one counter read instead of SWAPFILE_CLUSTER table
> > > reads.
> > 
> > Thanks.
> > 
> > Can you help us understand how significant this change is for users? 
> > If "not very" then I'd prefer to defer consideraton of the series until
> > after 7.3-rc1.
> 
> Hello Andrew
> 
> "Not very" in the common case, though there is a case where the win is clear.
> No bug and no user report.
> 
> For now I would rather defer to after 7.3-rc1.
> 
> And for your reference, here is the details.
> 
> Every swapoff does a little less work now, because the scan steps over an
> unused area one cluster at a time.
> But IMHO most of the swapoff time goes to unuse_mm() and to reading the pages back in.
> 
> The gain shows on a large swap device that is almost empty, when the last
> pages still in use are near the end of it.  The scan has to walk up to
> them, and today it looks at every slot on the way.  Now the empty clusters
> in between are skipped in one step.
> 
> I have no measured times yet, since that case has to be set up on purpose.
> What I did is the arithmetic for the case that skips best,
> For example 1T of swap with 256M slots with SWAPFILE_CLUSTER = 512
> and everything free but the far end:
> 
>   - today:          256M table reads
>   - with the skip:  512K counter reads


Maybe just use time to measure swapoff time consuming, just like below
as I did on a kvm guest, I guess a bare metal machine with larger system
ram could be more obvious?

root@fedora:~# free -h
               total        used        free      shared  buff/cache   available
Mem:           3.8Gi       181Mi       3.6Gi       924Ki        72Mi       3.7Gi
Swap:          2.0Gi        18Mi       2.0Gi
root@fedora:~# time swapoff /dev/vdb

real	0m0.101s
user	0m0.001s
sys	0m0.017s
root@fedora:~# swapon /dev/vdb
root@fedora:~# time swapoff /dev/vdb

real	0m0.014s
user	0m0.003s
sys	0m0.001s

Not sure if Andrew is asking for this.


> 
> That should be around half a second of scan saved.

Yeah, a concrete number is shown.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info
  2026-08-06 19:32 ` [PATCH v3 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info Youngjun Park
@ 2026-08-07  9:49   ` Baoquan He
  0 siblings, 0 replies; 12+ messages in thread
From: Baoquan He @ 2026-08-07  9:49 UTC (permalink / raw)
  To: Youngjun Park
  Cc: Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On 08/07/26 at 04:32am, Youngjun Park wrote:
> setup_swap_clusters_info() allocates cluster_info for every swap area,
> not only for SSDs.
> 
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> Acked-by: Kairui Song <kasong@tencent.com>
> Reviewed-by: Barry Song <baohua@kernel.org>
> ---
>  include/linux/swap.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Baoquan He <baoquan.he@linux.dev>

> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 2cb1d29307c5..2b14e2e9673b 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -246,7 +246,7 @@ struct swap_info_struct {
>  	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 */
> +	struct swap_cluster_info *cluster_info; /* array, one entry per cluster */
>  	struct list_head free_clusters; /* free clusters list */
>  	struct list_head full_clusters; /* full clusters list */
>  	struct list_head nonfull_clusters[SWAP_NR_ORDERS];
> -- 
> 2.48.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/2] mm/swap: scan by cluster in find_next_to_unuse()
  2026-08-06 19:32 ` [PATCH v3 2/2] mm/swap: scan by cluster in find_next_to_unuse() Youngjun Park
  2026-08-07  7:33   ` Kairui Song
@ 2026-08-07  9:52   ` Baoquan He
  1 sibling, 0 replies; 12+ messages in thread
From: Baoquan He @ 2026-08-07  9:52 UTC (permalink / raw)
  To: Youngjun Park
  Cc: Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On 08/07/26 at 04:32am, Youngjun Park wrote:
> find_next_to_unuse() walks every offset from 0 to si->max, and swapoff
> restarts that walk on each retry, so the cost scales with the size of
> the device rather than with the few slots the shmem and mmlist passes
> could not free.  It has caused stalls before.
> 
> The flat walk predates the swap table.  Slot state now lives in a per
> cluster table, and wait_for_allocation() stops all allocation before
> try_to_unuse() runs, so a cluster that holds no slot in use stays that
> way.  Skip such a cluster instead of reading all of its entries.
> 
> Commit dc644a073769 ("mm: add three more cond_resched() in swapoff")
> answered those stalls with a cond_resched() every 256 offsets.  A walk
> bounded by one cluster no longer needs that counter.  The loop now runs
> at most SWAPFILE_CLUSTER times before it returns or reschedules, the
> same bound swap_reclaim_full_clusters() already scans between
> cond_resched() calls.
> 
> The scan end is clamped to si->max, so the walk stops there rather than
> running into the masked tail of the last cluster.
> 
> ci->count is read without ci->lock, so READ_ONCE() marks the read for
> KCSAN.  Allocation is already stopped, so the count can only drop, and a
> slot stops being counted only after its folio has left the swap cache.
> An empty cluster therefore holds nothing for try_to_unuse() to act on.
> 
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> Reviewed-by: Barry Song <baohua@kernel.org>
> ---
>  mm/swapfile.c | 43 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index dea2d3b36e06..0d24efd32eb0 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -370,8 +370,6 @@ static void discard_swap_cluster(struct swap_info_struct *si,
>  	}
>  }
>  
> -#define LATENCY_LIMIT		256
> -
>  static inline bool cluster_is_empty(struct swap_cluster_info *info)
>  {
>  	return info->count == 0;
> @@ -2763,7 +2761,9 @@ static int unuse_mm(struct mm_struct *mm, unsigned int type)
>  static unsigned int find_next_to_unuse(struct swap_info_struct *si,
>  					unsigned int prev)
>  {
> -	unsigned int i;
> +	struct swap_cluster_info *ci;
> +	unsigned long i, end;
> +	unsigned int ci_off;
>  	unsigned long swp_tb;
>  
>  	/*
> @@ -2772,19 +2772,36 @@ static unsigned int find_next_to_unuse(struct swap_info_struct *si,
>  	 * hits are okay, and sys_swapoff() has already prevented new
>  	 * allocations from this area (while holding swap_lock).
>  	 */
> -	for (i = prev + 1; i < si->max; i++) {
> -		swp_tb = swap_table_get(__swap_offset_to_cluster(si, i),
> -					i % SWAPFILE_CLUSTER);
> -		if (!swp_tb_is_null(swp_tb) && !swp_tb_is_bad(swp_tb))
> -			break;
> -		if ((i % LATENCY_LIMIT) == 0)
> +	i = prev + 1;
> +	while (i < si->max) {
> +		ci = __swap_offset_to_cluster(si, i);
> +		end = min_t(unsigned long,
> +			    ALIGN_DOWN(i, SWAPFILE_CLUSTER) + SWAPFILE_CLUSTER,
> +			    si->max);
> +
> +		/*
> +		 * An empty cluster has no slot in use, so skip it whole.
> +		 * A slot is uncounted only after its folio left the swap
> +		 * cache, so there is nothing here for try_to_unuse() to act on.
> +		 * Count only drops here, so a READ_ONCE() without ci->lock is
> +		 * enough, unlike in every other cluster_is_empty() caller.
> +		 */
> +		if (!READ_ONCE(ci->count)) {
> +			i = end;
>  			cond_resched();
> -	}
> +			continue;
> +		}
>  
> -	if (i == si->max)
> -		i = 0;
> +		ci_off = i % SWAPFILE_CLUSTER;
> +		for (; i < end; ci_off++, i++) {
> +			swp_tb = swap_table_get(ci, ci_off);
> +			if (!swp_tb_is_null(swp_tb) && !swp_tb_is_bad(swp_tb))
> +				return i;

I would remove ci_off to save one local variable, but it's only personal
preference, not strong opinion.

	for (; i < end; i++) {
	    swp_tb = swap_table_get(ci, i % SWAPFILE_CLUSTER);
	    ...
	}

Other than the nitpick, this is a great optimization patch.

Reviewed-by: Baoquan He <baoquan.he@linux.dev>


> +		}
> +		cond_resched();
> +	}
>  
> -	return i;
> +	return 0;
>  }
>  
>  static int try_to_unuse(unsigned int type)
> -- 
> 2.48.1
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan
  2026-08-07  8:42     ` Baoquan He
@ 2026-08-07  9:59       ` Youngjun Park
  2026-08-07 21:42       ` Andrew Morton
  1 sibling, 0 replies; 12+ messages in thread
From: Youngjun Park @ 2026-08-07  9:59 UTC (permalink / raw)
  To: Baoquan He
  Cc: Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On Fri, Aug 07, 2026 at 04:42:22PM +0800, Baoquan He wrote:
> On 08/07/26 at 03:41pm, Youngjun Park wrote:
> > On Thu, Aug 06, 2026 at 01:06:55PM -0700, Andrew Morton wrote:
> > > On Fri,  7 Aug 2026 04:32:26 +0900 Youngjun Park <youngjun.park@lge.com> wrote:
> > > 
> > > > find_next_to_unuse() walks a swap device one offset at a time.  Slot
> > > > state now lives in a per cluster swap table, so patch 2 dismisses an
> > > > empty cluster with one counter read instead of SWAPFILE_CLUSTER table
> > > > reads.
> > > 
> > > Thanks.
> > > 
> > > Can you help us understand how significant this change is for users? 
> > > If "not very" then I'd prefer to defer consideraton of the series until
> > > after 7.3-rc1.
> > 
> > Hello Andrew
> > 
> > "Not very" in the common case, though there is a case where the win is clear.
> > No bug and no user report.
> > 
> > For now I would rather defer to after 7.3-rc1.
> > 
> > And for your reference, here is the details.
> > 
> > Every swapoff does a little less work now, because the scan steps over an
> > unused area one cluster at a time.
> > But IMHO most of the swapoff time goes to unuse_mm() and to reading the pages back in.
> > 
> > The gain shows on a large swap device that is almost empty, when the last
> > pages still in use are near the end of it.  The scan has to walk up to
> > them, and today it looks at every slot on the way.  Now the empty clusters
> > in between are skipped in one step.
> > 
> > I have no measured times yet, since that case has to be set up on purpose.
> > What I did is the arithmetic for the case that skips best,
> > For example 1T of swap with 256M slots with SWAPFILE_CLUSTER = 512
> > and everything free but the far end:
> > 
> >   - today:          256M table reads
> >   - with the skip:  512K counter reads
> 
> 
> Maybe just use time to measure swapoff time consuming, just like below
> as I did on a kvm guest, I guess a bare metal machine with larger system
> ram could be more obvious?
> 
> root@fedora:~# free -h
>                total        used        free      shared  buff/cache   available
> Mem:           3.8Gi       181Mi       3.6Gi       924Ki        72Mi       3.7Gi
> Swap:          2.0Gi        18Mi       2.0Gi
> root@fedora:~# time swapoff /dev/vdb
> 
> real	0m0.101s
> user	0m0.001s
> sys	0m0.017s
> root@fedora:~# swapon /dev/vdb
> root@fedora:~# time swapoff /dev/vdb
> 
> real	0m0.014s
> user	0m0.003s
> sys	0m0.001s
> 
> Not sure if Andrew is asking for this.

Thank you for looking into it.

Yes, it is possible to show this. However, it cannot provide the saturation
time, as that depends on the swap device size and swap slot distribution.

Since a naive swapoff test cannot provide stable evidence, I addressed the
approximate skip time (and also checked whether a skip occurred) using a logical time
calculation as shown below.

I try to take some time to measure the swapoff time under experiment conditions.
I will follow up this in the next patch iteration or sooner in this thread
(depending on Andrew's decision).

> > 
> > That should be around half a second of scan saved.
> 
> Yeah, a concrete number is shown.

Right, based on time complexity arithmetic.

Youngjun

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan
  2026-08-07  8:42     ` Baoquan He
  2026-08-07  9:59       ` Youngjun Park
@ 2026-08-07 21:42       ` Andrew Morton
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2026-08-07 21:42 UTC (permalink / raw)
  To: Baoquan He
  Cc: Youngjun Park, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
	Barry Song, her0gyugyu, linux-mm, linux-kernel

On Fri, 7 Aug 2026 16:42:22 +0800 Baoquan He <baoquan.he@linux.dev> wrote:

> > I have no measured times yet, since that case has to be set up on purpose.
> > What I did is the arithmetic for the case that skips best,
> > For example 1T of swap with 256M slots with SWAPFILE_CLUSTER = 512
> > and everything free but the far end:
> > 
> >   - today:          256M table reads
> >   - with the skip:  512K counter reads
> 
> 
> Maybe just use time to measure swapoff time consuming, just like below
> as I did on a kvm guest, I guess a bare metal machine with larger system
> ram could be more obvious?
> 
> root@fedora:~# free -h
>                total        used        free      shared  buff/cache   available
> Mem:           3.8Gi       181Mi       3.6Gi       924Ki        72Mi       3.7Gi
> Swap:          2.0Gi        18Mi       2.0Gi
> root@fedora:~# time swapoff /dev/vdb
> 
> real	0m0.101s
> user	0m0.001s
> sys	0m0.017s
> root@fedora:~# swapon /dev/vdb
> root@fedora:~# time swapoff /dev/vdb
> 
> real	0m0.014s
> user	0m0.003s
> sys	0m0.001s
> 
> Not sure if Andrew is asking for this.

I think it's helpful to include such info.  The audience for changelogs
is more than swap developers!  It's also an MM maintainer and -stable
maintainers and other people who are all wondering "should I backport
this for my users".  Let's give them the means to determine that.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-08-07 21:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 19:32 [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan Youngjun Park
2026-08-06 19:32 ` [PATCH v3 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info Youngjun Park
2026-08-07  9:49   ` Baoquan He
2026-08-06 19:32 ` [PATCH v3 2/2] mm/swap: scan by cluster in find_next_to_unuse() Youngjun Park
2026-08-07  7:33   ` Kairui Song
2026-08-07  9:52   ` Baoquan He
2026-08-06 20:06 ` [PATCH v3 0/2] mm/swap: skip empty clusters in the swapoff scan Andrew Morton
2026-08-07  6:41   ` Youngjun Park
2026-08-07  6:59     ` Youngjun Park
2026-08-07  8:42     ` Baoquan He
2026-08-07  9:59       ` Youngjun Park
2026-08-07 21:42       ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox