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

David Sterba
2020-08-13T14:11:25Z

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;
> }