On Sun, Nov 11, 2018 at 10:22:22PM +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.j...@oracle.com> > --- > 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);
While this looks ok, can you please rework it so there are no WARN_ON at random places in device-replace, poorly substituting error handling? The code flow in this case could be changed to make explicit checks for the know codes and then a catch-all branch like: if (ret == -EINPROGRESS) { ... } else (if == -ESOMETHINGELSE) { ... } else { unknown error, print error and do a proper cleanup } > } > > @@ -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); This one too, thanks.