[RFC] xfs: opting in or out of online repair

[RFC] xfs: opting in or out of online repair

[Darrick J. Wong]

Hi everyone,

Over in this thread:
https://lore.kernel.org/linux-xfs/20240724131825.GA12250@lst.de/

I originally proposed turning on background xfs_scrub by default in
Debian.  Christoph wondered if it was such a good idea to unleash this
upon hoards of debian sid users, considering that the introduction of
e2scrub by default didn't go entirely smoothly.  Debian policy is to
enable packaged services at installation time, which is reasonable for
httpd but perhaps not so much for experimental services.

I proposed modifying xfsprogs so that for Debian builds, it would spit
out a separate "xfsprogs-self-healing" package containing only the files
needed to activate the background scrub services by default.  People
would have to opt-in to installing it... insofar as one can configure
apt to pull in Suggests: and Recommends: packages.

Christoph suggested adding a compat flag to the superblock so that
sysadmins can mark filesystems for (or against) background online
repair.  Therefore, I'm proposing adding such a flag.  Before I start
any work on that, I thought I'd sketch out what I'd add to make that
happen:

On Disk Format Changes
======================

Add a new COMPAT flag XFS_SB_FEAT_COMPAT_SELF_HEALING.  If set,
the background xfs_scrub_all service can periodically scan the
filesystem and initiate repairs.  If not set, then background repairs
are not allowed.

Note that administrator-initated scans (e.g. invoking xfs_scrub from the
CLI) would not be blocked by this flag.

Question: Should this compat flag control background scrubs as well?

Userspace ABI Changes
=====================

The first thing is adding a new CLI option to mkfs.xfs to set the value
of the flag:

# mkfs.xfs -m self_healing=1

Question: If rmap and parent pointers are turned on, should we turn on
self healing by default?

Question: If self healing is turned on, should we turn on rmap and
parent pointers as well?

For xfs_scrub_all, we'd need a way to query the status of the self
healing bit; this could be snuck into the xfs geometry ioctl as a new
feature bit XFS_FSOP_GEOM_FLAGS_SELF_HEALING.

# xfs_info /dev/sdf
meta-data=/dev/sdf     isize=512    agcount=1, agsize=268435455 blks
         =             sectsz=512   attr=2, projid32bit=1
         =             crc=1        finobt=1, sparse=1, rmapbt=1
         =             reflink=1    bigtime=1 inobtcount=1 nrext64=1
         =             exchange=0   metadir=0 self_healing=1
data     =             bsize=4096   blocks=5192704, imaxpct=25
         =             sunit=0      swidth=0 blks
naming   =version 2    bsize=4096   ascii-ci=0, ftype=1, parent=0
log      =internal log bsize=4096   blocks=16384, version=2
         =             sectsz=512   sunit=0 blks, lazy-count=1
realtime =none         extsz=4096   blocks=0, rtextents=0
         =             rgcount=0    rgsize=0 extents

I think we also want to enable sysadmins to change this on the fly.
For mounted filesystems, this could be done through an administrative
ioctl or mount options.  Offline, this would become another xfs_admin
command.

# xfs_spaceman -c 'self_healing on' /usr
# xfs_spaceman -c 'self_healing off' /tmp/extraspace

# xfs_admin --self-healing=on /dev/sda1
# xfs_admin --self-healing=off /dev/sda2

# mount /dev/sda2 /mnt -o self_healing

Thoughts?  Flames?

--D

Re: [RFC] xfs: opting in or out of online repair

[Dave Chinner]

On Wed, Jul 24, 2024 at 02:38:52PM -0700, Darrick J. Wong wrote:
> Hi everyone,
> 
> Over in this thread:
> https://lore.kernel.org/linux-xfs/20240724131825.GA12250@lst.de/
> 
> I originally proposed turning on background xfs_scrub by default in
> Debian.  Christoph wondered if it was such a good idea to unleash this
> upon hoards of debian sid users, considering that the introduction of
> e2scrub by default didn't go entirely smoothly.  Debian policy is to
> enable packaged services at installation time, which is reasonable for
> httpd but perhaps not so much for experimental services.
> 
> I proposed modifying xfsprogs so that for Debian builds, it would spit
> out a separate "xfsprogs-self-healing" package containing only the files
> needed to activate the background scrub services by default.  People
> would have to opt-in to installing it... insofar as one can configure
> apt to pull in Suggests: and Recommends: packages.
> 
> Christoph suggested adding a compat flag to the superblock so that
> sysadmins can mark filesystems for (or against) background online
> repair.  Therefore, I'm proposing adding such a flag.  Before I start
> any work on that, I thought I'd sketch out what I'd add to make that
> happen:
> 
> On Disk Format Changes
> ======================
> 
> Add a new COMPAT flag XFS_SB_FEAT_COMPAT_SELF_HEALING.  If set,
> the background xfs_scrub_all service can periodically scan the
> filesystem and initiate repairs.  If not set, then background repairs
> are not allowed.

