[PATCH v2 0/2] xfs: Skip repetitive warnings about mount options

[PATCH v2 0/2] xfs: Skip repetitive warnings about mount options

[Pavel Reichl]

At least some version of mount will look in /proc/mounts and send in all of the 
options that it finds as part of a remount command. We also /do/ still emit
"attr2" in /proc/mounts (as we probably should), so remount passes that back
in, and we emit a warning, which is not great.

In other words mount passes in "attr2" and the kernel emits a deprecation
warning for attr2, even though the user/admin never explicitly asked for the
option.

So, lets skip the warning if (we are remounting && deprecated option
state is not changing).

I also attached test for xfstests that I used for testing (the test
will be proposed on xfstests-list after/if this patch is merged).

V2 vs. V1
Kernel:
* Added new patch that renames mp to parsing_mp in xfs_fs_parse_param()
* Added new function xfs_fs_warn_deprecated() to encapsulate the logic for displaying the deprecation warning.
* Fixed some white space issues.

Test:
* Skip test on old kernels that don't print any deprecation warning. 

Pavel Reichl (2):
  xfs: rename variable mp to parsing_mp
  xfs: Skip repetitive warnings about mount options

 fs/xfs/xfs_super.c | 118 +++++++++++++++++++++++++--------------------
 1 file changed, 67 insertions(+), 51 deletions(-)

-- 
2.29.2

[PATCH v2] xfs: Add test for printing deprec. mount options

[Pavel Reichl]

Verify that warnings about deprecated mount options are properly
printed.

Verify that no excessive warnings are printed during remounts.

Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
 tests/xfs/528     | 86 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/528.out |  2 ++
 tests/xfs/group   |  1 +
 3 files changed, 89 insertions(+)
 create mode 100755 tests/xfs/528
 create mode 100644 tests/xfs/528.out

diff --git a/tests/xfs/528 b/tests/xfs/528
new file mode 100755
index 00000000..111a3770
--- /dev/null
+++ b/tests/xfs/528
@@ -0,0 +1,86 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Red Hat, Inc.. All Rights Reserved.
+#
+# FS QA Test 528
+#
+# Verify that warnings about deprecated mount options are properly printed.
+#
+# Verify that no excessive warnings are printed during remounts.
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+_require_check_dmesg
+_supported_fs xfs
+_require_scratch
+
+log_tag()
+{
+	echo "fstests $seqnum [tag]" > /dev/kmsg
+}
+
+dmesg_since_test_tag()
+{
+	dmesg | tac | sed -ne "0,\#fstests $seqnum \[tag\]#p" | \
+		tac
+}
+
+check_dmesg_for_since_tag()
+{
+	dmesg_since_test_tag | egrep -q "$1"
+}
+
+echo "Silence is golden."
+
+
+# Skip old kernels that did not print the warning yet
+log_tag
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount -o attr2
+umount $SCRATCH_MNT
+check_dmesg_for_since_tag "XFS: attr2 mount option is deprecated" || \
+	_notrun "Deprecation warning are not printed at all."
+
+# Test mount with default options (attr2 and noikeep) and remount with
+# 2 groups of options
+# 1) the defaults (attr2, noikeep)
+# 2) non defaults (noattr2, ikeep)
+_scratch_mount
+for VAR in {attr2,noikeep}; do
+	log_tag
+	mount -o $VAR,remount $SCRATCH_MNT
+	check_dmesg_for_since_tag "XFS: $VAR mount option is deprecated." && \
+		echo "Should not be able to find deprecation warning for $VAR"
+done
+for VAR in {noattr2,ikeep}; do
+	log_tag
+	mount -o $VAR,remount $SCRATCH_MNT
+	check_dmesg_for_since_tag "XFS: $VAR mount option is deprecated" || \
+		echo "Could not find deprecation warning for $VAR"
+done
+umount $SCRATCH_MNT
+
+# success, all done
+status=0
+exit
+
diff --git a/tests/xfs/528.out b/tests/xfs/528.out
new file mode 100644
index 00000000..762dccc0
--- /dev/null
+++ b/tests/xfs/528.out
@@ -0,0 +1,2 @@
+QA output created by 528
+Silence is golden.
diff --git a/tests/xfs/group b/tests/xfs/group
index e861cec9..ad3bd223 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -525,3 +525,4 @@
 525 auto quick mkfs
 526 auto quick mkfs
 527 auto quick quota
+528 auto quick mount
-- 
2.29.2

[PATCH v2 1/2] xfs: rename variable mp to parsing_mp

[Pavel Reichl]

Rename mp variable to parsisng_mp so it is easy to distinguish
between current mount point handle and handle for mount point
which mount options are being parsed.

