From: Darrick J. Wong djwong@kernel.org

The deprecation of the 'attr2' mount option in 6.18 wasn't entirely successful because nobody noticed that the kernel never printed a warning about attr2 being set in fstab if the only xfs filesystem is the root fs; the initramfs mounts the root fs with no mount options; and the init scripts only conveyed the fstab options by remounting the root fs.

Fix this by making it complain all the time.

Cc: stable@vger.kernel.org # v5.13 Fixes: 92cf7d36384b99 ("xfs: Skip repetitive warnings about mount options") Signed-off-by: "Darrick J. Wong" djwong@kernel.org Reviewed-by: Christoph Hellwig hch@lst.de --- fs/xfs/xfs_super.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index e85a156dc17d16..ae9b17730eaf41 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -1373,16 +1373,25 @@ suffix_kstrtoull( static inline void xfs_fs_warn_deprecated( struct fs_context *fc, - struct fs_parameter *param, - uint64_t flag, - bool value) + struct fs_parameter *param) { - /* Don't print the warning if reconfiguring and current mount point - * already had the flag set + /* + * Always warn about someone passing in a deprecated mount option. + * Previously we wouldn't print the warning if we were reconfiguring + * and current mount point already had the flag set, but that was not + * the right thing to do. + * + * Many distributions mount the root filesystem with no options in the + * initramfs and rely on mount -a to remount the root fs with the + * options in fstab. However, the old behavior meant that there would + * never be a warning about deprecated mount options for the root fs in + * /etc/fstab. On a single-fs system, that means no warning at all. + * + * Compounding this problem are distribution scripts that copy + * /proc/mounts to fstab, which means that we can't remove mount + * options unless we're 100% sure they have only ever been advertised + * in /proc/mounts in response to explicitly provided mount options. */ - if ((fc->purpose & FS_CONTEXT_FOR_RECONFIGURE) && - !!(XFS_M(fc->root->d_sb)->m_features & flag) == value) - return; xfs_warn(fc->s_fs_info, "%s mount option is deprecated.", param->key); }

From: Darrick J. Wong djwong@kernel.org

Apparently we can never deprecate mount options in this project, because it will invariably turn out that some foolish userspace depends on some behavior and break. From Oleksandr Natalenko:

In v6.18, the attr2 XFS mount option is removed. This may silently break system boot if the attr2 option is still present in /etc/fstab for rootfs.

Consider Arch Linux that is being set up from scratch with / being formatted as XFS. The genfstab command that is used to generate /etc/fstab produces something like this by default:

/dev/sda2 on / type xfs (rw,relatime,attr2,discard,inode64,logbufs=8,logbsize=32k,noquota)

Once the system is set up and rebooted, there's no deprecation warning seen in the kernel log:

# cat /proc/cmdline root=UUID=77b42de2-397e-47ee-a1ef-4dfd430e47e9 rootflags=discard rd.luks.options=discard quiet

# dmesg | grep -i xfs [ 2.409818] SGI XFS with ACLs, security attributes, realtime, scrub, repair, quota, no debug enabled [ 2.415341] XFS (sda2): Mounting V5 Filesystem 77b42de2-397e-47ee-a1ef-4dfd430e47e9 [ 2.442546] XFS (sda2): Ending clean mount

Although as per the deprecation intention, it should be there.

Vlastimil (in Cc) suggests this is because xfs_fs_warn_deprecated() doesn't produce any warning by design if the XFS FS is set to be rootfs and gets remounted read-write during boot. This imposes two problems:

1) a user doesn't see the deprecation warning; and 2) with v6.18 kernel, the read-write remount fails because of unknown attr2 option rendering system unusable:

systemd[1]: Switching root. systemd-remount-fs[225]: /usr/bin/mount for / exited with exit status 32.

# mount -o rw / mount: /: fsconfig() failed: xfs: Unknown parameter 'attr2'.

Thorsten (in Cc) suggested reporting this as a user-visible regression.

From my PoV, although the deprecation is in place for 5 years already, it may not be visible enough as the warning is not emitted for rootfs. Considering the amount of systems set up with XFS on /, this may impose a mass problem for users.

Vlastimil suggested making attr2 option a complete noop instead of removing it.

IOWs, the initrd mounts the root fs with (I assume) no mount options, and mount -a remounts with whatever options are in fstab. However, XFS doesn't complain about deprecated mount options during a remount, so technically speaking we were not warning all users in all combinations that they were heading for a cliff.

Gotcha!!

Now, how did 'attr2' get slurped up on so many systems? The old code would put that in /proc/mounts if the filesystem happened to be in attr2 mode, even if user hadn't mounted with any such option. IOWs, this is because someone thought it would be a good idea to advertise system state via /proc/mounts.

The easy way to fix this is to reintroduce the four mount options but map them to a no-op option that ignores them, and hope that nobody's depending on attr2 to appear in /proc/mounts. (Hint: use the fsgeometry ioctl). But we've learned our lesson, so complain as LOUDLY as possible about the deprecation.

Lessons learned:

1. Don't expose system state via /proc/mounts; the only strings that ought to be there are options *explicitly* provided by the user. 2. Never tidy, it's not worth the stress and irritation.

