Re: [PATCH 4/4] btrfs: assert that relocation is protected with sb_start_write()

[Filipe Manana <fdmanana@kernel.org>]

On Fri, Mar 11, 2022 at 04:38:05PM +0900, Naohiro Aota wrote:
> btrfs_relocate_chunk() initiates new 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>

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

> ---
>  fs/btrfs/volumes.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 0d27d8d35c7a..b558fd293ffa 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3239,6 +3239,9 @@ int btrfs_relocate_chunk(struct btrfs_fs_info *fs_info, u64 chunk_offset)
>  	u64 length;
>  	int ret;
>  
> +	/* Assert we called sb_start_write(), not to race with FS freezing */
> +	ASSERT(sb_write_started(fs_info->sb));

Does this pass the scenario of patch 1/4 (resuming balance on mount)?

Because as commented in that patch, we have the sb_start_write() done
in the mount task, and not by the task that actually runs balance - the
balance kthread.

Anyway, this change looks good, my concerns are only about patch 1/4.

Thanks.

> +
>  	if (btrfs_fs_incompat(fs_info, EXTENT_TREE_V2)) {
>  		btrfs_err(fs_info,
>  			  "relocate: not supported on extent tree v2 yet");
> -- 
> 2.35.1
>