On Wed, Jul 25, 2018 at 06:06:00PM -0700, Eric Sandeen wrote:
> On 6/26/18 7:48 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Teach xfs_scrub to remember scrub requests that failed (or indicated
> > that optimization is a possibility) as action items. Depending on the
> > circumstances, certain items are acted upon immediately (e.g. metadata
> > that needs to be healthy in order to finish the scan, or files that are
> > already open) or deferred until later. Expand the repair phase to
> > deal with the deferred actions.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > man/man8/xfs_scrub.8 | 38 ++++++-
> > scrub/Makefile | 2
> > scrub/phase1.c | 7 +
> > scrub/phase2.c | 59 ++++++++++
> > scrub/phase3.c | 42 +++++--
> > scrub/phase4.c | 76 +++++++++++++
> > scrub/repair.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > scrub/repair.h | 42 +++++++
> > scrub/scrub.c | 130 ++++++++++++++++-------
> > scrub/scrub.h | 42 +++++--
> > scrub/xfs_scrub.c | 44 +++++++-
> > scrub/xfs_scrub.h | 1
> > 12 files changed, 690 insertions(+), 79 deletions(-)
> > create mode 100644 scrub/repair.c
> > create mode 100644 scrub/repair.h
> >
> >
> > diff --git a/man/man8/xfs_scrub.8 b/man/man8/xfs_scrub.8
> > index 680ef72b..18948a4e 100644
> > --- a/man/man8/xfs_scrub.8
> > +++ b/man/man8/xfs_scrub.8
> > @@ -1,6 +1,6 @@
> > .TH xfs_scrub 8
> > .SH NAME
> > -xfs_scrub \- check the contents of a mounted XFS filesystem
> > +xfs_scrub \- check and repair the contents of a mounted XFS filesystem
> > .SH SYNOPSIS
> > .B xfs_scrub
> > [
> > @@ -108,10 +108,40 @@ Optimizations supported by this program include, but are not limited to:
> > Instructing the underlying storage to discard unused extents via the
> > .B TRIM
> > ioctl.
> > +.IP \[bu]
> > +Updating secondary superblocks to match the primary superblock.
> > +.IP \[bu]
> > +Turning off shared block write checks for files that no longer share blocks.
> > .SH REPAIRS
> > -This program currently does not support making any repairs.
> > -Corruptions can only be fixed by unmounting the filesystem and running
> > -.BR xfs_repair (8).
> > +Repairs are performed by calling into the kernel.
> > +This limits the scope of repair activities to rebuilding primary data
> > +structures from secondary data structures, or secondary structures from
> > +primary structures.
> > +The existence of secondary data structures may require features that can
> > +only be turned on from
> > +.BR mkfs.xfs (8).
> > +If errors cannot be repaired, the filesystem must be
> > +unmounted and
> > +.BR xfs_repair (8)
> > +run.
> > +Repairs supported by the kernel include, but are not limited to:
> > +.IP \[bu] 2
> > +Reconstructing extent allocation data.
> > +.IP \[bu]
> > +Rebuilding free space information.
> > +.IP \[bu]
> > +Rebuilding inode indexes.
> > +.IP \[bu]
> > +Fixing minor corruptions of inode records.
> > +.IP \[bu]
> > +Recalculating reference count information.
> > +.IP \[bu]
> > +Reconstructing reverse mapping data from primary extent allocation data.
> > +.IP \[bu]
> > +Scheduling a quotacheck for the next mount.
> > +.PP
> > +If corrupt metadata is successfully repaired, this program will log that
> > +a repair has succeeded instead of a corruption report.
>
> for 4.18 I think we should only list what's possible, right?
> But, um, what is currently possible in 4.18?
Secondary superblocks only.
> Maybe I'll just add a "may include ..." and a "depending on the capabilities
> of the running kernel" to give us some wiggle room.
/me shrugs, happy to resubmit that list w/ just the secondary supers and
then to send more patches with each release as/if I get things
reviewed.
> As for the rest, um, seems ok AFAICT (which isn't that far). Minor nit:
>
> ...
>
> > static void
> > report_outcome(
> > struct scrub_ctx *ctx)
> > @@ -525,9 +555,16 @@ report_outcome(
> > * setting up the scrub and we actually saw corruptions. Warnings
> > * are not corruptions.
> > */
> > - if (ctx->scrub_setup_succeeded && total_errors > 0)
> > - fprintf(stderr, _("%s: Unmount and run xfs_repair.\n"),
> > - ctx->mntpoint);
> > + if (ctx->scrub_setup_succeeded && total_errors > 0) {
> > + char *msg;
> > +
> > + if (ctx->mode == SCRUB_MODE_DRY_RUN)
> > + msg = _("%s: Re-run xfs_scrub without -n.\n");
>
> "... to repair?"
Yes.
> Is it possible that we found something that can't actually be repaired
> online, and so they'll get:
>
> # xfs_scrub -n
> "Re-run xfs_scrub without -n"
> # xfs_scrub
> "Unmount and run xfs_repair."
Right. First scrub found something and told you how to fix it, but then
you tried that and it couldn't fix it so it told you about ye olde
repair tool.
>
> Maybe that's ok ... there's no way to keep track of which found problems
> are repairable by the running kernel, is there?
Not really, I'm afraid. Either the kernel has a function to repair
things or it doesn't, and there's no good way to track at a finer
grained level than that.
--D
> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