Suggested-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
 fs/xfs/xfs_super.c | 102 ++++++++++++++++++++++-----------------------
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 813be879a5e5..7e281d1139dc 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1165,7 +1165,7 @@ xfs_fs_parse_param(
 	struct fs_context	*fc,
 	struct fs_parameter	*param)
 {
-	struct xfs_mount	*mp = fc->s_fs_info;
+	struct xfs_mount	*parsing_mp = fc->s_fs_info;
 	struct fs_parse_result	result;
 	int			size = 0;
 	int			opt;
@@ -1176,142 +1176,142 @@ xfs_fs_parse_param(
 
 	switch (opt) {
 	case Opt_logbufs:
-		mp->m_logbufs = result.uint_32;
+		parsing_mp->m_logbufs = result.uint_32;
 		return 0;
 	case Opt_logbsize:
-		if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
+		if (suffix_kstrtoint(param->string, 10, &parsing_mp->m_logbsize))
 			return -EINVAL;
 		return 0;
 	case Opt_logdev:
-		kfree(mp->m_logname);
-		mp->m_logname = kstrdup(param->string, GFP_KERNEL);
-		if (!mp->m_logname)
+		kfree(parsing_mp->m_logname);
+		parsing_mp->m_logname = kstrdup(param->string, GFP_KERNEL);
+		if (!parsing_mp->m_logname)
 			return -ENOMEM;
 		return 0;
 	case Opt_rtdev:
-		kfree(mp->m_rtname);
-		mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
-		if (!mp->m_rtname)
+		kfree(parsing_mp->m_rtname);
+		parsing_mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
+		if (!parsing_mp->m_rtname)
 			return -ENOMEM;
 		return 0;
 	case Opt_allocsize:
 		if (suffix_kstrtoint(param->string, 10, &size))
 			return -EINVAL;
-		mp->m_allocsize_log = ffs(size) - 1;
-		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
+		parsing_mp->m_allocsize_log = ffs(size) - 1;
+		parsing_mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
 		return 0;
 	case Opt_grpid:
 	case Opt_bsdgroups:
-		mp->m_flags |= XFS_MOUNT_GRPID;
+		parsing_mp->m_flags |= XFS_MOUNT_GRPID;
 		return 0;
 	case Opt_nogrpid:
 	case Opt_sysvgroups:
-		mp->m_flags &= ~XFS_MOUNT_GRPID;
+		parsing_mp->m_flags &= ~XFS_MOUNT_GRPID;
 		return 0;
 	case Opt_wsync:
-		mp->m_flags |= XFS_MOUNT_WSYNC;
+		parsing_mp->m_flags |= XFS_MOUNT_WSYNC;
 		return 0;
 	case Opt_norecovery:
-		mp->m_flags |= XFS_MOUNT_NORECOVERY;
+		parsing_mp->m_flags |= XFS_MOUNT_NORECOVERY;
 		return 0;
 	case Opt_noalign:
-		mp->m_flags |= XFS_MOUNT_NOALIGN;
+		parsing_mp->m_flags |= XFS_MOUNT_NOALIGN;
 		return 0;
 	case Opt_swalloc:
-		mp->m_flags |= XFS_MOUNT_SWALLOC;
+		parsing_mp->m_flags |= XFS_MOUNT_SWALLOC;
 		return 0;
 	case Opt_sunit:
-		mp->m_dalign = result.uint_32;
+		parsing_mp->m_dalign = result.uint_32;
 		return 0;
 	case Opt_swidth:
-		mp->m_swidth = result.uint_32;
+		parsing_mp->m_swidth = result.uint_32;
 		return 0;
 	case Opt_inode32:
-		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+		parsing_mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
 		return 0;
 	case Opt_inode64:
-		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
+		parsing_mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
 		return 0;
 	case Opt_nouuid:
-		mp->m_flags |= XFS_MOUNT_NOUUID;
+		parsing_mp->m_flags |= XFS_MOUNT_NOUUID;
 		return 0;
 	case Opt_largeio:
-		mp->m_flags |= XFS_MOUNT_LARGEIO;
+		parsing_mp->m_flags |= XFS_MOUNT_LARGEIO;
 		return 0;
 	case Opt_nolargeio:
-		mp->m_flags &= ~XFS_MOUNT_LARGEIO;
+		parsing_mp->m_flags &= ~XFS_MOUNT_LARGEIO;
 		return 0;
 	case Opt_filestreams:
-		mp->m_flags |= XFS_MOUNT_FILESTREAMS;
+		parsing_mp->m_flags |= XFS_MOUNT_FILESTREAMS;
 		return 0;
 	case Opt_noquota:
-		mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
-		mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
-		mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
+		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
+		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
+		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
 		return 0;
 	case Opt_quota:
 	case Opt_uquota:
 	case Opt_usrquota:
-		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
+		parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
 				 XFS_UQUOTA_ENFD);
 		return 0;
 	case Opt_qnoenforce:
 	case Opt_uqnoenforce:
-		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
-		mp->m_qflags &= ~XFS_UQUOTA_ENFD;
+		parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
+		parsing_mp->m_qflags &= ~XFS_UQUOTA_ENFD;
 		return 0;
 	case Opt_pquota:
 	case Opt_prjquota:
-		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
+		parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
 				 XFS_PQUOTA_ENFD);
 		return 0;
 	case Opt_pqnoenforce:
-		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
-		mp->m_qflags &= ~XFS_PQUOTA_ENFD;
+		parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
+		parsing_mp->m_qflags &= ~XFS_PQUOTA_ENFD;
 		return 0;
 	case Opt_gquota:
 	case Opt_grpquota:
-		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
+		parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
 				 XFS_GQUOTA_ENFD);
 		return 0;
 	case Opt_gqnoenforce:
