Re: [PATCH 03/28] xfs: define the on-disk format for the metadir feature

[Dave Chinner <david@fromorbit.com>]

On Tue, Oct 15, 2024 at 05:20:51PM -0700, Darrick J. Wong wrote:
> On Wed, Oct 16, 2024 at 10:03:24AM +1100, Dave Chinner wrote:
> > On Tue, Oct 15, 2024 at 11:25:41AM -0700, Darrick J. Wong wrote:
> > > > > +	if (xfs_has_metadir(mp))
> > > > > +		xfs_warn(mp,
> > > > > +"EXPERIMENTAL metadata directory feature in use. Use at your own risk!");
> > > > > +
> > > > 
> > > > We really need a 'xfs_mark_experimental(mp, "Metadata directory")'
> > > > function to format all these experimental feature warnings the same
> > > > way....
> > > 
> > > We already have xfs_warn_mount for functionality that isn't sb feature
> > > bits.  Maybe xfs_warn_feat?
> > 
> > xfs_warn_mount() is only used for experimental warnings, so maybe we
> > should simply rename that xfs_mark_experiental().  Then we can use
> > it's inherent "warn once" behaviour for all the places where we
> > issue an experimental warning regardless of how the experimental
> > feature is enabled/detected. 
> > 
> > This means we'd have a single location that formats all experimental
> > feature warnings the same way. Having a single function explicitly
> > for this makes it trivial to audit and manage all the experimental
> > features supported by a given kernel version because we are no
> > longer reliant on grepping for custom format strings to find
> > experimental features.
> > 
> > It also means that adding a kernel taint flag indicating that the
> > kernel is running experimental code is trivial to do...
> 
> ...and I guess this means you can discover which forbidden features are
> turned on from crash dumps.  Ok, sounds good to me.

Yes, though I don't consider experimental features as "forbidden".

This is more about enabling experimental filesystem features to be
shipped under tech preview constraints(*). Knowing that an
experimental feature is in use will help manage support expectations
and workload. This, in turn, will also allow us to stay closer to
the upstream XFS code base and behaviour....

> Do you want it to return an int so that you (as a distributor, not you
> personally) can decide that nobody gets to use the experimental
> features?

I considered suggesting that earlier, but if we want to disable
specific experimental features we'll have to patch the kernel
anyway. Hence I don't think there's any reason for having the
upstream code doing anything other than tracking what experimental
features are in use.

-Dave.

(*) https://access.redhat.com/solutions/21101

"Technology Preview features are not fully supported, may not be
functionally complete, and are not suitable for deployment in
production. However, these features are provided to the customer as
a courtesy and the primary goal is for the feature to gain wider
exposure with the goal of full support in the future."

-- 
Dave Chinner
david@fromorbit.com