diff mbox

[10/13] btrfs: introduce helper functions to perform hot replace

Message ID 1459560651-14809-11-git-send-email-anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anand Jain April 2, 2016, 1:30 a.m. UTC
Hot replace / auto replace is important volume manager feature
and is critical to the data center operations, so that the degraded
volume can be brought back to a healthy state at the earliest and
without manual intervention.

This modifies the existing replace code to suite the need of auto
replace, in the long run I hope both the codes to be merged.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
---
 fs/btrfs/dev-replace.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/dev-replace.h |  1 +
 2 files changed, 44 insertions(+)

Comments

kernel test robot April 2, 2016, 5:40 a.m. UTC | #1
Hi Anand,

[auto build test ERROR on btrfs/next]
[also build test ERROR on v4.6-rc1 next-20160401]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Anand-Jain/Introduce-device-state-failed-Hot-spare-and-Auto-replace/20160402-093528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git next
config: x86_64-rhel (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/btrfs/dev-replace.c: In function 'btrfs_auto_replace_start':
   fs/btrfs/dev-replace.c:981:38: warning: passing argument 2 of 'btrfs_dev_replace_start' from incompatible pointer type
     ret = btrfs_dev_replace_start(root, tgt_path,
                                         ^
   fs/btrfs/dev-replace.c:308:5: note: expected 'struct btrfs_ioctl_dev_replace_args *' but argument is of type 'char *'
    int btrfs_dev_replace_start(struct btrfs_root *root,
        ^
>> fs/btrfs/dev-replace.c:981:8: error: too many arguments to function 'btrfs_dev_replace_start'
     ret = btrfs_dev_replace_start(root, tgt_path,
           ^
   fs/btrfs/dev-replace.c:308:5: note: declared here
    int btrfs_dev_replace_start(struct btrfs_root *root,
        ^

vim +/btrfs_dev_replace_start +981 fs/btrfs/dev-replace.c

   975		src_path = kstrdup(rcu_str_deref(src_device->name), GFP_ATOMIC);
   976		rcu_read_unlock();
   977		if (!src_path) {
   978			kfree(tgt_path);
   979			return -ENOMEM;
   980		}
 > 981		ret = btrfs_dev_replace_start(root, tgt_path,
   982						src_device->devid, src_path,
   983			BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID);
   984		if (ret)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Yauhen Kharuzhy April 7, 2016, 8 p.m. UTC | #2
On Sat, Apr 02, 2016 at 09:30:48AM +0800, Anand Jain wrote:
> Hot replace / auto replace is important volume manager feature
> and is critical to the data center operations, so that the degraded
> volume can be brought back to a healthy state at the earliest and
> without manual intervention.
> 
> This modifies the existing replace code to suite the need of auto
> replace, in the long run I hope both the codes to be merged.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
> ---
>  fs/btrfs/dev-replace.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/dev-replace.h |  1 +
>  2 files changed, 44 insertions(+)
> 
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 2b926867d136..ceab4c51db32 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -957,3 +957,46 @@ void btrfs_bio_counter_inc_blocked(struct btrfs_fs_info *fs_info)
>  				     &fs_info->fs_state));
>  	}
>  }
> +
> +int btrfs_auto_replace_start(struct btrfs_root *root,
> +				struct btrfs_device *src_device)
> +{
> +	int ret;
> +	char *tgt_path;
> +	char *src_path;
> +	struct btrfs_fs_info *fs_info = root->fs_info;
> +
> +	if (fs_info->sb->s_flags & MS_RDONLY)
> +		return -EROFS;
> +
> +	btrfs_dev_replace_lock(&fs_info->dev_replace, 0);
> +	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
> +		btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
> +		return -EBUSY;
> +	}
> +	btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
> +
> +	if (btrfs_get_spare_device(&tgt_path)) {
> +		btrfs_err(root->fs_info,
> +			"No spare device found/configured in the kernel");
> +		return -EINVAL;
> +	}
> +
> +	rcu_read_lock();
> +	src_path = kstrdup(rcu_str_deref(src_device->name), GFP_ATOMIC);
> +	rcu_read_unlock();
> +	if (!src_path) {

btrfs_put_spare_device(tgt_path) is needed here to put spare device
back in list.

> +		kfree(tgt_path);
> +		return -ENOMEM;
> +	}
> +	ret = btrfs_dev_replace_start(root, tgt_path,
> +					src_device->devid, src_path,
> +		BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID);
> +	if (ret)
> +		btrfs_put_spare_device(tgt_path);
> +
> +	kfree(tgt_path);
> +	kfree(src_path);
> +
> +	return 0;
> +}
> diff --git a/fs/btrfs/dev-replace.h b/fs/btrfs/dev-replace.h
> index e922b42d91df..b918b9d6e5df 100644
> --- a/fs/btrfs/dev-replace.h
> +++ b/fs/btrfs/dev-replace.h
> @@ -46,4 +46,5 @@ static inline void btrfs_dev_replace_stats_inc(atomic64_t *stat_value)
>  {
>  	atomic64_inc(stat_value);
>  }
> +int btrfs_auto_replace_start(struct btrfs_root *root, struct btrfs_device *src_device);
>  #endif
> -- 
> 2.7.0
Anand Jain April 8, 2016, 3:58 a.m. UTC | #3
On 04/08/2016 04:00 AM, Yauhen Kharuzhy wrote:
> On Sat, Apr 02, 2016 at 09:30:48AM +0800, Anand Jain wrote:
>> Hot replace / auto replace is important volume manager feature
>> and is critical to the data center operations, so that the degraded
>> volume can be brought back to a healthy state at the earliest and
>> without manual intervention.
>>
>> This modifies the existing replace code to suite the need of auto
>> replace, in the long run I hope both the codes to be merged.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>> ---
>>   fs/btrfs/dev-replace.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   fs/btrfs/dev-replace.h |  1 +
>>   2 files changed, 44 insertions(+)
>>
>> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
>> index 2b926867d136..ceab4c51db32 100644
>> --- a/fs/btrfs/dev-replace.c
>> +++ b/fs/btrfs/dev-replace.c
>> @@ -957,3 +957,46 @@ void btrfs_bio_counter_inc_blocked(struct btrfs_fs_info *fs_info)
>>   				     &fs_info->fs_state));
>>   	}
>>   }
>> +
>> +int btrfs_auto_replace_start(struct btrfs_root *root,
>> +				struct btrfs_device *src_device)
>> +{
>> +	int ret;
>> +	char *tgt_path;
>> +	char *src_path;
>> +	struct btrfs_fs_info *fs_info = root->fs_info;
>> +
>> +	if (fs_info->sb->s_flags & MS_RDONLY)
>> +		return -EROFS;
>> +
>> +	btrfs_dev_replace_lock(&fs_info->dev_replace, 0);
>> +	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
>> +		btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
>> +		return -EBUSY;
>> +	}
>> +	btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
>> +
>> +	if (btrfs_get_spare_device(&tgt_path)) {
>> +		btrfs_err(root->fs_info,
>> +			"No spare device found/configured in the kernel");
>> +		return -EINVAL;
>> +	}
>> +
>> +	rcu_read_lock();
>> +	src_path = kstrdup(rcu_str_deref(src_device->name), GFP_ATOMIC);
>> +	rcu_read_unlock();
>> +	if (!src_path) {
>
> btrfs_put_spare_device(tgt_path) is needed here to put spare device
> back in list.

  Right. Got that changed locally. Thanks.

Anand

>> +		kfree(tgt_path);
>> +		return -ENOMEM;
>> +	}
>> +	ret = btrfs_dev_replace_start(root, tgt_path,
>> +					src_device->devid, src_path,
>> +		BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID);
>> +	if (ret)
>> +		btrfs_put_spare_device(tgt_path);
>> +
>> +	kfree(tgt_path);
>> +	kfree(src_path);
>> +
>> +	return 0;
>> +}
>> diff --git a/fs/btrfs/dev-replace.h b/fs/btrfs/dev-replace.h
>> index e922b42d91df..b918b9d6e5df 100644
>> --- a/fs/btrfs/dev-replace.h
>> +++ b/fs/btrfs/dev-replace.h
>> @@ -46,4 +46,5 @@ static inline void btrfs_dev_replace_stats_inc(atomic64_t *stat_value)
>>   {
>>   	atomic64_inc(stat_value);
>>   }
>> +int btrfs_auto_replace_start(struct btrfs_root *root, struct btrfs_device *src_device);
>>   #endif
>> --
>> 2.7.0
>
--
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
Yauhen Kharuzhy April 8, 2016, 10:05 p.m. UTC | #4
On Sat, Apr 02, 2016 at 09:30:48AM +0800, Anand Jain wrote:
> Hot replace / auto replace is important volume manager feature
> and is critical to the data center operations, so that the degraded
> volume can be brought back to a healthy state at the earliest and
> without manual intervention.
> 
> This modifies the existing replace code to suite the need of auto
> replace, in the long run I hope both the codes to be merged.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
> ---
>  fs/btrfs/dev-replace.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/dev-replace.h |  1 +
>  2 files changed, 44 insertions(+)
> 
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 2b926867d136..ceab4c51db32 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -957,3 +957,46 @@ void btrfs_bio_counter_inc_blocked(struct btrfs_fs_info *fs_info)
>  				     &fs_info->fs_state));
>  	}
>  }
> +
> +int btrfs_auto_replace_start(struct btrfs_root *root,
> +				struct btrfs_device *src_device)
> +{
> +	int ret;
> +	char *tgt_path;
> +	char *src_path;
> +	struct btrfs_fs_info *fs_info = root->fs_info;
> +
> +	if (fs_info->sb->s_flags & MS_RDONLY)
> +		return -EROFS;
> +
> +	btrfs_dev_replace_lock(&fs_info->dev_replace, 0);
> +	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
> +		btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
> +		return -EBUSY;
> +	}
> +	btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
> +
> +	if (btrfs_get_spare_device(&tgt_path)) {
> +		btrfs_err(root->fs_info,
> +			"No spare device found/configured in the kernel");
> +		return -EINVAL;
> +	}
> +
> +	rcu_read_lock();
> +	src_path = kstrdup(rcu_str_deref(src_device->name), GFP_ATOMIC);
> +	rcu_read_unlock();
> +	if (!src_path) {
> +		kfree(tgt_path);
> +		return -ENOMEM;
> +	}
> +	ret = btrfs_dev_replace_start(root, tgt_path,
> +					src_device->devid, src_path,
> +		BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID);
> +	if (ret)
> +		btrfs_put_spare_device(tgt_path);
> +
> +	kfree(tgt_path);
> +	kfree(src_path);
> +
> +	return 0;
> +}

