[PATCH 0/2] delayed_node leak bug

[PATCH 0/2] delayed_node leak bug

[Leo Martins]

Currently investigating a bug I believe is caused by leaked
delayed_nodes. The following patches fix a potential delayed_node leak
in an assert function (I don't believe this is the cause of the bug) and
add a warning if a root still contains delayed_nodes when it is freed.

A little more on the bug I'm investigating in case anyone has seen
something similar...

Started seeing soft lockups in btrfs_kill_all_delayed_nodes due to an
infinte loop. Further investigation showed that there was a
delayed_node that was not being erased from the root->delayed_nodes xarray.
The delayed_node had a reference count of one meaning that it is failing
to be released somewhere.

Leo Martins (2):
  btrfs: fix refcount leak in debug assertion
  btrfs: warn if leaking delayed_nodes

 fs/btrfs/delayed-inode.c | 7 ++++++-
 fs/btrfs/disk-io.c       | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.47.1

[PATCH 1/2] btrfs: fix refcount leak in debug assertion

[Leo Martins]

If the delayed_root is not empty we are increasing the number of
references to a delayed_node without decreasing it, causing a leak.
Fix by decrementing the delayed_node reference count.

Signed-off-by: Leo Martins <loemra.dev@gmail.com>
---
 fs/btrfs/delayed-inode.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index c7cc24a5dd5e..f4e47bfc603b 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -1377,7 +1377,12 @@ static int btrfs_wq_run_delayed_node(struct btrfs_delayed_root *delayed_root,
 
 void btrfs_assert_delayed_root_empty(struct btrfs_fs_info *fs_info)
 {
-	WARN_ON(btrfs_first_delayed_node(fs_info->delayed_root));
+	struct btrfs_delayed_node *node = btrfs_first_delayed_node(fs_info->delayed_root);
+
+	WARN_ON(node);
+
+	if (node)
+		refcount_dec(&node->refs);
 }
 
 static bool could_end_wait(struct btrfs_delayed_root *delayed_root, int seq)
-- 
2.47.1

Re: [PATCH 1/2] btrfs: fix refcount leak in debug assertion

[Filipe Manana]

On Tue, May 27, 2025 at 8:29 PM Leo Martins <loemra.dev@gmail.com> wrote:
>
> If the delayed_root is not empty we are increasing the number of
> references to a delayed_node without decreasing it, causing a leak.
> Fix by decrementing the delayed_node reference count.
>
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> ---
>  fs/btrfs/delayed-inode.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index c7cc24a5dd5e..f4e47bfc603b 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -1377,7 +1377,12 @@ static int btrfs_wq_run_delayed_node(struct btrfs_delayed_root *delayed_root,
>
>  void btrfs_assert_delayed_root_empty(struct btrfs_fs_info *fs_info)
>  {
> -       WARN_ON(btrfs_first_delayed_node(fs_info->delayed_root));
> +       struct btrfs_delayed_node *node = btrfs_first_delayed_node(fs_info->delayed_root);
> +
> +       WARN_ON(node);
> +
> +       if (node)

This can shortened to:

if (WARN_ON(node))

Besides making the code shorter, it also allows the compiler to
generate better code since the WARN_ON() includes the unlikely tag and
this is an unexpected branch, so we benefit from doing it like that.

With that change:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

> +               refcount_dec(&node->refs);
>  }
>
>  static bool could_end_wait(struct btrfs_delayed_root *delayed_root, int seq)
> --
> 2.47.1
>
>

Re: [PATCH 1/2] btrfs: fix refcount leak in debug assertion

[Qu Wenruo]

在 2025/5/28 04:58, Leo Martins 写道:
> If the delayed_root is not empty we are increasing the number of
> references to a delayed_node without decreasing it, causing a leak.
> Fix by decrementing the delayed_node reference count.
> 
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> ---
>   fs/btrfs/delayed-inode.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index c7cc24a5dd5e..f4e47bfc603b 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -1377,7 +1377,12 @@ static int btrfs_wq_run_delayed_node(struct btrfs_delayed_root *delayed_root,
>   
>   void btrfs_assert_delayed_root_empty(struct btrfs_fs_info *fs_info)
>   {
> -	WARN_ON(btrfs_first_delayed_node(fs_info->delayed_root));
> +	struct btrfs_delayed_node *node = btrfs_first_delayed_node(fs_info->delayed_root);
> +
> +	WARN_ON(node);
> +
> +	if (node)

Just like what Filipe has pointed out, one line "if (WARN_ON(node))" 
will be good enough.

Otherwise looks good to me.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu> +		refcount_dec(&node->refs);
>   }
>   
>   static bool could_end_wait(struct btrfs_delayed_root *delayed_root, int seq)

[PATCH 2/2] btrfs: warn if leaking delayed_nodes

[Leo Martins]

Add a warning for leaked delayed_nodes when putting a root. We currently do this
for inodes, but not delayed_nodes.

Signed-off-by: Leo Martins <loemra.dev@gmail.com>
---
 fs/btrfs/disk-io.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1beb9458f622..3def93016963 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1835,6 +1835,8 @@ void btrfs_put_root(struct btrfs_root *root)
 	if (refcount_dec_and_test(&root->refs)) {
 		if (WARN_ON(!xa_empty(&root->inodes)))
 			xa_destroy(&root->inodes);
+		if (WARN_ON(!xa_empty(&root->delayed_nodes)))
+			xa_destroy(&root->delayed_nodes);
 		WARN_ON(test_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state));
 		if (root->anon_dev)
 			free_anon_bdev(root->anon_dev);
-- 
2.47.1

Re: [PATCH 2/2] btrfs: warn if leaking delayed_nodes

[Filipe Manana]

On Tue, May 27, 2025 at 8:29 PM Leo Martins <loemra.dev@gmail.com> wrote:
>
> Add a warning for leaked delayed_nodes when putting a root. We currently do this
> for inodes, but not delayed_nodes.
>
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, thanks.

> ---
>  fs/btrfs/disk-io.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 1beb9458f622..3def93016963 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1835,6 +1835,8 @@ void btrfs_put_root(struct btrfs_root *root)
>         if (refcount_dec_and_test(&root->refs)) {
>                 if (WARN_ON(!xa_empty(&root->inodes)))
>                         xa_destroy(&root->inodes);
> +               if (WARN_ON(!xa_empty(&root->delayed_nodes)))
> +                       xa_destroy(&root->delayed_nodes);
>                 WARN_ON(test_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state));
>                 if (root->anon_dev)
>                         free_anon_bdev(root->anon_dev);
> --
> 2.47.1
>
>

Re: [PATCH 2/2] btrfs: warn if leaking delayed_nodes

[Qu Wenruo]

在 2025/5/28 04:58, Leo Martins 写道:
> Add a warning for leaked delayed_nodes when putting a root. We currently do this
> for inodes, but not delayed_nodes.
> 
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/disk-io.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 1beb9458f622..3def93016963 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1835,6 +1835,8 @@ void btrfs_put_root(struct btrfs_root *root)
>   	if (refcount_dec_and_test(&root->refs)) {
>   		if (WARN_ON(!xa_empty(&root->inodes)))
>   			xa_destroy(&root->inodes);
> +		if (WARN_ON(!xa_empty(&root->delayed_nodes)))
> +			xa_destroy(&root->delayed_nodes);
>   		WARN_ON(test_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state));
>   		if (root->anon_dev)
>   			free_anon_bdev(root->anon_dev)