[PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[Rik van Riel]

collapse_file() calls xas_create_range() up front to populate the
xarray with nodes spanning the entire collapse range, including empty
slots for the holes (the nr_none entries).

On the success path those nodes are either consumed by the multi-index
store of new_folio or freed by the nr_none retry-entry dance.  But every
error path that branches straight to the rollback label -- e.g.
SCAN_TRUNCATED / SCAN_PAGE_LOCK / SCAN_FAIL detected in the main scan
loop (which then sets nr_none = 0 and jumps to rollback), or SCAN_COPY_MC
during the copy -- does neither.  The empty nodes are left dangling in
mapping->i_pages and are leaked once the inode is finally evicted.  This
is exactly the "THP collapse_file() failed" case that clear_inode()
documents and deliberately tolerates without warning.

syzkaller reproduces it trivially with MADV_COLLAPSE on a sparse shmem
mapping (collapse aborts with SCAN_TRUNCATED because the range is empty),
and also via slab fault injection, which forces xas_create_range() down
the xas_nomem() path before the same abort.  kmemleak then reports the
576-byte struct xa_node objects allocated in xas_alloc()/xas_nomem().

The leaked objects are struct xa_node (576 bytes each), left dangling in
mapping->i_pages and reclaimed only when the inode is finally evicted.
Nodes leak when a collapse takes one of the rollback paths (SCAN_TRUNCATED
/ SCAN_PAGE_LOCK / SCAN_FAIL / SCAN_COPY_MC), and only for the empty hole
slots (the nr_none entries); they are not leaked on the success path (the
nodes are consumed by the multi-index store and the nr_none dance), and
slots still holding folios are never touched.

Prune the now-empty nodes on the rollback path.  A node is only freed
once its slot count reaches zero, and storing NULL into an
already-empty slot is a no-op, so briefly store an XA_RETRY_ENTRY into
each empty slot and immediately clear it: the clear drops the count
back to zero and lets xas_store() -> xas_delete_node() free the node
and its now-empty ancestors.  Slots still holding the original folios
are left untouched.

Fixes: 77da9389b9d5 ("mm: Convert collapse_shmem to XArray")
Assisted-by: Claude:claude-opus-4-8 syzkaller
Signed-off-by: Rik van Riel <riel@surriel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@kernel.org>
Cc: Lorenzo Stoakes <ljs@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Hugh Dickins <hughd@kernel.org>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Liam R. Howlett <liam@infradead.org>
Cc: Nico Pache <npache@redhat.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
Cc: Lance Yang <lance.yang@linux.dev>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---
 mm/khugepaged.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index b8452dbdb043..d11a4c9610e1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2273,6 +2273,39 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
 
 rollback:
 	/* Something went wrong: roll back page cache changes */
+
+	/*
+	 * xas_create_range() above populated the xarray with nodes spanning
+	 * the whole collapse range, including empty slots for the holes
+	 * (nr_none entries).  On the success path these nodes are consumed by
+	 * the multi-index store of new_folio, and the nr_none handling further
+	 * up frees the ones covering the holes; but the error paths that branch
+	 * straight here do neither.  Prune the now-empty nodes explicitly,
+	 * otherwise they are leaked until the mapping is torn down -- one of
+	 * the two cases called out in the comment in clear_inode().
+	 *
+	 * A node can only be deleted once its slot count drops to zero, so
+	 * briefly store an XA_RETRY_ENTRY into each empty slot and then clear
+	 * it again: clearing the retry entry drops the count back to zero and
+	 * lets xas_store() -> xas_delete_node() free the node.  Slots that
+	 * still hold the original folios are left untouched.
+	 */
+	xas_lock_irq(&xas);
+	xas_set_order(&xas, start, 0);
+	for (index = start; index < end; index++) {
+		if (!xas_next(&xas)) {
+			xas_store(&xas, XA_RETRY_ENTRY);
+			if (xas_error(&xas))
+				break;
+		}
+	}
+	xas_set_order(&xas, start, 0);
+	for (index = start; index < end; index++) {
+		if (xas_next(&xas) == XA_RETRY_ENTRY)
+			xas_store(&xas, NULL);
+	}
+	xas_unlock_irq(&xas);
+
 	if (nr_none) {
 		xas_lock_irq(&xas);
 		mapping->nrpages -= nr_none;
-- 
2.53.0-Meta

Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[Zi Yan]

On 16 Jun 2026, at 10:54, Rik van Riel wrote:

> collapse_file() calls xas_create_range() up front to populate the
> xarray with nodes spanning the entire collapse range, including empty
> slots for the holes (the nr_none entries).
>
> On the success path those nodes are either consumed by the multi-index
> store of new_folio or freed by the nr_none retry-entry dance.  But every
> error path that branches straight to the rollback label -- e.g.
> SCAN_TRUNCATED / SCAN_PAGE_LOCK / SCAN_FAIL detected in the main scan
> loop (which then sets nr_none = 0 and jumps to rollback), or SCAN_COPY_MC
> during the copy -- does neither.  The empty nodes are left dangling in
> mapping->i_pages and are leaked once the inode is finally evicted.  This
> is exactly the "THP collapse_file() failed" case that clear_inode()
> documents and deliberately tolerates without warning.
>
> syzkaller reproduces it trivially with MADV_COLLAPSE on a sparse shmem
> mapping (collapse aborts with SCAN_TRUNCATED because the range is empty),
> and also via slab fault injection, which forces xas_create_range() down
> the xas_nomem() path before the same abort.  kmemleak then reports the
> 576-byte struct xa_node objects allocated in xas_alloc()/xas_nomem().
>
> The leaked objects are struct xa_node (576 bytes each), left dangling in
> mapping->i_pages and reclaimed only when the inode is finally evicted.
> Nodes leak when a collapse takes one of the rollback paths (SCAN_TRUNCATED
> / SCAN_PAGE_LOCK / SCAN_FAIL / SCAN_COPY_MC), and only for the empty hole
> slots (the nr_none entries); they are not leaked on the success path (the
> nodes are consumed by the multi-index store and the nr_none dance), and
> slots still holding folios are never touched.
>
> Prune the now-empty nodes on the rollback path.  A node is only freed
> once its slot count reaches zero, and storing NULL into an
> already-empty slot is a no-op, so briefly store an XA_RETRY_ENTRY into
> each empty slot and immediately clear it: the clear drops the count
> back to zero and lets xas_store() -> xas_delete_node() free the node
> and its now-empty ancestors.  Slots still holding the original folios
> are left untouched.

IIRC, there was a report[1] from Jinjiang (cc'd) about this leak.

[1] https://lore.kernel.org/all/86834731-02ba-43ea-9def-8b8ca156ec4a@huawei.com/
>
> Fixes: 77da9389b9d5 ("mm: Convert collapse_shmem to XArray")
> Assisted-by: Claude:claude-opus-4-8 syzkaller
> Signed-off-by: Rik van Riel <riel@surriel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@kernel.org>
> Cc: Lorenzo Stoakes <ljs@kernel.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Hugh Dickins <hughd@kernel.org>
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> Cc: Liam R. Howlett <liam@infradead.org>
> Cc: Nico Pache <npache@redhat.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Dev Jain <dev.jain@arm.com>
> Cc: Barry Song <baohua@kernel.org>
> Cc: Lance Yang <lance.yang@linux.dev>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  mm/khugepaged.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index b8452dbdb043..d11a4c9610e1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2273,6 +2273,39 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>
>  rollback:
>  	/* Something went wrong: roll back page cache changes */
> +
> +	/*
> +	 * xas_create_range() above populated the xarray with nodes spanning
> +	 * the whole collapse range, including empty slots for the holes
> +	 * (nr_none entries).  On the success path these nodes are consumed by
> +	 * the multi-index store of new_folio, and the nr_none handling further
> +	 * up frees the ones covering the holes; but the error paths that branch
> +	 * straight here do neither.  Prune the now-empty nodes explicitly,
> +	 * otherwise they are leaked until the mapping is torn down -- one of
> +	 * the two cases called out in the comment in clear_inode().
> +	 *
> +	 * A node can only be deleted once its slot count drops to zero, so
> +	 * briefly store an XA_RETRY_ENTRY into each empty slot and then clear
> +	 * it again: clearing the retry entry drops the count back to zero and
> +	 * lets xas_store() -> xas_delete_node() free the node.  Slots that
> +	 * still hold the original folios are left untouched.
> +	 */
> +	xas_lock_irq(&xas);
> +	xas_set_order(&xas, start, 0);
> +	for (index = start; index < end; index++) {
> +		if (!xas_next(&xas)) {
> +			xas_store(&xas, XA_RETRY_ENTRY);
> +			if (xas_error(&xas))
> +				break;
> +		}
> +	}
> +	xas_set_order(&xas, start, 0);
> +	for (index = start; index < end; index++) {
> +		if (xas_next(&xas) == XA_RETRY_ENTRY)
> +			xas_store(&xas, NULL);
> +	}
> +	xas_unlock_irq(&xas);
> +
>  	if (nr_none) {
>  		xas_lock_irq(&xas);
>  		mapping->nrpages -= nr_none;
> -- 
> 2.53.0-Meta


--
Best Regards,
Yan, Zi

Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[Matthew Wilcox]

On Tue, Jun 16, 2026 at 10:54:13AM -0400, Rik van Riel wrote:
> syzkaller reproduces it trivially with MADV_COLLAPSE on a sparse shmem
> mapping (collapse aborts with SCAN_TRUNCATED because the range is empty),
> and also via slab fault injection, which forces xas_create_range() down
> the xas_nomem() path before the same abort.  kmemleak then reports the
> 576-byte struct xa_node objects allocated in xas_alloc()/xas_nomem().

I think the ways to produce this problem are sufficiently rare/unlikely
to not merit this level of cleanup.  Why don't we defer it to
clear_inode() instead?

reasons not to:

1. the page cache is not the only user of xarrays and
theoretically either of these things could happen elsewhere

2. can a user do this on purpose to screw other users over?  i don't
think so, any more than they can dos the system by bringing a lot of
inodes into memory; we have cgroups and ulimits to protect against that

diff --git a/fs/inode.c b/fs/inode.c
index 6a3cbc7dcd28..d92c5a296504 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -779,21 +779,20 @@ void clear_inode(struct inode *inode)
 		fsverity_cleanup_inode(inode);
 
 	/*
-	 * We have to cycle the i_pages lock here because reclaim can be in the
-	 * process of removing the last page (in __filemap_remove_folio())
-	 * and we must not free the mapping under it.
+	 * We have to cycle the i_pages lock here because reclaim
+	 * can be in the process of removing the last page (in
+	 * __filemap_remove_folio()) and we must not free the mapping
+	 * under it.  We also remove nodes which are empty; these
+	 * can occur in two different ways.  The first is that radix
+	 * tree expansion can fail partway and the second is that THP
+	 * collapse_file() can allocate some temporary nodes and not
+	 * clean them up.
 	 */
 	xa_lock_irq(&inode->i_data.i_pages);
 	BUG_ON(inode->i_data.nrpages);
-	/*
-	 * Almost always, mapping_empty(&inode->i_data) here; but there are
-	 * two known and long-standing ways in which nodes may get left behind
-	 * (when deep radix-tree node allocation failed partway; or when THP
-	 * collapse_file() failed). Until those two known cases are cleaned up,
-	 * or a cleanup function is called here, do not BUG_ON(!mapping_empty),
-	 * nor even WARN_ON(!mapping_empty).
-	 */
+	__xa_destroy(&inode->i_data.i_pages);
 	xa_unlock_irq(&inode->i_data.i_pages);
+
 	BUG_ON(!(inode_state_read_once(inode) & I_FREEING));
 	BUG_ON(inode_state_read_once(inode) & I_CLEAR);
 	BUG_ON(!list_empty(&inode->i_wb_list));
diff --git a/include/linux/xarray.h b/include/linux/xarray.h
index be850174e802..d776a6e9ad18 100644
--- a/include/linux/xarray.h
+++ b/include/linux/xarray.h
@@ -571,6 +571,7 @@ int __must_check __xa_alloc_cyclic(struct xarray *, u32 *id, void *entry,
 		struct xa_limit, u32 *next, gfp_t);
 void __xa_set_mark(struct xarray *, unsigned long index, xa_mark_t);
 void __xa_clear_mark(struct xarray *, unsigned long index, xa_mark_t);
+void __xa_destroy(struct xarray *);
 
 /**
  * xa_store_bh() - Store this entry in the XArray.
diff --git a/lib/xarray.c b/lib/xarray.c
index 9a8b4916540c..ee2459ecdc9b 100644
--- a/lib/xarray.c
+++ b/lib/xarray.c
@@ -2370,6 +2370,21 @@ void xa_delete_node(struct xa_node *node, xa_update_node_t update)
 }
 EXPORT_SYMBOL_GPL(xa_delete_node);	/* For the benefit of the test suite */
 
+void __xa_destroy(struct xarray *xa)
+{
+	XA_STATE(xas, xa, 0);
+	void *entry;
+
+	xas.xa_node = NULL;
+	entry = xa_head_locked(xa);
+	RCU_INIT_POINTER(xa->xa_head, NULL);
+	xas_init_marks(&xas);
+	if (xa_zero_busy(xa))
+		xa_mark_clear(xa, XA_FREE_MARK);
+	if (xa_is_node(entry))
+		xas_free_nodes(&xas, xa_to_node(entry));
+}
+
 /**
  * xa_destroy() - Free all internal data structures.
  * @xa: XArray.
@@ -2382,21 +2397,11 @@ EXPORT_SYMBOL_GPL(xa_delete_node);	/* For the benefit of the test suite */
  */
 void xa_destroy(struct xarray *xa)
 {
-	XA_STATE(xas, xa, 0);
 	unsigned long flags;
-	void *entry;
 
-	xas.xa_node = NULL;
-	xas_lock_irqsave(&xas, flags);
-	entry = xa_head_locked(xa);
-	RCU_INIT_POINTER(xa->xa_head, NULL);
-	xas_init_marks(&xas);
-	if (xa_zero_busy(xa))
-		xa_mark_clear(xa, XA_FREE_MARK);
-	/* lockdep checks we're still holding the lock in xas_free_nodes() */
-	if (xa_is_node(entry))
-		xas_free_nodes(&xas, xa_to_node(entry));
-	xas_unlock_irqrestore(&xas, flags);
+	xa_lock_irqsave(xa, flags);
+	__xa_destroy(xa);
+	xa_unlock_irqrestore(xa, flags);
 }
 EXPORT_SYMBOL(xa_destroy);
 

Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[Jinjiang Tu]

在 2026/6/16 23:29, Matthew Wilcox 写道:
> On Tue, Jun 16, 2026 at 10:54:13AM -0400, Rik van Riel wrote:
>> syzkaller reproduces it trivially with MADV_COLLAPSE on a sparse shmem
>> mapping (collapse aborts with SCAN_TRUNCATED because the range is empty),
>> and also via slab fault injection, which forces xas_create_range() down
>> the xas_nomem() path before the same abort.  kmemleak then reports the
>> 576-byte struct xa_node objects allocated in xas_alloc()/xas_nomem().
> I think the ways to produce this problem are sufficiently rare/unlikely

I can reproduce it easily with following steps. [1]
1) create file /tmp/test_madvise_collapse and ftruncate to 4MB size, and
then mmap the file
2) memset for the first 2MB
3) madvise(MADV_COLLAPSE) for the second 2MB
4) unlink the file