Without of fs_info->mutually_exclusive_operation_running flag set in
btrfs_auto_replace_start(), device add/remove/balance etc. can be
started in parralel with autoreplace. Should this scenarios be permitted?

> diff --git a/fs/btrfs/dev-replace.h b/fs/btrfs/dev-replace.h
> index e922b42d91df..b918b9d6e5df 100644
> --- a/fs/btrfs/dev-replace.h
> +++ b/fs/btrfs/dev-replace.h
> @@ -46,4 +46,5 @@ static inline void btrfs_dev_replace_stats_inc(atomic64_t *stat_value)
>  {
>  	atomic64_inc(stat_value);
>  }
> +int btrfs_auto_replace_start(struct btrfs_root *root, struct btrfs_device *src_device);
>  #endif
> -- 
> 2.7.0
Anand Jain April 12, 2016, 2:16 p.m. UTC | #5
On 04/09/2016 06:05 AM, Yauhen Kharuzhy wrote:
> On Sat, Apr 02, 2016 at 09:30:48AM +0800, Anand Jain wrote:
>> Hot replace / auto replace is important volume manager feature
>> and is critical to the data center operations, so that the degraded
>> volume can be brought back to a healthy state at the earliest and
>> without manual intervention.
>>
>> This modifies the existing replace code to suite the need of auto
>> replace, in the long run I hope both the codes to be merged.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>> ---
>>   fs/btrfs/dev-replace.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   fs/btrfs/dev-replace.h |  1 +
>>   2 files changed, 44 insertions(+)
>>
>> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
>> index 2b926867d136..ceab4c51db32 100644
>> --- a/fs/btrfs/dev-replace.c
>> +++ b/fs/btrfs/dev-replace.c
>> @@ -957,3 +957,46 @@ void btrfs_bio_counter_inc_blocked(struct btrfs_fs_info *fs_info)
>>   				     &fs_info->fs_state));
>>   	}
>>   }
>> +
>> +int btrfs_auto_replace_start(struct btrfs_root *root,
>> +				struct btrfs_device *src_device)
>> +{
>> +	int ret;
>> +	char *tgt_path;
>> +	char *src_path;
>> +	struct btrfs_fs_info *fs_info = root->fs_info;
>> +
>> +	if (fs_info->sb->s_flags & MS_RDONLY)
>> +		return -EROFS;
>> +
>> +	btrfs_dev_replace_lock(&fs_info->dev_replace, 0);
>> +	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
>> +		btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
>> +		return -EBUSY;
>> +	}
>> +	btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
>> +
>> +	if (btrfs_get_spare_device(&tgt_path)) {
>> +		btrfs_err(root->fs_info,
>> +			"No spare device found/configured in the kernel");
>> +		return -EINVAL;
>> +	}
>> +
>> +	rcu_read_lock();
>> +	src_path = kstrdup(rcu_str_deref(src_device->name), GFP_ATOMIC);
>> +	rcu_read_unlock();
>> +	if (!src_path) {
>> +		kfree(tgt_path);
>> +		return -ENOMEM;
>> +	}
>> +	ret = btrfs_dev_replace_start(root, tgt_path,
>> +					src_device->devid, src_path,
>> +		BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID);
>> +	if (ret)
>> +		btrfs_put_spare_device(tgt_path);
>> +
>> +	kfree(tgt_path);
>> +	kfree(src_path);
>> +
>> +	return 0;
>> +}
>
> Without of fs_info->mutually_exclusive_operation_running flag set in
> btrfs_auto_replace_start(), device add/remove/balance etc. can be
> started in parralel with autoreplace. Should this scenarios be permitted?

  Its needs it in case of if device delete/add etc is running, added
  in v4.  Thanks.