-		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
-		mp->m_qflags &= ~XFS_GQUOTA_ENFD;
+		parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
+		parsing_mp->m_qflags &= ~XFS_GQUOTA_ENFD;
 		return 0;
 	case Opt_discard:
-		mp->m_flags |= XFS_MOUNT_DISCARD;
+		parsing_mp->m_flags |= XFS_MOUNT_DISCARD;
 		return 0;
 	case Opt_nodiscard:
-		mp->m_flags &= ~XFS_MOUNT_DISCARD;
+		parsing_mp->m_flags &= ~XFS_MOUNT_DISCARD;
 		return 0;
 #ifdef CONFIG_FS_DAX
 	case Opt_dax:
-		xfs_mount_set_dax_mode(mp, XFS_DAX_ALWAYS);
+		xfs_mount_set_dax_mode(parsing_mp, XFS_DAX_ALWAYS);
 		return 0;
 	case Opt_dax_enum:
-		xfs_mount_set_dax_mode(mp, result.uint_32);
+		xfs_mount_set_dax_mode(parsing_mp, result.uint_32);
 		return 0;
 #endif
 	/* Following mount options will be removed in September 2025 */
 	case Opt_ikeep:
-		xfs_warn(mp, "%s mount option is deprecated.", param->key);
-		mp->m_flags |= XFS_MOUNT_IKEEP;
+		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		parsing_mp->m_flags |= XFS_MOUNT_IKEEP;
 		return 0;
 	case Opt_noikeep:
-		xfs_warn(mp, "%s mount option is deprecated.", param->key);
-		mp->m_flags &= ~XFS_MOUNT_IKEEP;
+		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		parsing_mp->m_flags &= ~XFS_MOUNT_IKEEP;
 		return 0;
 	case Opt_attr2:
-		xfs_warn(mp, "%s mount option is deprecated.", param->key);
-		mp->m_flags |= XFS_MOUNT_ATTR2;
+		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		parsing_mp->m_flags |= XFS_MOUNT_ATTR2;
 		return 0;
 	case Opt_noattr2:
-		xfs_warn(mp, "%s mount option is deprecated.", param->key);
-		mp->m_flags &= ~XFS_MOUNT_ATTR2;
-		mp->m_flags |= XFS_MOUNT_NOATTR2;
+		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		parsing_mp->m_flags &= ~XFS_MOUNT_ATTR2;
+		parsing_mp->m_flags |= XFS_MOUNT_NOATTR2;
 		return 0;
 	default:
-		xfs_warn(mp, "unknown mount option [%s].", param->key);
+		xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
 		return -EINVAL;
 	}
 
-- 
2.29.2

Re: [PATCH v2 1/2] xfs: rename variable mp to parsing_mp

[Darrick J. Wong]

On Wed, Feb 24, 2021 at 10:43:22PM +0100, Pavel Reichl wrote:
> Rename mp variable to parsisng_mp so it is easy to distinguish
> between current mount point handle and handle for mount point
> which mount options are being parsed.
> 
> Suggested-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>

