* [PATCH] common/rc: Fix _check_s_dax()
@ 2020-12-02 21:41 ira.weiny
2020-12-02 21:46 ` [PATCH V2] " ira.weiny
2020-12-03 8:15 ` [PATCH] " Christoph Hellwig
0 siblings, 2 replies; 8+ messages in thread
From: ira.weiny @ 2020-12-02 21:41 UTC (permalink / raw)
To: fstests, Christoph Hellwig, Eric Sandeen
Cc: Ira Weiny, linux-kernel, Darrick J. Wong, Dan Williams,
Dave Chinner, Theodore Y. Ts'o, Jan Kara, Jeff Moyer,
linux-ext4, linux-xfs, linux-fsdevel, David Howells
From: Ira Weiny <ira.weiny@intel.com>
There is a conflict with the user visible statx bits 'mount root' and
'dax'. The kernel is changing the dax bit to correct this conflict.[1]
Adjust _check_s_dax() to use the new bit. Because DAX tests do not run
on root mounts, STATX_ATTR_MOUNT_ROOT should always be 0, therefore we
can allow either bit to indicate DAX and cover any kernel which may be
running.
[1] https://lore.kernel.org/lkml/3e28d2c7-fbe5-298a-13ba-dcd8fd504666@redhat.com/
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
I went ahead and used Christoph's suggestion regarding using both bits.
---
common/rc | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/common/rc b/common/rc
index b5a504e0dcb4..322e682e5239 100644
--- a/common/rc
+++ b/common/rc
@@ -3221,11 +3221,24 @@ _check_s_dax()
local exp_s_dax=$2
local attributes=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')
- if [ $exp_s_dax -eq 0 ]; then
- (( attributes & 0x2000 )) && echo "$target has unexpected S_DAX flag"
- else
- (( attributes & 0x2000 )) || echo "$target doesn't have expected S_DAX flag"
- fi
+
+ # The attribute bit value, STATX_ATTR_DAX (0x2000), conflicted with
+ # STATX_ATTR_MOUNT_ROOT. Therefore, STATX_ATTR_DAX was changed to
+ # 0x00200000.
+ #
+ # Because DAX tests do not run on root mounts, STATX_ATTR_MOUNT_ROOT
+ # should always be 0, therefore we can allow either bit to indicate DAX
+ # and cover any kernel which may be running.
+
+ if [ $(( attributes & 0x00200000 )) -ne 0 ] || [ $(( attributes & 0x2000 )) -ne 0 ]; then
+ if [ $exp_s_dax -eq 0 ]; then
+ echo "$target has unexpected S_DAX flag"
+ fi
+ else
+ if [ $exp_s_dax -ne 0 ]; then
+ echo "$target doesn't have expected S_DAX flag"
+ fi
+ fi
}
_check_xflag()
--
2.28.0.rc0.12.gb6a658bd00c9
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH V2] common/rc: Fix _check_s_dax()
2020-12-02 21:41 [PATCH] common/rc: Fix _check_s_dax() ira.weiny
@ 2020-12-02 21:46 ` ira.weiny
2020-12-04 1:45 ` [PATCH V3] " ira.weiny
2020-12-03 8:15 ` [PATCH] " Christoph Hellwig
1 sibling, 1 reply; 8+ messages in thread
From: ira.weiny @ 2020-12-02 21:46 UTC (permalink / raw)
To: fstests, Christoph Hellwig, Eric Sandeen
Cc: Ira Weiny, linux-kernel, Darrick J. Wong, Dan Williams,
Dave Chinner, Theodore Y. Ts'o, Jan Kara, Jeff Moyer,
linux-ext4, linux-xfs, linux-fsdevel, David Howells
From: Ira Weiny <ira.weiny@intel.com>
There is a conflict with the user visible statx bits 'mount root' and
'dax'. The kernel is changing the dax bit to correct this conflict.[1]
Adjust _check_s_dax() to use the new bit. Because DAX tests do not run
on root mounts, STATX_ATTR_MOUNT_ROOT should always be 0, therefore we
can allow either bit to indicate DAX and cover any kernel which may be
running.
[1] https://lore.kernel.org/lkml/3e28d2c7-fbe5-298a-13ba-dcd8fd504666@redhat.com/
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Changes for V2:
Fix bad indentation whitespace.
common/rc | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/common/rc b/common/rc
index b5a504e0dcb4..9418f7bc8dab 100644
--- a/common/rc
+++ b/common/rc
@@ -3221,10 +3221,23 @@ _check_s_dax()
local exp_s_dax=$2
local attributes=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')
- if [ $exp_s_dax -eq 0 ]; then
- (( attributes & 0x2000 )) && echo "$target has unexpected S_DAX flag"
+
+ # The attribute bit value, STATX_ATTR_DAX (0x2000), conflicted with
+ # STATX_ATTR_MOUNT_ROOT. Therefore, STATX_ATTR_DAX was changed to
+ # 0x00200000.
+ #
+ # Because DAX tests do not run on root mounts, STATX_ATTR_MOUNT_ROOT
+ # should always be 0, therefore we can allow either bit to indicate DAX
+ # and cover any kernel which may be running.
+
+ if [ $(( attributes & 0x00200000 )) -ne 0 ] || [ $(( attributes & 0x2000 )) -ne 0 ]; then
+ if [ $exp_s_dax -eq 0 ]; then
+ echo "$target has unexpected S_DAX flag"
+ fi
else
- (( attributes & 0x2000 )) || echo "$target doesn't have expected S_DAX flag"
+ if [ $exp_s_dax -ne 0 ]; then
+ echo "$target doesn't have expected S_DAX flag"
+ fi
fi
}
--
2.28.0.rc0.12.gb6a658bd00c9
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH V3] common/rc: Fix _check_s_dax()
2020-12-02 21:46 ` [PATCH V2] " ira.weiny
@ 2020-12-04 1:45 ` ira.weiny
2020-12-04 9:47 ` Christoph Hellwig
0 siblings, 1 reply; 8+ messages in thread
From: ira.weiny @ 2020-12-04 1:45 UTC (permalink / raw)
To: fstests, Christoph Hellwig, Eric Sandeen
Cc: Ira Weiny, linux-kernel, Darrick J. Wong, Dan Williams,
Dave Chinner, Theodore Y. Ts'o, Jan Kara, Jeff Moyer,
linux-ext4, linux-xfs, linux-fsdevel, David Howells
From: Ira Weiny <ira.weiny@intel.com>
There is a conflict with the user visible statx bits 'mount root' and
'dax'. The kernel is changing the dax bit to correct this conflict.[1]
Adjust _check_s_dax() to use the new bit. Because DAX tests do not run
on root mounts, STATX_ATTR_MOUNT_ROOT should always be 0. Therefore,
check for the old flag and fail the test if that occurs.
[1] https://lore.kernel.org/lkml/3e28d2c7-fbe5-298a-13ba-dcd8fd504666@redhat.com/
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Changes from V2:
As suggested by Christoph and Eric:
Fail the test with a hint as to why the wrong bit may be set.
common/rc | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/common/rc b/common/rc
index b5a504e0dcb4..5911a6c89a78 100644
--- a/common/rc
+++ b/common/rc
@@ -3221,10 +3221,27 @@ _check_s_dax()
local exp_s_dax=$2
local attributes=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')
+
+ # The original attribute bit value, STATX_ATTR_DAX (0x2000), conflicted
+ # with STATX_ATTR_MOUNT_ROOT. Therefore, STATX_ATTR_DAX was changed to
+ # 0x00200000.
+ #
+ # Because DAX tests do not run on root mounts, STATX_ATTR_MOUNT_ROOT
+ # should always be 0. Check for the old flag and fail the test if that
+ # occurs.
+
+ if [ $(( attributes & 0x2000 )) -ne 0 ]; then
+ echo "$target has an unexpected STATX_ATTR_MOUNT_ROOT flag set"
+ echo "which used to be STATX_ATTR_DAX"
+ echo " This test should not be running on the root inode..."
+ echo " Does the kernel have the following patch?"
+ echo " 72d1249e2ffd uapi: fix statx attribute value overlap for DAX & MOUNT_ROOT"
+ fi
+
if [ $exp_s_dax -eq 0 ]; then
- (( attributes & 0x2000 )) && echo "$target has unexpected S_DAX flag"
+ (( attributes & 0x00200000 )) && echo "$target has unexpected S_DAX flag"
else
- (( attributes & 0x2000 )) || echo "$target doesn't have expected S_DAX flag"
+ (( attributes & 0x00200000 )) || echo "$target doesn't have expected S_DAX flag"
fi
}
--
2.28.0.rc0.12.gb6a658bd00c9
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH V3] common/rc: Fix _check_s_dax()
2020-12-04 1:45 ` [PATCH V3] " ira.weiny
@ 2020-12-04 9:47 ` Christoph Hellwig
0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2020-12-04 9:47 UTC (permalink / raw)
To: ira.weiny
Cc: fstests, Christoph Hellwig, Eric Sandeen, linux-kernel,
Darrick J. Wong, Dan Williams, Dave Chinner,
Theodore Y. Ts'o, Jan Kara, Jeff Moyer, linux-ext4,
linux-xfs, linux-fsdevel, David Howells
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] common/rc: Fix _check_s_dax()
2020-12-02 21:41 [PATCH] common/rc: Fix _check_s_dax() ira.weiny
2020-12-02 21:46 ` [PATCH V2] " ira.weiny
@ 2020-12-03 8:15 ` Christoph Hellwig
2020-12-03 17:55 ` Eric Sandeen
1 sibling, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2020-12-03 8:15 UTC (permalink / raw)
To: ira.weiny
Cc: fstests, Christoph Hellwig, Eric Sandeen, linux-kernel,
Darrick J. Wong, Dan Williams, Dave Chinner,
Theodore Y. Ts'o, Jan Kara, Jeff Moyer, linux-ext4,
linux-xfs, linux-fsdevel, David Howells
On Wed, Dec 02, 2020 at 01:41:45PM -0800, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
>
> There is a conflict with the user visible statx bits 'mount root' and
> 'dax'. The kernel is changing the dax bit to correct this conflict.[1]
>
> Adjust _check_s_dax() to use the new bit. Because DAX tests do not run
> on root mounts, STATX_ATTR_MOUNT_ROOT should always be 0, therefore we
> can allow either bit to indicate DAX and cover any kernel which may be
> running.
>
> [1] https://lore.kernel.org/lkml/3e28d2c7-fbe5-298a-13ba-dcd8fd504666@redhat.com/
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
>
> I went ahead and used Christoph's suggestion regarding using both bits.
That wasn't my suggestion. I think we should always error out when
the bit value shared with STATX_ATTR_MOUNT_ROOT is seen. Because that
means the kernel is not using or fixed ABI we agreed to use going
forward.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] common/rc: Fix _check_s_dax()
2020-12-03 8:15 ` [PATCH] " Christoph Hellwig
@ 2020-12-03 17:55 ` Eric Sandeen
2020-12-03 18:08 ` Christoph Hellwig
0 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2020-12-03 17:55 UTC (permalink / raw)
To: Christoph Hellwig, ira.weiny
Cc: fstests, Eric Sandeen, linux-kernel, Darrick J. Wong,
Dan Williams, Dave Chinner, Theodore Y. Ts'o, Jan Kara,
Jeff Moyer, linux-ext4, linux-xfs, linux-fsdevel, David Howells
On 12/3/20 2:15 AM, Christoph Hellwig wrote:
> On Wed, Dec 02, 2020 at 01:41:45PM -0800, ira.weiny@intel.com wrote:
>> From: Ira Weiny <ira.weiny@intel.com>
>>
>> There is a conflict with the user visible statx bits 'mount root' and
>> 'dax'. The kernel is changing the dax bit to correct this conflict.[1]
>>
>> Adjust _check_s_dax() to use the new bit. Because DAX tests do not run
>> on root mounts, STATX_ATTR_MOUNT_ROOT should always be 0, therefore we
>> can allow either bit to indicate DAX and cover any kernel which may be
>> running.
>>
>> [1] https://lore.kernel.org/lkml/3e28d2c7-fbe5-298a-13ba-dcd8fd504666@redhat.com/
>>
>> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>> ---
>>
>> I went ahead and used Christoph's suggestion regarding using both bits.
>
> That wasn't my suggestion. I think we should always error out when
> the bit value shared with STATX_ATTR_MOUNT_ROOT is seen. Because that
> means the kernel is not using or fixed ABI we agreed to use going
> forward.
*nod* and my suggestion was to explicitly test for the old/wrong value and
offer the test-runner a hint about why it may have been set (missing the
fix commit), but we should still ultimately fail the test when it is seen.
-Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] common/rc: Fix _check_s_dax()
2020-12-03 17:55 ` Eric Sandeen
@ 2020-12-03 18:08 ` Christoph Hellwig
2020-12-04 1:44 ` Ira Weiny
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2020-12-03 18:08 UTC (permalink / raw)
To: Eric Sandeen
Cc: Christoph Hellwig, ira.weiny, fstests, Eric Sandeen,
linux-kernel, Darrick J. Wong, Dan Williams, Dave Chinner,
Theodore Y. Ts'o, Jan Kara, Jeff Moyer, linux-ext4,
linux-xfs, linux-fsdevel, David Howells
On Thu, Dec 03, 2020 at 11:55:50AM -0600, Eric Sandeen wrote:
> *nod* and my suggestion was to explicitly test for the old/wrong value and
> offer the test-runner a hint about why it may have been set (missing the
> fix commit), but we should still ultimately fail the test when it is seen.
Yes, that's what I'd prefer.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] common/rc: Fix _check_s_dax()
2020-12-03 18:08 ` Christoph Hellwig
@ 2020-12-04 1:44 ` Ira Weiny
0 siblings, 0 replies; 8+ messages in thread
From: Ira Weiny @ 2020-12-04 1:44 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Eric Sandeen, fstests, Eric Sandeen, linux-kernel,
Darrick J. Wong, Dan Williams, Dave Chinner,
Theodore Y. Ts'o, Jan Kara, Jeff Moyer, linux-ext4,
linux-xfs, linux-fsdevel, David Howells
On Thu, Dec 03, 2020 at 07:08:39PM +0100, Christoph Hellwig wrote:
> On Thu, Dec 03, 2020 at 11:55:50AM -0600, Eric Sandeen wrote:
> > *nod* and my suggestion was to explicitly test for the old/wrong value and
> > offer the test-runner a hint about why it may have been set (missing the
> > fix commit), but we should still ultimately fail the test when it is seen.
>
> Yes, that's what I'd prefer.
Sorry for the misunderstanding. V3 on it's way.
Ira
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-12-04 9:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 21:41 [PATCH] common/rc: Fix _check_s_dax() ira.weiny
2020-12-02 21:46 ` [PATCH V2] " ira.weiny
2020-12-04 1:45 ` [PATCH V3] " ira.weiny
2020-12-04 9:47 ` Christoph Hellwig
2020-12-03 8:15 ` [PATCH] " Christoph Hellwig
2020-12-03 17:55 ` Eric Sandeen
2020-12-03 18:08 ` Christoph Hellwig
2020-12-04 1:44 ` Ira Weiny
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).