[PATCH 0/3] fix bug in btrfs_init_new_device()

[PATCH 0/3] fix bug in btrfs_init_new_device()

[Anand Jain]

1/3 fixes a bug which failed to reset writable when sprouting failed
2/3 is cleanup and preparatory to fix 3/3
3/3 fixes BUG_ON in btrfs_init_new_device()

Anand Jain (3):
  btrfs: undo writable when sprouting fails
  btrfs: cleanup btrfs_init_new_device()
  btrfs: fix BUG_ON in btrfs_init_new_device()

 fs/btrfs/volumes.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

-- 
2.13.1

[PATCH 1/3] btrfs: undo writable when sprouting fails

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0e8f16c305df..9d64700cc9b6 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2501,6 +2501,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	return ret;
 
 error_trans:
+	if (seeding_dev)
+		sb->s_flags |= MS_RDONLY;
 	btrfs_end_transaction(trans);
 	rcu_string_free(device->name);
 	btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
-- 
2.13.1

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

[Anand Jain]

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);
 	if (seeding_dev)
 		sb->s_flags |= MS_RDONLY;
+	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);
-- 
2.13.1

[PATCH 3/3] btrfs: fix BUG_ON in btrfs_init_new_device()

[Anand Jain]

Here we will simply return the error to the caller. And handle the
fail condition by calling the abort transaction through the error
path.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5c34eea9d12d..93eaf314fe72 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2399,7 +2399,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	if (seeding_dev) {
 		sb->s_flags &= ~MS_RDONLY;
 		ret = btrfs_prepare_sprout(fs_info);
-		BUG_ON(ret); /* -ENOMEM */
+		if (ret)
+			goto error_trans;
 	}
 
 	device->fs_devices = fs_info->fs_devices;
@@ -2496,6 +2497,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 
 error_sysfs:
 	btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
+error_trans:
 	if (seeding_dev)
 		sb->s_flags |= MS_RDONLY;
 	btrfs_abort_transaction(trans, ret);
-- 
2.13.1

Re: [PATCH 1/3] btrfs: undo writable when sprouting fails

[Qu Wenruo]

On 2017年09月25日 20:31, Anand Jain wrote:
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

The error_trans tag needs to revert the RDONLY flag without doubt.


But just a small question.

Just lines before error_trans tag, we're committing transaction and then 
relocating system chunks.
In which we don't use goto error branch but returning directly.

Don't we need to to revert the RDONLY flag in that error handler?

Thanks,
Qu

> ---
>   fs/btrfs/volumes.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 0e8f16c305df..9d64700cc9b6 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2501,6 +2501,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>   	return ret;
>   
>   error_trans:
> +	if (seeding_dev)
> +		sb->s_flags |= MS_RDONLY;
>   	btrfs_end_transaction(trans);
>   	rcu_string_free(device->name);
>   	btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
> 

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

[Nikolay Borisov]

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;

> 

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

[Qu Wenruo]

On 2017年09月25日 20: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);
>   	if (seeding_dev)
>   		sb->s_flags |= MS_RDONLY;
> +	btrfs_abort_transaction(trans, ret);
>   	btrfs_end_transaction(trans);

Abort then end? Looks redundant at least.

>   	rcu_string_free(device->name);
> -	btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);

Any particular reason to move this line ahead?

I didn't see it has something to do with transaction, so just curious 
about the purpose.

Thanks,
Qu
>   	kfree(device);
>   error:
>   	blkdev_put(bdev, FMODE_EXCL);
> 

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

[Nikolay Borisov]

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


> 
>>

Re: [PATCH 1/3] btrfs: undo writable when sprouting fails

[David Sterba]

On Mon, Sep 25, 2017 at 08:31:02PM +0800, Anand Jain wrote:
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

This does not look like change trivial enough to just leave out the
description. Please explain in more detail.

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

[David Sterba]

On Mon, Sep 25, 2017 at 08:31:03PM +0800, 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().

Please keep all transaction abort exactly at the place where they
happened and do not merge them to one. This pattern should be used
everwhere and is important when debugging because we can pinpoint the
line in the code from the syslog message and do not have to guess which
way it got to the merged call.

This makes the binary large, looks like a code duplication in the code
but for a good reason.

[PATCH v2 0/3] fix bug in btrfs_init_new_device()

[Anand Jain]

1/3 fixes a bug which failed to reset writable when sprouting failed
2/3 is cleanup and preparatory to fix 3/3
3/3 fixes BUG_ON in btrfs_init_new_device()

