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

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, May 23, 2022 at 12:51:04PM +1000, Dave Chinner wrote:
> 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....

Ok, I'll do that tomorrow.

> > Also, any thoughts on the last two patches?
> 
> Not yet, been doing tree and test stuff so far today.

<nod>

--D

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