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@xxxxxxxxxxxxx
