[PATCH] fs: Free any excess xarray nodes in clear_inode()

[PATCH] fs: Free any excess xarray nodes in clear_inode()

[Matthew Wilcox (Oracle)]

For many years we've had a hard to hit leak of xarray nodes.  Hugh
documented it well in commit 786b31121a2c.  Recently people and
syzbot have found ways to force it to happen with madvise.  Rather
than fix the leaks where they happen, just call xa_destroy() which
has the side-effect of cycling the i_pages lock.

Cc: Rik van Riel <riel@surriel.com>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Jinjiang Tu <tujinjiang@huawei.com>
Cc: Dave Jones <davej@codemonkey.org.uk>
Link: https://lore.kernel.org/all/20260121062243.1893129-1-tujinjiang@huawei.com/
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/inode.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 6a3cbc7dcd28..4070ff27b318 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -779,21 +779,18 @@ 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);
+	xa_destroy(&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_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));
-- 
2.47.3

Re: [PATCH] fs: Free any excess xarray nodes in clear_inode()

[Rik van Riel]

On Tue, 2026-06-23 at 20:28 +0100, Matthew Wilcox (Oracle) wrote:
> For many years we've had a hard to hit leak of xarray nodes.  Hugh
> documented it well in commit 786b31121a2c.  Recently people and
> syzbot have found ways to force it to happen with madvise.  Rather
> than fix the leaks where they happen, just call xa_destroy() which
> has the side-effect of cycling the i_pages lock.
> 
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Jinjiang Tu <tujinjiang@huawei.com>
> Cc: Dave Jones <davej@codemonkey.org.uk>
> Link:
> https://lore.kernel.org/all/20260121062243.1893129-1-tujinjiang@huawei.com/
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
Reviewed-by: Rik van Riel <riel@surriel.com>


-- 
All Rights Reversed.

Re: [PATCH] fs: Free any excess xarray nodes in clear_inode()

[Christian Brauner]

On Tue, 23 Jun 2026 20:28:48 +0100, Matthew Wilcox (Oracle) wrote:
> fs: Free any excess xarray nodes in clear_inode()

Applied to the vfs-7.3.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.3.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.3.misc

[1/1] fs: Free any excess xarray nodes in clear_inode()
      https://git.kernel.org/vfs/vfs/c/aaecb408f8d3

Re: [PATCH] fs: Free any excess xarray nodes in clear_inode()

[Chen Xiaokun]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

> For many years we've had a hard to hit leak of xarray nodes.  Hugh
> documented it well in commit 786b31121a2c.  Recently people and
> syzbot have found ways to force it to happen with madvise.  Rather
> than fix the leaks where they happen, just call xa_destroy() which
> has the side-effect of cycling the i_pages lock.

Could this commit be considered for backporting to the supported stable
kernels?

Commit: ee3f01125010 ("fs: Free any excess xarray nodes in clear_inode()")

The xarray node leak fixed by this commit is present in older stable
kernels, and this fix should address the issue there as well.

Thanks,

Chen

[-- Attachment #2: Type: text/html, Size: 1175 bytes --]

Re: [PATCH] fs: Free any excess xarray nodes in clear_inode()

[shinka]

> For many years we've had a hard to hit leak of xarray nodes.  Hugh
> documented it well in commit 786b31121a2c.  Recently people and
> syzbot have found ways to force it to happen with madvise.  Rather
> than fix the leaks where they happen, just call xa_destroy() which
> has the side-effect of cycling the i_pages lock.

Could this commit be considered for backporting to the supported stable
kernels?

Commit: ee3f01125010 ("fs: Free any excess xarray nodes in clear_inode()")

The xarray node leak fixed by this commit is present in older stable
kernels, and this fix should address the issue there as well.

Thanks,

Chen

Re: [PATCH] fs: Free any excess xarray nodes in clear_inode()

[Sasha Levin]

> Could this commit be considered for backporting to the supported stable
> kernels?

Queued for 7.2, 6.18, 6.12, 6.6, 6.1, 5.15 and 5.10, thanks.

-- 
Thanks,
Sasha