Reported-by: Vlastimil Babka vbabka@suse.cz Reported-by: Oleksandr Natalenko oleksandr@natalenko.name Cc: stable@vger.kernel.org # v6.18-rc1 Fixes: b9a176e54162f8 ("xfs: remove deprecated mount options") Signed-off-by: "Darrick J. Wong" djwong@kernel.org --- fs/xfs/xfs_super.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index ae9b17730eaf41..d8f326d8838036 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -102,7 +102,7 @@ static const struct constant_table dax_param_enums[] = { * Table driven mount option parser. */ enum { - Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, + Op_deprecated, Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, Opt_nouuid, Opt_grpid, Opt_nogrpid, Opt_bsdgroups, Opt_sysvgroups, Opt_allocsize, Opt_norecovery, Opt_inode64, Opt_inode32, @@ -114,7 +114,21 @@ enum { Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write, };

+#define fsparam_dead(NAME) \ + __fsparam(NULL, (NAME), Op_deprecated, fs_param_deprecated, NULL) + static const struct fs_parameter_spec xfs_fs_parameters[] = { + /* + * These mount options were supposed to be deprecated in September 2025 + * but the deprecation warning was buggy, so not all users were + * notified. The deprecation is now obnoxiously loud and postponed to + * September 2030. + */ + fsparam_dead("attr2"), + fsparam_dead("noattr2"), + fsparam_dead("ikeep"), + fsparam_dead("noikeep"), + fsparam_u32("logbufs", Opt_logbufs), fsparam_string("logbsize", Opt_logbsize), fsparam_string("logdev", Opt_logdev), @@ -1417,6 +1431,9 @@ xfs_fs_parse_param( return opt;

switch (opt) { + case Op_deprecated: + xfs_fs_warn_deprecated(fc, param); + return 0; case Opt_logbufs: parsing_mp->m_logbufs = result.uint_32; return 0; @@ -1537,7 +1554,6 @@ xfs_fs_parse_param( xfs_mount_set_dax_mode(parsing_mp, result.uint_32); return 0; #endif - /* Following mount options will be removed in September 2025 */ case Opt_max_open_zones: parsing_mp->m_max_open_zones = result.uint_32; return 0;

From: Darrick J. Wong djwong@kernel.org

On a filesystem with parent pointers, xchk_nlinks_collect_dir walks both the directory entries (data fork) and the parent pointers (attr fork) to determine the correct link count. Unfortunately I forgot to update the lock mode logic to handle the case of a directory whose attr fork is in btree format and has not yet been loaded *and* whose data fork doesn't need loading.

This leads to a bunch of assertions from xfs/286 in xfs_iread_extents because we only took ILOCK_SHARED, not ILOCK_EXCL. You'd need the rare happenstance of a directory with a large number of non-pptr extended attributes set and enough memory pressure to cause the directory to be evicted and partially reloaded from disk.

I /think/ this only started in 6.18-rc1 because I've started seeing OOM errors with the maple tree slab using 70% of memory, and this didn't happen in 6.17. Yay dynamic systems!

Cc: stable@vger.kernel.org # v6.10 Fixes: 77ede5f44b0d86 ("xfs: walk directory parent pointers to determine backref count") Signed-off-by: "Darrick J. Wong" djwong@kernel.org --- fs/xfs/scrub/nlinks.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c index 26721fab5cab42..091c79e432e592 100644 --- a/fs/xfs/scrub/nlinks.c +++ b/fs/xfs/scrub/nlinks.c @@ -376,6 +376,36 @@ xchk_nlinks_collect_pptr( return error; }

+static uint +xchk_nlinks_ilock_dir( + struct xfs_inode *ip) +{ + uint lock_mode = XFS_ILOCK_SHARED; + + /* + * We're going to scan the directory entries, so we must be ready to + * pull the data fork mappings into memory if they aren't already. + */ + if (xfs_need_iread_extents(&ip->i_df)) + lock_mode = XFS_ILOCK_EXCL; + + /* + * We're going to scan the parent pointers, so we must be ready to + * pull the attr fork mappings into memory if they aren't already. + */ + if (xfs_has_parent(ip->i_mount) && xfs_inode_has_attr_fork(ip) && + xfs_need_iread_extents(&ip->i_af)) + lock_mode = XFS_ILOCK_EXCL; + + /* + * Take the IOLOCK so that other threads cannot start a directory + * update while we're scanning. + */ + lock_mode |= XFS_IOLOCK_SHARED; + xfs_ilock(ip, lock_mode); + return lock_mode; +} + /* Walk a directory to bump the observed link counts of the children. */ STATIC int xchk_nlinks_collect_dir( @@ -394,8 +424,7 @@ xchk_nlinks_collect_dir( return 0;

/* Prevent anyone from changing this directory while we walk it. */ - xfs_ilock(dp, XFS_IOLOCK_SHARED); - lock_mode = xfs_ilock_data_map_shared(dp); + lock_mode = xchk_nlinks_ilock_dir(dp);

/* * The dotdot entry of an unlinked directory still points to the last @@ -452,7 +481,6 @@ xchk_nlinks_collect_dir( xchk_iscan_abort(&xnc->collect_iscan); out_unlock: xfs_iunlock(dp, lock_mode); - xfs_iunlock(dp, XFS_IOLOCK_SHARED); return error; }