[PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[Qu Wenruo]

[PROBLEM]
Inside function update_inline_extent_backref(), we have several
BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
filesystem.

[ANAYLYSE]
Most of those BUG_ON()s and ASSERT()s are just a way of handling
unexpected on-disk data.

Although we have tree-checker to rule out obviously incorrect extent
tree blocks, it's not enough for those ones.

Thus we need proper error handling for them.

[FIX]
Thankfully all the callers of update_inline_extent_backref() would
eventually handle the errror by aborting the current transaction.

So this patch would do the proper error handling by:

- Make update_inline_extent_backref() to return int
  The return value would be either 0 or -EUCLEAN.

- Replace BUG_ON()s and ASSERT()s with proper error handling
  This includes:
  * Dump the bad extent tree leaf
  * Output an error message for the cause
    This would include the extent bytenr, num_bytes (if needed),
    the bad values and expected good values.
  * Return -EUCLEAN

  Note here we remove all the WARN_ON()s, as eventually the transaction
  would be aborted, thus a backtrace would be triggered anyway.

- Better comments on why we expect refs == 1 and refs_to_mode == -1 for
  tree blocks

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent-tree.c | 80 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 3cae798499e2..45e325523e81 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -381,11 +381,11 @@ int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
 		}
 	}
 
+	WARN_ON(1);
 	btrfs_print_leaf(eb);
 	btrfs_err(eb->fs_info,
 		  "eb %llu iref 0x%lx invalid extent inline ref type %d",
 		  eb->start, (unsigned long)iref, type);
-	WARN_ON(1);
 
 	return BTRFS_REF_TYPE_INVALID;
 }