This might be the only path easy to trigger; all other failure paths seem to be difficult to trigger.

[1] https://lore.kernel.org/all/20260121062243.1893129-1-tujinjiang@huawei.com/

> to not merit this level of cleanup.  Why don't we defer it to
> clear_inode() instead?
>
> reasons not to:
>
> 1. the page cache is not the only user of xarrays and
> theoretically either of these things could happen elsewhere
>
> 2. can a user do this on purpose to screw other users over?  i don't
> think so, any more than they can dos the system by bringing a lot of
> inodes into memory; we have cgroups and ulimits to protect against that
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 6a3cbc7dcd28..d92c5a296504 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -779,21 +779,20 @@ void clear_inode(struct inode *inode)
>   		fsverity_cleanup_inode(inode);
>   
>   	/*
> -	 * We have to cycle the i_pages lock here because reclaim can be in the
> -	 * process of removing the last page (in __filemap_remove_folio())
> -	 * and we must not free the mapping under it.
> +	 * We have to cycle the i_pages lock here because reclaim
> +	 * can be in the process of removing the last page (in
> +	 * __filemap_remove_folio()) and we must not free the mapping
> +	 * under it.  We also remove nodes which are empty; these
> +	 * can occur in two different ways.  The first is that radix
> +	 * tree expansion can fail partway and the second is that THP
> +	 * collapse_file() can allocate some temporary nodes and not
> +	 * clean them up.
>   	 */
>   	xa_lock_irq(&inode->i_data.i_pages);
>   	BUG_ON(inode->i_data.nrpages);
> -	/*
> -	 * Almost always, mapping_empty(&inode->i_data) here; but there are
> -	 * two known and long-standing ways in which nodes may get left behind
> -	 * (when deep radix-tree node allocation failed partway; or when THP
> -	 * collapse_file() failed). Until those two known cases are cleaned up,
> -	 * or a cleanup function is called here, do not BUG_ON(!mapping_empty),
> -	 * nor even WARN_ON(!mapping_empty).
> -	 */
> +	__xa_destroy(&inode->i_data.i_pages);
>   	xa_unlock_irq(&inode->i_data.i_pages);
> +
>   	BUG_ON(!(inode_state_read_once(inode) & I_FREEING));
>   	BUG_ON(inode_state_read_once(inode) & I_CLEAR);
>   	BUG_ON(!list_empty(&inode->i_wb_list));
> diff --git a/include/linux/xarray.h b/include/linux/xarray.h
> index be850174e802..d776a6e9ad18 100644
> --- a/include/linux/xarray.h
> +++ b/include/linux/xarray.h
> @@ -571,6 +571,7 @@ int __must_check __xa_alloc_cyclic(struct xarray *, u32 *id, void *entry,
>   		struct xa_limit, u32 *next, gfp_t);
>   void __xa_set_mark(struct xarray *, unsigned long index, xa_mark_t);
>   void __xa_clear_mark(struct xarray *, unsigned long index, xa_mark_t);
> +void __xa_destroy(struct xarray *);
>   
>   /**
>    * xa_store_bh() - Store this entry in the XArray.
> diff --git a/lib/xarray.c b/lib/xarray.c
> index 9a8b4916540c..ee2459ecdc9b 100644
> --- a/lib/xarray.c
> +++ b/lib/xarray.c
> @@ -2370,6 +2370,21 @@ void xa_delete_node(struct xa_node *node, xa_update_node_t update)
>   }
>   EXPORT_SYMBOL_GPL(xa_delete_node);	/* For the benefit of the test suite */
>   
> +void __xa_destroy(struct xarray *xa)
> +{
> +	XA_STATE(xas, xa, 0);
> +	void *entry;
> +
> +	xas.xa_node = NULL;
> +	entry = xa_head_locked(xa);
> +	RCU_INIT_POINTER(xa->xa_head, NULL);
> +	xas_init_marks(&xas);
> +	if (xa_zero_busy(xa))
> +		xa_mark_clear(xa, XA_FREE_MARK);
> +	if (xa_is_node(entry))
> +		xas_free_nodes(&xas, xa_to_node(entry));
> +}
> +
>   /**
>    * xa_destroy() - Free all internal data structures.
>    * @xa: XArray.
> @@ -2382,21 +2397,11 @@ EXPORT_SYMBOL_GPL(xa_delete_node);	/* For the benefit of the test suite */
>    */
>   void xa_destroy(struct xarray *xa)
>   {
> -	XA_STATE(xas, xa, 0);
>   	unsigned long flags;
> -	void *entry;
>   
> -	xas.xa_node = NULL;
> -	xas_lock_irqsave(&xas, flags);
> -	entry = xa_head_locked(xa);
> -	RCU_INIT_POINTER(xa->xa_head, NULL);
> -	xas_init_marks(&xas);
> -	if (xa_zero_busy(xa))
> -		xa_mark_clear(xa, XA_FREE_MARK);
> -	/* lockdep checks we're still holding the lock in xas_free_nodes() */
> -	if (xa_is_node(entry))
> -		xas_free_nodes(&xas, xa_to_node(entry));
> -	xas_unlock_irqrestore(&xas, flags);
> +	xa_lock_irqsave(xa, flags);
> +	__xa_destroy(xa);
> +	xa_unlock_irqrestore(xa, flags);
>   }
>   EXPORT_SYMBOL(xa_destroy);
>   
>

Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[Matthew Wilcox]

