[PATCH] mm/swap: publish cluster tables after full initialization

[PATCH] mm/swap: publish cluster tables after full initialization

[Longlong Xia]

From: Longlong Xia <xialonglong@kylinos.cn>

swap_cluster_populate() drops the local, global, and cluster locks
before its sleeping allocation.  The allocation helper publishes ci->table
before allocating the memcg table and, on some 32-bit configurations, the
zero bitmap.

A stale per-CPU or global cluster cursor can reach the isolated cluster in
that window.  Since CLUSTER_FLAG_NONE and a non-NULL table make the cluster
appear usable, it can allocate a slot without the auxiliary state.  An
auxiliary allocation failure can then tear down a table which is already in
use.

Allocate the complete set of tables into a private carrier.  Install the
auxiliary pointers and publish the main table only while holding ci->lock;
the slow path does this after reacquiring all allocator locks.  Allocation
failures now free only unpublished resources.

Fixes: b197d41462c2 ("mm/memcg, swap: store cgroup id in cluster table directly")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
---
 mm/swapfile.c | 167 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 117 insertions(+), 50 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 78b49b0658ad..2ca947c540e9 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -410,6 +410,99 @@ static inline unsigned int cluster_offset(struct swap_info_struct *si,
 	return cluster_index(si, ci) * SWAPFILE_CLUSTER;
 }
 
+struct swap_cluster_tables {
+	struct swap_table *table;
+#ifdef CONFIG_MEMCG
+	struct swap_memcg_table *memcg_table;
+#endif
+#if !SWAP_TABLE_HAS_ZEROFLAG
+	unsigned long *zero_bitmap;
+#endif
+};
+
+static void swap_cluster_tables_free(struct swap_cluster_tables *tables)
+{
+#ifdef CONFIG_MEMCG
+	kfree(tables->memcg_table);
+	tables->memcg_table = NULL;
+#endif
+
+#if !SWAP_TABLE_HAS_ZEROFLAG
+	kfree(tables->zero_bitmap);
+	tables->zero_bitmap = NULL;
+#endif
+
+	if (!tables->table)
+		return;
+
+	if (SWP_TABLE_USE_PAGE)
+		folio_put(virt_to_folio(tables->table));
+	else
+		kmem_cache_free(swap_table_cachep, tables->table);
+	tables->table = NULL;
+}
+
+static int swap_cluster_tables_alloc(struct swap_cluster_tables *tables,
+				     gfp_t gfp)
+{
+	struct folio *folio;
+
+	if (SWP_TABLE_USE_PAGE) {
+		folio = folio_alloc(gfp | __GFP_ZERO, 0);
+		if (folio)
+			tables->table = folio_address(folio);
+	} else {
+		tables->table = kmem_cache_zalloc(swap_table_cachep, gfp);
+	}
+	if (!tables->table)
+		return -ENOMEM;
+
+#ifdef CONFIG_MEMCG
+	if (!mem_cgroup_disabled()) {
+		tables->memcg_table = kzalloc_obj(*tables->memcg_table, gfp);
+		if (!tables->memcg_table)
+			goto free_tables;
+	}
+#endif
+
+#if !SWAP_TABLE_HAS_ZEROFLAG
+	tables->zero_bitmap = bitmap_zalloc(SWAPFILE_CLUSTER, gfp);
+	if (!tables->zero_bitmap)
+		goto free_tables;
+#endif
+
+	return 0;
+
+#if defined(CONFIG_MEMCG) || !SWAP_TABLE_HAS_ZEROFLAG
+free_tables:
+	swap_cluster_tables_free(tables);
+	return -ENOMEM;
+#endif
+}
+
+static void swap_cluster_tables_install(struct swap_cluster_info *ci,
+					struct swap_cluster_tables *tables)
+{
+	lockdep_assert_held(&ci->lock);
+	VM_WARN_ON_ONCE(ci->flags || !cluster_is_empty(ci));
+	VM_WARN_ON_ONCE(rcu_access_pointer(ci->table));
+
+#ifdef CONFIG_MEMCG
+	VM_WARN_ON_ONCE(ci->memcg_table);
+	ci->memcg_table = tables->memcg_table;
+	tables->memcg_table = NULL;
+#endif
+
+#if !SWAP_TABLE_HAS_ZEROFLAG
+	VM_WARN_ON_ONCE(ci->zero_bitmap);
+	ci->zero_bitmap = tables->zero_bitmap;
+	tables->zero_bitmap = NULL;
+#endif
+
+	rcu_assign_pointer(ci->table, tables->table);
+	tables->table = NULL;
+}
+
 static void swap_cluster_free_table_folio_rcu_cb(struct rcu_head *head)
 {
 	struct folio *folio;
@@ -446,50 +539,6 @@ static void swap_cluster_free_table(struct swap_cluster_info *ci)
 		 swap_cluster_free_table_folio_rcu_cb);
 }
 
