[PATCH 0/2] mm/swap: skip empty clusters in the swapoff scan

[PATCH 0/2] mm/swap: skip empty clusters in the swapoff scan

[Youngjun Park]

From: Youngjun Park <her0gyugyu@gmail.com>

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 counter in the skip branch confirmed it runs, and swapoff completed
with the device under load.  No DEBUG_VM or lockdep splats.

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        | 39 ++++++++++++++++++++++++++-------------
 2 files changed, 27 insertions(+), 14 deletions(-)


base-commit: 2a4faffc103297c4822bb269d521249b75cc2261
-- 
2.48.1

[PATCH 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info

[Youngjun Park]

From: Youngjun Park <her0gyugyu@gmail.com>

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

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

Signed-off-by: Youngjun Park <youngjun.park@lge.com>
---
 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..e12c73891b48 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; /* one per cluster, on every device */
 	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

Re: [PATCH 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info

[Youngjun Park]

On Tue, Aug 04, 2026 at 05:51:52PM +0800, Barry Song wrote:
> On Tue, Jul 28, 2026 at 11:59 PM Youngjun Park <her0gyugyu@gmail.com> wrote:
> >
> > From: Youngjun Park <her0gyugyu@gmail.com>
> >
> > From: Youngjun Park <youngjun.park@lge.com>
> >
> > setup_swap_clusters_info() allocates cluster_info for every swap area,
> > not only for SSDs.
> >
> > Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> > ---
> >  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..e12c73891b48 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; /* one per cluster, on every device */
> 
> Do we still need "on every device", since it is on every device?
> 
> Thanks
> Barry

Hi Barry!

Right, Will trim it to /* one per cluster *

Youngjun

Re: [PATCH 1/2] mm/swap: fix stale comment on swap_info_struct::cluster_info

[Youngjun Park]

On Tue, Aug 04, 2026 at 05:48:51PM +0800, Kairui Song wrote:
> On Wed, Jul 29, 2026 at 12:43 AM Youngjun Park <her0gyugyu@gmail.com> wrote:
> >
> > From: Youngjun Park <her0gyugyu@gmail.com>
> >
> > From: Youngjun Park <youngjun.park@lge.com>
> 
> Hi Youngjun, Thanks for the patch.
> 
> Double From:? I think you want to keep the later one right?

my git config was wrong :(
fixed it locally 

> >
> > setup_swap_clusters_info() allocates cluster_info for every swap area,
> > not only for SSDs.
> >
> > Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> > ---
> >  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..e12c73891b48 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; /* one per cluster, on every device */
> 
> Nice catch.
> 
> Acked-by: Kairui Song <kasong@tencent.com>

[PATCH 2/2] mm/swap: scan by cluster in find_next_to_unuse()

[Youngjun Park]

From: Youngjun Park <her0gyugyu@gmail.com>

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

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 with cluster_is_empty() 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.

cluster_is_empty() reads ci->count without ci->lock, like the rest of
this scan.  A slot stops being counted only after its folio has left the
swap cache, so an empty cluster holds nothing for try_to_unuse() to act
on and skipping it loses no work.

Signed-off-by: Youngjun Park <youngjun.park@lge.com>
---
 mm/swapfile.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 5d15913dcf86..230abb276ceb 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,32 @@ 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);
+		ci_off = i % SWAPFILE_CLUSTER;
+		end = min(si->max, i - ci_off + SWAPFILE_CLUSTER);
+
+		/*
+		 * 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.
+		 */
+		if (cluster_is_empty(ci)) {
+			i = end;
 			cond_resched();
-	}
+			continue;
+		}
 
-	if (i == si->max)
-		i = 0;
+		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

Re: [PATCH 2/2] mm/swap: scan by cluster in find_next_to_unuse()

[Kairui Song]

On Wed, Jul 29, 2026 at 12:52 AM Youngjun Park <her0gyugyu@gmail.com> wrote:
>
> From: Youngjun Park <her0gyugyu@gmail.com>
>
> From: Youngjun Park <youngjun.park@lge.com>
>
> 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 with cluster_is_empty() 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.
>
> cluster_is_empty() reads ci->count without ci->lock, like the rest of
> this scan.  A slot stops being counted only after its folio has left the
> swap cache, so an empty cluster holds nothing for try_to_unuse() to act
> on and skipping it loses no work.

A really nice optimization, thanks!

>
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> ---
>  mm/swapfile.c | 39 ++++++++++++++++++++++++++-------------
>  1 file changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 5d15913dcf86..230abb276ceb 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,32 @@ 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);
> +               ci_off = i % SWAPFILE_CLUSTER;
> +               end = min(si->max, i - ci_off + SWAPFILE_CLUSTER);

Do we need the min here? Table size is always SWAPFILE_CLUSTER aligned.

> +
> +               /*
> +                * 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.
> +                */
> +               if (cluster_is_empty(ci)) {

Hmm, it's not wrong, but this is indeed the only user calling
cluster_is_empty without holding a lock, and not in initilization
path, perhaps we should at least make it READ_ONCE? Maybe KCSAN will
not be happy, I guess? Just nitpick.

Re: [PATCH 2/2] mm/swap: scan by cluster in find_next_to_unuse()

[Youngjun Park]

...

Hello Kairui
Thanks for the review.

> > -       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);
> > +               ci_off = i % SWAPFILE_CLUSTER;
> > +               end = min(si->max, i - ci_off + SWAPFILE_CLUSTER);
> 
> Do we need the min here? Table size is always SWAPFILE_CLUSTER aligned.

Yes. I remove it.

> > +
> > +               /*
> > +                * 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.
> > +                */
> > +               if (cluster_is_empty(ci)) {
> 
> Hmm, it's not wrong, but this is indeed the only user calling
> cluster_is_empty without holding a lock, and not in initilization
> path, perhaps we should at least make it READ_ONCE? Maybe KCSAN will
> not be happy, I guess? Just nitpick.

Right it is better to use READ_ONCE. I also applied it and submited v2!

Youngjun