[PATCH 0/4] mm, swap: some random fixes and cleanups

[PATCH 0/4] mm, swap: some random fixes and cleanups

[Kemeng Shi]

Hello all,

This series contains some random fixes and cleanups. More details
can be found in respective patches.

Thanks!

Kemeng Shi (4):
  mm, swap: Fix potential NULL dereference when trying a sleep table
    allocation
  mm, swap: Move setup_swap_clusters_info() after SWP_SOLIDSTATE
    initialization
  mm, swap: return early from swap_extend_table_try_free() on first
    non-zero entry
  mm, swap: Remove unneeded swap_extend_table_try_free() in
    swap_dup_entries_cluster()

 mm/swapfile.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

-- 
2.36.1

[PATCH 1/4] mm, swap: Fix potential NULL dereference when trying a sleep table allocation

[Kemeng Shi]

The root cause of this issue is because multi-tables are updated in non
atomic context. To be more specific, the issue could be triggerred as
following:

swap_alloc_fast         swap_cluster_populate()
                         /* Try a sleep allocation */
                         spin_unlock(&ci->lock);
                         swap_cluster_alloc_table()
                          rcu_assign_pointer(ci->table, table);

 ci = swap_cluster_lock(si, offset)
 cluster_is_usable(ci, order）
  if (!cluster_table_is_alloced(ci)) // ok
 alloc_swap_scan_cluster()
  cluster_scan_range()
   __swap_table_get()

                          /* free table when more table allocation fails */
                          ci->memcg_table = kzalloc_obj(*ci->memcg_table,
                                                        gfp);
                          if (!ci->memcg_table)
                           swap_cluster_free_table()
                            rcu_assign_pointer(ci->table, NULL);

    table = rcu_dereference_check(ci->table, lockdep_is_held(&ci->lock));
     atomic_long_read(&table[off]); // NULL dereference

Fix the issue by updating allocated tables in atomic context.

Fixes: 2fe7a6f5024b8 ("mm/memcg, swap: store cgroup id in cluster table directly")
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 mm/swapfile.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 615d90867111..d29062d9c3cd 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -490,6 +490,20 @@ static int swap_cluster_alloc_table(struct swap_cluster_info *ci, gfp_t gfp)
 	return 0;
 }
 
+static void swap_cluster_copy_table(struct swap_cluster_info *d_ci,
+				    struct swap_cluster_info *s_ci)
+{
+	rcu_assign_pointer(d_ci->table, rcu_access_pointer(s_ci->table));
+
+#ifdef CONFIG_MEMCG
+	d_ci->memcg_table = s_ci->memcg_table;
+#endif
+
+#if !SWAP_TABLE_HAS_ZEROFLAG
+	d_ci->zero_bitmap = s_ci->zero_bitmap;
+#endif
+}
+
 /*
  * Sanity check to ensure nothing leaked, and the specified range is empty.
  * One special case is that bad slots can't be freed, so check the number of
@@ -527,6 +541,7 @@ static struct swap_cluster_info *
 swap_cluster_populate(struct swap_info_struct *si,
 			 struct swap_cluster_info *ci)
 {
+	struct swap_cluster_info tmp_ci;
 	int ret;
 
 	/*
@@ -552,7 +567,8 @@ swap_cluster_populate(struct swap_info_struct *si,
 		spin_unlock(&si->global_cluster_lock);
 	local_unlock(&percpu_swap_cluster.lock);
 
-	ret = swap_cluster_alloc_table(ci, __GFP_HIGH | __GFP_NOMEMALLOC |
+	tmp_ci = *ci;
+	ret = swap_cluster_alloc_table(&tmp_ci, __GFP_HIGH | __GFP_NOMEMALLOC |
 					   GFP_KERNEL);
 
 	/*
@@ -573,6 +589,9 @@ swap_cluster_populate(struct swap_info_struct *si,
 		spin_unlock(&ci->lock);
 		return NULL;
 	}
+
+	/* Update tables in atomic context */
+	swap_cluster_copy_table(ci, &tmp_ci);
 	return ci;
 }
 
-- 
2.36.1

[PATCH 2/4] mm, swap: Move setup_swap_clusters_info() after SWP_SOLIDSTATE initialization

[Kemeng Shi]

