[PATCH v3] btrfs: ctree: Dump the leaf before BUG_ON()

[PATCH v3] btrfs: ctree: Dump the leaf before BUG_ON()

[Qu Wenruo]

We have a user reporting BUG_ON() triggered in
btrfs_set_item_key_safe().

Let's dump the leaf content before triggering BUG_ON() so that we can
have some clue on what's going wrong.
The output of tree locks should help us to debug such problem.

Reviewed-by: Filip Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changelog:
v2:
- Dump slot/old key/new key.
v3:
- Output message update, to avoid "on-disk" wording.
- Use BUG() to replace BUG_ON(1) to make clang quite.

 fs/btrfs/ctree.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 5116c2a1f0f9..5df76c17775a 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -3185,11 +3185,31 @@ void btrfs_set_item_key_safe(struct btrfs_fs_info *fs_info,
 	slot = path->slots[0];
 	if (slot > 0) {
 		btrfs_item_key(eb, &disk_key, slot - 1);
-		BUG_ON(comp_keys(&disk_key, new_key) >= 0);
+		if (unlikely(comp_keys(&disk_key, new_key) >= 0)) {
+			btrfs_crit(fs_info,
+		"slot %u key (%llu %u %llu) new key (%llu %u %llu)",
+				   slot, btrfs_disk_key_objectid(&disk_key),
+				   btrfs_disk_key_type(&disk_key),
+				   btrfs_disk_key_offset(&disk_key),
+				   new_key->objectid, new_key->type,
+				   new_key->offset);
+			btrfs_print_leaf(eb);
+			BUG();
+		}
 	}
 	if (slot < btrfs_header_nritems(eb) - 1) {
 		btrfs_item_key(eb, &disk_key, slot + 1);
-		BUG_ON(comp_keys(&disk_key, new_key) <= 0);
+		if (unlikely(comp_keys(&disk_key, new_key) <= 0)) {
+			btrfs_crit(fs_info,
+		"slot %u key (%llu %u %llu) new key (%llu %u %llu)",
+				   slot, btrfs_disk_key_objectid(&disk_key),
+				   btrfs_disk_key_type(&disk_key),
+				   btrfs_disk_key_offset(&disk_key),
+				   new_key->objectid, new_key->type,
+				   new_key->offset);
+			btrfs_print_leaf(eb);
+			BUG();
+		}
 	}
 
 	btrfs_cpu_key_to_disk(&disk_key, new_key);
-- 
2.21.0

Re: [PATCH v3] btrfs: ctree: Dump the leaf before BUG_ON()

[Filipe Manana]

On Thu, Apr 25, 2019 at 8:52 AM Qu Wenruo <wqu@suse.com> wrote:
>
> We have a user reporting BUG_ON() triggered in
> btrfs_set_item_key_safe().
>
> Let's dump the leaf content before triggering BUG_ON() so that we can
> have some clue on what's going wrong.
> The output of tree locks should help us to debug such problem.
>
> Reviewed-by: Filip Manana <fdmanana@suse.com>

Filip -> Filipe

David can probably amend that at commit time.

> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Dump slot/old key/new key.
> v3:
> - Output message update, to avoid "on-disk" wording.
> - Use BUG() to replace BUG_ON(1) to make clang quite.
>
>  fs/btrfs/ctree.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 5116c2a1f0f9..5df76c17775a 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -3185,11 +3185,31 @@ void btrfs_set_item_key_safe(struct btrfs_fs_info *fs_info,
>         slot = path->slots[0];
>         if (slot > 0) {
>                 btrfs_item_key(eb, &disk_key, slot - 1);
> -               BUG_ON(comp_keys(&disk_key, new_key) >= 0);
> +               if (unlikely(comp_keys(&disk_key, new_key) >= 0)) {
> +                       btrfs_crit(fs_info,
> +               "slot %u key (%llu %u %llu) new key (%llu %u %llu)",
> +                                  slot, btrfs_disk_key_objectid(&disk_key),
> +                                  btrfs_disk_key_type(&disk_key),
> +                                  btrfs_disk_key_offset(&disk_key),
> +                                  new_key->objectid, new_key->type,
> +                                  new_key->offset);
> +                       btrfs_print_leaf(eb);
> +                       BUG();
> +               }
>         }
>         if (slot < btrfs_header_nritems(eb) - 1) {
>                 btrfs_item_key(eb, &disk_key, slot + 1);
> -               BUG_ON(comp_keys(&disk_key, new_key) <= 0);
> +               if (unlikely(comp_keys(&disk_key, new_key) <= 0)) {
> +                       btrfs_crit(fs_info,
> +               "slot %u key (%llu %u %llu) new key (%llu %u %llu)",
> +                                  slot, btrfs_disk_key_objectid(&disk_key),
> +                                  btrfs_disk_key_type(&disk_key),
> +                                  btrfs_disk_key_offset(&disk_key),
> +                                  new_key->objectid, new_key->type,
> +                                  new_key->offset);
> +                       btrfs_print_leaf(eb);
> +                       BUG();
> +               }
>         }
>
>         btrfs_cpu_key_to_disk(&disk_key, new_key);
> --
> 2.21.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH v3] btrfs: ctree: Dump the leaf before BUG_ON()

[David Sterba]

On Thu, Apr 25, 2019 at 08:55:53AM +0800, Qu Wenruo wrote:
> We have a user reporting BUG_ON() triggered in
> btrfs_set_item_key_safe().
> 
> Let's dump the leaf content before triggering BUG_ON() so that we can
> have some clue on what's going wrong.
> The output of tree locks should help us to debug such problem.
> 
> Reviewed-by: Filip Manana <fdmanana@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Dump slot/old key/new key.
> v3:
> - Output message update, to avoid "on-disk" wording.
> - Use BUG() to replace BUG_ON(1) to make clang quite.
> 
>  fs/btrfs/ctree.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 5116c2a1f0f9..5df76c17775a 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -3185,11 +3185,31 @@ void btrfs_set_item_key_safe(struct btrfs_fs_info *fs_info,
>  	slot = path->slots[0];
>  	if (slot > 0) {
>  		btrfs_item_key(eb, &disk_key, slot - 1);
> -		BUG_ON(comp_keys(&disk_key, new_key) >= 0);
> +		if (unlikely(comp_keys(&disk_key, new_key) >= 0)) {
> +			btrfs_crit(fs_info,
> +		"slot %u key (%llu %u %llu) new key (%llu %u %llu)",
> +				   slot, btrfs_disk_key_objectid(&disk_key),
> +				   btrfs_disk_key_type(&disk_key),
> +				   btrfs_disk_key_offset(&disk_key),
> +				   new_key->objectid, new_key->type,
> +				   new_key->offset);
> +			btrfs_print_leaf(eb);
> +			BUG();

This is going to produce a lot of lines, with the stacktrace at the end.
We'll understand what's wrong just by seeing the stack, though the
contents of the leaf will be only in the logs and not on screen. This
hopefully will be available for analysis. I've seen this bug too and
will be able to provide the logs eventually.

Re: [PATCH v3] btrfs: ctree: Dump the leaf before BUG_ON()

[David Sterba]

On Thu, Apr 25, 2019 at 08:55:53AM +0800, Qu Wenruo wrote:
> We have a user reporting BUG_ON() triggered in
> btrfs_set_item_key_safe().
> 
> Let's dump the leaf content before triggering BUG_ON() so that we can
> have some clue on what's going wrong.
> The output of tree locks should help us to debug such problem.
> 
> Reviewed-by: Filip Manana <fdmanana@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Dump slot/old key/new key.
> v3:
> - Output message update, to avoid "on-disk" wording.
> - Use BUG() to replace BUG_ON(1) to make clang quite.

Name fixed and I've added a sample stacktrace that I had in the logs.
Patch added to misc-next, thanks.