Seems reasonable...

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_super.c | 102 ++++++++++++++++++++++-----------------------
>  1 file changed, 51 insertions(+), 51 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 813be879a5e5..7e281d1139dc 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1165,7 +1165,7 @@ xfs_fs_parse_param(
>  	struct fs_context	*fc,
>  	struct fs_parameter	*param)
>  {
> -	struct xfs_mount	*mp = fc->s_fs_info;
> +	struct xfs_mount	*parsing_mp = fc->s_fs_info;
>  	struct fs_parse_result	result;
>  	int			size = 0;
>  	int			opt;
> @@ -1176,142 +1176,142 @@ xfs_fs_parse_param(
>  
>  	switch (opt) {
>  	case Opt_logbufs:
> -		mp->m_logbufs = result.uint_32;
> +		parsing_mp->m_logbufs = result.uint_32;
>  		return 0;
>  	case Opt_logbsize:
> -		if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
> +		if (suffix_kstrtoint(param->string, 10, &parsing_mp->m_logbsize))
>  			return -EINVAL;
>  		return 0;
>  	case Opt_logdev:
> -		kfree(mp->m_logname);
> -		mp->m_logname = kstrdup(param->string, GFP_KERNEL);
> -		if (!mp->m_logname)
> +		kfree(parsing_mp->m_logname);
> +		parsing_mp->m_logname = kstrdup(param->string, GFP_KERNEL);
> +		if (!parsing_mp->m_logname)
>  			return -ENOMEM;
>  		return 0;
>  	case Opt_rtdev:
> -		kfree(mp->m_rtname);
> -		mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
> -		if (!mp->m_rtname)
> +		kfree(parsing_mp->m_rtname);
> +		parsing_mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
> +		if (!parsing_mp->m_rtname)
>  			return -ENOMEM;
>  		return 0;
>  	case Opt_allocsize:
>  		if (suffix_kstrtoint(param->string, 10, &size))
>  			return -EINVAL;
> -		mp->m_allocsize_log = ffs(size) - 1;
> -		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
> +		parsing_mp->m_allocsize_log = ffs(size) - 1;
> +		parsing_mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
>  		return 0;
>  	case Opt_grpid:
>  	case Opt_bsdgroups:
> -		mp->m_flags |= XFS_MOUNT_GRPID;
> +		parsing_mp->m_flags |= XFS_MOUNT_GRPID;
>  		return 0;
>  	case Opt_nogrpid:
>  	case Opt_sysvgroups:
> -		mp->m_flags &= ~XFS_MOUNT_GRPID;
> +		parsing_mp->m_flags &= ~XFS_MOUNT_GRPID;
>  		return 0;
>  	case Opt_wsync:
> -		mp->m_flags |= XFS_MOUNT_WSYNC;
> +		parsing_mp->m_flags |= XFS_MOUNT_WSYNC;
>  		return 0;
>  	case Opt_norecovery:
> -		mp->m_flags |= XFS_MOUNT_NORECOVERY;
> +		parsing_mp->m_flags |= XFS_MOUNT_NORECOVERY;
>  		return 0;
>  	case Opt_noalign:
> -		mp->m_flags |= XFS_MOUNT_NOALIGN;
> +		parsing_mp->m_flags |= XFS_MOUNT_NOALIGN;
>  		return 0;
>  	case Opt_swalloc:
> -		mp->m_flags |= XFS_MOUNT_SWALLOC;
> +		parsing_mp->m_flags |= XFS_MOUNT_SWALLOC;
>  		return 0;
>  	case Opt_sunit:
> -		mp->m_dalign = result.uint_32;
> +		parsing_mp->m_dalign = result.uint_32;
>  		return 0;
>  	case Opt_swidth:
> -		mp->m_swidth = result.uint_32;
> +		parsing_mp->m_swidth = result.uint_32;
>  		return 0;
>  	case Opt_inode32:
> -		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> +		parsing_mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
>  		return 0;
>  	case Opt_inode64:
> -		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> +		parsing_mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
>  		return 0;
>  	case Opt_nouuid:
> -		mp->m_flags |= XFS_MOUNT_NOUUID;
> +		parsing_mp->m_flags |= XFS_MOUNT_NOUUID;
>  		return 0;
>  	case Opt_largeio:
> -		mp->m_flags |= XFS_MOUNT_LARGEIO;
> +		parsing_mp->m_flags |= XFS_MOUNT_LARGEIO;
>  		return 0;
>  	case Opt_nolargeio:
> -		mp->m_flags &= ~XFS_MOUNT_LARGEIO;
> +		parsing_mp->m_flags &= ~XFS_MOUNT_LARGEIO;
>  		return 0;
>  	case Opt_filestreams:
> -		mp->m_flags |= XFS_MOUNT_FILESTREAMS;
> +		parsing_mp->m_flags |= XFS_MOUNT_FILESTREAMS;
>  		return 0;
>  	case Opt_noquota:
> -		mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> -		mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> -		mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> +		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> +		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> +		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
>  		return 0;
>  	case Opt_quota:
>  	case Opt_uquota:
>  	case Opt_usrquota:
> -		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> +		parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
>  				 XFS_UQUOTA_ENFD);
>  		return 0;
>  	case Opt_qnoenforce:
>  	case Opt_uqnoenforce:
> -		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> -		mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> +		parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> +		parsing_mp->m_qflags &= ~XFS_UQUOTA_ENFD;
>  		return 0;
>  	case Opt_pquota:
>  	case Opt_prjquota:
> -		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> +		parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
>  				 XFS_PQUOTA_ENFD);
>  		return 0;
>  	case Opt_pqnoenforce:
> -		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> -		mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> +		parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> +		parsing_mp->m_qflags &= ~XFS_PQUOTA_ENFD;
>  		return 0;
>  	case Opt_gquota:
>  	case Opt_grpquota:
> -		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> +		parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
>  				 XFS_GQUOTA_ENFD);
>  		return 0;
>  	case Opt_gqnoenforce:
> -		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> -		mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> +		parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> +		parsing_mp->m_qflags &= ~XFS_GQUOTA_ENFD;
>  		return 0;
>  	case Opt_discard:
> -		mp->m_flags |= XFS_MOUNT_DISCARD;
> +		parsing_mp->m_flags |= XFS_MOUNT_DISCARD;
>  		return 0;
>  	case Opt_nodiscard:
> -		mp->m_flags &= ~XFS_MOUNT_DISCARD;
> +		parsing_mp->m_flags &= ~XFS_MOUNT_DISCARD;
>  		return 0;
>  #ifdef CONFIG_FS_DAX
>  	case Opt_dax:
> -		xfs_mount_set_dax_mode(mp, XFS_DAX_ALWAYS);
> +		xfs_mount_set_dax_mode(parsing_mp, XFS_DAX_ALWAYS);
>  		return 0;
>  	case Opt_dax_enum:
> -		xfs_mount_set_dax_mode(mp, result.uint_32);
> +		xfs_mount_set_dax_mode(parsing_mp, result.uint_32);
>  		return 0;
>  #endif
>  	/* Following mount options will be removed in September 2025 */
>  	case Opt_ikeep:
> -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> -		mp->m_flags |= XFS_MOUNT_IKEEP;
> +		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		parsing_mp->m_flags |= XFS_MOUNT_IKEEP;
>  		return 0;
>  	case Opt_noikeep:
> -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> -		mp->m_flags &= ~XFS_MOUNT_IKEEP;
> +		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		parsing_mp->m_flags &= ~XFS_MOUNT_IKEEP;
>  		return 0;
>  	case Opt_attr2:
> -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> -		mp->m_flags |= XFS_MOUNT_ATTR2;
> +		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		parsing_mp->m_flags |= XFS_MOUNT_ATTR2;
>  		return 0;
>  	case Opt_noattr2:
> -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> -		mp->m_flags &= ~XFS_MOUNT_ATTR2;
> -		mp->m_flags |= XFS_MOUNT_NOATTR2;
> +		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		parsing_mp->m_flags &= ~XFS_MOUNT_ATTR2;
> +		parsing_mp->m_flags |= XFS_MOUNT_NOATTR2;
>  		return 0;
>  	default:
> -		xfs_warn(mp, "unknown mount option [%s].", param->key);
> +		xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
>  		return -EINVAL;
>  	}
>  
> -- 
> 2.29.2
> 