In setup_swap_clusters_info(), SWP_SOLIDSTATE is used to decide
global_cluster allocation. Move setup_swap_clusters_info() after
SWP_SOLIDSTATE initialization to avoid unneeded global_cluster
allocation.

Fixes: 451c6326105b2 ("mm, swap: clean up swapon process and locking")
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 mm/swapfile.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index d29062d9c3cd..81c4040912be 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -3607,11 +3607,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 
 	maxpages = si->max;
 
-	/* Set up the swap cluster info */
-	error = setup_swap_clusters_info(si, swap_header, maxpages);
-	if (error)
-		goto bad_swap_unlock_inode;
-
 	if (si->bdev && bdev_stable_writes(si->bdev))
 		si->flags |= SWP_STABLE_WRITES;
 
@@ -3625,6 +3620,14 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		inced_nr_rotate_swap = true;
 	}
 
+	/*
+	 * Set up the swap cluster info. SWP_SOLIDSTATE is used for
+	 * global_cluster allocation
+	 */
+	error = setup_swap_clusters_info(si, swap_header, maxpages);
+	if (error)
+		goto bad_swap_unlock_inode;
+
 	if ((swap_flags & SWAP_FLAG_DISCARD) &&
 	    si->bdev && bdev_max_discard_sectors(si->bdev)) {
 		/*
-- 
2.36.1

[PATCH 3/4] mm, swap: return early from swap_extend_table_try_free() on first non-zero entry

[Kemeng Shi]

Return immediately when the first non-zero swap count is found as
any non-zero swap count prevents freeing extend_table and further
iteration is pointless.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 mm/swapfile.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 81c4040912be..3fcbd10c6353 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1530,20 +1530,17 @@ int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp)
 static void swap_extend_table_try_free(struct swap_cluster_info *ci)
 {
 	unsigned long i;
-	bool can_free = true;
 
 	if (!ci->extend_table)
 		return;
 
 	for (i = 0; i < SWAPFILE_CLUSTER; i++) {
 		if (ci->extend_table[i])
-			can_free = false;
+			return;
 	}
 
-	if (can_free) {
-		kfree(ci->extend_table);
-		ci->extend_table = NULL;
-	}
+	kfree(ci->extend_table);
+	ci->extend_table = NULL;
 }
 
 /* Decrease the swap count of one slot, without freeing it */
-- 
2.36.1

[PATCH 4/4] mm, swap: Remove unneeded swap_extend_table_try_free() in swap_dup_entries_cluster()

[Kemeng Shi]

Since commit 0475fde0f68de ("mm, swap: avoid leaving unused extend table
after alloc race"), extend table is always allocated when any swap count
reach MAX - 1 and is always freed when swap count decrease to MAX - 1
or MAX - 2 with cluster lock held. So the extend table will always be
freed properly when decrease swap count in __swap_cluster_put_entry().
So swap_extend_table_try_free() outside of __swap_cluster_put_entry()
is unneeded and can be removed.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 mm/swapfile.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 3fcbd10c6353..fc9f16bc2e4e 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1732,7 +1732,6 @@ static int swap_dup_entries_cluster(struct swap_info_struct *si,
 failed:
 	while (ci_off-- > ci_start)
 		__swap_cluster_put_entry(ci, ci_off);
-	swap_extend_table_try_free(ci);
 	swap_cluster_unlock(ci);
 	return err;
 }
-- 
2.36.1

Re: [PATCH 1/4] mm, swap: Fix potential NULL dereference when trying a sleep table allocation

[Youngjun Park]

On Mon, Jul 20, 2026 at 03:13:39PM +0800, Kemeng Shi wrote:

Hello Kemeng Shi,

Good catch!

it indeed looks like a possible race condition 
(though I haven't verified it at runtime either).

> The root cause of this issue is because multi-tables are updated in non
> atomic context. To be more specific, the issue could be triggerred as
> following:

Here the cluster is isolated (CLUSTER_FLAG_NONE).

> swap_alloc_fast         swap_cluster_populate()
>                          /* Try a sleep allocation */
>                          spin_unlock(&ci->lock);

And from here, the table becomes visible,

>                          swap_cluster_alloc_table()
>                           rcu_assign_pointer(ci->table, table);

All entry on this cluster is freed(e.g process dead) , 
so this might be the free cluster.

>  ci = swap_cluster_lock(si, offset)
>  cluster_is_usable(ci, order）

Since it's a normal cluster (CLUSTER_FLAG_NONE) and the table exists,
it passes here...

>   if (!cluster_table_is_alloced(ci)) // ok 
>  alloc_swap_scan_cluster()
>   cluster_scan_range()
>    __swap_table_get()
> 
>                           /* free table when more table allocation fails */
>                           ci->memcg_table = kzalloc_obj(*ci->memcg_table,
>                                                         gfp);
>                           if (!ci->memcg_table)
>                            swap_cluster_free_table()

Nullified

>                             rcu_assign_pointer(ci->table, NULL);

Now it happens.

>     table = rcu_dereference_check(ci->table, lockdep_is_held(&ci->lock));
>      atomic_long_read(&table[off]); // NULL dereference
> 
> Fix the issue by updating allocated tables in atomic context.
> 
> Fixes: 2fe7a6f5024b8 ("mm/memcg, swap: store cgroup id in cluster table directly")
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  mm/swapfile.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 615d90867111..d29062d9c3cd 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -490,6 +490,20 @@ static int swap_cluster_alloc_table(struct swap_cluster_info *ci, gfp_t gfp)
>  	return 0;
>  }
>  
> +static void swap_cluster_copy_table(struct swap_cluster_info *d_ci,
> +				    struct swap_cluster_info *s_ci)
> +{
> +	rcu_assign_pointer(d_ci->table, rcu_access_pointer(s_ci->table));
> +
> +#ifdef CONFIG_MEMCG
> +	d_ci->memcg_table = s_ci->memcg_table;
> +#endif
> +
> +#if !SWAP_TABLE_HAS_ZEROFLAG
> +	d_ci->zero_bitmap = s_ci->zero_bitmap;
> +#endif
> +}
> +
>  /*
>   * Sanity check to ensure nothing leaked, and the specified range is empty.
>   * One special case is that bad slots can't be freed, so check the number of
> @@ -527,6 +541,7 @@ static struct swap_cluster_info *
>  swap_cluster_populate(struct swap_info_struct *si,
>  			 struct swap_cluster_info *ci)
>  {
> +	struct swap_cluster_info tmp_ci;
>  	int ret;

IMHO, How about resolving everything inside swap_cluster_alloc_table() instead?

We could consider the following two things.

- Assign the table at the very end inside swap_cluster_alloc_table().
- Handle the table freeing properly if subsequent allocations fail.

Rather than allocating a temp_ci (which adds special handling
for this case), wouldn't it be better to maintain the original intention
of the swap_cluster_alloc_table() function?

Although this is not a hot path and table allocation will usually succeed with
the ATOMIC allocator, this approach would also save the memcpy overhead
and stack memory usage.

(Another option might be checking memcg_table and zero_bitmap inside
cluster_table_is_alloced(). However, I personally dislike this idea as
it might introduce side effects regarding its coverage.)

Your current approach is also a good direction, but please review this
idea as well :)

Thanks,
Youngjun

Re: [PATCH 1/4] mm, swap: Fix potential NULL dereference when trying a sleep table allocation

[Kairui Song]

On Mon, Jul 20, 2026 at 03:13:39PM +0800, Kemeng Shi wrote:
> The root cause of this issue is because multi-tables are updated in non
> atomic context. To be more specific, the issue could be triggerred as
> following:
> 
> swap_alloc_fast         swap_cluster_populate()
>                          /* Try a sleep allocation */
>                          spin_unlock(&ci->lock);
>                          swap_cluster_alloc_table()
>                           rcu_assign_pointer(ci->table, table);
> 
>  ci = swap_cluster_lock(si, offset)
>  cluster_is_usable(ci, order）
>   if (!cluster_table_is_alloced(ci)) // ok
>  alloc_swap_scan_cluster()
>   cluster_scan_range()
>    __swap_table_get()
> 
>                           /* free table when more table allocation fails */
>                           ci->memcg_table = kzalloc_obj(*ci->memcg_table,
>                                                         gfp);
>                           if (!ci->memcg_table)
>                            swap_cluster_free_table()
>                             rcu_assign_pointer(ci->table, NULL);
> 
>     table = rcu_dereference_check(ci->table, lockdep_is_held(&ci->lock));
>      atomic_long_read(&table[off]); // NULL dereference
> 
> Fix the issue by updating allocated tables in atomic context.

Thanks, that's a really tricky one, good catch.

> 
> Fixes: 2fe7a6f5024b8 ("mm/memcg, swap: store cgroup id in cluster table directly")

This commit doesn't exist, you mean b197d41462c2 right?

> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  mm/swapfile.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 615d90867111..d29062d9c3cd 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -490,6 +490,20 @@ static int swap_cluster_alloc_table(struct swap_cluster_info *ci, gfp_t gfp)
>  	return 0;
>  }
>  
> +static void swap_cluster_copy_table(struct swap_cluster_info *d_ci,
> +				    struct swap_cluster_info *s_ci)

Nit: The name is a bit misleading, it not copying the table content,
just filled the d_ci with s_ci. And do we really need a helper for this?

> +{
> +	rcu_assign_pointer(d_ci->table, rcu_access_pointer(s_ci->table));
> +
> +#ifdef CONFIG_MEMCG
> +	d_ci->memcg_table = s_ci->memcg_table;
> +#endif
> +
> +#if !SWAP_TABLE_HAS_ZEROFLAG
> +	d_ci->zero_bitmap = s_ci->zero_bitmap;
> +#endif
> +}
> +
>  /*
>   * Sanity check to ensure nothing leaked, and the specified range is empty.
>   * One special case is that bad slots can't be freed, so check the number of
> @@ -527,6 +541,7 @@ static struct swap_cluster_info *
>  swap_cluster_populate(struct swap_info_struct *si,
>  			 struct swap_cluster_info *ci)
>  {
> +	struct swap_cluster_info tmp_ci;
>  	int ret;
>  
>  	/*
> @@ -552,7 +567,8 @@ swap_cluster_populate(struct swap_info_struct *si,
>  		spin_unlock(&si->global_cluster_lock);
>  	local_unlock(&percpu_swap_cluster.lock);
>  
> -	ret = swap_cluster_alloc_table(ci, __GFP_HIGH | __GFP_NOMEMALLOC |
> +	tmp_ci = *ci;
> +	ret = swap_cluster_alloc_table(&tmp_ci, __GFP_HIGH | __GFP_NOMEMALLOC |
>  					   GFP_KERNEL);

This allocation could be largely simplified with mempool, the reason I didn't
use that is due to lack of RCU support, which can be done later to clean
this up.

Please fix the Fixes tags, other parts are mostly fine, thanks agian!

Re: [PATCH 2/4] mm, swap: Move setup_swap_clusters_info() after SWP_SOLIDSTATE initialization

[Luiz Capitulino]

On 2026-07-20 03:13, Kemeng Shi wrote:
> In setup_swap_clusters_info(), SWP_SOLIDSTATE is used to decide
> global_cluster allocation. Move setup_swap_clusters_info() after
> SWP_SOLIDSTATE initialization to avoid unneeded global_cluster
> allocation.
> 
> Fixes: 451c6326105b2 ("mm, swap: clean up swapon process and locking")
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

I was about to send the same fix :) (one nit below)

> ---
>   mm/swapfile.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index d29062d9c3cd..81c4040912be 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3607,11 +3607,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>   
>   	maxpages = si->max;
>   
> -	/* Set up the swap cluster info */
> -	error = setup_swap_clusters_info(si, swap_header, maxpages);
> -	if (error)
> -		goto bad_swap_unlock_inode;
> -
>   	if (si->bdev && bdev_stable_writes(si->bdev))
>   		si->flags |= SWP_STABLE_WRITES;
>   
> @@ -3625,6 +3620,14 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>   		inced_nr_rotate_swap = true;
>   	}
>   
> +	/*
> +	 * Set up the swap cluster info. SWP_SOLIDSTATE is used for
> +	 * global_cluster allocation
> +	 */

Maybe something like:

/*
  * Set up the swap cluster info after SWP_ flags handling as
  * setup_swap_clusters_info() checks SWP_SOLIDSTATE.
  */

As it's minor:

Reviewed-by: Luiz Capitulino <luizcap@redhat.com>

> +	error = setup_swap_clusters_info(si, swap_header, maxpages);
> +	if (error)
> +		goto bad_swap_unlock_inode;
> +
>   	if ((swap_flags & SWAP_FLAG_DISCARD) &&
>   	    si->bdev && bdev_max_discard_sectors(si->bdev)) {
>   		/*