[v2] btrfs: ctree: Dump the leaf before BUG_ON()
diff mbox series

Message ID 20190424143134.32699-1-wqu@suse.com
State New
Headers show
Series
  • [v2] btrfs: ctree: Dump the leaf before BUG_ON()
Related show

Commit Message

Qu Wenruo April 24, 2019, 2:31 p.m. UTC
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.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v2:
- Also dump the slot/disk_key/new_key.
---
 fs/btrfs/ctree.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Comments

Filipe Manana April 24, 2019, 2:42 p.m. UTC | #1
On Wed, Apr 24, 2019 at 3:33 PM 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.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> changelog:
> v2:
> - Also dump the slot/disk_key/new_key.
> ---
>  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 f52eb952597b..61e6e2a23a8a 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,
> +               "on-disk slot %u key (%llu, %u, %llu) new key (%llu %u %llu)",

On-disk?  Not necessarily, the leaf might have been changed multiple
times after the last time it was persisted to disk.
I would remove that.

Also remove the commas after each element of key, just like you do for
new key (which is the prevalent style we have).

> +                                  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_ON(1);

Use BUG(); instead please, see
https://lore.kernel.org/linux-btrfs/20190325130256.1437810-1-arnd@arndb.de/

> +               }
>         }
>         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,
> +               "on-disk slot %u key (%llu, %u, %llu) new key (%llu %u %llu)",

Same here.

> +                                  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_ON(1);

Same here.

Other then that, it looks ok to me and you can add Reviewed-by:
fdmanana@suse.com

Thanks.

> +               }
>         }
>
>         btrfs_cpu_key_to_disk(&disk_key, new_key);
> --
> 2.21.0
>

Patch
diff mbox series

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index f52eb952597b..61e6e2a23a8a 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,
+		"on-disk 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_ON(1);
+		}
 	}
 	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,
+		"on-disk 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_ON(1);
+		}
 	}
 
 	btrfs_cpu_key_to_disk(&disk_key, new_key);