On 2020/7/15 12:15, Ira Weiny wrote:
On Wed, Jul 15, 2020 at 11:19:58AM +0800, Xiao Yang wrote:
On 2020/7/15 9:59, Ira Weiny wrote:
On Tue, Jul 14, 2020 at 05:40:03PM +0800, Xiao Yang wrote:
1) _require_scratch_dax_mountopt() checks both old and new DAX mount option
2) _require_dax_iflag() checks FS_XFLAG_DAX
Signed-off-by: Xiao Yang<yangx.jy@xxxxxxxxxxxxxx>
Reviewed-by: Ira Weiny<ira.weiny@xxxxxxxxx>
Hi Ira,
Do you want to check if invalid parameter is passed to
_require_scratch_dax_mountopt? Thought
I think it is not necessary(user should pass correct parament).
Like this:
[ "$mountopt" != "dax" ]&& [ "$mountopt" != "dax=always" ]&& _notrun
"$mountopt is invalid"
I thought this patch was just checking if the filesystem/device supported DAX
such that DAX tests could run using either old _or_ new dax support...
But after thinking about it I think we need something more complicated.
Something like:
if ('-o dax=inode')
// FS is DAX with per file support (ie ext4 || XFS kernel>= 5.8)
Hi Ira,
dax=inode/dax=never cannot check if underlying device supports dax, so I
perfer to use dax=always
For example:
XFS:
# mount -o dax=inode /dev/sda8 /mnt/xfstests/test/
# mount | grep sda8
/dev/sda8 on /mnt/xfstests/test type xfs
(rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
...
# mount -o dax=always /dev/sda8 /mnt/xfstests/test/
# mount | grep sda8
/dev/sda8 on /mnt/xfstests/test type xfs
(rw,relatime,seclabel,attr2,dax=never,inode64,logbufs=8,logbsize=32k,noquota)
EXT4:
# mount -o dax=inode /dev/sda8 /mnt/xfstests/test/
# mount | grep sda8
/dev/sda8 on /mnt/xfstests/test type ext4 (rw,relatime,seclabel,dax=inode)
...
# mount -o dax=always /dev/sda8 /mnt/xfstests/test/
mount: /mnt/xfstests/test: wrong fs type, bad option, bad superblock on
/dev/sda8, missing codepage or helper program, or other error.
# dmesg | grep DAX
[597988.165120] EXT4-fs (sda8): DAX unsupported by block device.
If we want to test new dax(i.e. dax=inode, dax=always or dax=never), so
pass "dax=always" to _require_scratch_dax_mountopt
If we want to test old dax or new dax=always(either of them), so pass
"dax" to _require_scratch_dax_mountopt
Best Regards,
Xiao Yang
else if ('-o dax')
// FS is DAX (ie ext4 || XFS on kernel< 5.8)
else
// FS has no dax support.
Do we do that with 2 functions?
_require_scratch_dax_mountopt()
_require_scratch_new_dax_mountopt()
If both fail we are in the else clause above?
We can use the dax=inode because it is a new option and is not necessarily the
option used while running the tests. (I think. as I said my xfstests skills
are not great so I'm not always sure of the correct patterns)
Ira
Best Regards,
Xiao Yang
---
V5->V6:
Simplify _require_scratch_dax_mountopt because it is enough to only check
dax/dax=always mount option. See the following reasons:
1) we cannot detect if underlying device supports dax by mounting dax=inode
or dax=never.
2) dax=always, dax=inode, dax=never are always introduced together.
common/rc | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/common/rc b/common/rc
index f17b19f2..aeec1f11 100644
--- a/common/rc
+++ b/common/rc
@@ -3188,6 +3188,28 @@ _require_scratch_dax()
_scratch_unmount
}
+# Only accept dax/dax=always mount option becasue dax=always, dax=inode
+# and dax=never are always introduced together.
+_require_scratch_dax_mountopt()
+{
+ local mountopt=$1
+
+ _require_scratch
+ _scratch_mkfs> /dev/null 2>&1
+ _try_scratch_mount "-o $mountopt"> /dev/null 2>&1 || \
+ _notrun "mount $SCRATCH_DEV with $mountopt failed"
+
+ _fs_options $SCRATCH_DEV | egrep -q "dax(=always|,|$)" || \
+ _notrun "$SCRATCH_DEV $FSTYP does not support -o $mountopt"
+
+ _scratch_unmount
+}
+
+_require_dax_iflag()
+{
+ _require_xfs_io_command "chattr" "x"
+}
+
# Does norecovery support by this fs?
_require_norecovery()
{
--
2.21.0
.
.