-static int swap_cluster_alloc_table(struct swap_cluster_info *ci, gfp_t gfp)
-{
-	struct swap_table *table = NULL;
-	struct folio *folio;
-
-	/* The cluster must be empty and not on any list during allocation. */
-	VM_WARN_ON_ONCE(ci->flags || !cluster_is_empty(ci));
-	if (rcu_access_pointer(ci->table))
-		return 0;
-
-	if (SWP_TABLE_USE_PAGE) {
-		folio = folio_alloc(gfp | __GFP_ZERO, 0);
-		if (folio)
-			table = folio_address(folio);
-	} else {
-		table = kmem_cache_zalloc(swap_table_cachep, gfp);
-	}
-	if (!table)
-		return -ENOMEM;
-
-	rcu_assign_pointer(ci->table, table);
-
-#ifdef CONFIG_MEMCG
-	if (!mem_cgroup_disabled()) {
-		VM_WARN_ON_ONCE(ci->memcg_table);
-		ci->memcg_table = kzalloc_obj(*ci->memcg_table, gfp);
-		if (!ci->memcg_table) {
-			swap_cluster_free_table(ci);
-			return -ENOMEM;
-		}
-	}
-#endif
-
-#if !SWAP_TABLE_HAS_ZEROFLAG
-	VM_WARN_ON_ONCE(ci->zero_bitmap);
-	ci->zero_bitmap = bitmap_zalloc(SWAPFILE_CLUSTER, gfp);
-	if (!ci->zero_bitmap) {
-		swap_cluster_free_table(ci);
-		return -ENOMEM;
-	}
-#endif
-	return 0;
-}
-
 /*
  * 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 +576,7 @@ static struct swap_cluster_info *
 swap_cluster_populate(struct swap_info_struct *si,
 			 struct swap_cluster_info *ci)
 {
+	struct swap_cluster_tables tables = {};
 	int ret;
 
 	/*
@@ -538,9 +588,12 @@ swap_cluster_populate(struct swap_info_struct *si,
 		lockdep_assert_held(&si->global_cluster_lock);
 	lockdep_assert_held(&ci->lock);
 
-	if (!swap_cluster_alloc_table(ci, __GFP_HIGH | __GFP_NOMEMALLOC |
-					  __GFP_NOWARN))
+	ret = swap_cluster_tables_alloc(&tables, __GFP_HIGH | __GFP_NOMEMALLOC |
+					       __GFP_NOWARN);
+	if (!ret) {
+		swap_cluster_tables_install(ci, &tables);
 		return ci;
+	}
 
 	/*
 	 * Try a sleep allocation. Each isolated free cluster may cause
@@ -552,8 +605,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 |
-					   GFP_KERNEL);
+	ret = swap_cluster_tables_alloc(&tables, __GFP_HIGH | __GFP_NOMEMALLOC |
+						GFP_KERNEL);
 
 	/*
 	 * Back to atomic context. We might have migrated to a new CPU with a
@@ -568,11 +621,19 @@ swap_cluster_populate(struct swap_info_struct *si,
 		spin_lock(&si->global_cluster_lock);
 	spin_lock(&ci->lock);
 
+	/* Nothing except this helper should populate an isolated cluster. */
+	if (WARN_ON_ONCE(cluster_table_is_alloced(ci))) {
+		swap_cluster_tables_free(&tables);
+		return ci;
+	}
+
 	if (ret) {
 		move_cluster(si, ci, &si->free_clusters, CLUSTER_FLAG_FREE);
 		spin_unlock(&ci->lock);
 		return NULL;
 	}
+
+	swap_cluster_tables_install(ci, &tables);
 	return ci;
 }
 
