Re: [PATCH 2/3] fs: wait for partially frozen filesystems

Christoph Hellwig
2023-06-12T04:01:52Z

On Sun, Jun 11, 2023 at 08:15:28PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Jan Kara suggested that when one thread is in the middle of freezing a
> filesystem, another thread trying to freeze the same fs but with a
> different freeze_holder should wait until the freezer reaches either end
> state (UNFROZEN or COMPLETE) instead of returning EBUSY immediately.
>
> Plumb in the extra coded needed to wait for the fs freezer to reach an
> end state and try the freeze again.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/super.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/super.c b/fs/super.c
> index 36adccecc828..151e0eeff2c2 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1647,6 +1647,15 @@ static int freeze_frozen_super(struct super_block *sb, enum freeze_holder who)
> return 0;
> }
>
> +static void wait_for_partially_frozen(struct super_block *sb)
> +{
> + up_write(&sb->s_umount);
> + wait_var_event(&sb->s_writers.frozen,
> + sb->s_writers.frozen == SB_UNFROZEN ||
> + sb->s_writers.frozen == SB_FREEZE_COMPLETE);
> + down_write(&sb->s_umount);

Does sb->s_writers.frozen need WRITE_ONCE/READ_ONCE treatment if we want
to check it outside of s_umount? Or should we maybe just open code
wait_var_event and only drop the lock after checking the variable?

> if (sb->s_writers.frozen != SB_UNFROZEN) {
> - deactivate_locked_super(sb);
> - return -EBUSY;
> + if (!try_again) {
> + deactivate_locked_super(sb);
> + return -EBUSY;
> + }
> +
> + wait_for_partially_frozen(sb);
> + try_again = false;
> + goto retry;

Can you throw in a comment on wait we're only waiting for a partial
freeze one here?