v2:
 add commit log to 1/3
 do not consolidate btrfs_abort_transaction() in 2/3

Anand Jain (3):
  btrfs: undo writable when sprouting fails
  btrfs: cleanup btrfs_init_new_device()
  btrfs: fix BUG_ON in btrfs_init_new_device()

 fs/btrfs/volumes.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

-- 
2.13.1

[PATCH v2 1/3] btrfs: undo writable when sprouting fails

[Anand Jain]

When new device is being added to seed FS, seed FS is marked writable,
but when we fail to bring in the new device, we missed to undo the
writable part. This patch fixes it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: add commit log
 fs/btrfs/volumes.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0e8f16c305df..9d64700cc9b6 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2501,6 +2501,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	return ret;
 
 error_trans:
+	if (seeding_dev)
+		sb->s_flags |= MS_RDONLY;
 	btrfs_end_transaction(trans);
 	rcu_string_free(device->name);
 	btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
-- 
2.13.1

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

[Anand Jain]

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 | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9d64700cc9b6..22eb81794375 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2445,14 +2445,14 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 		mutex_unlock(&fs_info->chunk_mutex);
 		if (ret) {
 			btrfs_abort_transaction(trans, ret);
-			goto error_trans;
+			goto error_sysfs;
 		}
 	}
 
 	ret = btrfs_add_device(trans, fs_info, device);
 	if (ret) {
 		btrfs_abort_transaction(trans, ret);
-		goto error_trans;
+		goto error_sysfs;
 	}
 
 	if (seeding_dev) {
@@ -2461,7 +2461,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 		ret = btrfs_finish_sprout(trans, fs_info);
 		if (ret) {
 			btrfs_abort_transaction(trans, ret);
-			goto error_trans;
+			goto error_sysfs;
 		}
 
 		/* Sprouting would change fsid of the mounted root,
@@ -2500,12 +2500,12 @@ 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);
 	if (seeding_dev)
 		sb->s_flags |= MS_RDONLY;
 	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);
-- 
2.13.1

[PATCH v2 3/3] btrfs: fix BUG_ON in btrfs_init_new_device()

[Anand Jain]

Instead of BUG_ON return error to the caller. And handle the fail
condition by calling the abort transaction through the error path.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: do not consolidate btrfs_abort_transaction()
 fs/btrfs/volumes.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 22eb81794375..4cb575fbf643 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2399,7 +2399,10 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	if (seeding_dev) {
 		sb->s_flags &= ~MS_RDONLY;
 		ret = btrfs_prepare_sprout(fs_info);
-		BUG_ON(ret); /* -ENOMEM */
+		if (ret) {
+			btrfs_abort_transaction(trans, ret);
+			goto error_trans;
+		}
 	}
 
 	device->fs_devices = fs_info->fs_devices;
@@ -2502,6 +2505,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 
 error_sysfs:
 	btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
+error_trans:
 	if (seeding_dev)
 		sb->s_flags |= MS_RDONLY;
 	btrfs_end_transaction(trans);
-- 
2.13.1

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

[Anand Jain]

> The
> sysfs rm stands though

  This is a preparatory patch for 3/3, at BUG_ON it needs an exit 
without  the btrfs_sysfs_add_device_link() part.

-Anand

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

[Anand Jain]

On 09/26/2017 03:57 PM, Anand Jain wrote:
> 
>> The
>> sysfs rm stands though
> 
>   This is a preparatory patch for 3/3, at BUG_ON it needs an exit 
> without  the btrfs_sysfs_add_device_link() part.

typo
s/btrfs_sysfs_add_device_link/btrfs_sysfs_rm_device_link/


> -Anand
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Nikolay Borisov]

On 26.09.2017 10:57, Anand Jain wrote:
> 
>> The
>> sysfs rm stands though
> 
>  This is a preparatory patch for 3/3, at BUG_ON it needs an exit
> without  the btrfs_sysfs_add_device_link() part.

In this case I'd rather you have this change in 3/3, because on its own
it doesn't really make sense and cannot easily be figured out if doing
git blame.

> 
> -Anand
> 

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

[Anand Jain]

>>> The
>>> sysfs rm stands though
>>
>>   This is a preparatory patch for 3/3, at BUG_ON it needs an exit
>> without  the btrfs_sysfs_add_device_link() part.
> 
> In this case I'd rather you have this change in 3/3, because on its own
> it doesn't really make sense and cannot easily be figured out if doing
> git blame.

  Fixed in v3. Thanks for suggesting.

-Anand