Maybe I'm missing something important - this doesn't feel like
on-disk format stuff. Why would having online repair enabled make
the fileystem unmountable on older kernels?

Hmmm. Could this be implemented with an xattr on the root inode
that says "self healing allowed"?

That way installing the xfsprogs-self-healing package can turn
itself on at install/upgrade time when we decide that online reapir
is no longer experimental without on-disk format implications. In
the mean time we could add an xfs_admin command to set/clear the
xattr on the mounted filesystem for the admin to chose to enable it.

> Note that administrator-initated scans (e.g. invoking xfs_scrub from the
> CLI) would not be blocked by this flag.
> 
> Question: Should this compat flag control background scrubs as well?

Probably. scrub is less intrusive, but I can see people wanting to
avoid it because it can have a perf impact. Could this be done with
a different xattr on the root inode?

> Userspace ABI Changes
> =====================

Answers from here are purely about mkfs based compat bit, though I'd
think this is unnecesary if we use an xattr.

> The first thing is adding a new CLI option to mkfs.xfs to set the value
> of the flag:
> 
> # mkfs.xfs -m self_healing=1
> 
> Question: If rmap and parent pointers are turned on, should we turn on
> self healing by default?

No.

> Question: If self healing is turned on, should we turn on rmap and
> parent pointers as well?

Yes.

> For xfs_scrub_all, we'd need a way to query the status of the self
> healing bit; this could be snuck into the xfs geometry ioctl as a new
> feature bit XFS_FSOP_GEOM_FLAGS_SELF_HEALING.
> 
> # xfs_info /dev/sdf
> meta-data=/dev/sdf     isize=512    agcount=1, agsize=268435455 blks
>          =             sectsz=512   attr=2, projid32bit=1
>          =             crc=1        finobt=1, sparse=1, rmapbt=1
>          =             reflink=1    bigtime=1 inobtcount=1 nrext64=1
>          =             exchange=0   metadir=0 self_healing=1
> data     =             bsize=4096   blocks=5192704, imaxpct=25
>          =             sunit=0      swidth=0 blks
> naming   =version 2    bsize=4096   ascii-ci=0, ftype=1, parent=0
> log      =internal log bsize=4096   blocks=16384, version=2
>          =             sectsz=512   sunit=0 blks, lazy-count=1
> realtime =none         extsz=4096   blocks=0, rtextents=0
>          =             rgcount=0    rgsize=0 extents

OK.

> I think we also want to enable sysadmins to change this on the fly.
> For mounted filesystems, this could be done through an administrative
> ioctl or mount options.  Offline, this would become another xfs_admin
> command.
> 
> # xfs_spaceman -c 'self_healing on' /usr
> # xfs_spaceman -c 'self_healing off' /tmp/extraspace
> 
> # xfs_admin --self-healing=on /dev/sda1
> # xfs_admin --self-healing=off /dev/sda2

I'd prefer the primary admin to be done through an xfs_admin
command.

> # mount /dev/sda2 /mnt -o self_healing
> 
> Thoughts?  Flames?

This really seems like information that userspace can maintain on
the filesystem itself, not something that belongs in the on-disk
format for the filesystem. Implementing it as an xattr the
xfsprogs-self-healing package maintains means no
kernel, mkfs, spaceman, db or repair changes are needed.

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

Re: [RFC] xfs: opting in or out of online repair

[Christoph Hellwig]

On Thu, Jul 25, 2024 at 12:05:26PM +1000, Dave Chinner wrote:
> Maybe I'm missing something important - this doesn't feel like
> on-disk format stuff. Why would having online repair enabled make
> the fileystem unmountable on older kernels?

Yes, that's the downside of the feature flag.

> Hmmm. Could this be implemented with an xattr on the root inode
> that says "self healing allowed"?

The annoying thing about stuff in the public file system namespace
is that chowning the root of a file system to a random user isn't
that uncommon, an that would give that user more privileges than
intended.  So it could not hust be a normal xattr but would have
to be a privileged one, and with my VFS hat on I'd really like
to avoid creating all these toally overloaded random non-user
namespace xattrs that are a complete mess.

One option would be an xattr on the metadir root (once we merge
that, hopefully for 6.12).  That would still require a new ioctl
or whatever interface to change (or carve out an exception to
the attr by handle interface), but it would not require kernel
and tools to fully understand it.

