Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device()

[Nikolay Borisov <nborisov@suse.com>]

On 25.09.2017 15:31, Anand Jain wrote:
> Moves btrfs_abort_transaction() to the error goto and renames
> error_trans to error_sysfs. This is a preparatory patch to
> remove the BUG_ON() in btrfs_init_new_device().
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/volumes.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 9d64700cc9b6..5c34eea9d12d 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2443,26 +2443,20 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>  		mutex_lock(&fs_info->chunk_mutex);
>  		ret = init_first_rw_device(trans, fs_info);
>  		mutex_unlock(&fs_info->chunk_mutex);
> -		if (ret) {
> -			btrfs_abort_transaction(trans, ret);
> -			goto error_trans;
> -		}
> +		if (ret)
> +			goto error_sysfs;
>  	}
>  
>  	ret = btrfs_add_device(trans, fs_info, device);
> -	if (ret) {
> -		btrfs_abort_transaction(trans, ret);
> -		goto error_trans;
> -	}
> +	if (ret)
> +		goto error_sysfs;
>  
>  	if (seeding_dev) {
>  		char fsid_buf[BTRFS_UUID_UNPARSED_SIZE];
>  
>  		ret = btrfs_finish_sprout(trans, fs_info);
> -		if (ret) {
> -			btrfs_abort_transaction(trans, ret);
> -			goto error_trans;
> -		}
> +		if (ret)
> +			goto error_sysfs;
>  
>  		/* Sprouting would change fsid of the mounted root,
>  		 * so rename the fsid on the sysfs
> @@ -2500,12 +2494,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>  	update_dev_time(device_path);
>  	return ret;
>  
> -error_trans:
> +error_sysfs:
> +	btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);

Any reason why you move btrfs_sysfs_rm_device_link?

>  	if (seeding_dev)
>  		sb->s_flags |= MS_RDONL> +	btrfs_abort_transaction(trans, ret);
>  	btrfs_end_transaction(trans);
>  	rcu_string_free(device->name);
> -	btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
>  	kfree(device);>  error:
>  	blkdev_put(bdev, FMODE_EXCL);

Also which branch is your code based on since in Linus' master the error
handling (before your patch) looks a bit different. E.g. the MS_RDONL
stuff look wrong.

error_trans:

        btrfs_end_transaction(trans);

        rcu_string_free(device->name);

        btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);

        kfree(device);

error:

        blkdev_put(bdev, FMODE_EXCL);

        if (seeding_dev) {

                mutex_unlock(&uuid_mutex);

                up_write(&sb->s_umount);

        }

        return ret;

>