btrfs: fix clearing of BTRFS_FS_RELOC_RUNNING if relocation already running

When starting relocation, at reloc_chunk_start(), if we happen to find
the flag BTRFS_FS_RELOC_RUNNING is already set we return an error
(-EINPROGRESS) to the callers, however the callers call reloc_chunk_end()
which will clear the flag BTRFS_FS_RELOC_RUNNING, which is wrong since
relocation was started by another task and still running.

Finding the BTRFS_FS_RELOC_RUNNING flag already set is an unexpected
scenario, but still our current behaviour is not correct.

Fix this by never calling reloc_chunk_end() if reloc_chunk_start() has
returned an error, which is what logically makes sense, since the general
widespread pattern is to have end functions called only if the counterpart
start functions succeeded. This requires changing reloc_chunk_start() to
clear BTRFS_FS_RELOC_RUNNING if there's a pending cancel request.

Fixes: 907d2710d727 ("btrfs: add cancellable chunk relocation support")
CC: stable@vger.kernel.org # 5.15+
Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 8dd8de6b9fb89e84e43d2a5d15e8ca6a6bfcf41f..0765e06d00b80f10c1fa4b6c7c322253dfe88c00 100644 (file)
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3780,6 +3780,7 @@ out:
 /*
  * Mark start of chunk relocation that is cancellable. Check if the cancellation
  * has been requested meanwhile and don't start in that case.
+ * NOTE: if this returns an error, reloc_chunk_end() must not be called.
  *
  * Return:
  *   0             success
@@ -3796,10 +3797,8 @@ static int reloc_chunk_start(struct btrfs_fs_info *fs_info)
 
        if (atomic_read(&fs_info->reloc_cancel_req) > 0) {
                btrfs_info(fs_info, "chunk relocation canceled on start");
-               /*
-                * On cancel, clear all requests but let the caller mark
-                * the end after cleanup operations.
-                */
+               /* On cancel, clear all requests. */
+               clear_and_wake_up_bit(BTRFS_FS_RELOC_RUNNING, &fs_info->flags);
                atomic_set(&fs_info->reloc_cancel_req, 0);
                return -ECANCELED;
        }
@@ -3808,9 +3807,11 @@ static int reloc_chunk_start(struct btrfs_fs_info *fs_info)
 
 /*
  * Mark end of chunk relocation that is cancellable and wake any waiters.
+ * NOTE: call only if a previous call to reloc_chunk_start() succeeded.
  */
 static void reloc_chunk_end(struct btrfs_fs_info *fs_info)
 {
+       ASSERT(test_bit(BTRFS_FS_RELOC_RUNNING, &fs_info->flags));
        /* Requested after start, clear bit first so any waiters can continue */
        if (atomic_read(&fs_info->reloc_cancel_req) > 0)
                btrfs_info(fs_info, "chunk relocation canceled during operation");
@@ -4023,9 +4024,9 @@ out:
        if (err && rw)
                btrfs_dec_block_group_ro(rc->block_group);
        iput(rc->data_inode);
+       reloc_chunk_end(fs_info);
 out_put_bg:
        btrfs_put_block_group(bg);
-       reloc_chunk_end(fs_info);
        free_reloc_control(rc);
        return err;
 }
@@ -4208,8 +4209,8 @@ out_clean:
                ret = ret2;
 out_unset:
        unset_reloc_control(rc);
-out_end:
        reloc_chunk_end(fs_info);
+out_end:
        free_reloc_control(rc);
 out:
        free_reloc_roots(&reloc_roots);