Re: [PATCH 3/5] xfs: warn about LARP once per day

[Dave Chinner <david@fromorbit.com>]

On Sun, May 22, 2022 at 06:16:57PM -0700, Darrick J. Wong wrote:
> On Mon, May 23, 2022 at 08:54:04AM +1000, Dave Chinner wrote:
> > On Sun, May 22, 2022 at 08:28:30AM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Since LARP is an experimental debug-only feature, we should try to warn
> > > about it being in use once per day, not once per reboot.
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > >  fs/xfs/xfs_log.c |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > 
> > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > > index 9dc748abdf33..edd077e055d5 100644
> > > --- a/fs/xfs/xfs_log.c
> > > +++ b/fs/xfs/xfs_log.c
> > > @@ -3910,8 +3910,8 @@ xfs_attr_use_log_assist(
> > >  	if (error)
> > >  		goto drop_incompat;
> > >  
> > > -	xfs_warn_once(mp,
> > > -"EXPERIMENTAL logged extended attributes feature added. Use at your own risk!");
> > > +	xfs_warn_daily(mp,
> > > + "EXPERIMENTAL logged extended attributes feature added. Use at your own risk!");
> > 
> > I think even this is wrong. We need this to warn once per *mount*
> > like we do with all other experimental features, not once or once
> > per day.  i.e. we could have 10 filesystems mounted and only one of
> > them will warn that EXPERIMENTAL features are in use.
> > 
> > We really need all filesystems that use an experimental feature to
> > warn about the use of said feature, not just a single filesystem.
> > That will make this consistent with the way we warn once (and once
> > only) at mount time about EXPERIMENTAL features that are enabled at
> > mount time...
> 
> Ok.  I was thinking we could have an unsigned long m_warned then all
> we'd need to do is convert the existing three callers (scrub, shrink,
> larp) to something like:
> 
> 	if (!test_and_set_bit(XFS_WARNED_FUBAR, &mp->m_warned))
> 		xfs_warn(mp,
>  "EXPERIMENTAL fubar feature is enabled, use at your own risk!");

Just use an m_opstate bit. We've got heaps, and these will
eventually get reclaimed anyway....

> Also, any thoughts on the last two patches?

Not yet, been doing tree and test stuff so far today.

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