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

[Nikolay Borisov <nborisov@suse.com>]

On 25.09.2017 16:07, Nikolay Borisov wrote:
> 
> 
> 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;

Ok, turned out I hadn't received 1/3 so this answers this question. The
sysfs rm stands though


> 
>>