Re: [PATCH] btrfs: disk-io: Kill the BUG_ON()s in write_and_map_eb()

[Su Yue <Damenly_Su@gmx.com>]

On Mon 02 Mar 2020 at 09:41, Qu Wenruo <wqu@suse.com> wrote:

> All callers of write_and_map_eb(), except btrfs-corrupt-block,
> have handled error, but inside write_and_map_eb() itself, the
> only error handling is BUG_ON().
>
> This patch will kill all the BUG_ON()s inside
> write_and_map_eb(), and enhance the the caller in
> btrfs-corrupt-block() to handle the error.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com> ---
>  btrfs-corrupt-block.c       |  9 ++++++++-
>  cmds/rescue-super-recover.c |  3 ++-

So may the subject be prefixed with "btrfs-progs"?

>  disk-io.c                   | 26 +++++++++++++++++++++++--- 3
>  files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index
> 95df871a7822..3c236e146176 100644 --- a/btrfs-corrupt-block.c
> +++ b/btrfs-corrupt-block.c @@ -771,8 +771,15 @@ static int
> corrupt_metadata_block(struct btrfs_fs_info *fs_info, u64 block,
>  		u64 bogus = generate_u64(orig);
>  btrfs_set_header_generation(eb, bogus);
> -		write_and_map_eb(fs_info, eb); +		ret =
> write_and_map_eb(fs_info, eb);
>  		free_extent_buffer(eb);
> +		if (ret < 0) { +			errno = -ret; +
> fprintf(stderr, +				"failed to write extent buffer at
> %llu: %m", +				eb->start); +			return ret; +
> }
>  		break; } case BTRFS_METADATA_BLOCK_SHIFT_ITEMS:
> diff --git a/cmds/rescue-super-recover.c
> b/cmds/rescue-super-recover.c index 5d6bea836c8b..62f4f7754720
> 100644 --- a/cmds/rescue-super-recover.c +++
> b/cmds/rescue-super-recover.c @@ -276,7 +276,8 @@ int
> btrfs_recover_superblocks(const char *dname,
>  				  struct super_block_record, list);  root =
>  open_ctree(record->device_name, record->bytenr,
> -			  OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES); +
> OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES | +
> OPEN_CTREE_EXCLUSIVE);

Any explanation about this change?

Thanks

>  	if (!root) {
>  		ret = 3;
>  		goto no_recover;
> diff --git a/disk-io.c b/disk-io.c
> index e8a2e4afa93a..9ff62fcd54d1 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -487,20 +487,40 @@ int write_and_map_eb(struct btrfs_fs_info *fs_info, struct extent_buffer *eb)
>  	length = eb->len;
>  	ret = btrfs_map_block(fs_info, WRITE, eb->start, &length,
>  			      &multi, 0, &raid_map);
> +	if (ret < 0) {
> +		errno = -ret;
> +		error("failed to map bytenr %llu length %u: %m",
> +			eb->start, eb->len);
> +		goto out;
> +	}
>
>  	if (raid_map) {
>  		ret = write_raid56_with_parity(fs_info, eb, multi,
>  					       length, raid_map);
> -		BUG_ON(ret);
> +		if (ret < 0) {
> +			errno = -ret;
> +			error(
> +		"failed to write raid56 stripe for bytenr %llu length %llu: %m",
> +				eb->start, length);
> +			goto out;
> +		}
>  	} else while (dev_nr < multi->num_stripes) {
> -		BUG_ON(ret);
>  		eb->fd = multi->stripes[dev_nr].dev->fd;
>  		eb->dev_bytenr = multi->stripes[dev_nr].physical;
>  		multi->stripes[dev_nr].dev->total_ios++;
>  		dev_nr++;
>  		ret = write_extent_to_disk(eb);
> -		BUG_ON(ret);
> +		if (ret < 0) {
> +			errno = -ret;
> +			error(
> +"failed to write bytenr %llu length %u devid %llu dev_bytenr %llu: %m",
> +				eb->start, eb->len,
> +				multi->stripes[dev_nr].dev->devid,
> +				eb->dev_bytenr);
> +			goto out;
> +		}
>  	}
> +out:
>  	kfree(raid_map);
>  	kfree(multi);
>  	return 0;