Re: [PATCH v2 1/2] xfs: rename variable mp to parsing_mp

[Carlos Maiolino]

On Wed, Feb 24, 2021 at 10:43:22PM +0100, Pavel Reichl wrote:
> Rename mp variable to parsisng_mp so it is easy to distinguish
> between current mount point handle and handle for mount point
> which mount options are being parsed.
> 
> Suggested-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
>  fs/xfs/xfs_super.c | 102 ++++++++++++++++++++++-----------------------
>  1 file changed, 51 insertions(+), 51 deletions(-)

Looks good,
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 813be879a5e5..7e281d1139dc 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1165,7 +1165,7 @@ xfs_fs_parse_param(
>  	struct fs_context	*fc,
>  	struct fs_parameter	*param)
>  {
> -	struct xfs_mount	*mp = fc->s_fs_info;
> +	struct xfs_mount	*parsing_mp = fc->s_fs_info;
>  	struct fs_parse_result	result;
>  	int			size = 0;
>  	int			opt;
> @@ -1176,142 +1176,142 @@ xfs_fs_parse_param(
>  
>  	switch (opt) {
>  	case Opt_logbufs:
> -		mp->m_logbufs = result.uint_32;
> +		parsing_mp->m_logbufs = result.uint_32;
>  		return 0;
>  	case Opt_logbsize:
> -		if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
> +		if (suffix_kstrtoint(param->string, 10, &parsing_mp->m_logbsize))
>  			return -EINVAL;
>  		return 0;
>  	case Opt_logdev:
> -		kfree(mp->m_logname);
> -		mp->m_logname = kstrdup(param->string, GFP_KERNEL);
> -		if (!mp->m_logname)
> +		kfree(parsing_mp->m_logname);
> +		parsing_mp->m_logname = kstrdup(param->string, GFP_KERNEL);
> +		if (!parsing_mp->m_logname)
>  			return -ENOMEM;
>  		return 0;
>  	case Opt_rtdev:
> -		kfree(mp->m_rtname);
> -		mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
> -		if (!mp->m_rtname)
> +		kfree(parsing_mp->m_rtname);
> +		parsing_mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
> +		if (!parsing_mp->m_rtname)
>  			return -ENOMEM;
>  		return 0;
>  	case Opt_allocsize:
>  		if (suffix_kstrtoint(param->string, 10, &size))
>  			return -EINVAL;
> -		mp->m_allocsize_log = ffs(size) - 1;
> -		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
> +		parsing_mp->m_allocsize_log = ffs(size) - 1;
> +		parsing_mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
>  		return 0;
>  	case Opt_grpid:
>  	case Opt_bsdgroups:
> -		mp->m_flags |= XFS_MOUNT_GRPID;
> +		parsing_mp->m_flags |= XFS_MOUNT_GRPID;
>  		return 0;
>  	case Opt_nogrpid:
>  	case Opt_sysvgroups:
> -		mp->m_flags &= ~XFS_MOUNT_GRPID;
> +		parsing_mp->m_flags &= ~XFS_MOUNT_GRPID;
>  		return 0;
>  	case Opt_wsync:
> -		mp->m_flags |= XFS_MOUNT_WSYNC;
> +		parsing_mp->m_flags |= XFS_MOUNT_WSYNC;
>  		return 0;
>  	case Opt_norecovery:
> -		mp->m_flags |= XFS_MOUNT_NORECOVERY;
> +		parsing_mp->m_flags |= XFS_MOUNT_NORECOVERY;
>  		return 0;
>  	case Opt_noalign:
> -		mp->m_flags |= XFS_MOUNT_NOALIGN;
> +		parsing_mp->m_flags |= XFS_MOUNT_NOALIGN;
>  		return 0;
>  	case Opt_swalloc:
> -		mp->m_flags |= XFS_MOUNT_SWALLOC;
> +		parsing_mp->m_flags |= XFS_MOUNT_SWALLOC;
>  		return 0;
>  	case Opt_sunit:
> -		mp->m_dalign = result.uint_32;
> +		parsing_mp->m_dalign = result.uint_32;
>  		return 0;
>  	case Opt_swidth:
> -		mp->m_swidth = result.uint_32;
> +		parsing_mp->m_swidth = result.uint_32;
>  		return 0;
>  	case Opt_inode32:
> -		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> +		parsing_mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
>  		return 0;
>  	case Opt_inode64:
> -		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> +		parsing_mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
>  		return 0;
>  	case Opt_nouuid:
> -		mp->m_flags |= XFS_MOUNT_NOUUID;
> +		parsing_mp->m_flags |= XFS_MOUNT_NOUUID;
>  		return 0;
>  	case Opt_largeio:
> -		mp->m_flags |= XFS_MOUNT_LARGEIO;
> +		parsing_mp->m_flags |= XFS_MOUNT_LARGEIO;
>  		return 0;
>  	case Opt_nolargeio:
> -		mp->m_flags &= ~XFS_MOUNT_LARGEIO;
> +		parsing_mp->m_flags &= ~XFS_MOUNT_LARGEIO;
>  		return 0;
>  	case Opt_filestreams:
> -		mp->m_flags |= XFS_MOUNT_FILESTREAMS;
> +		parsing_mp->m_flags |= XFS_MOUNT_FILESTREAMS;
>  		return 0;
>  	case Opt_noquota:
> -		mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> -		mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> -		mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> +		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> +		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> +		parsing_mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
>  		return 0;
>  	case Opt_quota:
>  	case Opt_uquota:
>  	case Opt_usrquota:
> -		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> +		parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
>  				 XFS_UQUOTA_ENFD);
>  		return 0;
>  	case Opt_qnoenforce:
>  	case Opt_uqnoenforce:
> -		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> -		mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> +		parsing_mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> +		parsing_mp->m_qflags &= ~XFS_UQUOTA_ENFD;
>  		return 0;
>  	case Opt_pquota:
>  	case Opt_prjquota:
> -		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> +		parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
>  				 XFS_PQUOTA_ENFD);
>  		return 0;
>  	case Opt_pqnoenforce:
> -		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> -		mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> +		parsing_mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> +		parsing_mp->m_qflags &= ~XFS_PQUOTA_ENFD;
>  		return 0;
>  	case Opt_gquota:
>  	case Opt_grpquota:
> -		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> +		parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
>  				 XFS_GQUOTA_ENFD);
>  		return 0;
>  	case Opt_gqnoenforce:
> -		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> -		mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> +		parsing_mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> +		parsing_mp->m_qflags &= ~XFS_GQUOTA_ENFD;
>  		return 0;
>  	case Opt_discard:
> -		mp->m_flags |= XFS_MOUNT_DISCARD;
> +		parsing_mp->m_flags |= XFS_MOUNT_DISCARD;
>  		return 0;
>  	case Opt_nodiscard:
> -		mp->m_flags &= ~XFS_MOUNT_DISCARD;
> +		parsing_mp->m_flags &= ~XFS_MOUNT_DISCARD;
>  		return 0;
>  #ifdef CONFIG_FS_DAX
>  	case Opt_dax:
> -		xfs_mount_set_dax_mode(mp, XFS_DAX_ALWAYS);
> +		xfs_mount_set_dax_mode(parsing_mp, XFS_DAX_ALWAYS);
>  		return 0;
>  	case Opt_dax_enum:
> -		xfs_mount_set_dax_mode(mp, result.uint_32);
> +		xfs_mount_set_dax_mode(parsing_mp, result.uint_32);
>  		return 0;
>  #endif
>  	/* Following mount options will be removed in September 2025 */
>  	case Opt_ikeep:
> -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> -		mp->m_flags |= XFS_MOUNT_IKEEP;
> +		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		parsing_mp->m_flags |= XFS_MOUNT_IKEEP;
>  		return 0;
>  	case Opt_noikeep:
> -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> -		mp->m_flags &= ~XFS_MOUNT_IKEEP;
> +		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		parsing_mp->m_flags &= ~XFS_MOUNT_IKEEP;
>  		return 0;
>  	case Opt_attr2:
> -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> -		mp->m_flags |= XFS_MOUNT_ATTR2;
> +		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		parsing_mp->m_flags |= XFS_MOUNT_ATTR2;
>  		return 0;
>  	case Opt_noattr2:
> -		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> -		mp->m_flags &= ~XFS_MOUNT_ATTR2;
> -		mp->m_flags |= XFS_MOUNT_NOATTR2;
> +		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		parsing_mp->m_flags &= ~XFS_MOUNT_ATTR2;
> +		parsing_mp->m_flags |= XFS_MOUNT_NOATTR2;
>  		return 0;
>  	default:
> -		xfs_warn(mp, "unknown mount option [%s].", param->key);
> +		xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
>  		return -EINVAL;
>  	}
>  
> -- 
> 2.29.2
> 

