Re: [PATCH 03/27] xfs: create an ioctl to scrub AG metadata

Darrick J. Wong
2017-09-22T03:26:21Z

On Thu, Sep 21, 2017 at 01:52:19PM -0400, Brian Foster wrote:
> On Thu, Sep 21, 2017 at 10:35:10AM -0700, Darrick J. Wong wrote:
> > On Thu, Sep 21, 2017 at 10:36:39AM -0400, Brian Foster wrote:
> > > On Wed, Sep 20, 2017 at 05:17:56PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > >
> > > > Create an ioctl that can be used to scrub internal filesystem metadata.
> > > > The new ioctl takes the metadata type, an (optional) AG number, an
> > > > (optional) inode number and generation, and a flags argument. This will
> > > > be used by the upcoming XFS online scrub tool.
> > > >
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > > fs/xfs/Kconfig | 17 ++++++++++++++
> > > > fs/xfs/Makefile | 11 +++++++++
> > > > fs/xfs/libxfs/xfs_fs.h | 53 +++++++++++++++++++++++++++++++++++++++++++++
> > > > fs/xfs/scrub/scrub.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > fs/xfs/scrub/scrub.h | 25 +++++++++++++++++++++
> > > > fs/xfs/scrub/trace.c | 41 +++++++++++++++++++++++++++++++++++
> > > > fs/xfs/scrub/trace.h | 33 ++++++++++++++++++++++++++++
> > > > fs/xfs/scrub/xfs_scrub.h | 29 +++++++++++++++++++++++++
> > > > fs/xfs/xfs_ioctl.c | 28 ++++++++++++++++++++++++
> > > > fs/xfs/xfs_ioctl32.c | 1 +
> > > > 10 files changed, 292 insertions(+)
> > > > create mode 100644 fs/xfs/scrub/scrub.c
> > > > create mode 100644 fs/xfs/scrub/scrub.h
> > > > create mode 100644 fs/xfs/scrub/trace.c
> > > > create mode 100644 fs/xfs/scrub/trace.h
> > > > create mode 100644 fs/xfs/scrub/xfs_scrub.h
> > > >
> > > >
> > >
> > > The code looks sane, though I think I need to understand the error codes
> > > a bit better. Perhaps once I get further into the series..
> >
> > Yes, the ioctl needs documentation, though I'm unclear on where's an
> > appropriate place to put them. The ioctl is not intended for general
> > consumption, so man-pages.git seems inappropriate. I was thinking
> > either in xfs_fs.h directly, or perhaps xfsprogs' man pages?
> >
>
> For now (while all of this is still experimental and until you get a
> better answer :P), it seems reasonable enough to me to add basic
> interface documentation to the kernel source. Then we at least have the
> content and can move it out into a manpage or something later, if
> needed.

It's more or less documented in the source... but in a somewhat
piecemeal fashion as we add functionality. I've drafted a manpage to
describe all of what the ioctl wants to do, so I'll tack that on the end
of this series (and cc you and Dave).

--D