On Wed, Jun 17, 2026 at 10:00:59AM +0800, Jinjiang Tu wrote:
> 
> 在 2026/6/16 23:29, Matthew Wilcox 写道:
> > On Tue, Jun 16, 2026 at 10:54:13AM -0400, Rik van Riel wrote:
> > > syzkaller reproduces it trivially with MADV_COLLAPSE on a sparse shmem
> > > mapping (collapse aborts with SCAN_TRUNCATED because the range is empty),
> > > and also via slab fault injection, which forces xas_create_range() down
> > > the xas_nomem() path before the same abort.  kmemleak then reports the
> > > 576-byte struct xa_node objects allocated in xas_alloc()/xas_nomem().
> > I think the ways to produce this problem are sufficiently rare/unlikely
> 
> I can reproduce it easily with following steps. [1]
> 1) create file /tmp/test_madvise_collapse and ftruncate to 4MB size, and
> then mmap the file
> 2) memset for the first 2MB
> 3) madvise(MADV_COLLAPSE) for the second 2MB
> 4) unlink the file

Yes, but is there a problem?  That is, do we need to clear this up
before we get to clear_inode()?

Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[Jinjiang Tu]

在 2026/6/17 10:18, Matthew Wilcox 写道:
> On Wed, Jun 17, 2026 at 10:00:59AM +0800, Jinjiang Tu wrote:
>> 在 2026/6/16 23:29, Matthew Wilcox 写道:
>>> On Tue, Jun 16, 2026 at 10:54:13AM -0400, Rik van Riel wrote:
>>>> syzkaller reproduces it trivially with MADV_COLLAPSE on a sparse shmem
>>>> mapping (collapse aborts with SCAN_TRUNCATED because the range is empty),
>>>> and also via slab fault injection, which forces xas_create_range() down
>>>> the xas_nomem() path before the same abort.  kmemleak then reports the
>>>> 576-byte struct xa_node objects allocated in xas_alloc()/xas_nomem().
>>> I think the ways to produce this problem are sufficiently rare/unlikely
>> I can reproduce it easily with following steps. [1]
>> 1) create file /tmp/test_madvise_collapse and ftruncate to 4MB size, and
>> then mmap the file
>> 2) memset for the first 2MB
>> 3) madvise(MADV_COLLAPSE) for the second 2MB
>> 4) unlink the file
> Yes, but is there a problem?  That is, do we need to clear this up
> before we get to clear_inode()?

