Re: [PATCH 1/1] xfs: Skip repetitive warnings about mount options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 22, 2021 at 01:28:30PM -0800, Darrick J. Wong wrote:
> On Sat, Feb 20, 2021 at 11:15:49PM +0100, Pavel Reichl wrote:
> > Skip the warnings about mount option being deprecated if we are
> > remounting and deprecated option state is not changing.
> > 
> > Bug: https://bugzilla.kernel.org/show_bug.cgi?id=211605
> > Fix-suggested-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> > Signed-off-by: Pavel Reichl <preichl@xxxxxxxxxx>
> > ---
> >  fs/xfs/xfs_super.c | 23 +++++++++++++++++++----
> >  1 file changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 813be879a5e5..6724a7018d1f 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -1169,6 +1169,13 @@ xfs_fs_parse_param(
> >  	struct fs_parse_result	result;
> >  	int			size = 0;
> >  	int			opt;
> > +	uint64_t                prev_m_flags = 0; /* Mount flags of prev. mount */
> 
> Nit: spaces here^^^^^^^^^^^^^^^^ should be tabs.
> 
> > +	bool			remounting = fc->purpose & FS_CONTEXT_FOR_RECONFIGURE;
> > +
> > +	/* if reconfiguring then get mount flags of previous flags */
> > +	if (remounting) {
> > +		prev_m_flags  = XFS_M(fc->root->d_sb)->m_flags;
> > +	}
> >  
> >  	opt = fs_parse(fc, xfs_fs_parameters, param, &result);
> >  	if (opt < 0)
> > @@ -1294,19 +1301,27 @@ xfs_fs_parse_param(
> >  #endif
> >  	/* Following mount options will be removed in September 2025 */
> >  	case Opt_ikeep:
> > -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > +		if (!remounting ||  !(prev_m_flags & XFS_MOUNT_IKEEP)) {
> > +			xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > +		}
> 
> /me wonders if these could be refactored into a common helper, though I
> can't really think of anything less clunky than:
> 
> static inline void
> xfs_fs_warn_deprecated(
> 	struct fs_context	*fc,
> 	struct fs_parameter	*param)
> 	uint64_t		flag,
> 	bool			value);
> {
> 	uint64_t		prev_m_flags;
> 
> 	if (!(fc->purpose & FS_CONTEXT_FOR_RECONFIGURE))
> 		goto warn;
> 	prev_m_flags  = XFS_M(fc->root->d_sb)->m_flags;
> 	if (!!(prev_m_flags & flag) == value)
> 		goto warn;
> 	return;
> warn:
> 	xfs_warn(mp, "%s mount option is deprecated.", param->key);
> }
> ...
> 	case Opt_ikeep:
> 		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, true);
> 		mp->m_flags |= XFS_MOUNT_IKEEP;
> 		break;
> 	case Opt_noikeep:
> 		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, false);
> 		mp->m_flags &= ~XFS_MOUNT_IKEEP;
> 		break;

	case Opt_ikeep:
		xfs_fs_warn_deprecated(fc, param,
				(mp->m_flags & XFS_MOUNT_IKEEP), true);
		mp->m_flags |= XFS_MOUNT_IKEEP;
		break;
	case Opt_noikeep:
		xfs_fs_warn_deprecated(fc, param,
				(mp->m_flags & XFS_MOUNT_IKEEP), false);
		mp->m_flags &= ~XFS_MOUNT_IKEEP;
		break;

static inline void
xfs_fs_warn_deprecated(
	struct fs_context	*fc,
	struct fs_parameter	*param)
	bool			old_value,
	bool			new_value);
{
	if ((fc->purpose & FS_CONTEXT_FOR_RECONFIGURE) &&
	    old_value == new_value)
		return;
	xfs_warn(mp, "%s mount option is deprecated.", param->key);
}

-Dave.

> 
> Thoughts?
> 
> --D
> 
> >  		mp->m_flags |= XFS_MOUNT_IKEEP;
> >  		return 0;
> >  	case Opt_noikeep:
> > -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > +		if (!remounting || prev_m_flags & XFS_MOUNT_IKEEP) {
> > +			xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > +		}
> >  		mp->m_flags &= ~XFS_MOUNT_IKEEP;
> >  		return 0;
> >  	case Opt_attr2:
> > -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > +		if (!remounting || !(prev_m_flags & XFS_MOUNT_ATTR2)) {
> > +			xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > +		}
> >  		mp->m_flags |= XFS_MOUNT_ATTR2;
> >  		return 0;
> >  	case Opt_noattr2:
> > -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > +		if (!remounting || !(prev_m_flags & XFS_MOUNT_NOATTR2)) {
> > +			xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > +		}
> >  		mp->m_flags &= ~XFS_MOUNT_ATTR2;
> >  		mp->m_flags |= XFS_MOUNT_NOATTR2;
> >  		return 0;
> > -- 
> > 2.29.2
> > 
> 

-- 
Dave Chinner
david@xxxxxxxxxxxxx



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux