Re: [PATCH v4 2/4] btrfs: extent-tree: Kill BUG_ON() in __btrfs_free_extent() and do better comment

[David Sterba <dsterba@suse.cz>]

On Wed, Aug 12, 2020 at 02:05:07PM +0800, Qu Wenruo wrote:
> @@ -2987,13 +3049,20 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
>  				found_extent = 1;
>  				break;
>  			}
> +
> +			/* Quick path didn't find the EXTEMT/METADATA_ITEM */
>  			if (path->slots[0] - extent_slot > 5)
>  				break;
>  			extent_slot--;
>  		}
>  
>  		if (!found_extent) {
> -			BUG_ON(iref);
> +			if (unlikely(iref)) {
> +				btrfs_crit(info,
> +"invalid iref, no EXTENT/METADATA_ITEM found but has inline extent ref");
> +				goto err_dump_abort;

This is not calling transaction abort at the place where it happens,
here and several other places too.

> @@ -3164,6 +3267,21 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
>  out:
>  	btrfs_free_path(path);
>  	return ret;
> +err_dump_abort:
> +	/*
> +	 * Leaf dump can take up a lot of dmesg buffer since default nodesize
> +	 * is already 16K.
> +	 * So we only do full leaf dump for debug build.
> +	 */
> +	if (IS_ENABLED(CONFIG_BTRFS_DEBUG)) {
> +		btrfs_crit(info, "path->slots[0]=%d extent_slot=%d",
> +			   path->slots[0], extent_slot);
> +		btrfs_print_leaf(path->nodes[0]);

Preceded by a potentially long-running action, so this will delay the
acutal abort.

> +	}
> +
> +	btrfs_abort_transaction(trans, -EUCLEAN);
> +	btrfs_free_path(path);
> +	return -EUCLEAN;
>  }