-- 
Carlos

[PATCH v2 2/2] xfs: Skip repetitive warnings about mount options

[Pavel Reichl]

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@redhat.com>
Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
 fs/xfs/xfs_super.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 7e281d1139dc..ba113a28b631 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1155,6 +1155,22 @@ suffix_kstrtoint(
 	return ret;
 }
 
+static inline void
+xfs_fs_warn_deprecated(
+	struct fs_context	*fc,
+	struct fs_parameter	*param,
+	uint64_t		flag,
+	bool			value)
+{
+	/* Don't print the warning if reconfiguring and current mount point
+	 * already had the flag set
+	 */
+	if ((fc->purpose & FS_CONTEXT_FOR_RECONFIGURE) &&
+			!!(XFS_M(fc->root->d_sb)->m_flags & flag) == value)
+		return;
+	xfs_warn(fc->s_fs_info, "%s mount option is deprecated.", param->key);
+}
+
 /*
  * Set mount state from a mount option.
  *
@@ -1294,19 +1310,19 @@ xfs_fs_parse_param(
 #endif
 	/* Following mount options will be removed in September 2025 */
 	case Opt_ikeep:
-		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, true);
 		parsing_mp->m_flags |= XFS_MOUNT_IKEEP;
 		return 0;
 	case Opt_noikeep:
