[PATCH] btrfs: WARN_ON_ONCE() in our leak detection code

[PATCH] btrfs: WARN_ON_ONCE() in our leak detection code

[Josef Bacik]

fstests looks for WARN_ON's in dmesg.  Add WARN_ON_ONCE() to our leak
detection code so that fstests will fail if these things trip at all.
This will allow us to easily catch problems with our reference counting
that may otherwise go unnoticed.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/disk-io.c        | 1 +
 fs/btrfs/extent-io-tree.c | 1 +
 fs/btrfs/extent_io.c      | 1 +
 3 files changed, 3 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c6907d533fe8..5f350702a4d9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1244,6 +1244,7 @@ void btrfs_check_leaked_roots(struct btrfs_fs_info *fs_info)
 		btrfs_err(fs_info, "leaked root %s refcount %d",
 			  btrfs_root_name(&root->root_key, buf),
 			  refcount_read(&root->refs));
+		WARN_ON_ONCE(1);
 		while (refcount_read(&root->refs) > 1)
 			btrfs_put_root(root);
 		btrfs_put_root(root);
diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index e3ee5449cc4a..1544e7b1eaed 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -48,6 +48,7 @@ static inline void btrfs_extent_state_leak_debug_check(void)
 		       extent_state_in_tree(state),
 		       refcount_read(&state->refs));
 		list_del(&state->leak_list);
+		WARN_ON_ONCE(1);
 		kmem_cache_free(extent_state_cache, state);
 	}
 }
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a0ffd41c5cc1..a173cf08eb8f 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -82,6 +82,7 @@ void btrfs_extent_buffer_leak_debug_check(struct btrfs_fs_info *fs_info)
 		       eb->start, eb->len, atomic_read(&eb->refs), eb->bflags,
 		       btrfs_header_owner(eb));
 		list_del(&eb->leak_list);
+		WARN_ON_ONCE(1);
 		kmem_cache_free(extent_buffer_cache, eb);
 	}
 	spin_unlock_irqrestore(&fs_info->eb_leak_lock, flags);
-- 
2.43.0

Re: [PATCH] btrfs: WARN_ON_ONCE() in our leak detection code

[Neal Gompa]

On Tue, Jan 2, 2024 at 3:18 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> fstests looks for WARN_ON's in dmesg.  Add WARN_ON_ONCE() to our leak
> detection code so that fstests will fail if these things trip at all.
> This will allow us to easily catch problems with our reference counting
> that may otherwise go unnoticed.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/disk-io.c        | 1 +
>  fs/btrfs/extent-io-tree.c | 1 +
>  fs/btrfs/extent_io.c      | 1 +
>  3 files changed, 3 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c6907d533fe8..5f350702a4d9 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1244,6 +1244,7 @@ void btrfs_check_leaked_roots(struct btrfs_fs_info *fs_info)
>                 btrfs_err(fs_info, "leaked root %s refcount %d",
>                           btrfs_root_name(&root->root_key, buf),
>                           refcount_read(&root->refs));
> +               WARN_ON_ONCE(1);
>                 while (refcount_read(&root->refs) > 1)
>                         btrfs_put_root(root);
>                 btrfs_put_root(root);
> diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
> index e3ee5449cc4a..1544e7b1eaed 100644
> --- a/fs/btrfs/extent-io-tree.c
> +++ b/fs/btrfs/extent-io-tree.c
> @@ -48,6 +48,7 @@ static inline void btrfs_extent_state_leak_debug_check(void)
>                        extent_state_in_tree(state),
>                        refcount_read(&state->refs));
>                 list_del(&state->leak_list);
> +               WARN_ON_ONCE(1);
>                 kmem_cache_free(extent_state_cache, state);
>         }
>  }
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index a0ffd41c5cc1..a173cf08eb8f 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -82,6 +82,7 @@ void btrfs_extent_buffer_leak_debug_check(struct btrfs_fs_info *fs_info)
>                        eb->start, eb->len, atomic_read(&eb->refs), eb->bflags,
>                        btrfs_header_owner(eb));
>                 list_del(&eb->leak_list);
> +               WARN_ON_ONCE(1);
>                 kmem_cache_free(extent_buffer_cache, eb);
>         }
>         spin_unlock_irqrestore(&fs_info->eb_leak_lock, flags);
> --
> 2.43.0
>

Great, simple, and useful! A nice trifecta.

Reviewed-by: Neal Gompa <neal@gompa.dev>


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH] btrfs: WARN_ON_ONCE() in our leak detection code

[Anand Jain]

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thx, Anand

Re: [PATCH] btrfs: WARN_ON_ONCE() in our leak detection code

[David Sterba]

On Tue, Jan 02, 2024 at 03:18:07PM -0500, Josef Bacik wrote:
> fstests looks for WARN_ON's in dmesg.  Add WARN_ON_ONCE() to our leak
> detection code so that fstests will fail if these things trip at all.
> This will allow us to easily catch problems with our reference counting
> that may otherwise go unnoticed.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Added to misc-next, thanks.