@@ -1059,12 +1059,13 @@ static int lookup_extent_backref(struct btrfs_trans_handle *trans,
  * helper to update/remove inline back ref
  */
 static noinline_for_stack
-void update_inline_extent_backref(struct btrfs_path *path,
-				  struct btrfs_extent_inline_ref *iref,
-				  int refs_to_mod,
-				  struct btrfs_delayed_extent_op *extent_op)
+int update_inline_extent_backref(struct btrfs_path *path,
+				 struct btrfs_extent_inline_ref *iref,
+				 int refs_to_mod,
+				 struct btrfs_delayed_extent_op *extent_op)
 {
 	struct extent_buffer *leaf = path->nodes[0];
+	struct btrfs_fs_info *fs_info = leaf->fs_info;
 	struct btrfs_extent_item *ei;
 	struct btrfs_extent_data_ref *dref = NULL;
 	struct btrfs_shared_data_ref *sref = NULL;
@@ -1077,18 +1078,33 @@ void update_inline_extent_backref(struct btrfs_path *path,
 
 	ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
 	refs = btrfs_extent_refs(leaf, ei);
-	WARN_ON(refs_to_mod < 0 && refs + refs_to_mod <= 0);
+	if (unlikely(refs_to_mod < 0 && refs + refs_to_mod <= 0)) {
+		struct btrfs_key key;
+		u32 extent_size;
+
+		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
+		if (key.type == BTRFS_METADATA_ITEM_KEY)
+			extent_size = fs_info->nodesize;
+		else
+			extent_size = key.offset;
+		btrfs_print_leaf(leaf);
+		btrfs_err(fs_info,
+"invalid refs_to_mod for extent %llu num_bytes %u, has %d expect >= -%llu",
+			  key.objectid, extent_size, refs_to_mod, refs);
+		return -EUCLEAN;
+	}
 	refs += refs_to_mod;
 	btrfs_set_extent_refs(leaf, ei, refs);
 	if (extent_op)
 		__run_delayed_extent_op(extent_op, leaf, ei);
 
-	/*
-	 * If type is invalid, we should have bailed out after
-	 * lookup_inline_extent_backref().
-	 */
 	type = btrfs_get_extent_inline_ref_type(leaf, iref, BTRFS_REF_TYPE_ANY);
-	ASSERT(type != BTRFS_REF_TYPE_INVALID);
+	/*
+	 * Function btrfs_get_extent_inline_ref_type() has already output
+	 * error messages.
+	 */
+	if (unlikely(type == BTRFS_REF_TYPE_INVALID))
+		return -EUCLEAN;
 
 	if (type == BTRFS_EXTENT_DATA_REF_KEY) {
 		dref = (struct btrfs_extent_data_ref *)(&iref->offset);
@@ -1098,10 +1114,43 @@ void update_inline_extent_backref(struct btrfs_path *path,
 		refs = btrfs_shared_data_ref_count(leaf, sref);
 	} else {
 		refs = 1;
-		BUG_ON(refs_to_mod != -1);
+		/*
+		 * For tree blocks we can only drop one ref for it, and
+		 * tree blocks should not have refs > 1.
+		 *
+		 * Furthermore if we're inserting a new inline backref,
+		 * we won't reach this path either.
+		 * (that would be setup_inline_extent_backref())
+		 */
+		if (unlikely(refs_to_mod != -1)) {
+			struct btrfs_key key;
+
+			btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
+
+			btrfs_print_leaf(leaf);
+			btrfs_err(fs_info,
+		"invalid refs_to_mod for tree block %llu, has %d expect -1",
+				  key.objectid, refs_to_mod);
+			return -EUCLEAN;
+		}
 	}
 
-	BUG_ON(refs_to_mod < 0 && refs < -refs_to_mod);
+	if (unlikely(refs_to_mod < 0 && refs < -refs_to_mod)) {
+		struct btrfs_key key;
+		u32 extent_size;
+
+		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
+		if (key.type == BTRFS_METADATA_ITEM_KEY)
+			extent_size = fs_info->nodesize;
+		else
+			extent_size = key.offset;
+		btrfs_print_leaf(leaf);
+		btrfs_err(fs_info,
+"invalid refs_to_mod for backref entry, iref %lu extent %llu num_bytes %u, has %d expect >= -%llu",
+			  (unsigned long)iref, key.objectid, extent_size,
+			  refs_to_mod, refs);
+		return -EUCLEAN;
+	}
 	refs += refs_to_mod;
 
 	if (refs > 0) {
@@ -1121,6 +1170,7 @@ void update_inline_extent_backref(struct btrfs_path *path,
 		btrfs_truncate_item(path, item_size, 1);
 	}
 	btrfs_mark_buffer_dirty(leaf);
+	return 0;
 }
 
 static noinline_for_stack
@@ -1149,7 +1199,7 @@ int insert_inline_extent_backref(struct btrfs_trans_handle *trans,
 				   bytenr, num_bytes, root_objectid, path->slots[0]);
 			return -EUCLEAN;
 		}
-		update_inline_extent_backref(path, iref, refs_to_add, extent_op);
+		ret = update_inline_extent_backref(path, iref, refs_to_add, extent_op);
 	} else if (ret == -ENOENT) {
 		setup_inline_extent_backref(trans->fs_info, path, iref, parent,
 					    root_objectid, owner, offset,
@@ -1169,7 +1219,7 @@ static int remove_extent_backref(struct btrfs_trans_handle *trans,
 
 	BUG_ON(!is_data && refs_to_drop != 1);
 	if (iref)
-		update_inline_extent_backref(path, iref, -refs_to_drop, NULL);
+		ret = update_inline_extent_backref(path, iref, -refs_to_drop, NULL);
 	else if (is_data)
 		ret = remove_extent_data_ref(trans, root, path, refs_to_drop);
 	else
-- 
2.41.0

Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[David Sterba]

On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
> [PROBLEM]
> Inside function update_inline_extent_backref(), we have several
> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
> filesystem.
> 
> [ANAYLYSE]
> Most of those BUG_ON()s and ASSERT()s are just a way of handling
> unexpected on-disk data.
> 
> Although we have tree-checker to rule out obviously incorrect extent
> tree blocks, it's not enough for those ones.
> 
> Thus we need proper error handling for them.
> 
> [FIX]
> Thankfully all the callers of update_inline_extent_backref() would
> eventually handle the errror by aborting the current transaction.
> 
> So this patch would do the proper error handling by:
> 
> - Make update_inline_extent_backref() to return int
>   The return value would be either 0 or -EUCLEAN.
> 
> - Replace BUG_ON()s and ASSERT()s with proper error handling
>   This includes:
>   * Dump the bad extent tree leaf
>   * Output an error message for the cause
>     This would include the extent bytenr, num_bytes (if needed),
>     the bad values and expected good values.
>   * Return -EUCLEAN
> 
>   Note here we remove all the WARN_ON()s, as eventually the transaction
>   would be aborted, thus a backtrace would be triggered anyway.
> 
> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
>   tree blocks
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Is this fix for syzbot report
https://lore.kernel.org/linux-btrfs/000000000000287928060275b914@google.com/
?

Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[Qu Wenruo]

On 2023/8/9 19:20, David Sterba wrote:
> On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
>> [PROBLEM]
>> Inside function update_inline_extent_backref(), we have several
>> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
>> filesystem.
>>
>> [ANAYLYSE]
>> Most of those BUG_ON()s and ASSERT()s are just a way of handling
>> unexpected on-disk data.
>>
>> Although we have tree-checker to rule out obviously incorrect extent
>> tree blocks, it's not enough for those ones.
>>
>> Thus we need proper error handling for them.
>>
>> [FIX]
>> Thankfully all the callers of update_inline_extent_backref() would
>> eventually handle the errror by aborting the current transaction.
>>
>> So this patch would do the proper error handling by:
>>
>> - Make update_inline_extent_backref() to return int
>>    The return value would be either 0 or -EUCLEAN.
>>
>> - Replace BUG_ON()s and ASSERT()s with proper error handling
>>    This includes:
>>    * Dump the bad extent tree leaf
>>    * Output an error message for the cause
>>      This would include the extent bytenr, num_bytes (if needed),
>>      the bad values and expected good values.
>>    * Return -EUCLEAN
>>
>>    Note here we remove all the WARN_ON()s, as eventually the transaction
>>    would be aborted, thus a backtrace would be triggered anyway.
>>
>> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
>>    tree blocks
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> Is this fix for syzbot report
> https://lore.kernel.org/linux-btrfs/000000000000287928060275b914@google.com/
> ?

Unfortunately, I don't think this is the proper fix.

Since there is no reproducer, I'm not sure if it's really corrupted fs
causing the bug.

If it's something else, then the patch won't help at all, except a
little better debug output.

Thanks,
Qu

Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[Josef Bacik]

On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
> [PROBLEM]
> Inside function update_inline_extent_backref(), we have several
> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
> filesystem.
> 
> [ANAYLYSE]
> Most of those BUG_ON()s and ASSERT()s are just a way of handling
> unexpected on-disk data.
> 
> Although we have tree-checker to rule out obviously incorrect extent
> tree blocks, it's not enough for those ones.
> 
> Thus we need proper error handling for them.
> 
> [FIX]
> Thankfully all the callers of update_inline_extent_backref() would
> eventually handle the errror by aborting the current transaction.
> 
> So this patch would do the proper error handling by:
> 
> - Make update_inline_extent_backref() to return int
>   The return value would be either 0 or -EUCLEAN.
> 
> - Replace BUG_ON()s and ASSERT()s with proper error handling
>   This includes:
>   * Dump the bad extent tree leaf
>   * Output an error message for the cause
>     This would include the extent bytenr, num_bytes (if needed),
>     the bad values and expected good values.
>   * Return -EUCLEAN
> 
>   Note here we remove all the WARN_ON()s, as eventually the transaction
>   would be aborted, thus a backtrace would be triggered anyway.
> 
> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
>   tree blocks
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

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

Thanks,

Josef

Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[David Sterba]

On Wed, Aug 09, 2023 at 07:29:20PM +0800, Qu Wenruo wrote:
> 
> 
> On 2023/8/9 19:20, David Sterba wrote:
> > On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
> > > [PROBLEM]
> > > Inside function update_inline_extent_backref(), we have several
> > > BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
> > > filesystem.
> > > 
> > > [ANAYLYSE]
> > > Most of those BUG_ON()s and ASSERT()s are just a way of handling
> > > unexpected on-disk data.
> > > 
> > > Although we have tree-checker to rule out obviously incorrect extent
> > > tree blocks, it's not enough for those ones.
> > > 
> > > Thus we need proper error handling for them.
> > > 
> > > [FIX]
> > > Thankfully all the callers of update_inline_extent_backref() would
> > > eventually handle the errror by aborting the current transaction.
> > > 
> > > So this patch would do the proper error handling by:
> > > 
> > > - Make update_inline_extent_backref() to return int
> > >    The return value would be either 0 or -EUCLEAN.
> > > 
> > > - Replace BUG_ON()s and ASSERT()s with proper error handling
> > >    This includes:
> > >    * Dump the bad extent tree leaf
> > >    * Output an error message for the cause
> > >      This would include the extent bytenr, num_bytes (if needed),
> > >      the bad values and expected good values.
> > >    * Return -EUCLEAN
> > > 
> > >    Note here we remove all the WARN_ON()s, as eventually the transaction
> > >    would be aborted, thus a backtrace would be triggered anyway.
> > > 
> > > - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
> > >    tree blocks
> > > 
> > > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > 
> > Is this fix for syzbot report
> > https://lore.kernel.org/linux-btrfs/000000000000287928060275b914@google.com/
> > ?
> 
> Unfortunately, I don't think this is the proper fix.
> 
> Since there is no reproducer, I'm not sure if it's really corrupted fs
> causing the bug.
> 
> If it's something else, then the patch won't help at all, except a
> little better debug output.

Ok, thanks. Adding more error handling instead of BUG_ON or ASSERT that
could be possibly hit by corrupted images is good on its own.

Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[David Sterba]

On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
> [PROBLEM]
> Inside function update_inline_extent_backref(), we have several
> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
> filesystem.
> 
> [ANAYLYSE]
> Most of those BUG_ON()s and ASSERT()s are just a way of handling
> unexpected on-disk data.
> 
> Although we have tree-checker to rule out obviously incorrect extent
> tree blocks, it's not enough for those ones.

Yeah we know tree-checker does not have and cannot have complete
coverage of the input data so we may need to add more sanity checks and
definitely convert the BUG_ONs if they "act" as error handling.

> Thus we need proper error handling for them.
> 
> [FIX]
> Thankfully all the callers of update_inline_extent_backref() would
> eventually handle the errror by aborting the current transaction.
> 
> So this patch would do the proper error handling by:
> 
> - Make update_inline_extent_backref() to return int
>   The return value would be either 0 or -EUCLEAN.
> 
> - Replace BUG_ON()s and ASSERT()s with proper error handling
>   This includes:
>   * Dump the bad extent tree leaf
>   * Output an error message for the cause
>     This would include the extent bytenr, num_bytes (if needed),
>     the bad values and expected good values.
>   * Return -EUCLEAN
> 
>   Note here we remove all the WARN_ON()s, as eventually the transaction
>   would be aborted, thus a backtrace would be triggered anyway.
> 
> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
>   tree blocks
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to misc-next, thanks.

Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[David Sterba]

On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
> [PROBLEM]
> Inside function update_inline_extent_backref(), we have several
> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
> filesystem.
> 
> [ANAYLYSE]
> Most of those BUG_ON()s and ASSERT()s are just a way of handling
> unexpected on-disk data.
> 
> Although we have tree-checker to rule out obviously incorrect extent
> tree blocks, it's not enough for those ones.
> 
> Thus we need proper error handling for them.
> 
> [FIX]
> Thankfully all the callers of update_inline_extent_backref() would
> eventually handle the errror by aborting the current transaction.
> 
> So this patch would do the proper error handling by:
> 
> - Make update_inline_extent_backref() to return int
>   The return value would be either 0 or -EUCLEAN.
> 
> - Replace BUG_ON()s and ASSERT()s with proper error handling
>   This includes:
>   * Dump the bad extent tree leaf
>   * Output an error message for the cause
>     This would include the extent bytenr, num_bytes (if needed),
>     the bad values and expected good values.
>   * Return -EUCLEAN
> 
>   Note here we remove all the WARN_ON()s, as eventually the transaction
>   would be aborted, thus a backtrace would be triggered anyway.
> 
> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
>   tree blocks
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/extent-tree.c | 80 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 65 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 3cae798499e2..45e325523e81 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -381,11 +381,11 @@ int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
>  		}
>  	}
>  
> +	WARN_ON(1);
>  	btrfs_print_leaf(eb);
>  	btrfs_err(eb->fs_info,
>  		  "eb %llu iref 0x%lx invalid extent inline ref type %d",
>  		  eb->start, (unsigned long)iref, type);
> -	WARN_ON(1);

Do we even want to print the warning here? There's the whole leaf, error
message, why would we need the stack trace?

Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[Qu Wenruo]

On 2023/8/10 02:59, David Sterba wrote:
> On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
>> [PROBLEM]
>> Inside function update_inline_extent_backref(), we have several
>> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
>> filesystem.
>>
>> [ANAYLYSE]
>> Most of those BUG_ON()s and ASSERT()s are just a way of handling
>> unexpected on-disk data.
>>
>> Although we have tree-checker to rule out obviously incorrect extent
>> tree blocks, it's not enough for those ones.
>>
>> Thus we need proper error handling for them.
>>
>> [FIX]
>> Thankfully all the callers of update_inline_extent_backref() would
>> eventually handle the errror by aborting the current transaction.
>>
>> So this patch would do the proper error handling by:
>>
>> - Make update_inline_extent_backref() to return int
>>    The return value would be either 0 or -EUCLEAN.
>>
>> - Replace BUG_ON()s and ASSERT()s with proper error handling
>>    This includes:
>>    * Dump the bad extent tree leaf
>>    * Output an error message for the cause
>>      This would include the extent bytenr, num_bytes (if needed),
>>      the bad values and expected good values.
>>    * Return -EUCLEAN
>>
>>    Note here we remove all the WARN_ON()s, as eventually the transaction
>>    would be aborted, thus a backtrace would be triggered anyway.
>>
>> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
>>    tree blocks
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/extent-tree.c | 80 ++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 65 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 3cae798499e2..45e325523e81 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -381,11 +381,11 @@ int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
>>   		}
>>   	}
>>
>> +	WARN_ON(1);
>>   	btrfs_print_leaf(eb);
>>   	btrfs_err(eb->fs_info,
>>   		  "eb %llu iref 0x%lx invalid extent inline ref type %d",
>>   		  eb->start, (unsigned long)iref, type);
>> -	WARN_ON(1);
>
> Do we even want to print the warning here? There's the whole leaf, error
> message, why would we need the stack trace?

Following the principle I mentioned in another thread, you're right, we
don't need the warning as long as we move the transaction abort closer
to the error site.

Otherwise we have two possible sites calling this function, thus harder
to locate the real call chain.

Thanks,
Qu

Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[David Sterba]

On Thu, Aug 10, 2023 at 09:13:43AM +0800, Qu Wenruo wrote:
> On 2023/8/10 02:59, David Sterba wrote:
> > On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
> >> [PROBLEM]
> >> Inside function update_inline_extent_backref(), we have several
> >> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
> >> filesystem.
> >>
> >> [ANAYLYSE]
> >> Most of those BUG_ON()s and ASSERT()s are just a way of handling
> >> unexpected on-disk data.
> >>
> >> Although we have tree-checker to rule out obviously incorrect extent
> >> tree blocks, it's not enough for those ones.
> >>
> >> Thus we need proper error handling for them.
> >>
> >> [FIX]
> >> Thankfully all the callers of update_inline_extent_backref() would
> >> eventually handle the errror by aborting the current transaction.
> >>
> >> So this patch would do the proper error handling by:
> >>
> >> - Make update_inline_extent_backref() to return int
> >>    The return value would be either 0 or -EUCLEAN.
> >>
> >> - Replace BUG_ON()s and ASSERT()s with proper error handling
> >>    This includes:
> >>    * Dump the bad extent tree leaf
> >>    * Output an error message for the cause
> >>      This would include the extent bytenr, num_bytes (if needed),
> >>      the bad values and expected good values.
> >>    * Return -EUCLEAN
> >>
> >>    Note here we remove all the WARN_ON()s, as eventually the transaction
> >>    would be aborted, thus a backtrace would be triggered anyway.
> >>
> >> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
> >>    tree blocks
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >> ---
> >>   fs/btrfs/extent-tree.c | 80 ++++++++++++++++++++++++++++++++++--------
> >>   1 file changed, 65 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> >> index 3cae798499e2..45e325523e81 100644
> >> --- a/fs/btrfs/extent-tree.c
> >> +++ b/fs/btrfs/extent-tree.c
> >> @@ -381,11 +381,11 @@ int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
> >>   		}
> >>   	}
> >>
> >> +	WARN_ON(1);
> >>   	btrfs_print_leaf(eb);
> >>   	btrfs_err(eb->fs_info,
> >>   		  "eb %llu iref 0x%lx invalid extent inline ref type %d",
> >>   		  eb->start, (unsigned long)iref, type);
> >> -	WARN_ON(1);
> >
> > Do we even want to print the warning here? There's the whole leaf, error
> > message, why would we need the stack trace?
> 
> Following the principle I mentioned in another thread, you're right, we
> don't need the warning as long as we move the transaction abort closer
> to the error site.
> 
> Otherwise we have two possible sites calling this function, thus harder
> to locate the real call chain.

So I've kept the warning for now, removing it deserves the reasoning and
review of the other call sites.