>> diff --git a/fs/btrfs/dev-replace.h b/fs/btrfs/dev-replace.h
>> index e922b42d91df..b918b9d6e5df 100644
>> --- a/fs/btrfs/dev-replace.h
>> +++ b/fs/btrfs/dev-replace.h
>> @@ -46,4 +46,5 @@ static inline void btrfs_dev_replace_stats_inc(atomic64_t *stat_value)
>>   {
>>   	atomic64_inc(stat_value);
>>   }
>> +int btrfs_auto_replace_start(struct btrfs_root *root, struct btrfs_device *src_device);
>>   #endif
>> --
>> 2.7.0
>
--
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
diff mbox

Patch

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 2b926867d136..ceab4c51db32 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -957,3 +957,46 @@  void btrfs_bio_counter_inc_blocked(struct btrfs_fs_info *fs_info)
 				     &fs_info->fs_state));
 	}
 }
+
+int btrfs_auto_replace_start(struct btrfs_root *root,
+				struct btrfs_device *src_device)
+{
+	int ret;
+	char *tgt_path;
+	char *src_path;
+	struct btrfs_fs_info *fs_info = root->fs_info;
+
+	if (fs_info->sb->s_flags & MS_RDONLY)
+		return -EROFS;
+
+	btrfs_dev_replace_lock(&fs_info->dev_replace, 0);
+	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
+		btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
+		return -EBUSY;
+	}
+	btrfs_dev_replace_unlock(&fs_info->dev_replace, 0);
+
+	if (btrfs_get_spare_device(&tgt_path)) {
+		btrfs_err(root->fs_info,
+			"No spare device found/configured in the kernel");
+		return -EINVAL;
+	}
+
+	rcu_read_lock();
+	src_path = kstrdup(rcu_str_deref(src_device->name), GFP_ATOMIC);
+	rcu_read_unlock();
+	if (!src_path) {
+		kfree(tgt_path);
+		return -ENOMEM;
+	}
+	ret = btrfs_dev_replace_start(root, tgt_path,
+					src_device->devid, src_path,
+		BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID);
+	if (ret)
+		btrfs_put_spare_device(tgt_path);
+
+	kfree(tgt_path);
+	kfree(src_path);
+
+	return 0;
+}
diff --git a/fs/btrfs/dev-replace.h b/fs/btrfs/dev-replace.h
index e922b42d91df..b918b9d6e5df 100644
--- a/fs/btrfs/dev-replace.h
+++ b/fs/btrfs/dev-replace.h
@@ -46,4 +46,5 @@  static inline void btrfs_dev_replace_stats_inc(atomic64_t *stat_value)
 {
 	atomic64_inc(stat_value);
 }
+int btrfs_auto_replace_start(struct btrfs_root *root, struct btrfs_device *src_device);
 #endif