Re: [PATCH] xfs: restore speculative_cow_prealloc_lifetime sysctl

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

On Sat, Feb 13, 2021 at 08:21:47AM +1100, Dave Chinner wrote:
> On Fri, Feb 12, 2021 at 09:24:36AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > In commit 9669f51de5c0 I tried to get rid of the undocumented cow gc
> > lifetime knob.  The knob's function was never documented and it now
> > doesn't really have a function since eof and cow gc have been
> > consolidated.
> > 
> > Regrettably, xfs/231 relies on it and regresses on for-next.  I did not
> > succeed at getting far enough through fstests patch review for the fixup
> > to land in time.
> > 
> > Restore the sysctl knob, document what it did (does?), put it on the
> > deprecation schedule, and rip out a redundant function.
> > 
> > Fixes: 9669f51de5c0 ("xfs: consolidate the eofblocks and cowblocks workers")
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> 
> Thnanks for doing this, Darrick!
> 
> >  STATIC int
> > -xfs_deprecate_irix_sgid_inherit_proc_handler(
> > +xfs_deprecated_dointvec_minmax(
> >  	struct ctl_table	*ctl,
> >  	int			write,
> >  	void			*buffer,
> > @@ -60,23 +60,7 @@ xfs_deprecate_irix_sgid_inherit_proc_handler(
> >  {
> >  	if (write) {
> >  		printk_once(KERN_WARNING
> > -				"XFS: " "%s sysctl option is deprecated.\n",
> > -				ctl->procname);
> > -	}
> > -	return proc_dointvec_minmax(ctl, write, buffer, lenp, ppos);
> > -}
> > -
> > -STATIC int
> > -xfs_deprecate_irix_symlink_mode_proc_handler(
> > -	struct ctl_table	*ctl,
> > -	int			write,
> > -	void			*buffer,
> > -	size_t			*lenp,
> > -	loff_t			*ppos)
> > -{
> > -	if (write) {
> > -		printk_once(KERN_WARNING
> > -				"XFS: " "%s sysctl option is deprecated.\n",
> > +				"XFS: %s sysctl option is deprecated.\n",
> >  				ctl->procname);
> >  	}
> 
> The use of printk_once means it will only warn on the first
> deprecated sysctl written to, not the first write to each of the
> deprecated sysctls.
> 
> Is there any evidence that anyone is writing these with any regualr
> frequency, and if not, maybe just a ratelimited warning is
> sufficient here?

I've never seen any evidence that people hammer on sysctls with great
frequency.  Will change to printk_ratelimited.

--D

> Otherwise looks fine.
> 
> Cheers,
> 
> Dave.
> 
> -- 
> Dave Chinner
> david@fromorbit.com