Re: [PATCH 03/16] xfs: rework attr2 feature and mount options
From: Dave Chinner <david@fromorbit.com>
Date: 2021-08-18 01:18:18
On Wed, Aug 11, 2021 at 04:04:05PM -0700, Darrick J. Wong wrote:
On Tue, Aug 10, 2021 at 03:24:38PM +1000, Dave Chinner wrote:quoted
From: Dave Chinner <redacted> The attr2 feature is somewhat unique in that it has both a superblock feature bit to enable it and mount options to enable and disable it. Back when it was first introduced in 2005, attr2 was disabled unless either the attr2 superblock feature bit was set, or the attr2 mount option was set. If the superblock feature bit was not set but the mount option was set, then when the first attr2 format inode fork was created, it would set the superblock feature bit. This is as it should be - the superblock feature bit indicated the presence of the attr2 on disk format.
......
quoted hunk ↗ jump to hunk
I see the following regression on xfs/187 with this patch applied:--- /tmp/fstests/tests/xfs/187.out 2021-05-13 11:47:55.849859833 -0700 +++ /var/tmp/fstests/xfs/187.out.bad 2021-08-11 15:59:15.692618610 -0700@@ -9,6 +9,8 @@ noattr2 fs +MOREBITS +ATTR2 *** 2. test attr2 mkfs and then noattr2 mount with 1 EA ***@@ -23,6 +25,8 @@ user.test ATTR +MOREBITS +ATTR2 *** 3. test noattr2 mount and lazy sb ***@@ -36,4 +40,5 @@ noattr2 fs MOREBITS +ATTR2 LAZYSBCOUNTI am pretty sure this is a direct result of "This will not remove the superblock feature bit", correct? Do you have an adjustment to xfs/187 to avoid regressing QA?
I've been looking at 187, and the tests that are failing are testing the specific behaviour of "mkfs w/attr2 enabled; mount noattr2 and ensure the attr2 version bits are not set". Basically, the premise of the noattr2 verification tests are not valid anymore, but adding filters to mask out the addition of MOREBITS and ATTR2 basically renders the test useless for validating the old behaviour is still correct. IOWs, there's no way to detect if a kernel is going to behave like the old code and remove the bits, or whether it is going to just leave the attr2 fields set on disk. Hence I've got no idea how we'd change xfs/187 to validate both the old and new behaviour because we have no way of knowing which behaviour to expect. So I'd vote for just throwing away xfs/187 because a) it looks unfixable to me, and b) it's trying to validate what should be considered broken behaviour (i.e. on-disk feature bit gets removed even though the feature is still present on disk)... Your thoughts? Cheers, Dave. -- Dave Chinner david@fromorbit.com