Thread (34 messages) 34 messages, 3 authors, 2021-08-18

Re: [PATCH 05/16] xfs: replace xfs_sb_version checks with feature flag checks

From: Dave Chinner <david@fromorbit.com>
Date: 2021-08-18 01:33:45

On Wed, Aug 11, 2021 at 03:13:29PM -0700, Darrick J. Wong wrote:
quoted hunk ↗ jump to hunk
On Tue, Aug 10, 2021 at 03:24:40PM +1000, Dave Chinner wrote:
quoted
From: Dave Chinner <redacted>

Convert the xfs_sb_version_hasfoo() to checks against
mp->m_features. Checks of the superblock itself during disk
operations (e.g. in the read/write verifiers and the to/from disk
formatters) are not converted - they operate purely on the
superblock state. Everything else should use the mount features.

Large parts of this conversion were done with sed with commands like
this:

for f in `git grep -l xfs_sb_version_has fs/xfs/*.c`; do
	sed -i -e 's/xfs_sb_version_has\(.*\)(&\(.*\)->m_sb)/xfs_has_\1(\2)/' $f
done

With manual cleanups for things like "xfs_has_extflgbit" and other
little inconsistencies in naming.

The result is ia lot less typing to check features and an XFS binary
size reduced by a bit over 3kB:

$ size -t fs/xfs/built-in.a
	text	   data	    bss	    dec	    hex	filenam
before	1130866  311352     484 1442702  16038e (TOTALS)
after	1127727  311352     484 1439563  15f74b (TOTALS)

Signed-off-by: Dave Chinner <redacted>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
<snip>
quoted
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 699066fb9052..7361830163d7 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -951,8 +951,7 @@ xfs_growfs_rt(
 		return -EINVAL;
 
 	/* Unsupported realtime features. */
-	if (xfs_sb_version_hasrmapbt(&mp->m_sb) ||
-	    xfs_sb_version_hasreflink(&mp->m_sb))
+	if (xfs_has_rmapbt(mp) || xfs_has_reflink(mp))
A regression test that I developed to test the act of adding a realtime
volume to an existing filesystem exposed the following failure:
--- /tmp/fstests/tests/xfs/779.out      2021-08-08 08:47:08.535033170 -0700
+++ /var/tmp/fstests/xfs/779.out.bad    2021-08-11 14:54:18.389346401 -0700
@@ -1,2 +1,4 @@
 QA output created by 779
+/opt/a is not a realtime file?
+expected file extszhint 0, got 12288
 Silence is golden
Since this test is not yet upstream, I will describe the sequence of
events:

1. Suppress SCRATCH_RTDEV when invoking _scratch_mkfs.  This creates a
   filesystem with no realtime volume attached.
2. Mount the fs with SCRATCH_RTDEV in the mount options.  The rt volume
   still has not been attached.
3. Set RTINHERIT (and EXTSZINHERIT) on the root directory.  Make sure
   the extent size hint is not a multiple of the rt extent size.
4. Call xfs_growfs -r to add the rt volume into the filesystem.
5. Create a file.  Due to RTINHERIT, the new file should be a realtime
   file.
6. Check that the file is actually a realtime file and does not have an
   extent size hint.

The regression of course happens in step 6, because xfs_growfs_rt can
add a realtime volume to an existing filesystem.  Prior to this patch,
the "has realtime?" predicate always looked at the mp->m_sb.  Now that
the feature state has been turned into a separate xfs_mount state, we
must set the REALTIME m_feature flag explicitly.

The following patch solves the regression:
I've added a variant of this to the patch - it just sets
XFS_FEAT_REALTIME directly without the "xfs_add_realtime()" wrapper.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help