-		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, false);
 		parsing_mp->m_flags &= ~XFS_MOUNT_IKEEP;
 		return 0;
 	case Opt_attr2:
-		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_ATTR2, true);
 		parsing_mp->m_flags |= XFS_MOUNT_ATTR2;
 		return 0;
 	case Opt_noattr2:
-		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_NOATTR2, true);
 		parsing_mp->m_flags &= ~XFS_MOUNT_ATTR2;
 		parsing_mp->m_flags |= XFS_MOUNT_NOATTR2;
 		return 0;
-- 
2.29.2

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

[Darrick J. Wong]

On Wed, Feb 24, 2021 at 10:43:23PM +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@redhat.com>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>

Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_super.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 7e281d1139dc..ba113a28b631 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1155,6 +1155,22 @@ suffix_kstrtoint(
>  	return ret;
>  }
>  
> +static inline void
> +xfs_fs_warn_deprecated(
> +	struct fs_context	*fc,
> +	struct fs_parameter	*param,
> +	uint64_t		flag,
> +	bool			value)
> +{
> +	/* Don't print the warning if reconfiguring and current mount point
> +	 * already had the flag set
> +	 */
> +	if ((fc->purpose & FS_CONTEXT_FOR_RECONFIGURE) &&
> +			!!(XFS_M(fc->root->d_sb)->m_flags & flag) == value)
> +		return;
> +	xfs_warn(fc->s_fs_info, "%s mount option is deprecated.", param->key);
> +}
> +
>  /*
>   * Set mount state from a mount option.
>   *
> @@ -1294,19 +1310,19 @@ xfs_fs_parse_param(
>  #endif
>  	/* Following mount options will be removed in September 2025 */
>  	case Opt_ikeep:
> -		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, true);
>  		parsing_mp->m_flags |= XFS_MOUNT_IKEEP;
>  		return 0;
>  	case Opt_noikeep:
> -		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, false);
>  		parsing_mp->m_flags &= ~XFS_MOUNT_IKEEP;
>  		return 0;
>  	case Opt_attr2:
> -		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_ATTR2, true);
>  		parsing_mp->m_flags |= XFS_MOUNT_ATTR2;
>  		return 0;
>  	case Opt_noattr2:
> -		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_NOATTR2, true);
>  		parsing_mp->m_flags &= ~XFS_MOUNT_ATTR2;
>  		parsing_mp->m_flags |= XFS_MOUNT_NOATTR2;
>  		return 0;
> -- 
> 2.29.2
> 

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

[Carlos Maiolino]

On Wed, Feb 24, 2021 at 10:43:23PM +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@redhat.com>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
>  fs/xfs/xfs_super.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 7e281d1139dc..ba113a28b631 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1155,6 +1155,22 @@ suffix_kstrtoint(
>  	return ret;
>  }
>  

