Re: [PATCH RFC] btrfs: exit scrub and balance early if the fs is being frozen

[Qu Wenruo <wqu@suse.com>]

在 2025/7/7 09:55, Qu Wenruo 写道:
> [PROBLEM]
> There are some reports that btrfs is unable to be frozen if there is a
> running scrub.
> 
> [CAUSE]
> If there is a running scrub, freeze_super() will wait for the running
> scrub as read-write scrub is holding sb_start_write():
> 
>             Scrub process             |         Freeze process
> -------------------------------------+--------------------------------
> btrfs_ioctl_scrub()                  |
> |- mnt_want_write_file()             |
> |  |- sb_start_write()               |
> |     This will block freezing       |
> |                                    | freeze_super()
> |- mnt_drop_write_file()             | |
>                                       | |- sb_wait_write()
>                                       | |  This will wait for any
>                                       | |  sb_start_write() to finish
> 
> This means freeze_super() will wait for any running scrub to finish.
> The same applies to all ioctls that requires mnt_want_write_file().
> 
> The most common long running ones are scrub and balance.
> 
> Since scrub and balance can be very long running operations, this will
> cause freezing to timeout.
> And since freezing the fs is required before suspension/hibernation,
> this means those two operations will fail too.
> 
> [FIX]
> Check if the fs is being frozen, and if so cancel the current running
> scrub or balance.
> 
> So far I didn't find a better way to solve the problem, the only way to
> drop the mnt_want_write_file() is to finish or cancel the scrub/balance.
> 
> There is no way to drop the mnt_want_write_file() meanwhile just pausing
> scrub/balance, at least not for now.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Reason for RFC:
> I'm not sure if cancelling is the best solution, but it is the easiest
> one to implementation.
> 
> Pause the scrub/balance is not really feasible yet, as it will still hold the
> mnt_want_write_file(), thus blocking freezing.
> 
> Meanwhile for end users, pausing scrub/balance when freezing, and resume
> when thawing should be the best outcome.

I have explored some other solutions, like dropping and grabbing the 
s_writers.rw_sem during the balance/scrub.

The problem of that solution is the reserved lock sequence, thus it will 
be deadlock prune.


Currently I guess the best solution would be introducing a special error 
code (maybe >0? -EGAIN may be a little too generic in this case) so that 
if we hit that specific error code, we error out as usual.

But at the top level where we call mnt_want_write*() function, we drop 
the rw_sem, and retry other than exit.

By this, we split the original long-running ioctl into several different 
smaller sections (the split only happens after the fs being frozen), so 
that they can properly follow the fs freeze behavior.

The challenge is how to resume from such interruption.
Currently neither scrub nor balance can properly handle such resume and 
will restart from the beginning.

And even with that resume implementation, the checks in this patch will 
still be needed.

Thanks,
Qu

> ---
>   fs/btrfs/relocation.c | 3 ++-
>   fs/btrfs/scrub.c      | 6 ++++++
>   2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 175fc3acc38b..f173e36a69f8 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -2797,7 +2797,8 @@ noinline int btrfs_should_cancel_balance(const struct btrfs_fs_info *fs_info)
>   {
>   	return atomic_read(&fs_info->balance_cancel_req) ||
>   		atomic_read(&fs_info->reloc_cancel_req) ||
> -		fatal_signal_pending(current);
> +		fatal_signal_pending(current) ||
> +		fs_info->sb->s_writers.frozen > SB_UNFROZEN;
>   }
>   ALLOW_ERROR_INJECTION(btrfs_should_cancel_balance, TRUE);
>   
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 6776e6ab8d10..bf8e4c411b60 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -2244,6 +2244,12 @@ static int scrub_simple_mirror(struct scrub_ctx *sctx,
>   		u64 found_logical = U64_MAX;
>   		u64 cur_physical = physical + cur_logical - logical_start;
>   
> +		/* Fs being frozen, need to exit early or freezing will timeout. */
> +		if (fs_info->sb->s_writers.frozen > SB_UNFROZEN) {
> +			ret = -ECANCELED;
> +			break;
> +		}
> +
>   		/* Canceled? */
>   		if (atomic_read(&fs_info->scrub_cancel_req) ||
>   		    atomic_read(&sctx->cancel_req)) {