diff mbox series

[v4,3/3] btrfs: assert that relocation is protected with sb_start_write()

Message ID 28e3e02ed14fe7c0859707e1a10a447fe4338c16.1648535838.git.naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series protect relocation with sb_start_write | expand

Commit Message

Naohiro Aota March 29, 2022, 6:56 a.m. UTC
Relocation of a data block group creates ordered extents. They can cause a
hang when a process is trying to thaw the filesystem.

We should have called sb_start_write(), so the filesystem is not being
frozen. Add an ASSERT to check it is protected.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/relocation.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Filipe Manana March 29, 2022, 10:10 a.m. UTC | #1
On Tue, Mar 29, 2022 at 03:56:00PM +0900, Naohiro Aota wrote:
> Relocation of a data block group creates ordered extents. They can cause a
> hang when a process is trying to thaw the filesystem.
> 
> We should have called sb_start_write(), so the filesystem is not being
> frozen. Add an ASSERT to check it is protected.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  fs/btrfs/relocation.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index fdc2c4b411f0..5e52cd8d5f23 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -3977,6 +3977,16 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
>  	if (!bg)
>  		return -ENOENT;
>  
> +	/*
> +	 * Relocation of a data block group creates ordered extents.
> +	 * Without sb_start_write(), we can freeze the FS while unfinished
> +	 * ordered extents are left. Such ordered extents can cause a
> +	 * deadlock e.g, when syncfs() is trying to finish them because

syncfs() is not trying to finish them, it's waiting for them to complete.

> +	 * they never finish as the FS is already frozen.

More specifically they can't finish because they block when joining a transaction,
due to the fact that the freeze locks are being held in write mode.

Anyway, I won't make you send yet another version. Perhaps this is something
David can fixup when he picks the patch.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

> +	 */
> +	if (bg->flags & BTRFS_BLOCK_GROUP_DATA)
> +		ASSERT(sb_write_started(fs_info->sb));
> +
>  	if (btrfs_pinned_by_swapfile(fs_info, bg)) {
>  		btrfs_put_block_group(bg);
>  		return -ETXTBSY;
> -- 
> 2.35.1
>
diff mbox series

Patch

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index fdc2c4b411f0..5e52cd8d5f23 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3977,6 +3977,16 @@  int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 	if (!bg)
 		return -ENOENT;
 
+	/*
+	 * Relocation of a data block group creates ordered extents.
+	 * Without sb_start_write(), we can freeze the FS while unfinished
+	 * ordered extents are left. Such ordered extents can cause a
+	 * deadlock e.g, when syncfs() is trying to finish them because
+	 * they never finish as the FS is already frozen.
+	 */
+	if (bg->flags & BTRFS_BLOCK_GROUP_DATA)
+		ASSERT(sb_write_started(fs_info->sb));
+
 	if (btrfs_pinned_by_swapfile(fs_info, bg)) {
 		btrfs_put_block_group(bg);
 		return -ETXTBSY;