> > Note that administrator-initated scans (e.g. invoking xfs_scrub from the
> > CLI) would not be blocked by this flag.
> > 
> > Question: Should this compat flag control background scrubs as well?
> 
> Probably. scrub is less intrusive, but I can see people wanting to
> avoid it because it can have a perf impact. Could this be done with
> a different xattr on the root inode?

Yes, scrub vs repair should probably be separate.

Re: [RFC] xfs: opting in or out of online repair

[Dave Chinner]

On Thu, Jul 25, 2024 at 04:14:13PM +0200, Christoph Hellwig wrote:
> On Thu, Jul 25, 2024 at 12:05:26PM +1000, Dave Chinner wrote:
> > Maybe I'm missing something important - this doesn't feel like
> > on-disk format stuff. Why would having online repair enabled make
> > the fileystem unmountable on older kernels?
> 
> Yes, that's the downside of the feature flag.
> 
> > Hmmm. Could this be implemented with an xattr on the root inode
> > that says "self healing allowed"?
> 
> The annoying thing about stuff in the public file system namespace
> is that chowning the root of a file system to a random user isn't
> that uncommon, an that would give that user more privileges than
> intended.  So it could not hust be a normal xattr but would have
> to be a privileged one,


I'm not sure I understand what the problem is. We have a generic
xattr namespace for this sort of userspace sysadmin info already.

$ man 7 xattr
....
Trusted extended attributes
       Trusted extended attributes are visible and accessible only
       to processes that have the CAP_SYS_ADMIN capability.
       Attributes in this class are used to implement mechanisms
       in user space (i.e., outside the kernel) which keep
       information in extended attributes to which ordinary
       processes should not have access.

> and with my VFS hat on I'd really like
> to avoid creating all these toally overloaded random non-user
> namespace xattrs that are a complete mess.

There's no need to create a new xattr namespace at all here.
Userspace could manipulate a trusted.xfs.self_healing xattr to do
exactly what we need. It's automatically protected by
CAP_SYS_ADMIN in the init namespace, hence it provides all the
requirements that have been presented so far...

> One option would be an xattr on the metadir root (once we merge
> that, hopefully for 6.12).  That would still require a new ioctl
> or whatever interface to change (or carve out an exception to
> the attr by handle interface), but it would not require kernel
> and tools to fully understand it.

That seems awfully complex. It requires a new on-disk
filesystem format and a new user API to support storing this
userspace only information. I think this is more work than Darrick's
original compat flag idea.

This is information that is only relevant to a specific userspace
utility, and it can maintain that information itself without needing
to modify the on-disk format. Indeed, the information doesn't need
to be in the filesystem at all - it could just as easily be stored
in a config file in /etc/xfs/ that the xfs-self-healing start
scripts parse, yes? The config is still privileged information
requiring root to modify it, so it's no different to a trusted xattr
except for where the config information is stored.

Userspace package config information doesn't belong in the on-disk
format. It belongs in userspace configuration files (i.e. as file
data) or in trusted named xattrs (file metadata).

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

Re: [RFC] xfs: opting in or out of online repair

[Darrick J. Wong]

On Fri, Jul 26, 2024 at 08:33:02AM +1000, Dave Chinner wrote:
> On Thu, Jul 25, 2024 at 04:14:13PM +0200, Christoph Hellwig wrote:
> > On Thu, Jul 25, 2024 at 12:05:26PM +1000, Dave Chinner wrote:
> > > Maybe I'm missing something important - this doesn't feel like
> > > on-disk format stuff. Why would having online repair enabled make
> > > the fileystem unmountable on older kernels?
> > 
> > Yes, that's the downside of the feature flag.
> > 
> > > Hmmm. Could this be implemented with an xattr on the root inode
> > > that says "self healing allowed"?
> > 
> > The annoying thing about stuff in the public file system namespace
> > is that chowning the root of a file system to a random user isn't
> > that uncommon, an that would give that user more privileges than
> > intended.  So it could not hust be a normal xattr but would have
> > to be a privileged one,
> 
> 
> I'm not sure I understand what the problem is. We have a generic
> xattr namespace for this sort of userspace sysadmin info already.
> 
> $ man 7 xattr
> ....
> Trusted extended attributes
>        Trusted extended attributes are visible and accessible only
>        to processes that have the CAP_SYS_ADMIN capability.
>        Attributes in this class are used to implement mechanisms
>        in user space (i.e., outside the kernel) which keep
>        information in extended attributes to which ordinary
>        processes should not have access.
> 
> > and with my VFS hat on I'd really like
> > to avoid creating all these toally overloaded random non-user
> > namespace xattrs that are a complete mess.
> 
> There's no need to create a new xattr namespace at all here.
> Userspace could manipulate a trusted.xfs.self_healing xattr to do
> exactly what we need. It's automatically protected by
> CAP_SYS_ADMIN in the init namespace, hence it provides all the
> requirements that have been presented so far...

