On Sun, Jun 11, 2023 at 09:01:48PM -0700, Christoph Hellwig wrote:
> On Sun, Jun 11, 2023 at 08:15:28PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> > 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?
How about something like:
do {
up_write(&sb->s_umount);
down_write(&sb->s_umount);
} while (sb->s_writers.frozen != SB_UNFROZEN &&
sb->s_writers.frozen != SB_FREEZE_COMPLETE);
so that we always return in either end state of a freezer transition?
> > 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?
I didn't want a thread to get stuck in the retry forever if it always
loses the race. However, I think any other threads running freeze_super
will always end at UNFROZEN or COMPLETE; and thaw_super always goes
straight froM COMPLETE to UNFROZEN, so I think I'll get rid of the retry
flag logic entirely.
--D