@@ -788,6 +849,7 @@ static int swap_cluster_setup_bad_slot(struct swap_info_struct *si,
 				       struct swap_cluster_info *cluster_info,
 				       unsigned int offset, bool mask)
 {
+	struct swap_cluster_tables tables = {};
 	unsigned int ci_off = offset % SWAPFILE_CLUSTER;
 	unsigned long idx = offset / SWAPFILE_CLUSTER;
 	struct swap_cluster_info *ci;
@@ -812,9 +874,14 @@ static int swap_cluster_setup_bad_slot(struct swap_info_struct *si,
 
 	ci = cluster_info + idx;
 	/* Need to allocate swap table first for initial bad slot marking. */
-	if (!ci->count && swap_cluster_alloc_table(ci, GFP_KERNEL))
-		return -ENOMEM;
+	if (!ci->count) {
+		ret = swap_cluster_tables_alloc(&tables, GFP_KERNEL);
+		if (ret)
+			return ret;
+	}
 	spin_lock(&ci->lock);
+	if (tables.table)
+		swap_cluster_tables_install(ci, &tables);
 	/* Check for duplicated bad swap slots. */
 	if (__swap_table_xchg(ci, ci_off, SWP_TB_BAD) != SWP_TB_NULL) {
 		pr_warn("Duplicated bad slot offset %d\n", offset);
-- 
2.43.0

Re: [PATCH] mm/swap: publish cluster tables after full initialization

[Andrew Morton]

On Thu, 13 Aug 2026 23:03:16 +0800 Longlong Xia <xialonglong2025@163.com> wrote:

> From: Longlong Xia <xialonglong@kylinos.cn>
> 
> swap_cluster_populate() drops the local, global, and cluster locks
> before its sleeping allocation.  The allocation helper publishes ci->table
> before allocating the memcg table and, on some 32-bit configurations, the
> zero bitmap.
> 
> A stale per-CPU or global cluster cursor can reach the isolated cluster in
> that window.  Since CLUSTER_FLAG_NONE and a non-NULL table make the cluster
> appear usable, it can allocate a slot without the auxiliary state.  An
> auxiliary allocation failure can then tear down a table which is already in
> use.
> 
> Allocate the complete set of tables into a private carrier.  Install the
> auxiliary pointers and publish the main table only while holding ci->lock;
> the slow path does this after reacquiring all allocator locks.  Allocation
> failures now free only unpublished resources.

This is a complex patch, for a problem which I'm assuming nobody has
ever encountered.

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

For the eleven thousandth time (I'm not blaming you - this is endemic). 
When fixing a bug, please fully describe the userspace-visible runtime
effects of this bug.

If there is a report then document this.  If there is a testcase which
triggers this then document it.  If there is neither then document
that.

> Cc: stable@vger.kernel.org

Especially when proposing a backport.

Think of the -stable maintainers, and of everyone downstream of them. 
They're looking at this thing wondering "should I merge this into my
kernel".  We're providing them no reason for doing that!  We should do
this.  We're the swap experts, aren't we?  They depend upon us to
explain these things.

> Assisted-by: Codex:gpt-5.6-sol

Doesn't have a great track record, in my drive-by experience.  Sashiko
has found real bugs in things which gpt accepted.  And a second opinion
is always welcome.

Sashiko review says "No regressions", but it also says "Status:
Skipped".  I cannot figure out what this means.  Let me
cc:sashiko@lists.linux.dev.  Folks, can you please explain?

https://sashiko.dev/#/patchset/20260813150316.2793642-1-xialonglong2025@163.com

> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>

Thanks.

>  mm/swapfile.c | 167 +++++++++++++++++++++++++++++++++++---------------

I'll take a pass on this and shall await comment from the swap
maintainers.  And please understand that it'll be a lot of work for
them, and without that statement of "userspace-visible runtime
effects", their motivation will not be high.

Re: [PATCH] mm/swap: publish cluster tables after full initialization

[Youngjun Park]

On Thu, Aug 13, 2026 at 11:03:16PM +0800, Longlong Xia wrote:

+Cc: Kemeng Shi <shikemeng@huaweicloud.com>

Hello Longlong Xia

> From: Longlong Xia <xialonglong@kylinos.cn>
> 
> swap_cluster_populate() drops the local, global, and cluster locks
> before its sleeping allocation.  The allocation helper publishes ci->table
> before allocating the memcg table and, on some 32-bit configurations, the
> zero bitmap.
> 
> A stale per-CPU or global cluster cursor can reach the isolated cluster in
> that window.  Since CLUSTER_FLAG_NONE and a non-NULL table make the cluster
> appear usable, it can allocate a slot without the auxiliary state.  An
> auxiliary allocation failure can then tear down a table which is already in
> use.

I think this issue is addressed in Kemeng Shi's patch, which provides a
detailed reproduce scenario.
https://lore.kernel.org/linux-mm/20260720071342.50742-2-shikemeng@huaweicloud.com/

In short, referencing the table while auxiliary table allocation fails can
lead to a panic or something due to a NULL pointer dereference. That patch only fixed
the populate code rather than the entire allocation path.

However, as I mentioned in my previous review.
https://lore.kernel.org/linux-mm/al7WTNXanPVSELAH@yjaykim-PowerEdge-T330/
I suggested making it visible only when the ci->table installation succeeds,
even in the CLUSTER_FLAG_NONE case. If we do this, seeing the table in the
CLUSTER_FLAG_NONE case would not trigger the auxiliary allocation teardown
mentioned in the problem description.

> Allocate the complete set of tables into a private carrier.  Install the
> auxiliary pointers and publish the main table only while holding ci->lock;
> the slow path does this after reacquiring all allocator locks.  Allocation
> failures now free only unpublished resources.
> 
> Fixes: b197d41462c2 ("mm/memcg, swap: store cgroup id in cluster table directly")
> Cc: stable@vger.kernel.org

I see this commit is included in v7.2-rc1:
$ git describe --contains b197d41462c2
v7.2-rc1\~94^2\~147

Since it's in v7.2-rc1, is `Cc: stable@vger.kernel.org` necessary?

> @@ -788,6 +849,7 @@ static int swap_cluster_setup_bad_slot(struct swap_info_struct *si,
>  				       struct swap_cluster_info *cluster_info,
>  				       unsigned int offset, bool mask)
>  {
> +	struct swap_cluster_tables tables = {};
>  	unsigned int ci_off = offset % SWAPFILE_CLUSTER;
>  	unsigned long idx = offset / SWAPFILE_CLUSTER;
>  	struct swap_cluster_info *ci;
> @@ -812,9 +874,14 @@ static int swap_cluster_setup_bad_slot(struct swap_info_struct *si,
>  
>  	ci = cluster_info + idx;
>  	/* Need to allocate swap table first for initial bad slot marking. */
> -	if (!ci->count && swap_cluster_alloc_table(ci, GFP_KERNEL))
> -		return -ENOMEM;
> +	if (!ci->count) {
> +		ret = swap_cluster_tables_alloc(&tables, GFP_KERNEL);
> +		if (ret)
> +			return ret;
> +	}
>  	spin_lock(&ci->lock);
> +	if (tables.table)
> +		swap_cluster_tables_install(ci, &tables);
>  	/* Check for duplicated bad swap slots. */
>  	if (__swap_table_xchg(ci, ci_off, SWP_TB_BAD) != SWP_TB_NULL) {

As discussed before, this routine is set up before the swap device becomes
visible, so it doesn't actually benefit from this fix (although it inevitably
needs to be modified to align with the current direction).

Overall, I prefer the direction I suggested in my previous review. (If my assumtion is right)
https://lore.kernel.org/linux-mm/al7WTNXanPVSELAH@yjaykim-PowerEdge-T330/
or Kemeng's current suggestion.

(+ Also, if this needs to go into the 7.2 hotfix, Kemeng Shi or Longlong Xia
can submit it. Also, I can help to submit it based on my reviewed direction 
with both of your tags included.)

Thanks
Youngjun

Re: [PATCH] mm/swap: publish cluster tables after full initialization

[Kairui Song]

On Thu, Aug 13, 2026 at 11:03:16PM +0800, Longlong Xia wrote:
> From: Longlong Xia <xialonglong@kylinos.cn>
> 
> swap_cluster_populate() drops the local, global, and cluster locks
> before its sleeping allocation.  The allocation helper publishes ci->table
> before allocating the memcg table and, on some 32-bit configurations, the
> zero bitmap.
> 
> A stale per-CPU or global cluster cursor can reach the isolated cluster in
> that window.  Since CLUSTER_FLAG_NONE and a non-NULL table make the cluster
> appear usable, it can allocate a slot without the auxiliary state.  An
> auxiliary allocation failure can then tear down a table which is already in
> use.
> 
> Allocate the complete set of tables into a private carrier.  Install the
> auxiliary pointers and publish the main table only while holding ci->lock;
> the slow path does this after reacquiring all allocator locks.  Allocation
> failures now free only unpublished resources.
> 
> Fixes: b197d41462c2 ("mm/memcg, swap: store cgroup id in cluster table directly")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> ---
>  mm/swapfile.c | 167 +++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 117 insertions(+), 50 deletions(-)
> 

Hi Longlong, thanks for the patch and report.

> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 78b49b0658ad..2ca947c540e9 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -410,6 +410,99 @@ static inline unsigned int cluster_offset(struct swap_info_struct *si,
>  	return cluster_index(si, ci) * SWAPFILE_CLUSTER;
>  }
>  
> +struct swap_cluster_tables {
> +	struct swap_table *table;
> +#ifdef CONFIG_MEMCG
> +	struct swap_memcg_table *memcg_table;
> +#endif
> +#if !SWAP_TABLE_HAS_ZEROFLAG
> +	unsigned long *zero_bitmap;
> +#endif
> +};
> +
> +static void swap_cluster_tables_free(struct swap_cluster_tables *tables)
> +{
> +#ifdef CONFIG_MEMCG
> +	kfree(tables->memcg_table);
> +	tables->memcg_table = NULL;
> +#endif
> +
> +#if !SWAP_TABLE_HAS_ZEROFLAG
> +	kfree(tables->zero_bitmap);
> +	tables->zero_bitmap = NULL;
> +#endif
> +
> +	if (!tables->table)
> +		return;
> +
> +	if (SWP_TABLE_USE_PAGE)
> +		folio_put(virt_to_folio(tables->table));
> +	else
> +		kmem_cache_free(swap_table_cachep, tables->table);
> +	tables->table = NULL;
> +}
> +
> +static int swap_cluster_tables_alloc(struct swap_cluster_tables *tables,
> +				     gfp_t gfp)
> +{
> +	struct folio *folio;
> +
> +	if (SWP_TABLE_USE_PAGE) {
> +		folio = folio_alloc(gfp | __GFP_ZERO, 0);
> +		if (folio)
> +			tables->table = folio_address(folio);
> +	} else {
> +		tables->table = kmem_cache_zalloc(swap_table_cachep, gfp);
> +	}
> +	if (!tables->table)
> +		return -ENOMEM;
> +
> +#ifdef CONFIG_MEMCG
> +	if (!mem_cgroup_disabled()) {
> +		tables->memcg_table = kzalloc_obj(*tables->memcg_table, gfp);
> +		if (!tables->memcg_table)
> +			goto free_tables;
> +	}
> +#endif
> +
> +#if !SWAP_TABLE_HAS_ZEROFLAG
> +	tables->zero_bitmap = bitmap_zalloc(SWAPFILE_CLUSTER, gfp);
> +	if (!tables->zero_bitmap)
> +		goto free_tables;
> +#endif
> +
> +	return 0;
> +
> +#if defined(CONFIG_MEMCG) || !SWAP_TABLE_HAS_ZEROFLAG
> +free_tables:
> +	swap_cluster_tables_free(tables);
> +	return -ENOMEM;
> +#endif
> +}
> +
> +static void swap_cluster_tables_install(struct swap_cluster_info *ci,
> +					struct swap_cluster_tables *tables)
> +{
> +	lockdep_assert_held(&ci->lock);
> +	VM_WARN_ON_ONCE(ci->flags || !cluster_is_empty(ci));
> +	VM_WARN_ON_ONCE(rcu_access_pointer(ci->table));
> +
> +#ifdef CONFIG_MEMCG
> +	VM_WARN_ON_ONCE(ci->memcg_table);
> +	ci->memcg_table = tables->memcg_table;
> +	tables->memcg_table = NULL;
> +#endif
> +
> +#if !SWAP_TABLE_HAS_ZEROFLAG
> +	VM_WARN_ON_ONCE(ci->zero_bitmap);
> +	ci->zero_bitmap = tables->zero_bitmap;
> +	tables->zero_bitmap = NULL;
> +#endif
> +
> +	rcu_assign_pointer(ci->table, tables->table);
> +	tables->table = NULL;
> +}
> +

You don't need to shuffle all the code for a simple bug fix, you can use
forward declaration if some functions are needed earlier.

...

>  	/*
>  	 * Back to atomic context. We might have migrated to a new CPU with a
> @@ -568,11 +621,19 @@ swap_cluster_populate(struct swap_info_struct *si,
>  		spin_lock(&si->global_cluster_lock);
>  	spin_lock(&ci->lock);
>  
> +	/* Nothing except this helper should populate an isolated cluster. */
> +	if (WARN_ON_ONCE(cluster_table_is_alloced(ci))) {
> +		swap_cluster_tables_free(&tables);
> +		return ci;
> +	}
> +
>  	if (ret) {
>  		move_cluster(si, ci, &si->free_clusters, CLUSTER_FLAG_FREE);
>  		spin_unlock(&ci->lock);
>  		return NULL;
>  	}
> +
> +	swap_cluster_tables_install(ci, &tables);

It seems the same fix with Kemeng's patch? Youngjun have notice this too.
https://lore.kernel.org/linux-mm/20260720071342.50742-2-shikemeng@huaweicloud.com/

And I think you missed some Cc, maybe you can try tools like b4 which
automatically generate the Cc list for you.

Hello Kemeng, can you help check and see if an updated can be sent?

Re: [PATCH] mm/swap: publish cluster tables after full initialization

[Longlong Xia]

Hi Kairui, Youngjun, Kemeng,

Thanks for the comments.

I missed Kemeng's earlier patch and the follow-up discussion. Sorry for the
noise and for the incomplete Cc list.

Since this issue is already being discussed there, I will step back and let
Kemeng continue with the updated version.

Thanks,
Longlong


在 2026/8/14 10:35, Kairui Song 写道:
> On Thu, Aug 13, 2026 at 11:03:16PM +0800, Longlong Xia wrote:
>> From: Longlong Xia <xialonglong@kylinos.cn>
>>
>> swap_cluster_populate() drops the local, global, and cluster locks
>> before its sleeping allocation.  The allocation helper publishes ci->table
>> before allocating the memcg table and, on some 32-bit configurations, the
>> zero bitmap.
>>
>> A stale per-CPU or global cluster cursor can reach the isolated cluster in
>> that window.  Since CLUSTER_FLAG_NONE and a non-NULL table make the cluster
>> appear usable, it can allocate a slot without the auxiliary state.  An
>> auxiliary allocation failure can then tear down a table which is already in
>> use.
>>
>> Allocate the complete set of tables into a private carrier.  Install the
>> auxiliary pointers and publish the main table only while holding ci->lock;
>> the slow path does this after reacquiring all allocator locks.  Allocation
>> failures now free only unpublished resources.
>>
>> Fixes: b197d41462c2 ("mm/memcg, swap: store cgroup id in cluster table directly")
>> Cc: stable@vger.kernel.org
>> Assisted-by: Codex:gpt-5.6-sol
>> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
>> ---
>>   mm/swapfile.c | 167 +++++++++++++++++++++++++++++++++++---------------
>>   1 file changed, 117 insertions(+), 50 deletions(-)
>>
> Hi Longlong, thanks for the patch and report.
>
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index 78b49b0658ad..2ca947c540e9 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -410,6 +410,99 @@ static inline unsigned int cluster_offset(struct swap_info_struct *si,
>>   	return cluster_index(si, ci) * SWAPFILE_CLUSTER;
>>   }
>>   
>> +struct swap_cluster_tables {
>> +	struct swap_table *table;
>> +#ifdef CONFIG_MEMCG
>> +	struct swap_memcg_table *memcg_table;
>> +#endif
>> +#if !SWAP_TABLE_HAS_ZEROFLAG
>> +	unsigned long *zero_bitmap;
>> +#endif
>> +};
>> +
>> +static void swap_cluster_tables_free(struct swap_cluster_tables *tables)
>> +{
>> +#ifdef CONFIG_MEMCG
>> +	kfree(tables->memcg_table);
>> +	tables->memcg_table = NULL;
>> +#endif
>> +
>> +#if !SWAP_TABLE_HAS_ZEROFLAG
>> +	kfree(tables->zero_bitmap);
>> +	tables->zero_bitmap = NULL;
>> +#endif
>> +
>> +	if (!tables->table)
>> +		return;
>> +
>> +	if (SWP_TABLE_USE_PAGE)
>> +		folio_put(virt_to_folio(tables->table));
>> +	else
>> +		kmem_cache_free(swap_table_cachep, tables->table);
>> +	tables->table = NULL;
>> +}
>> +
>> +static int swap_cluster_tables_alloc(struct swap_cluster_tables *tables,
>> +				     gfp_t gfp)
>> +{
>> +	struct folio *folio;
>> +
>> +	if (SWP_TABLE_USE_PAGE) {
>> +		folio = folio_alloc(gfp | __GFP_ZERO, 0);
>> +		if (folio)
>> +			tables->table = folio_address(folio);
>> +	} else {
>> +		tables->table = kmem_cache_zalloc(swap_table_cachep, gfp);
>> +	}
>> +	if (!tables->table)
>> +		return -ENOMEM;
>> +
>> +#ifdef CONFIG_MEMCG
>> +	if (!mem_cgroup_disabled()) {
>> +		tables->memcg_table = kzalloc_obj(*tables->memcg_table, gfp);
>> +		if (!tables->memcg_table)
>> +			goto free_tables;
>> +	}
>> +#endif
>> +
>> +#if !SWAP_TABLE_HAS_ZEROFLAG
>> +	tables->zero_bitmap = bitmap_zalloc(SWAPFILE_CLUSTER, gfp);
>> +	if (!tables->zero_bitmap)
>> +		goto free_tables;
>> +#endif
>> +
>> +	return 0;
>> +
>> +#if defined(CONFIG_MEMCG) || !SWAP_TABLE_HAS_ZEROFLAG
>> +free_tables:
>> +	swap_cluster_tables_free(tables);
>> +	return -ENOMEM;
>> +#endif
>> +}
>> +
>> +static void swap_cluster_tables_install(struct swap_cluster_info *ci,
>> +					struct swap_cluster_tables *tables)
>> +{
>> +	lockdep_assert_held(&ci->lock);
>> +	VM_WARN_ON_ONCE(ci->flags || !cluster_is_empty(ci));
>> +	VM_WARN_ON_ONCE(rcu_access_pointer(ci->table));
>> +
>> +#ifdef CONFIG_MEMCG
>> +	VM_WARN_ON_ONCE(ci->memcg_table);
>> +	ci->memcg_table = tables->memcg_table;
>> +	tables->memcg_table = NULL;
>> +#endif
>> +
>> +#if !SWAP_TABLE_HAS_ZEROFLAG
>> +	VM_WARN_ON_ONCE(ci->zero_bitmap);
>> +	ci->zero_bitmap = tables->zero_bitmap;
>> +	tables->zero_bitmap = NULL;
>> +#endif
>> +
>> +	rcu_assign_pointer(ci->table, tables->table);
>> +	tables->table = NULL;
>> +}
>> +
> You don't need to shuffle all the code for a simple bug fix, you can use
> forward declaration if some functions are needed earlier.
>
> ...
>
>>   	/*
>>   	 * Back to atomic context. We might have migrated to a new CPU with a
>> @@ -568,11 +621,19 @@ swap_cluster_populate(struct swap_info_struct *si,
>>   		spin_lock(&si->global_cluster_lock);
>>   	spin_lock(&ci->lock);
>>   
>> +	/* Nothing except this helper should populate an isolated cluster. */
>> +	if (WARN_ON_ONCE(cluster_table_is_alloced(ci))) {
>> +		swap_cluster_tables_free(&tables);
>> +		return ci;
>> +	}
>> +
>>   	if (ret) {
>>   		move_cluster(si, ci, &si->free_clusters, CLUSTER_FLAG_FREE);
>>   		spin_unlock(&ci->lock);
>>   		return NULL;
>>   	}
>> +
>> +	swap_cluster_tables_install(ci, &tables);
> It seems the same fix with Kemeng's patch? Youngjun have notice this too.
> https://lore.kernel.org/linux-mm/20260720071342.50742-2-shikemeng@huaweicloud.com/
>
> And I think you missed some Cc, maybe you can try tools like b4 which
> automatically generate the Cc list for you.
>
> Hello Kemeng, can you help check and see if an updated can be sent?