Re: [PATCH] xfs: stabilize fs summary counters for online fsck

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Aug 03, 2023 at 04:34:48PM +1000, Dave Chinner wrote:
> On Wed, Aug 02, 2023 at 10:22:18PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > If the fscounters scrubber notices incorrect summary counters, it's
> > entirely possible that scrub is simply racing with other threads that
> > are updating the incore counters.  There isn't a good way to stabilize
> > percpu counters or set ourselves up to observe live updates with hooks
> > like we do for the quotacheck or nlinks scanners, so we instead choose
> > to freeze the filesystem long enough to walk the incore per-AG
> > structures.
> > 
> > Past me thought that it was going to be commonplace to have to freeze
> > the filesystem to perform some kind of repair and set up a whole
> > separate infrastructure to freeze the filesystem in such a way that
> > userspace could not unfreeze while we were running.  This involved
> > adding a mutex and freeze_super/thaw_super functions and dealing with
> > the fact that the VFS freeze/thaw functions can free the VFS superblock
> > references on return.
> > 
> > This was all very overwrought, since fscounters turned out to be the
> > only user of scrub freezes, and it doesn't require the log to quiesce,
> > only the incore superblock counters.  We prevent other threads from
> > changing the freeze level by calling freeze_super_excl with a custom
> > freeze cookie to keep everyone else out of the filesystem.
> > 
> > The end result is that fscounters should be much more efficient.  When
> > we're checking a busy system and we can't stabilize the counters, the
> > custom freeze will do less work, which should result in less downtime.
> > Repair should be similarly speedy, but that's in the next patch.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  fs/xfs/scrub/fscounters.c |  198 ++++++++++++++++++++++++++++++++++-----------
> >  fs/xfs/scrub/fscounters.h |   20 +++++
> >  fs/xfs/scrub/scrub.c      |    6 +
> >  fs/xfs/scrub/scrub.h      |    1 
> >  fs/xfs/scrub/trace.h      |   26 ++++++
> >  5 files changed, 203 insertions(+), 48 deletions(-)
> >  create mode 100644 fs/xfs/scrub/fscounters.h
> 
> Code changes look ok, though I am wondering why struct
> xchk_fscounters needs to be moved to it's own header file? AFAICT it
> is still only used by fs/xfs/scrub/fscounters.c, so I'm not sure
> what purpose the new header file serves....

The header file is the basis for the repair patch which I've previously
sent to the list.
https://lore.kernel.org/linux-xfs/168506061531.3732954.7713322896089390150.stgit@frogsfrogsfrogs/

It's not strictly needed at this time.  I'll resend without that bit.

--D

> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com