>
> Brian
>
> > (We don't seem to document the xfs ioctls afaict?)
> >
> > --D
> >
> > >
> > > Brian
> > >
> > > > diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
> > > > index 1b98cfa..f42fcf1 100644
> > > > --- a/fs/xfs/Kconfig
> > > > +++ b/fs/xfs/Kconfig
> > > > @@ -71,6 +71,23 @@ config XFS_RT
> > > >
> > > > If unsure, say N.
> > > >
> > > > +config XFS_ONLINE_SCRUB
> > > > + bool "XFS online metadata check support"
> > > > + default n
> > > > + depends on XFS_FS
> > > > + help
> > > > + If you say Y here you will be able to check metadata on a
> > > > + mounted XFS filesystem. This feature is intended to reduce
> > > > + filesystem downtime by supplementing xfs_repair. The key
> > > > + advantage here is to look for problems proactively so that
> > > > + they can be dealt with in a controlled manner.
> > > > +
> > > > + This feature is considered EXPERIMENTAL. Use with caution!
> > > > +
> > > > + See the xfs_scrub man page in section 8 for additional information.
> > > > +
> > > > + If unsure, say N.
> > > > +
> > > > config XFS_WARN
> > > > bool "XFS Verbose Warnings"
> > > > depends on XFS_FS && !XFS_DEBUG
> > > > diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> > > > index dbc33e0..f4312bc 100644
> > > > --- a/fs/xfs/Makefile
> > > > +++ b/fs/xfs/Makefile
> > > > @@ -138,3 +138,14 @@ xfs-$(CONFIG_XFS_POSIX_ACL) += xfs_acl.o
> > > > xfs-$(CONFIG_SYSCTL) += xfs_sysctl.o
> > > > xfs-$(CONFIG_COMPAT) += xfs_ioctl32.o
> > > > xfs-$(CONFIG_EXPORTFS_BLOCK_OPS) += xfs_pnfs.o
> > > > +
> > > > +# online scrub/repair
> > > > +ifeq ($(CONFIG_XFS_ONLINE_SCRUB),y)
> > > > +
> > > > +# Tracepoints like to blow up, so build that before everything else
> > > > +
> > > > +xfs-y += $(addprefix scrub/, \
> > > > + trace.o \
> > > > + scrub.o \
> > > > + )
> > > > +endif
> > > > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > > > index 2c26c38..a4b4c8c 100644
> > > > --- a/fs/xfs/libxfs/xfs_fs.h
> > > > +++ b/fs/xfs/libxfs/xfs_fs.h
> > > > @@ -468,6 +468,58 @@ typedef struct xfs_swapext
> > > > #define XFS_FSOP_GOING_FLAGS_LOGFLUSH 0x1 /* flush log but not data */
> > > > #define XFS_FSOP_GOING_FLAGS_NOLOGFLUSH 0x2 /* don't flush log nor data */
> > > >
> > > > +/* metadata scrubbing */
> > > > +struct xfs_scrub_metadata {
> > > > + __u32 sm_type; /* What to check? */
> > > > + __u32 sm_flags; /* flags; see below. */
> > > > + __u64 sm_ino; /* inode number. */
> > > > + __u32 sm_gen; /* inode generation. */
> > > > + __u32 sm_agno; /* ag number. */
> > > > + __u64 sm_reserved[5]; /* pad to 64 bytes */
> > > > +};
> > > > +
> > > > +/*
> > > > + * Metadata types and flags for scrub operation.
> > > > + */
> > > > +
> > > > +/* Scrub subcommands. */
> > > > +
> > > > +/* Number of scrub subcommands. */
> > > > +#define XFS_SCRUB_TYPE_NR 0
> > > > +
> > > > +/* i: Repair this metadata. */
> > > > +#define XFS_SCRUB_IFLAG_REPAIR (1 << 0)
> > > > +
> > > > +/* o: Metadata object needs repair. */
> > > > +#define XFS_SCRUB_OFLAG_CORRUPT (1 << 1)
> > > > +
> > > > +/*
> > > > + * o: Metadata object could be optimized. It's not corrupt, but
> > > > + * we could improve on it somehow.
> > > > + */
> > > > +#define XFS_SCRUB_OFLAG_PREEN (1 << 2)
> > > > +
> > > > +/* o: Cross-referencing failed. */
> > > > +#define XFS_SCRUB_OFLAG_XFAIL (1 << 3)
> > > > +
> > > > +/* o: Metadata object disagrees with cross-referenced metadata. */
> > > > +#define XFS_SCRUB_OFLAG_XCORRUPT (1 << 4)
> > > > +
> > > > +/* o: Scan was not complete. */
> > > > +#define XFS_SCRUB_OFLAG_INCOMPLETE (1 << 5)
> > > > +
> > > > +/* o: Metadata object looked funny but isn't corrupt. */
> > > > +#define XFS_SCRUB_OFLAG_WARNING (1 << 6)
> > > > +
> > > > +#define XFS_SCRUB_FLAGS_IN (XFS_SCRUB_IFLAG_REPAIR)
> > > > +#define XFS_SCRUB_FLAGS_OUT (XFS_SCRUB_OFLAG_CORRUPT | \
> > > > + XFS_SCRUB_OFLAG_PREEN | \
> > > > + XFS_SCRUB_OFLAG_XFAIL | \
> > > > + XFS_SCRUB_OFLAG_XCORRUPT | \
> > > > + XFS_SCRUB_OFLAG_INCOMPLETE | \
> > > > + XFS_SCRUB_OFLAG_WARNING)
> > > > +#define XFS_SCRUB_FLAGS_ALL (XFS_SCRUB_FLAGS_IN | XFS_SCRUB_FLAGS_OUT)
> > > > +
> > > > /*
> > > > * AG reserved block counters
> > > > */
> > > > @@ -522,6 +574,7 @@ struct xfs_fsop_ag_resblks {
> > > > #define XFS_IOC_ZERO_RANGE _IOW ('X', 57, struct xfs_flock64)
> > > > #define XFS_IOC_FREE_EOFBLOCKS _IOR ('X', 58, struct xfs_fs_eofblocks)
> > > > /* XFS_IOC_GETFSMAP ------ hoisted 59 */
> > > > +#define XFS_IOC_SCRUB_METADATA _IOWR('X', 60, struct xfs_scrub_metadata)
> > > >
> > > > /*
> > > > * ioctl commands that replace IRIX syssgi()'s
> > > > diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
> > > > new file mode 100644
> > > > index 0000000..5db2a6f
> > > > --- /dev/null
> > > > +++ b/fs/xfs/scrub/scrub.c
> > > > @@ -0,0 +1,54 @@
> > > > +/*
> > > > + * Copyright (C) 2017 Oracle. All Rights Reserved.
> > > > + *
> > > > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > + * modify it under the terms of the GNU General Public License
> > > > + * as published by the Free Software Foundation; either version 2
> > > > + * of the License, or (at your option) any later version.
> > > > + *
> > > > + * This program is distributed in the hope that it would be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > + * GNU General Public License for more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public License
> > > > + * along with this program; if not, write the Free Software Foundation,
> > > > + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
> > > > + */
> > > > +#include "xfs.h"
> > > > +#include "xfs_fs.h"
> > > > +#include "xfs_shared.h"
> > > > +#include "xfs_format.h"
> > > > +#include "xfs_trans_resv.h"
> > > > +#include "xfs_mount.h"
> > > > +#include "xfs_defer.h"
> > > > +#include "xfs_btree.h"
> > > > +#include "xfs_bit.h"
> > > > +#include "xfs_log_format.h"
> > > > +#include "xfs_trans.h"
> > > > +#include "xfs_sb.h"
> > > > +#include "xfs_inode.h"
> > > > +#include "xfs_alloc.h"
> > > > +#include "xfs_alloc_btree.h"
> > > > +#include "xfs_bmap.h"
> > > > +#include "xfs_bmap_btree.h"
> > > > +#include "xfs_ialloc.h"
> > > > +#include "xfs_ialloc_btree.h"
> > > > +#include "xfs_refcount.h"
> > > > +#include "xfs_refcount_btree.h"
> > > > +#include "xfs_rmap.h"
> > > > +#include "xfs_rmap_btree.h"
> > > > +#include "scrub/xfs_scrub.h"
> > > > +#include "scrub/scrub.h"
> > > > +#include "scrub/trace.h"
> > > > +
> > > > +/* Dispatch metadata scrubbing. */
> > > > +int
> > > > +xfs_scrub_metadata(
> > > > + struct xfs_inode *ip,
> > > > + struct xfs_scrub_metadata *sm)
> > > > +{
> > > > + return -EOPNOTSUPP;
> > > > +}
> > > > diff --git a/fs/xfs/scrub/scrub.h b/fs/xfs/scrub/scrub.h
> > > > new file mode 100644
> > > > index 0000000..eb1cd9d
> > > > --- /dev/null
> > > > +++ b/fs/xfs/scrub/scrub.h
> > > > @@ -0,0 +1,25 @@
> > > > +/*
> > > > + * Copyright (C) 2017 Oracle. All Rights Reserved.
> > > > + *
> > > > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > + * modify it under the terms of the GNU General Public License
> > > > + * as published by the Free Software Foundation; either version 2
> > > > + * of the License, or (at your option) any later version.
> > > > + *
> > > > + * This program is distributed in the hope that it would be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > + * GNU General Public License for more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public License
> > > > + * along with this program; if not, write the Free Software Foundation,
> > > > + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
> > > > + */
> > > > +#ifndef __XFS_SCRUB_SCRUB_H__
> > > > +#define __XFS_SCRUB_SCRUB_H__
> > > > +
> > > > +/* Metadata scrubbers */
> > > > +
> > > > +#endif /* __XFS_SCRUB_SCRUB_H__ */
> > > > diff --git a/fs/xfs/scrub/trace.c b/fs/xfs/scrub/trace.c
> > > > new file mode 100644
> > > > index 0000000..c59fd41
> > > > --- /dev/null
> > > > +++ b/fs/xfs/scrub/trace.c
> > > > @@ -0,0 +1,41 @@
> > > > +/*
> > > > + * Copyright (C) 2017 Oracle. All Rights Reserved.
> > > > + *
> > > > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > + * modify it under the terms of the GNU General Public License
> > > > + * as published by the Free Software Foundation; either version 2
> > > > + * of the License, or (at your option) any later version.
> > > > + *
> > > > + * This program is distributed in the hope that it would be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > + * GNU General Public License for more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public License
> > > > + * along with this program; if not, write the Free Software Foundation,
> > > > + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
> > > > + */
> > > > +#include "xfs.h"
> > > > +#include "xfs_fs.h"
> > > > +#include "xfs_shared.h"
> > > > +#include "xfs_format.h"
> > > > +#include "xfs_log_format.h"
> > > > +#include "xfs_trans_resv.h"
> > > > +#include "xfs_mount.h"
> > > > +#include "xfs_defer.h"
> > > > +#include "xfs_da_format.h"
> > > > +#include "xfs_defer.h"
> > > > +#include "xfs_inode.h"
> > > > +#include "xfs_btree.h"
> > > > +#include "xfs_trans.h"
> > > > +#include "scrub/xfs_scrub.h"
> > > > +#include "scrub/scrub.h"
> > > > +
> > > > +/*
> > > > + * We include this last to have the helpers above available for the trace
> > > > + * event implementations.
> > > > + */
> > > > +#define CREATE_TRACE_POINTS
> > > > +#include "scrub/trace.h"
> > > > diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> > > > new file mode 100644
> > > > index 0000000..a95a7c8
> > > > --- /dev/null
> > > > +++ b/fs/xfs/scrub/trace.h
> > > > @@ -0,0 +1,33 @@
> > > > +/*
> > > > + * Copyright (C) 2017 Oracle. All Rights Reserved.
> > > > + *
> > > > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > + * modify it under the terms of the GNU General Public License
> > > > + * as published by the Free Software Foundation; either version 2
> > > > + * of the License, or (at your option) any later version.
> > > > + *
> > > > + * This program is distributed in the hope that it would be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > + * GNU General Public License for more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public License
> > > > + * along with this program; if not, write the Free Software Foundation,
> > > > + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
> > > > + */
> > > > +#undef TRACE_SYSTEM
> > > > +#define TRACE_SYSTEM xfs_scrub
> > > > +
> > > > +#if !defined(_TRACE_XFS_SCRUB_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
> > > > +#define _TRACE_XFS_SCRUB_TRACE_H
> > > > +
> > > > +#include <linux/tracepoint.h>
> > > > +
> > > > +#endif /* _TRACE_XFS_SCRUB_TRACE_H */
> > > > +
> > > > +#undef TRACE_INCLUDE_PATH
> > > > +#define TRACE_INCLUDE_PATH .
> > > > +#define TRACE_INCLUDE_FILE scrub/trace
> > > > +#include <trace/define_trace.h>
> > > > diff --git a/fs/xfs/scrub/xfs_scrub.h b/fs/xfs/scrub/xfs_scrub.h
> > > > new file mode 100644
> > > > index 0000000..e00e0ea
> > > > --- /dev/null
> > > > +++ b/fs/xfs/scrub/xfs_scrub.h
> > > > @@ -0,0 +1,29 @@
> > > > +/*
> > > > + * Copyright (C) 2017 Oracle. All Rights Reserved.
> > > > + *
> > > > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > + * modify it under the terms of the GNU General Public License
> > > > + * as published by the Free Software Foundation; either version 2
> > > > + * of the License, or (at your option) any later version.
> > > > + *
> > > > + * This program is distributed in the hope that it would be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > + * GNU General Public License for more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public License
> > > > + * along with this program; if not, write the Free Software Foundation,
> > > > + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
> > > > + */
> > > > +#ifndef __XFS_SCRUB_H__
> > > > +#define __XFS_SCRUB_H__
> > > > +
> > > > +#ifndef CONFIG_XFS_ONLINE_SCRUB
> > > > +# define xfs_scrub_metadata(ip, sm) (-ENOTTY)
> > > > +#else
> > > > +int xfs_scrub_metadata(struct xfs_inode *ip, struct xfs_scrub_metadata *sm);
> > > > +#endif /* CONFIG_XFS_ONLINE_SCRUB */
> > > > +
> > > > +#endif /* __XFS_SCRUB_H__ */
> > > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > > > index 44dc178..ab7a7f8 100644
> > > > --- a/fs/xfs/xfs_ioctl.c
> > > > +++ b/fs/xfs/xfs_ioctl.c
> > > > @@ -44,6 +44,7 @@
> > > > #include "xfs_btree.h"
> > > > #include <linux/fsmap.h>
> > > > #include "xfs_fsmap.h"
> > > > +#include "scrub/xfs_scrub.h"
> > > >
> > > > #include <linux/capability.h>
> > > > #include <linux/cred.h>
> > > > @@ -1702,6 +1703,30 @@ xfs_ioc_getfsmap(
> > > > return 0;
> > > > }
> > > >
> > > > +STATIC int
> > > > +xfs_ioc_scrub_metadata(
> > > > + struct xfs_inode *ip,
> > > > + void __user *arg)
> > > > +{
> > > > + struct xfs_scrub_metadata scrub;
> > > > + int error;
> > > > +
> > > > + if (!capable(CAP_SYS_ADMIN))
> > > > + return -EPERM;
> > > > +
> > > > + if (copy_from_user(&scrub, arg, sizeof(scrub)))
> > > > + return -EFAULT;
> > > > +
> > > > + error = xfs_scrub_metadata(ip, &scrub);
> > > > + if (error)
> > > > + return error;
> > > > +
> > > > + if (copy_to_user(arg, &scrub, sizeof(scrub)))
> > > > + return -EFAULT;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > int
> > > > xfs_ioc_swapext(
> > > > xfs_swapext_t *sxp)
> > > > @@ -1906,6 +1931,9 @@ xfs_file_ioctl(
> > > > case FS_IOC_GETFSMAP:
> > > > return xfs_ioc_getfsmap(ip, arg);
> > > >
> > > > + case XFS_IOC_SCRUB_METADATA:
> > > > + return xfs_ioc_scrub_metadata(ip, arg);
> > > > +
> > > > case XFS_IOC_FD_TO_HANDLE:
> > > > case XFS_IOC_PATH_TO_HANDLE:
> > > > case XFS_IOC_PATH_TO_FSHANDLE: {
> > > > diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> > > > index e8b4de3..972d4bd 100644
> > > > --- a/fs/xfs/xfs_ioctl32.c
> > > > +++ b/fs/xfs/xfs_ioctl32.c
> > > > @@ -557,6 +557,7 @@ xfs_file_compat_ioctl(
> > > > case XFS_IOC_ERROR_CLEARALL:
> > > > case FS_IOC_GETFSMAP:
> > > > case XFS_IOC_GET_AG_RESBLKS:
> > > > + case XFS_IOC_SCRUB_METADATA:
> > > > return xfs_file_ioctl(filp, cmd, p);
> > > > #ifndef BROKEN_X86_ALIGNMENT
> > > > /* These are handled fine if no alignment issues */
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > 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@vger.kernel.org
> > > 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@vger.kernel.org
> > 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html