Looks good indeed.

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> +static inline void
> +xfs_fs_warn_deprecated(
> +	struct fs_context	*fc,
> +	struct fs_parameter	*param,
> +	uint64_t		flag,
> +	bool			value)
> +{
> +	/* Don't print the warning if reconfiguring and current mount point
> +	 * already had the flag set
> +	 */
> +	if ((fc->purpose & FS_CONTEXT_FOR_RECONFIGURE) &&
> +			!!(XFS_M(fc->root->d_sb)->m_flags & flag) == value)
> +		return;
> +	xfs_warn(fc->s_fs_info, "%s mount option is deprecated.", param->key);
> +}
> +
>  /*
>   * Set mount state from a mount option.
>   *
> @@ -1294,19 +1310,19 @@ xfs_fs_parse_param(
>  #endif
>  	/* Following mount options will be removed in September 2025 */
>  	case Opt_ikeep:
> -		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, true);
>  		parsing_mp->m_flags |= XFS_MOUNT_IKEEP;
>  		return 0;
>  	case Opt_noikeep:
> -		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, false);
>  		parsing_mp->m_flags &= ~XFS_MOUNT_IKEEP;
>  		return 0;
>  	case Opt_attr2:
> -		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_ATTR2, true);
>  		parsing_mp->m_flags |= XFS_MOUNT_ATTR2;
>  		return 0;
>  	case Opt_noattr2:
> -		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
> +		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_NOATTR2, true);
>  		parsing_mp->m_flags &= ~XFS_MOUNT_ATTR2;
>  		parsing_mp->m_flags |= XFS_MOUNT_NOATTR2;
>  		return 0;
> -- 
> 2.29.2
> 

-- 
Carlos

[PATCH v2 0/2] xfs: Skip repetitive warnings about mount options

[Pavel Reichl]

Hello,

I belive that this patch set was missed in recent for-next branch update.

Patches are the same as before I just added the reviewed-by tags.

Thanks!

...
At least some version of mount will look in /proc/mounts and send in all of the 
options that it finds as part of a remount command. We also /do/ still emit
"attr2" in /proc/mounts (as we probably should), so remount passes that back
in, and we emit a warning, which is not great.

In other words mount passes in "attr2" and the kernel emits a deprecation
warning for attr2, even though the user/admin never explicitly asked for the
option.

So, lets skip the warning if (we are remounting && deprecated option
state is not changing).

I also attached test for xfstests that I used for testing (the test
will be proposed on xfstests-list after/if this patch is merged).

V2 vs. V1

* Added new patch that renames mp to parsing_mp in xfs_fs_parse_param()
* Added new function xfs_fs_warn_deprecated() to encapsulate the logic for displaying the deprecation warning.
* Fixed some white space issues.

Pavel Reichl (2):
  xfs: rename variable mp to parsing_mp
  xfs: Skip repetitive warnings about mount options

 fs/xfs/xfs_super.c | 118 +++++++++++++++++++++++++--------------------
 1 file changed, 67 insertions(+), 51 deletions(-)

-- 
2.30.2

[PATCH v2 2/2] xfs: Skip repetitive warnings about mount options

[Pavel Reichl]

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@redhat.com>
Signed-off-by: Pavel Reichl <preichl@redhat.com>

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_super.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 7e281d1139dc..ba113a28b631 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1155,6 +1155,22 @@ suffix_kstrtoint(
 	return ret;
 }
 
+static inline void
+xfs_fs_warn_deprecated(
+	struct fs_context	*fc,
+	struct fs_parameter	*param,
+	uint64_t		flag,
+	bool			value)
+{
+	/* Don't print the warning if reconfiguring and current mount point
+	 * already had the flag set
+	 */
+	if ((fc->purpose & FS_CONTEXT_FOR_RECONFIGURE) &&
+			!!(XFS_M(fc->root->d_sb)->m_flags & flag) == value)
+		return;
+	xfs_warn(fc->s_fs_info, "%s mount option is deprecated.", param->key);
+}
+
 /*
  * Set mount state from a mount option.
  *
@@ -1294,19 +1310,19 @@ xfs_fs_parse_param(
 #endif
 	/* Following mount options will be removed in September 2025 */
 	case Opt_ikeep:
-		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, true);
 		parsing_mp->m_flags |= XFS_MOUNT_IKEEP;
 		return 0;
 	case Opt_noikeep:
-		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_IKEEP, false);
 		parsing_mp->m_flags &= ~XFS_MOUNT_IKEEP;
 		return 0;
 	case Opt_attr2:
-		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_ATTR2, true);
 		parsing_mp->m_flags |= XFS_MOUNT_ATTR2;
 		return 0;
 	case Opt_noattr2:
-		xfs_warn(parsing_mp, "%s mount option is deprecated.", param->key);
+		xfs_fs_warn_deprecated(fc, param, XFS_MOUNT_NOATTR2, true);
 		parsing_mp->m_flags &= ~XFS_MOUNT_ATTR2;
 		parsing_mp->m_flags |= XFS_MOUNT_NOATTR2;
 		return 0;
-- 
2.30.2