* [PATCH 1/3] btrfs: use fs_info directly
@ 2016-03-24 10:48 Anand Jain
2016-03-24 10:48 ` [PATCH 2/3] btrfs: keep sysfs target add in the last Anand Jain
2016-03-24 10:48 ` [PATCH 3/3] btrfs: refactor btrfs_dev_replace_start for reuse Anand Jain
0 siblings, 2 replies; 6+ messages in thread
From: Anand Jain @ 2016-03-24 10:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
Local variable fs_info, contains root->fs_info, use it.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/dev-replace.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index a1d6652e0c47..d38cad37ba27 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -375,7 +375,7 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
WARN_ON(!tgt_device);
dev_replace->tgtdev = tgt_device;
- btrfs_info_in_rcu(root->fs_info,
+ btrfs_info_in_rcu(fs_info,
"dev_replace from %s (devid %llu) to %s started",
src_device->missing ? "<missing disk>" :
rcu_str_deref(src_device->name),
@@ -399,9 +399,9 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
ret = btrfs_sysfs_add_device_link(tgt_device->fs_devices, tgt_device);
if (ret)
- btrfs_err(root->fs_info, "kobj add dev failed %d\n", ret);
+ btrfs_err(fs_info, "kobj add dev failed %d\n", ret);
- btrfs_wait_ordered_roots(root->fs_info, -1);
+ btrfs_wait_ordered_roots(fs_info, -1);
/* force writing the updated state information to disk */
trans = btrfs_start_transaction(root, 0);
@@ -419,7 +419,7 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
btrfs_device_get_total_bytes(src_device),
&dev_replace->scrub_progress, 0, 1);
- ret = btrfs_dev_replace_finishing(root->fs_info, ret);
+ ret = btrfs_dev_replace_finishing(fs_info, ret);
/* don't warn if EINPROGRESS, someone else might be running scrub */
if (ret == -EINPROGRESS) {
args->result = BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS;
--
2.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] btrfs: keep sysfs target add in the last
2016-03-24 10:48 [PATCH 1/3] btrfs: use fs_info directly Anand Jain
@ 2016-03-24 10:48 ` Anand Jain
2016-04-25 16:23 ` David Sterba
2016-03-24 10:48 ` [PATCH 3/3] btrfs: refactor btrfs_dev_replace_start for reuse Anand Jain
1 sibling, 1 reply; 6+ messages in thread
From: Anand Jain @ 2016-03-24 10:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
Sysfs create context should come in the last, so that we
don't have to undo sysfs operation for the reason that any
other operation has failed.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/dev-replace.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index d38cad37ba27..025f42ef5ab3 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -397,10 +397,6 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
args->result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR;
btrfs_dev_replace_unlock(dev_replace, 1);
- ret = btrfs_sysfs_add_device_link(tgt_device->fs_devices, tgt_device);
- if (ret)
- btrfs_err(fs_info, "kobj add dev failed %d\n", ret);
-
btrfs_wait_ordered_roots(fs_info, -1);
/* force writing the updated state information to disk */
@@ -428,6 +424,9 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
WARN_ON(ret);
}
+ if (btrfs_sysfs_add_device_link(tgt_device->fs_devices, tgt_device))
+ btrfs_err(fs_info, "kobj add dev failed during replace\n");
+
return ret;
leave:
--
2.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] btrfs: refactor btrfs_dev_replace_start for reuse
2016-03-24 10:48 [PATCH 1/3] btrfs: use fs_info directly Anand Jain
2016-03-24 10:48 ` [PATCH 2/3] btrfs: keep sysfs target add in the last Anand Jain
@ 2016-03-24 10:48 ` Anand Jain
2016-04-25 16:32 ` David Sterba
1 sibling, 1 reply; 6+ messages in thread
From: Anand Jain @ 2016-03-24 10:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
A refactor patch, and avoids user input verification in the
btrfs_dev_replace_start(), and so this function can be reused.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/dev-replace.c | 62 ++++++++++++++++++++++++++++++--------------------
fs/btrfs/dev-replace.h | 4 +++-
fs/btrfs/ioctl.c | 2 +-
3 files changed, 41 insertions(+), 27 deletions(-)
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 025f42ef5ab3..2b926867d136 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -305,8 +305,8 @@ void btrfs_after_dev_replace_commit(struct btrfs_fs_info *fs_info)
dev_replace->cursor_left_last_write_of_item;
}
-int btrfs_dev_replace_start(struct btrfs_root *root,
- struct btrfs_ioctl_dev_replace_args *args)
+int btrfs_dev_replace_start(struct btrfs_root *root, char *tgtdev_name,
+ u64 srcdevid, char *srcdev_name, int read_src)
{
struct btrfs_trans_handle *trans;
struct btrfs_fs_info *fs_info = root->fs_info;
@@ -315,29 +315,16 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
struct btrfs_device *tgt_device = NULL;
struct btrfs_device *src_device = NULL;
- switch (args->start.cont_reading_from_srcdev_mode) {
- case BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_ALWAYS:
- case BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID:
- break;
- default:
- return -EINVAL;
- }
-
- if ((args->start.srcdevid == 0 && args->start.srcdev_name[0] == '\0') ||
- args->start.tgtdev_name[0] == '\0')
- return -EINVAL;
-
/* the disk copy procedure reuses the scrub code */
mutex_lock(&fs_info->volume_mutex);
- ret = btrfs_dev_replace_find_srcdev(root, args->start.srcdevid,
- args->start.srcdev_name,
- &src_device);
+ ret = btrfs_dev_replace_find_srcdev(root, srcdevid,
+ srcdev_name, &src_device);
if (ret) {
mutex_unlock(&fs_info->volume_mutex);
return ret;
}
- ret = btrfs_init_dev_replace_tgtdev(root, args->start.tgtdev_name,
+ ret = btrfs_init_dev_replace_tgtdev(root, tgtdev_name,
src_device, &tgt_device);
mutex_unlock(&fs_info->volume_mutex);
if (ret)
@@ -364,12 +351,11 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
break;
case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED:
case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED:
- args->result = BTRFS_IOCTL_DEV_REPLACE_RESULT_ALREADY_STARTED;
+ ret = BTRFS_IOCTL_DEV_REPLACE_RESULT_ALREADY_STARTED;
goto leave;
}
- dev_replace->cont_reading_from_srcdev_mode =
- args->start.cont_reading_from_srcdev_mode;
+ dev_replace->cont_reading_from_srcdev_mode = read_src;
WARN_ON(!src_device);
dev_replace->srcdev = src_device;
WARN_ON(!tgt_device);
@@ -394,7 +380,6 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
dev_replace->cursor_right = 0;
dev_replace->is_valid = 1;
dev_replace->item_needs_writeback = 1;
- args->result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR;
btrfs_dev_replace_unlock(dev_replace, 1);
btrfs_wait_ordered_roots(fs_info, -1);
@@ -416,10 +401,8 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
&dev_replace->scrub_progress, 0, 1);
ret = btrfs_dev_replace_finishing(fs_info, ret);
- /* don't warn if EINPROGRESS, someone else might be running scrub */
if (ret == -EINPROGRESS) {
- args->result = BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS;
- ret = 0;
+ ret = BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS;
} else {
WARN_ON(ret);
}
@@ -437,6 +420,35 @@ leave:
return ret;
}
+int btrfs_dev_replace_by_ioctl(struct btrfs_root *root,
+ struct btrfs_ioctl_dev_replace_args *args)
+{
+ int ret;
+
+ switch (args->start.cont_reading_from_srcdev_mode) {
+ case BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_ALWAYS:
+ case BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if ((args->start.srcdevid == 0 && args->start.srcdev_name[0] == '\0') ||
+ args->start.tgtdev_name[0] == '\0')
+ return -EINVAL;
+
+ ret = btrfs_dev_replace_start(root, args->start.tgtdev_name,
+ args->start.srcdevid,
+ args->start.srcdev_name,
+ args->start.cont_reading_from_srcdev_mode);
+ args->result = ret;
+ /* don't warn if EINPROGRESS, someone else might be running scrub */
+ if (ret == BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS)
+ ret = 0;
+
+ return ret;
+}
+
/*
* blocked until all flighting bios are finished.
*/
diff --git a/fs/btrfs/dev-replace.h b/fs/btrfs/dev-replace.h
index 29e3ef5f96bd..e922b42d91df 100644
--- a/fs/btrfs/dev-replace.h
+++ b/fs/btrfs/dev-replace.h
@@ -25,8 +25,10 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info);
int btrfs_run_dev_replace(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info);
void btrfs_after_dev_replace_commit(struct btrfs_fs_info *fs_info);
-int btrfs_dev_replace_start(struct btrfs_root *root,
+int btrfs_dev_replace_by_ioctl(struct btrfs_root *root,
struct btrfs_ioctl_dev_replace_args *args);
+int btrfs_dev_replace_start(struct btrfs_root *root, char *tgtdev_name,
+ u64 srcdevid, char *srcdev_name, int read_src);
void btrfs_dev_replace_status(struct btrfs_fs_info *fs_info,
struct btrfs_ioctl_dev_replace_args *args);
int btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 053e677839fe..a811089c3890 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4376,7 +4376,7 @@ static long btrfs_ioctl_dev_replace(struct btrfs_root *root, void __user *arg)
1)) {
ret = BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
} else {
- ret = btrfs_dev_replace_start(root, p);
+ ret = btrfs_dev_replace_by_ioctl(root, p);
atomic_set(
&root->fs_info->mutually_exclusive_operation_running,
0);
--
2.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] btrfs: keep sysfs target add in the last
2016-03-24 10:48 ` [PATCH 2/3] btrfs: keep sysfs target add in the last Anand Jain
@ 2016-04-25 16:23 ` David Sterba
2016-05-01 20:57 ` Anand Jain
0 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2016-04-25 16:23 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, dsterba
On Thu, Mar 24, 2016 at 06:48:13PM +0800, Anand Jain wrote:
> Sysfs create context should come in the last, so that we
> don't have to undo sysfs operation for the reason that any
> other operation has failed.
Moving the sysfs call will make a visible change: in the old code, the
sysfs node exists during the whole replace process, while in the new
code it appears only after it finishes. While this is not necessarily a
problem, I'd like to check that his is an intended change, as it's not
mentioned in the changelog.
Besides, the sysfs node seems to be added unconditionally, so if the
scrub is running in parallel (checked a few lines above the new code),
we'll happily add the target device although no replace happened.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] btrfs: refactor btrfs_dev_replace_start for reuse
2016-03-24 10:48 ` [PATCH 3/3] btrfs: refactor btrfs_dev_replace_start for reuse Anand Jain
@ 2016-04-25 16:32 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2016-04-25 16:32 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, dsterba
On Thu, Mar 24, 2016 at 06:48:14PM +0800, Anand Jain wrote:
> A refactor patch, and avoids user input verification in the
> btrfs_dev_replace_start(), and so this function can be reused.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
Added on top of the delete-by-id patchset as there's a dependency, plus
the 1/3 patch "btrfs: use fs_info directly".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] btrfs: keep sysfs target add in the last
2016-04-25 16:23 ` David Sterba
@ 2016-05-01 20:57 ` Anand Jain
0 siblings, 0 replies; 6+ messages in thread
From: Anand Jain @ 2016-05-01 20:57 UTC (permalink / raw)
To: dsterba, linux-btrfs
On 04/26/2016 12:23 AM, David Sterba wrote:
> On Thu, Mar 24, 2016 at 06:48:13PM +0800, Anand Jain wrote:
>> Sysfs create context should come in the last, so that we
>> don't have to undo sysfs operation for the reason that any
>> other operation has failed.
>
> Moving the sysfs call will make a visible change: in the old code, the
> sysfs node exists during the whole replace process, while in the new
> code it appears only after it finishes.
> While this is not necessarily a
> problem, I'd like to check that his is an intended change, as it's not
> mentioned in the changelog.
Ah. I missed this point.
> Besides, the sysfs node seems to be added unconditionally, so if the
> scrub is running in parallel (checked a few lines above the new code),
> we'll happily add the target device although no replace happened.
Replace may take a long time, We definitely need devid 0 shown at
sysfs. Apparently we don't have the undo-sysfs part in the original
code, which this patch tried to mitigate, but it needs a deeper fix.
I have to pull this patch out and leave the original bug as is for now.
Further we need to handle fail return of btrfs_dev_replace_finishing()
which seeks sysfs and rest of the stuff un-done as well.
Thanks, Anand
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-05-01 20:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-24 10:48 [PATCH 1/3] btrfs: use fs_info directly Anand Jain
2016-03-24 10:48 ` [PATCH 2/3] btrfs: keep sysfs target add in the last Anand Jain
2016-04-25 16:23 ` David Sterba
2016-05-01 20:57 ` Anand Jain
2016-03-24 10:48 ` [PATCH 3/3] btrfs: refactor btrfs_dev_replace_start for reuse Anand Jain
2016-04-25 16:32 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).