diff mbox series

[5/5] btrfs: improve error message in setup_items_for_insert

Message ID 20200901144001.4265-6-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series Improve setup_items_for_insert | expand

Commit Message

Nikolay Borisov Sept. 1, 2020, 2:40 p.m. UTC
While at it also print the leaf content after the error.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Josef Bacik Sept. 1, 2020, 7:23 p.m. UTC | #1
On 9/1/20 10:40 AM, Nikolay Borisov wrote:
> While at it also print the leaf content after the error.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
David Sterba Sept. 10, 2020, 4:14 p.m. UTC | #2
On Tue, Sep 01, 2020 at 05:40:01PM +0300, Nikolay Borisov wrote:
> While at it also print the leaf content after the error.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/ctree.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index bb89c0954ca1..2c3f78cc6aa2 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -4799,9 +4799,9 @@ void setup_items_for_insert(struct btrfs_root *root, struct btrfs_path *path,
>  		unsigned int old_data = btrfs_item_end_nr(leaf, slot);
>  
>  		if (old_data < data_end) {
> -			btrfs_print_leaf(leaf);
> -			btrfs_crit(fs_info, "slot %d old_data %d data_end %d",
> +			btrfs_crit(fs_info, "item's (slot %d) data offset (%d) beyond data end of leaf (%d)",
>  				   slot, old_data, data_end);
> +			btrfs_print_leaf(leaf);

This is a question if the order is right or vice versa. When something
goes wrong, the leaf dump fills a few pages or screens and it's not
always possible to scroll back.

The idea of 1) dump leaf 2) print the error message, is to keep the
specific error last hoping that it's still visible when somebody takes a
screenshot for a bugreport.

I've checked instances where we call print_leaf and this is the most
common ordering, so I'd rather keep it like that as it has some practial
impacts.
David Sterba Sept. 11, 2020, 2:50 p.m. UTC | #3
On Tue, Sep 01, 2020 at 05:40:01PM +0300, Nikolay Borisov wrote:
> While at it also print the leaf content after the error.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/ctree.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index bb89c0954ca1..2c3f78cc6aa2 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -4799,9 +4799,9 @@ void setup_items_for_insert(struct btrfs_root *root, struct btrfs_path *path,
>  		unsigned int old_data = btrfs_item_end_nr(leaf, slot);
>  
>  		if (old_data < data_end) {
> -			btrfs_print_leaf(leaf);
> -			btrfs_crit(fs_info, "slot %d old_data %d data_end %d",
> +			btrfs_crit(fs_info, "item's (slot %d) data offset (%d) beyond data end of leaf (%d)",
>  				   slot, old_data, data_end);

I've changed it to

"item at slot %d with data offset %u beyond data end of leaf %u"

as we don't use the ( ) elsewhere. The formats were mismatching old_data
and data_end, so they're now %u.

> +			btrfs_print_leaf(leaf);

The leaf is dumped as before, so the changelog got updated too.
diff mbox series

Patch

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index bb89c0954ca1..2c3f78cc6aa2 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -4799,9 +4799,9 @@  void setup_items_for_insert(struct btrfs_root *root, struct btrfs_path *path,
 		unsigned int old_data = btrfs_item_end_nr(leaf, slot);
 
 		if (old_data < data_end) {
-			btrfs_print_leaf(leaf);
-			btrfs_crit(fs_info, "slot %d old_data %d data_end %d",
+			btrfs_crit(fs_info, "item's (slot %d) data offset (%d) beyond data end of leaf (%d)",
 				   slot, old_data, data_end);
+			btrfs_print_leaf(leaf);
 			BUG();
 		}
 		/*