btrfs: fix clearing of BTRFS_FS_RELOC_RUNNING if relocation already running

commit 7e5a5983edda664e8e4bb20af17b80f5135c655c upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 04af8687759d858b59d9d79bcdbb16c6b657d9ac..96c89884988b4a431817d56b3bfa3a782789f6cf 100644 (file)
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3915,6 +3915,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
@@ -3931,10 +3932,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;
        }
@@ -3943,9 +3942,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");
@@ -4144,9 +4145,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;
 }
@@ -4337,8 +4338,8 @@ out_clean:
                err = ret;
 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);