[PATCH RESEND 0/9 v2] fix warn_on for replace cancel

[PATCH RESEND 0/9 v2] fix warn_on for replace cancel

[Anand Jain]

These two patches were sent as part of
   [PATCH 0/9 v2] fix replace-start and replace-cancel racing
before but as these aren't integrated so I am sending these again.

The patch [1] which is in misc-next, calls btrfs_dev_replace_finishing()
after replace is canceled, so the ret argument passed to the
btrfs_dev_replace_finishing() is -ECANCEL, which is not a real error.
So these patches quieten the warn and error log if its -ECANCEL.

These should be integrated otherwise we see the WARN_ON and btrfs_error()
after replace cancel.

[1] 
08bdfc3a0611 btrfs: fix use-after-free due to race between replace start and cancel

Anand Jain (2):
  btrfs: quieten warn if the replace is canceled at finish
  btrfs: user requsted replace cancel is not an error

[PATCH RESEND v2 1/2] btrfs: quieten warn if replace is canceled at finish

[Anand Jain]

When we successfully cancel the replace its scrub returns -ECANCELED,
which then passed to btrfs_dev_replace_finishing(), it cleans up based
on the scrub returned status and propagates the same -ECANCELED back
the parent function. As of now only user can cancel the replace-scrub,
so its ok to quieten the warn here.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
[fix: quieten spelling]
v1->v2: Use the condition within the WARN_ON()

 fs/btrfs/dev-replace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 1dc8e86546db..9031a362921a 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -497,7 +497,7 @@ static int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
 	ret = btrfs_dev_replace_finishing(fs_info, ret);
 	if (ret == -EINPROGRESS) {
 		ret = BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS;
-	} else {
+	} else if (ret != -ECANCELED) {
 		WARN_ON(ret);
 	}
 
@@ -954,7 +954,7 @@ static int btrfs_dev_replace_kthread(void *data)
 			      btrfs_device_get_total_bytes(dev_replace->srcdev),
 			      &dev_replace->scrub_progress, 0, 1);
 	ret = btrfs_dev_replace_finishing(fs_info, ret);
-	WARN_ON(ret);
+	WARN_ON(ret && ret != -ECANCELED);
 
 	clear_bit(BTRFS_FS_EXCL_OP, &fs_info->flags);
 	return 0;
-- 
1.8.3.1

Re: [PATCH RESEND v2 1/2] btrfs: quieten warn if replace is canceled at finish

[David Sterba]

On Tue, Nov 20, 2018 at 07:56:15PM +0800, Anand Jain wrote:
> When we successfully cancel the replace its scrub returns -ECANCELED,
> which then passed to btrfs_dev_replace_finishing(), it cleans up based
> on the scrub returned status and propagates the same -ECANCELED back
> the parent function. As of now only user can cancel the replace-scrub,
> so its ok to quieten the warn here.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Ok for getting rid if the ECANCELED warning, though it would be better
to replace the WARN_ON too.

Reviewed-by: David Sterba <dsterba@suse.com>

[PATCH RESEND v2 2/2] btrfs: user requsted replace cancel is not an error

[Anand Jain]

As of now only user requested replace cancel can cancel the replace-scrub
so no need to log error for it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1->v2: none.

 fs/btrfs/dev-replace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 9031a362921a..40a0942b4659 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -622,7 +622,8 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
 								src_device,
 								tgt_device);
 	} else {
-		btrfs_err_in_rcu(fs_info,
+		if (scrub_ret != -ECANCELED)
+			btrfs_err_in_rcu(fs_info,
 				 "btrfs_scrub_dev(%s, %llu, %s) failed %d",
 				 btrfs_dev_name(src_device),
 				 src_device->devid,
-- 
1.8.3.1

Re: [PATCH RESEND 0/9 v2] fix warn_on for replace cancel

[David Sterba]

On Tue, Nov 20, 2018 at 07:56:14PM +0800, Anand Jain wrote:
> These two patches were sent as part of
>    [PATCH 0/9 v2] fix replace-start and replace-cancel racing
> before but as these aren't integrated so I am sending these again.
> 
> The patch [1] which is in misc-next, calls btrfs_dev_replace_finishing()
> after replace is canceled, so the ret argument passed to the
> btrfs_dev_replace_finishing() is -ECANCEL, which is not a real error.
> So these patches quieten the warn and error log if its -ECANCEL.
> 
> These should be integrated otherwise we see the WARN_ON and btrfs_error()
> after replace cancel.
> 
> [1] 
> 08bdfc3a0611 btrfs: fix use-after-free due to race between replace start and cancel
> 
> Anand Jain (2):
>   btrfs: quieten warn if the replace is canceled at finish
>   btrfs: user requsted replace cancel is not an error

Thanks, patches added to misc-next, right after the other dev-replace
patches.