<nod> Ok, how about an ATTR_ROOT xattr "xfs.self_healing" that can be
one of "none", "check", or "repair".  No xattr means "check".

> > One option would be an xattr on the metadir root (once we merge
> > that, hopefully for 6.12).  That would still require a new ioctl
> > or whatever interface to change (or carve out an exception to
> > the attr by handle interface), but it would not require kernel
> > and tools to fully understand it.
> 
> That seems awfully complex. It requires a new on-disk
> filesystem format and a new user API to support storing this
> userspace only information. I think this is more work than Darrick's
> original compat flag idea.
> 
> This is information that is only relevant to a specific userspace
> utility, and it can maintain that information itself without needing
> to modify the on-disk format. Indeed, the information doesn't need
> to be in the filesystem at all - it could just as easily be stored
> in a config file in /etc/xfs/ that the xfs-self-healing start
> scripts parse, yes? The config is still privileged information
> requiring root to modify it, so it's no different to a trusted xattr
> except for where the config information is stored.

Sysadmins can already do that via systemctl mask, as I pointed out in
the earlier thread.  I think between that and the xattr we're covered.

> Userspace package config information doesn't belong in the on-disk
> format. It belongs in userspace configuration files (i.e. as file
> data) or in trusted named xattrs (file metadata).

Sounds reasonable to me.

--D

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

Re: [RFC] xfs: opting in or out of online repair

[Dave Chinner]

On Thu, Jul 25, 2024 at 05:41:54PM -0700, Darrick J. Wong wrote:
> On Fri, Jul 26, 2024 at 08:33:02AM +1000, Dave Chinner wrote:
> > On Thu, Jul 25, 2024 at 04:14:13PM +0200, Christoph Hellwig wrote:
> > > On Thu, Jul 25, 2024 at 12:05:26PM +1000, Dave Chinner wrote:
> > > > Maybe I'm missing something important - this doesn't feel like
> > > > on-disk format stuff. Why would having online repair enabled make
> > > > the fileystem unmountable on older kernels?
> > > 
> > > Yes, that's the downside of the feature flag.
> > > 
> > > > Hmmm. Could this be implemented with an xattr on the root inode
> > > > that says "self healing allowed"?
> > > 
> > > The annoying thing about stuff in the public file system namespace
> > > is that chowning the root of a file system to a random user isn't
> > > that uncommon, an that would give that user more privileges than
> > > intended.  So it could not hust be a normal xattr but would have
> > > to be a privileged one,
> > 
> > 
> > I'm not sure I understand what the problem is. We have a generic
> > xattr namespace for this sort of userspace sysadmin info already.
> > 
> > $ man 7 xattr
> > ....
> > Trusted extended attributes
> >        Trusted extended attributes are visible and accessible only
> >        to processes that have the CAP_SYS_ADMIN capability.
> >        Attributes in this class are used to implement mechanisms
> >        in user space (i.e., outside the kernel) which keep
> >        information in extended attributes to which ordinary
> >        processes should not have access.
> > 
> > > and with my VFS hat on I'd really like
> > > to avoid creating all these toally overloaded random non-user
> > > namespace xattrs that are a complete mess.
> > 
> > There's no need to create a new xattr namespace at all here.
> > Userspace could manipulate a trusted.xfs.self_healing xattr to do
> > exactly what we need. It's automatically protected by
> > CAP_SYS_ADMIN in the init namespace, hence it provides all the
> > requirements that have been presented so far...
> 
> <nod> Ok, how about an ATTR_ROOT xattr "xfs.self_healing" that can be
> one of "none", "check", or "repair".  No xattr means "check".

Sounds good to me.

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

Re: [RFC] xfs: opting in or out of online repair

[Christoph Hellwig]

On Thu, Jul 25, 2024 at 05:41:54PM -0700, Darrick J. Wong wrote:
> > requirements that have been presented so far...
> 
> <nod> Ok, how about an ATTR_ROOT xattr "xfs.self_healing" that can be
> one of "none", "check", or "repair".  No xattr means "check".

Fine with me.

Re: [RFC] xfs: opting in or out of online repair

[Darrick J. Wong]

On Fri, Jul 26, 2024 at 03:59:48PM +0200, Christoph Hellwig wrote:
> On Thu, Jul 25, 2024 at 05:41:54PM -0700, Darrick J. Wong wrote:
> > > requirements that have been presented so far...
> > 
> > <nod> Ok, how about an ATTR_ROOT xattr "xfs.self_healing" that can be
> > one of "none", "check", or "repair".  No xattr means "check".
> 
> Fine with me.

I will also add "optimize" for -p.

--D