A user cannot exhaust memory by continuously creating xa_node structures. So
it is enough to free empty  xa_nodes in clear_inode().

We shouldn't free empty xa_nodes in collapse_file() rollback path, because we
couldn't avoid a concurrent call of collapse_file(), which creates xa_nodes
for the same region and may use these nodes.

Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[Rik van Riel]

On Tue, Jun 16, 2026 at 10:18 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jun 17, 2026 at 10:00:59AM +0800, Jinjiang Tu wrote:

> > I can reproduce it easily with following steps. [1]
> > 1) create file /tmp/test_madvise_collapse and ftruncate to 4MB size, and
> > then mmap the file
> > 2) memset for the first 2MB
> > 3) madvise(MADV_COLLAPSE) for the second 2MB
> > 4) unlink the file
>
> Yes, but is there a problem?  That is, do we need to clear this up
> before we get to clear_inode()?

If there is no harm in leaving this xarray node hang around until
clear_inode() time, I agree we do not need this patch.

Are there no issues with large folios getting inserted into the
xarray at a later time, after zapping little ones, and leaving
the empty xarray node dangling?

If none of this causes issues, we can keep things simple.

Re: [PATCH] mm: khugepaged: free xarray nodes left behind by failed collapse_file()

[Matthew Wilcox]

On Wed, Jun 17, 2026 at 11:40:28AM -0400, Rik van Riel wrote:
> On Tue, Jun 16, 2026 at 10:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Wed, Jun 17, 2026 at 10:00:59AM +0800, Jinjiang Tu wrote:
> 
> > > I can reproduce it easily with following steps. [1]
> > > 1) create file /tmp/test_madvise_collapse and ftruncate to 4MB size, and
> > > then mmap the file
> > > 2) memset for the first 2MB
> > > 3) madvise(MADV_COLLAPSE) for the second 2MB
> > > 4) unlink the file
> >
> > Yes, but is there a problem?  That is, do we need to clear this up
> > before we get to clear_inode()?
> 
> If there is no harm in leaving this xarray node hang around until
> clear_inode() time, I agree we do not need this patch.
> 
> Are there no issues with large folios getting inserted into the
> xarray at a later time, after zapping little ones, and leaving
> the empty xarray node dangling?

That's not an issue.  The code which handles this is in
__filemap_add_folio().  You can see that we handle value (ie shadow)
entries here:

                xas_for_each_conflict(&xas, entry) {
                        old = entry;
                        if (!xa_is_value(entry)) {
                                xas_set_err(&xas, -EEXIST);
                                goto unlock;
                        }

(this loop will iterate 0 times if there are only empty nodes in the
"conflict" area)

Then we just do xas_store() which will overwrite the nodes.  That calls
xas_free_nodes() on any nodes which are overwritten.