* [PATCH v6 0/7] Make fstests support new behavior of DAX
@ 2020-07-14 9:40 Xiao Yang
2020-07-14 9:40 ` [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX Xiao Yang
` (6 more replies)
0 siblings, 7 replies; 29+ messages in thread
From: Xiao Yang @ 2020-07-14 9:40 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, ira.weiny, Xiao Yang
The new behavior of DAX on xfs/ext4 has been merged into main kernel
tree/ext4-dax branch so it is time for fstests to support new behavior
of DAX.
1) Refactor common functions and take use of them.
2) Move and update xfs/260.
3) Add two new tests to verify some features.
References:
https://lkml.org/lkml/2019/10/20/96
https://lkml.org/lkml/2020/5/28/949
Xiao Yang (7):
common/rc: Introduce new helpers for DAX mount options and
FS_XFLAG_DAX
fstests: Use _require_scratch_dax_mountopt() and _require_dax_iflag()
generic/223: Don't clear all mkfs options for _scratch_mkfs_geom()
roughly
generic/413, xfs/260: Improve format operation for PMD fault testing
xfs/260: Move and update xfs/260
generic: Verify if statx() can qurey S_DAX flag on regular file
correctly
generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in
various combinations
common/rc | 47 ++++++--
tests/ext4/030 | 2 +-
tests/ext4/031 | 4 +-
tests/generic/223 | 1 -
tests/generic/413 | 12 +-
tests/generic/462 | 2 +-
tests/{xfs/260 => generic/603} | 74 ++++++------
tests/generic/603.out | 2 +
tests/generic/604 | 100 +++++++++++++++++
tests/generic/604.out | 2 +
tests/generic/605 | 199 +++++++++++++++++++++++++++++++++
tests/generic/605.out | 2 +
tests/generic/group | 3 +
tests/xfs/260.out | 2 -
tests/xfs/group | 1 -
15 files changed, 390 insertions(+), 63 deletions(-)
rename tests/{xfs/260 => generic/603} (53%)
create mode 100644 tests/generic/603.out
create mode 100644 tests/generic/604
create mode 100644 tests/generic/604.out
create mode 100644 tests/generic/605
create mode 100644 tests/generic/605.out
delete mode 100644 tests/xfs/260.out
--
2.21.0
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX
2020-07-14 9:40 [PATCH v6 0/7] Make fstests support new behavior of DAX Xiao Yang
@ 2020-07-14 9:40 ` Xiao Yang
2020-07-15 1:59 ` Ira Weiny
2020-07-14 9:40 ` [PATCH v6 2/7] fstests: Use _require_scratch_dax_mountopt() and _require_dax_iflag() Xiao Yang
` (5 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-14 9:40 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, ira.weiny, Xiao Yang
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@cn.fujitsu.com>
---
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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 2/7] fstests: Use _require_scratch_dax_mountopt() and _require_dax_iflag()
2020-07-14 9:40 [PATCH v6 0/7] Make fstests support new behavior of DAX Xiao Yang
2020-07-14 9:40 ` [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX Xiao Yang
@ 2020-07-14 9:40 ` Xiao Yang
2020-07-15 16:08 ` Darrick J. Wong
2020-07-14 9:40 ` [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly Xiao Yang
` (4 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-14 9:40 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, ira.weiny, Xiao Yang
1) Make related tests use _require_scratch_dax_mountopt() and _require_dax_iflag()
2) Remove unused _require_scratch_dax()
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
---
V5->V6:
Merge the third patch into the second patch.
common/rc | 13 -------------
tests/ext4/030 | 2 +-
tests/ext4/031 | 4 ++--
tests/generic/413 | 2 +-
tests/generic/462 | 2 +-
tests/xfs/260 | 4 ++--
6 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/common/rc b/common/rc
index aeec1f11..6c908f2e 100644
--- a/common/rc
+++ b/common/rc
@@ -3175,19 +3175,6 @@ _require_scratch_shutdown()
}
# Does dax mount option work on this dev/fs?
-_require_scratch_dax()
-{
- _require_scratch
- _scratch_mkfs > /dev/null 2>&1
- _try_scratch_mount -o dax || \
- _notrun "mount $SCRATCH_DEV with dax failed"
- # Check options to be sure. XFS ignores dax option
- # and goes on if dev underneath does not support dax.
- _fs_options $SCRATCH_DEV | grep -qw "dax" || \
- _notrun "$SCRATCH_DEV $FSTYP does not support -o 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()
diff --git a/tests/ext4/030 b/tests/ext4/030
index 93bbca86..fb5cf451 100755
--- a/tests/ext4/030
+++ b/tests/ext4/030
@@ -33,7 +33,7 @@ rm -f $seqres.full
# Modify as appropriate.
_supported_os Linux
_supported_fs ext4
-_require_scratch_dax
+_require_scratch_dax_mountopt "dax"
_require_test_program "t_ext4_dax_journal_corruption"
_require_command "$CHATTR_PROG" chattr
diff --git a/tests/ext4/031 b/tests/ext4/031
index dc58214e..20e2fab7 100755
--- a/tests/ext4/031
+++ b/tests/ext4/031
@@ -37,7 +37,7 @@ MOUNT_OPTIONS=""
# Modify as appropriate.
_supported_os Linux
_supported_fs ext4
-_require_scratch_dax
+_require_scratch_dax_mountopt "dax"
_require_test_program "t_ext4_dax_inline_corruption"
_require_scratch_ext4_feature "inline_data"
@@ -56,7 +56,7 @@ _scratch_unmount >> $seqres.full 2>&1
_try_scratch_mount "-o dax" >> $seqres.full 2>&1
if [[ $? != 0 ]]; then
- # _require_scratch_dax already verified that we could mount with DAX.
+ # _require_scratch_dax_mountopt already verified that we could mount with DAX.
# Failure here is expected because we have inline data.
echo "Silence is golden"
status=0
diff --git a/tests/generic/413 b/tests/generic/413
index 1ce89aff..19e1b926 100755
--- a/tests/generic/413
+++ b/tests/generic/413
@@ -31,7 +31,7 @@ rm -f $seqres.full
_supported_fs generic
_supported_os Linux
_require_test
-_require_scratch_dax
+_require_scratch_dax_mountopt "dax"
_require_test_program "feature"
_require_test_program "t_mmap_dio"
_require_xfs_io_command "falloc"
diff --git a/tests/generic/462 b/tests/generic/462
index 1ab6cadc..4a940239 100755
--- a/tests/generic/462
+++ b/tests/generic/462
@@ -37,7 +37,7 @@ rm -f $seqres.full
_supported_fs generic
_supported_os Linux
_require_test
-_require_scratch_dax
+_require_scratch_dax_mountopt "dax"
_require_test_program "t_mmap_write_ro"
# running by unpriviliged user is not necessary to reproduce
# this bug, just trying to test more.
diff --git a/tests/xfs/260 b/tests/xfs/260
index 3464ffef..bcdc6041 100755
--- a/tests/xfs/260
+++ b/tests/xfs/260
@@ -30,10 +30,10 @@ rm -f $seqres.full
_supported_fs xfs
_supported_os Linux
-_require_scratch_dax
+_require_scratch_dax_mountopt "dax"
_require_test_program "feature"
_require_test_program "t_mmap_dio"
-_require_xfs_io_command "chattr" "x"
+_require_dax_iflag
_require_xfs_io_command "falloc"
prep_files()
--
2.21.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly
2020-07-14 9:40 [PATCH v6 0/7] Make fstests support new behavior of DAX Xiao Yang
2020-07-14 9:40 ` [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX Xiao Yang
2020-07-14 9:40 ` [PATCH v6 2/7] fstests: Use _require_scratch_dax_mountopt() and _require_dax_iflag() Xiao Yang
@ 2020-07-14 9:40 ` Xiao Yang
2020-07-15 2:31 ` Ira Weiny
2020-07-14 9:40 ` [PATCH v6 4/7] generic/413, xfs/260: Improve format operation for PMD fault testing Xiao Yang
` (3 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-14 9:40 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, ira.weiny, Xiao Yang
ext4 can accept the last one if the same mkfs options are passed but xfs cannot
accept the same mkfs options and reports "xxx option is respecified" error. I
prefer to override the same mkfs option which is defined in MKFS_OPTION so that
we can have a chance to pass other mkfs options to _scratch_mkfs_geom().
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
common/rc | 14 +++++++++++++-
tests/generic/223 | 1 -
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/common/rc b/common/rc
index 6c908f2e..567cf83b 100644
--- a/common/rc
+++ b/common/rc
@@ -1051,7 +1051,19 @@ _scratch_mkfs_geom()
case $FSTYP in
xfs)
- MKFS_OPTIONS+=" -b size=$blocksize, -d su=$sunit_bytes,sw=$swidth_mult"
+ if echo "$MKFS_OPTIONS" | egrep -q "b?size="; then
+ MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r "s/(b?size=)[0-9]+/\1$blocksize/")
+ else
+ MKFS_OPTIONS+=" -b size=$blocksize"
+ fi
+
+ if echo "$MKFS_OPTIONS" | egrep -q "(su|sunit|sw|swidth)="; then
+ MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r \
+ -e "s/(su|sunit)=[0-9kmg]+/su=$sunit_bytes/" \
+ -e "s/(sw|swidth)=[0-9kmg]+/sw=$swidth_mult/")
+ else
+ MKFS_OPTIONS+=" -d su=$sunit_bytes,sw=$swidth_mult"
+ fi
;;
ext4|ext4dev)
MKFS_OPTIONS+=" -b $blocksize -E stride=$sunit_blocks,stripe_width=$swidth_blocks"
diff --git a/tests/generic/223 b/tests/generic/223
index 6cfd00dd..ba7c9a44 100755
--- a/tests/generic/223
+++ b/tests/generic/223
@@ -41,7 +41,6 @@ for SUNIT_K in 8 16 32 64 128; do
let SUNIT_BLOCKS=$SUNIT_BYTES/$BLOCKSIZE
echo "=== mkfs with su $SUNIT_BLOCKS blocks x 4 ==="
- export MKFS_OPTIONS=""
_scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >> $seqres.full 2>&1
_scratch_mount
--
2.21.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 4/7] generic/413, xfs/260: Improve format operation for PMD fault testing
2020-07-14 9:40 [PATCH v6 0/7] Make fstests support new behavior of DAX Xiao Yang
` (2 preceding siblings ...)
2020-07-14 9:40 ` [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly Xiao Yang
@ 2020-07-14 9:40 ` Xiao Yang
2020-07-15 16:09 ` Darrick J. Wong
2020-07-14 9:40 ` [PATCH v6 5/7] xfs/260: Move and update xfs/260 Xiao Yang
` (2 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-14 9:40 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, ira.weiny, Xiao Yang
1) Simple code and fix the wrong value of stripe_width by _scratch_mkfs_geom().
2) Get hugepage size by _get_hugepagesize() and replace fixed 2M with
hugepage size because hugepage size/PMD_SIZE is not 2M on some
arches(e.g. hugepage size/PMD_SIZE is 512M on arm64).
3) For debugging, redirect the output of mkfs to $seqres.full.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
common/rc | 10 ++++++++++
tests/generic/413 | 10 ++--------
tests/xfs/260 | 4 ++--
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/common/rc b/common/rc
index 567cf83b..b6a48241 100644
--- a/common/rc
+++ b/common/rc
@@ -170,6 +170,16 @@ _get_filesize()
stat -c %s "$1"
}
+# Get hugepagesize in bytes
+_get_hugepagesize()
+{
+ local hugepgsz=$(awk '/Hugepagesize/ {print $2}' /proc/meminfo)
+ # Call _notrun if $hugepgsz is not a number
+ echo "$hugepgsz" | egrep -q ^[0-9]+$ || \
+ _notrun "Cannot get the value of Hugepagesize"
+ echo $((hugepgsz * 1024))
+}
+
_mount()
{
$MOUNT_PROG `_mount_ops_filter $*`
diff --git a/tests/generic/413 b/tests/generic/413
index 19e1b926..dfe2912b 100755
--- a/tests/generic/413
+++ b/tests/generic/413
@@ -111,14 +111,8 @@ do_tests()
t_mmap_dio_dax $((64 * 1024 * 1024))
}
-# make fs 2Mb aligned for PMD fault testing
-mkfs_opts=""
-if [ "$FSTYP" == "ext4" ]; then
- mkfs_opts="-E stride=512,stripe_width=1"
-elif [ "$FSTYP" == "xfs" ]; then
- mkfs_opts="-d su=2m,sw=1"
-fi
-_scratch_mkfs "$mkfs_opts" > /dev/null 2>&1
+# make fs aligned for PMD fault testing
+_scratch_mkfs_geom $(_get_hugepagesize) 1 >> $seqres.full 2>&1
# mount SCRATCH_DEV with dax option, TEST_DEV not
export MOUNT_OPTIONS=""
diff --git a/tests/xfs/260 b/tests/xfs/260
index bcdc6041..81b42f01 100755
--- a/tests/xfs/260
+++ b/tests/xfs/260
@@ -121,8 +121,8 @@ do_tests()
t_dax_flag_mmap_dio $((64 * 1024 * 1024))
}
-# make xfs 2Mb aligned for PMD fault testing
-_scratch_mkfs "-d su=2m,sw=1" > /dev/null 2>&1
+# make xfs aligned for PMD fault testing
+_scratch_mkfs_geom $(_get_hugepagesize) 1 >> $seqres.full 2>&1
# mount with dax option
_scratch_mount "-o dax"
--
2.21.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 5/7] xfs/260: Move and update xfs/260
2020-07-14 9:40 [PATCH v6 0/7] Make fstests support new behavior of DAX Xiao Yang
` (3 preceding siblings ...)
2020-07-14 9:40 ` [PATCH v6 4/7] generic/413, xfs/260: Improve format operation for PMD fault testing Xiao Yang
@ 2020-07-14 9:40 ` Xiao Yang
2020-07-15 16:10 ` Darrick J. Wong
2020-07-14 9:40 ` [PATCH v6 6/7] generic: Verify if statx() can qurey S_DAX flag on regular file correctly Xiao Yang
2020-07-14 9:40 ` [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations Xiao Yang
6 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-14 9:40 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, ira.weiny, Xiao Yang
1) Both ext4 and xfs have supported FS_XFLAG_DAX so move it to generic.
2) Modifying FS_XFLAG_DAX on flies does not take effect immediately so
make files inherit the DAX state of parent directory.
3) Setting/clearing FS_XFLAG_DAX have no chance to change S_DAX flag if
mount with dax option so remove the related subtest.
4) Setting/clearing FS_XFLAG_DAX doesn't change S_DAX flag on older xfs
due to commit 742d84290739 ("xfs: disable per-inode DAX flag") so
only do test when fs supports new dax=inode option.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
---
tests/{xfs/260 => generic/603} | 68 +++++++++++++++++-----------------
tests/generic/603.out | 2 +
tests/generic/group | 1 +
tests/xfs/260.out | 2 -
tests/xfs/group | 1 -
5 files changed, 36 insertions(+), 38 deletions(-)
rename tests/{xfs/260 => generic/603} (57%)
create mode 100644 tests/generic/603.out
delete mode 100644 tests/xfs/260.out
diff --git a/tests/xfs/260 b/tests/generic/603
similarity index 57%
rename from tests/xfs/260
rename to tests/generic/603
index 81b42f01..5dabe447 100755
--- a/tests/xfs/260
+++ b/tests/generic/603
@@ -2,7 +2,7 @@
# SPDX-License-Identifier: GPL-2.0
# Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
#
-# FS QA Test 260
+# FS QA Test 603
#
# Test per-inode DAX flag by mmap direct/buffered IO.
#
@@ -28,76 +28,80 @@ _cleanup()
# remove previous $seqres.full before test
rm -f $seqres.full
-_supported_fs xfs
+_supported_fs generic
_supported_os Linux
-_require_scratch_dax_mountopt "dax"
+_require_scratch_dax_mountopt "dax=always"
_require_test_program "feature"
_require_test_program "t_mmap_dio"
_require_dax_iflag
_require_xfs_io_command "falloc"
-prep_files()
+SRC_DIR=$SCRATCH_MNT/src
+SRC_FILE=$SRC_DIR/tf_s
+DST_DIR=$SCRATCH_MNT/dst
+DST_FILE=$DST_DIR/tf_d
+
+prep_directories()
{
- rm -f $SCRATCH_MNT/tf_{s,d}
+ mkdir -p $SRC_DIR $DST_DIR
+}
+prep_files()
+{
+ rm -f $SRC_FILE $DST_FILE
$XFS_IO_PROG -f -c "falloc 0 $tsize" \
- $SCRATCH_MNT/tf_{s,d} >> $seqres.full 2>&1
+ $SRC_FILE $DST_FILE >> $seqres.full 2>&1
}
t_both_dax()
{
+ $XFS_IO_PROG -c "chattr +x" $SRC_DIR $DST_DIR
prep_files
- $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_{s,d}
# with O_DIRECT first
- $here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} $1 "dio both dax"
+ $here/src/t_mmap_dio $SRC_FILE $DST_FILE \
+ $1 "dio both dax"
prep_files
- $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_{s,d}
# again with buffered IO
- $here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
+ $here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
$1 "buffered both dax"
}
t_nondax_to_dax()
{
+ $XFS_IO_PROG -c "chattr -x" $SRC_DIR
+ $XFS_IO_PROG -c "chattr +x" $DST_DIR
prep_files
- $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_s
- $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_d
- $here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
+ $here/src/t_mmap_dio $SRC_FILE $DST_FILE \
$1 "dio nondax to dax"
prep_files
- $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_s
- $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_d
- $here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
+ $here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
$1 "buffered nondax to dax"
}
t_dax_to_nondax()
{
+ $XFS_IO_PROG -c "chattr +x" $SRC_DIR
+ $XFS_IO_PROG -c "chattr -x" $DST_DIR
prep_files
- $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_s
- $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_d
- $here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
+ $here/src/t_mmap_dio $SRC_FILE $DST_FILE \
$1 "dio dax to nondax"
prep_files
- $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_s
- $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_d
- $here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
+ $here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
$1 "buffered dax to nondax"
}
t_both_nondax()
{
+ $XFS_IO_PROG -c "chattr -x" $SRC_DIR $DST_DIR
prep_files
- $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_{s,d}
- $here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
+ $here/src/t_mmap_dio $SRC_FILE $DST_FILE \
$1 "dio both nondax"
prep_files
- $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_{s,d}
- $here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
+ $here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
$1 "buffered both nondax"
}
@@ -112,6 +116,7 @@ t_dax_flag_mmap_dio()
do_tests()
{
+ prep_directories
# less than page size
t_dax_flag_mmap_dio 1024
# page size
@@ -124,17 +129,10 @@ do_tests()
# make xfs aligned for PMD fault testing
_scratch_mkfs_geom $(_get_hugepagesize) 1 >> $seqres.full 2>&1
-# mount with dax option
-_scratch_mount "-o dax"
-
tsize=$((128 * 1024 * 1024))
-do_tests
-_scratch_unmount
-
-# mount again without dax option
-export MOUNT_OPTIONS=""
-_scratch_mount
+# mount with dax=inode option
+_scratch_mount "-o dax=inode"
do_tests
# success, all done
diff --git a/tests/generic/603.out b/tests/generic/603.out
new file mode 100644
index 00000000..6810da89
--- /dev/null
+++ b/tests/generic/603.out
@@ -0,0 +1,2 @@
+QA output created by 603
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index d9ab9a31..1d0d5606 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -605,3 +605,4 @@
600 auto quick quota
601 auto quick quota
602 auto quick encrypt
+603 auto attr quick dax
diff --git a/tests/xfs/260.out b/tests/xfs/260.out
deleted file mode 100644
index 18ca517c..00000000
--- a/tests/xfs/260.out
+++ /dev/null
@@ -1,2 +0,0 @@
-QA output created by 260
-Silence is golden
diff --git a/tests/xfs/group b/tests/xfs/group
index daf54add..71c30898 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -257,7 +257,6 @@
257 auto quick clone
258 auto quick clone
259 auto quick
-260 auto attr quick dax
261 auto quick quota
262 dangerous_fuzzers dangerous_scrub dangerous_online_repair
263 auto quick quota
--
2.21.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 6/7] generic: Verify if statx() can qurey S_DAX flag on regular file correctly
2020-07-14 9:40 [PATCH v6 0/7] Make fstests support new behavior of DAX Xiao Yang
` (4 preceding siblings ...)
2020-07-14 9:40 ` [PATCH v6 5/7] xfs/260: Move and update xfs/260 Xiao Yang
@ 2020-07-14 9:40 ` Xiao Yang
2020-07-14 9:40 ` [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations Xiao Yang
6 siblings, 0 replies; 29+ messages in thread
From: Xiao Yang @ 2020-07-14 9:40 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, ira.weiny, Xiao Yang
With new kernel(e.g. v5.8-rc1), statx() can be used to qurey S_DAX flag
on regular file, so add a test to verify the feature.
Reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=712b2698e4c024b561694cbcc1abba13eb0fd9ce
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
tests/generic/604 | 100 ++++++++++++++++++++++++++++++++++++++++++
tests/generic/604.out | 2 +
tests/generic/group | 1 +
3 files changed, 103 insertions(+)
create mode 100644 tests/generic/604
create mode 100644 tests/generic/604.out
diff --git a/tests/generic/604 b/tests/generic/604
new file mode 100644
index 00000000..59963dbc
--- /dev/null
+++ b/tests/generic/604
@@ -0,0 +1,100 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Fujitsu. All Rights Reserved.
+#
+# FS QA Test 604
+#
+# By the following cases, verify if statx() can query S_DAX flag
+# on regular file correctly.
+# 1) With dax=always option, FS_XFLAG_DAX is ignored and S_DAX flag
+# always exists on regular file.
+# 2) With dax=inode option, setting/clearing FS_XFLAG_DAX can change
+# S_DAX flag on regular file.
+# 3) With dax=never option, FS_XFLAG_DAX is ignored and S_DAX flag
+# never exists on regular file.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+_supported_fs generic
+_supported_os Linux
+_require_scratch_dax_mountopt "dax=always"
+_require_dax_iflag
+_require_xfs_io_command "statx" "-r"
+
+PARENT_DIR=$SCRATCH_MNT/testdir
+TEST_FILE=$PARENT_DIR/testfile
+
+check_s_dax()
+{
+ local exp_s_dax=$1
+
+ local attributes=$($XFS_IO_PROG -c 'statx -r' $TEST_FILE | awk '/stat.attributes / { print $3 }')
+ if [ $exp_s_dax -eq 0 ]; then
+ (( attributes & 0x2000 )) && echo "$TEST_FILE has unexpected S_DAX flag"
+ else
+ (( attributes & 0x2000 )) || echo "$TEST_FILE doen't have expected S_DAX flag"
+ fi
+}
+
+test_s_dax()
+{
+ local dax_option=$1
+ local exp_s_dax1=$2
+ local exp_s_dax2=$3
+
+ # Mount with specified dax option
+ _scratch_mount "-o $dax_option"
+
+ # Prepare directory
+ mkdir -p $PARENT_DIR
+
+ rm -f $TEST_FILE
+ $XFS_IO_PROG -c "chattr +x" $PARENT_DIR
+ touch $TEST_FILE
+ # Check if setting FS_XFLAG_DAX changes S_DAX flag
+ check_s_dax $exp_s_dax1
+
+ rm -f $TEST_FILE
+ $XFS_IO_PROG -c "chattr -x" $PARENT_DIR
+ touch $TEST_FILE
+ # Check if clearing FS_XFLAG_DAX changes S_DAX flag
+ check_s_dax $exp_s_dax2
+
+ _scratch_unmount
+}
+
+do_tests()
+{
+ test_s_dax "dax=always" 1 1
+ test_s_dax "dax=inode" 1 0
+ test_s_dax "dax=never" 0 0
+}
+
+_scratch_mkfs >> $seqres.full 2>&1
+
+do_tests
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/604.out b/tests/generic/604.out
new file mode 100644
index 00000000..c06a1c7f
--- /dev/null
+++ b/tests/generic/604.out
@@ -0,0 +1,2 @@
+QA output created by 604
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 1d0d5606..676e0d2e 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -606,3 +606,4 @@
601 auto quick quota
602 auto quick encrypt
603 auto attr quick dax
+604 auto attr quick dax
--
2.21.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-14 9:40 [PATCH v6 0/7] Make fstests support new behavior of DAX Xiao Yang
` (5 preceding siblings ...)
2020-07-14 9:40 ` [PATCH v6 6/7] generic: Verify if statx() can qurey S_DAX flag on regular file correctly Xiao Yang
@ 2020-07-14 9:40 ` Xiao Yang
2020-07-15 2:48 ` Ira Weiny
6 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-14 9:40 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, ira.weiny, Xiao Yang
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
tests/generic/605 | 199 ++++++++++++++++++++++++++++++++++++++++++
tests/generic/605.out | 2 +
tests/generic/group | 1 +
3 files changed, 202 insertions(+)
create mode 100644 tests/generic/605
create mode 100644 tests/generic/605.out
diff --git a/tests/generic/605 b/tests/generic/605
new file mode 100644
index 00000000..6924223a
--- /dev/null
+++ b/tests/generic/605
@@ -0,0 +1,199 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Fujitsu. All Rights Reserved.
+#
+# FS QA Test 605
+#
+# Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations.
+# 1) New files and directories automatically inherit FS_XFLAG_DAX from their parent directory.
+# 2) cp operation make files and directories inherit the FS_XFLAG_DAX from new parent directory.
+# 3) mv operation make files and directories preserve the FS_XFLAG_DAX from old parent directory.
+# In addition, setting/clearing FS_XFLAG_DAX flag is not impacted by dax mount options.
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_dax_iflag
+_require_xfs_io_command "lsattr" "-v"
+
+check_xflag()
+{
+ local target=$1
+ local exp_xflag=$2
+
+ if [ $exp_xflag -eq 0 ]; then
+ _test_inode_flag dax $target && echo "$target has unexpected FS_XFLAG_DAX flag"
+ else
+ _test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
+ fi
+}
+
+test_xflag_inheritance1()
+{
+ mkdir -p a
+ $XFS_IO_PROG -c "chattr +x" a
+ mkdir -p a/b/c
+ touch a/b/c/d
+
+ check_xflag a 1
+ check_xflag a/b 1
+ check_xflag a/b/c 1
+ check_xflag a/b/c/d 1
+
+ rm -rf a
+}
+
+test_xflag_inheritance2()
+{
+ mkdir -p a/b
+ $XFS_IO_PROG -c "chattr +x" a
+ mkdir -p a/b/c a/d
+ touch a/b/c/e a/d/f
+
+ check_xflag a 1
+ check_xflag a/b 0
+ check_xflag a/b/c 0
+ check_xflag a/b/c/e 0
+ check_xflag a/d 1
+ check_xflag a/d/f 1
+
+ rm -rf a
+}
+
+test_xflag_inheritance3()
+{
+ mkdir -p a/b
+ $XFS_IO_PROG -c "chattr +x" a/b
+ mkdir -p a/b/c a/d
+ touch a/b/c/e a/d/f
+
+ check_xflag a 0
+ check_xflag a/b 1
+ check_xflag a/b/c 1
+ check_xflag a/b/c/e 1
+ check_xflag a/d 0
+ check_xflag a/d/f 0
+
+ rm -rf a
+}
+
+test_xflag_inheritance4()
+{
+ mkdir -p a
+ $XFS_IO_PROG -c "chattr +x" a
+ mkdir -p a/b/c
+ $XFS_IO_PROG -c "chattr -x" a/b
+ mkdir -p a/b/c/d a/b/e
+ touch a/b/c/d/f a/b/e/g
+
+ check_xflag a 1
+ check_xflag a/b 0
+ check_xflag a/b/c 1
+ check_xflag a/b/c/d 1
+ check_xflag a/b/c/d/f 1
+ check_xflag a/b/e 0
+ check_xflag a/b/e/g 0
+
+ rm -rf a
+}
+
+test_xflag_inheritance5()
+{
+ mkdir -p a b
+ $XFS_IO_PROG -c "chattr +x" a
+ mkdir -p a/c a/d b/e b/f
+ touch a/g b/h
+
+ cp -r a/c b/
+ cp -r b/e a/
+ cp -r a/g b/
+ mv a/d b/
+ mv b/f a/
+ mv b/h a/
+
+ check_xflag b/c 0
+ check_xflag b/d 1
+ check_xflag a/e 1
+ check_xflag a/f 0
+ check_xflag b/g 0
+ check_xflag a/h 0
+
+ rm -rf a b
+}
+
+do_xflag_tests()
+{
+ local option=$1
+
+ _scratch_mount "$option"
+ cd $SCRATCH_MNT
+
+ for i in $(seq 1 5); do
+ test_xflag_inheritance${i}
+ done
+
+ cd - > /dev/null
+ _scratch_unmount
+}
+
+check_dax_mountopt()
+{
+ local option=$1
+ local ret=0
+
+ _try_scratch_mount "-o $option" >> $seqres.full 2>&1 || return 1
+
+ # Match option name exactly
+ _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
+
+ _scratch_unmount
+
+ return $ret
+}
+
+do_tests()
+{
+ # Mount without dax option
+ do_xflag_tests
+
+ # Mount with old dax option if fs only supports it.
+ check_dax_mountopt "dax" && do_xflag_tests "-o dax"
+
+ # Mount with new dax options if fs supports them.
+ if check_dax_mountopt "dax=always"; then
+ for dax_option in "dax=always" "dax=inode" "dax=never"; do
+ do_xflag_tests "-o $dax_option"
+ done
+ fi
+}
+
+_scratch_mkfs >> $seqres.full 2>&1
+
+do_tests
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/605.out b/tests/generic/605.out
new file mode 100644
index 00000000..1ae20049
--- /dev/null
+++ b/tests/generic/605.out
@@ -0,0 +1,2 @@
+QA output created by 605
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 676e0d2e..a8451862 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -607,3 +607,4 @@
602 auto quick encrypt
603 auto attr quick dax
604 auto attr quick dax
+605 auto attr quick dax
--
2.21.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX
2020-07-14 9:40 ` [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX Xiao Yang
@ 2020-07-15 1:59 ` Ira Weiny
2020-07-15 3:19 ` Xiao Yang
0 siblings, 1 reply; 29+ messages in thread
From: Ira Weiny @ 2020-07-15 1:59 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests, darrick.wong
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@cn.fujitsu.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> ---
>
> 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
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly
2020-07-14 9:40 ` [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly Xiao Yang
@ 2020-07-15 2:31 ` Ira Weiny
2020-07-15 3:12 ` Xiao Yang
0 siblings, 1 reply; 29+ messages in thread
From: Ira Weiny @ 2020-07-15 2:31 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests, darrick.wong
On Tue, Jul 14, 2020 at 05:40:05PM +0800, Xiao Yang wrote:
> ext4 can accept the last one if the same mkfs options are passed but xfs cannot
I'm having trouble parsing this commit message. What does 'last one' refer to?
> accept the same mkfs options and reports "xxx option is respecified" error.
Ok I think I understand now. Some FS's (XFS) do not accept an option more than
once. So we can't just blindly add options to the end of the MKFS_OPTIONS. Is
that correct?
> I
> prefer to override the same mkfs option which is defined in MKFS_OPTION so that
> we can have a chance to pass other mkfs options to _scratch_mkfs_geom().
Instead the patch parses the current option string and replaces the value if
the option is already there. This allows us to specify MKFS_OPTIONS to
generic/223.
I think the code is reasonable although my sed skills are not good enough to
tell for sure... ;-)
Ira
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> common/rc | 14 +++++++++++++-
> tests/generic/223 | 1 -
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 6c908f2e..567cf83b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1051,7 +1051,19 @@ _scratch_mkfs_geom()
>
> case $FSTYP in
> xfs)
> - MKFS_OPTIONS+=" -b size=$blocksize, -d su=$sunit_bytes,sw=$swidth_mult"
> + if echo "$MKFS_OPTIONS" | egrep -q "b?size="; then
> + MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r "s/(b?size=)[0-9]+/\1$blocksize/")
> + else
> + MKFS_OPTIONS+=" -b size=$blocksize"
> + fi
> +
> + if echo "$MKFS_OPTIONS" | egrep -q "(su|sunit|sw|swidth)="; then
> + MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r \
> + -e "s/(su|sunit)=[0-9kmg]+/su=$sunit_bytes/" \
> + -e "s/(sw|swidth)=[0-9kmg]+/sw=$swidth_mult/")
> + else
> + MKFS_OPTIONS+=" -d su=$sunit_bytes,sw=$swidth_mult"
> + fi
> ;;
> ext4|ext4dev)
> MKFS_OPTIONS+=" -b $blocksize -E stride=$sunit_blocks,stripe_width=$swidth_blocks"
> diff --git a/tests/generic/223 b/tests/generic/223
> index 6cfd00dd..ba7c9a44 100755
> --- a/tests/generic/223
> +++ b/tests/generic/223
> @@ -41,7 +41,6 @@ for SUNIT_K in 8 16 32 64 128; do
> let SUNIT_BLOCKS=$SUNIT_BYTES/$BLOCKSIZE
>
> echo "=== mkfs with su $SUNIT_BLOCKS blocks x 4 ==="
> - export MKFS_OPTIONS=""
> _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >> $seqres.full 2>&1
> _scratch_mount
>
> --
> 2.21.0
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-14 9:40 ` [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations Xiao Yang
@ 2020-07-15 2:48 ` Ira Weiny
2020-07-15 5:39 ` Xiao Yang
0 siblings, 1 reply; 29+ messages in thread
From: Ira Weiny @ 2020-07-15 2:48 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests, darrick.wong
On Tue, Jul 14, 2020 at 05:40:09PM +0800, Xiao Yang wrote:
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> tests/generic/605 | 199 ++++++++++++++++++++++++++++++++++++++++++
> tests/generic/605.out | 2 +
> tests/generic/group | 1 +
> 3 files changed, 202 insertions(+)
> create mode 100644 tests/generic/605
> create mode 100644 tests/generic/605.out
>
> diff --git a/tests/generic/605 b/tests/generic/605
> new file mode 100644
> index 00000000..6924223a
> --- /dev/null
> +++ b/tests/generic/605
> @@ -0,0 +1,199 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Fujitsu. All Rights Reserved.
> +#
> +# FS QA Test 605
> +#
> +# Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations.
> +# 1) New files and directories automatically inherit FS_XFLAG_DAX from their parent directory.
> +# 2) cp operation make files and directories inherit the FS_XFLAG_DAX from new parent directory.
> +# 3) mv operation make files and directories preserve the FS_XFLAG_DAX from old parent directory.
> +# In addition, setting/clearing FS_XFLAG_DAX flag is not impacted by dax mount options.
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +_require_dax_iflag
> +_require_xfs_io_command "lsattr" "-v"
> +
> +check_xflag()
> +{
> + local target=$1
> + local exp_xflag=$2
> +
> + if [ $exp_xflag -eq 0 ]; then
> + _test_inode_flag dax $target && echo "$target has unexpected FS_XFLAG_DAX flag"
> + else
> + _test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
> + fi
> +}
> +
> +test_xflag_inheritance1()
> +{
> + mkdir -p a
> + $XFS_IO_PROG -c "chattr +x" a
> + mkdir -p a/b/c
> + touch a/b/c/d
> +
> + check_xflag a 1
> + check_xflag a/b 1
> + check_xflag a/b/c 1
> + check_xflag a/b/c/d 1
> +
> + rm -rf a
> +}
> +
> +test_xflag_inheritance2()
> +{
> + mkdir -p a/b
> + $XFS_IO_PROG -c "chattr +x" a
> + mkdir -p a/b/c a/d
> + touch a/b/c/e a/d/f
> +
> + check_xflag a 1
> + check_xflag a/b 0
> + check_xflag a/b/c 0
> + check_xflag a/b/c/e 0
> + check_xflag a/d 1
> + check_xflag a/d/f 1
> +
> + rm -rf a
> +}
> +
> +test_xflag_inheritance3()
> +{
> + mkdir -p a/b
> + $XFS_IO_PROG -c "chattr +x" a/b
> + mkdir -p a/b/c a/d
> + touch a/b/c/e a/d/f
> +
> + check_xflag a 0
> + check_xflag a/b 1
> + check_xflag a/b/c 1
> + check_xflag a/b/c/e 1
> + check_xflag a/d 0
> + check_xflag a/d/f 0
> +
> + rm -rf a
> +}
It really seems like 2 and 3 test the same thing?
> +
> +test_xflag_inheritance4()
> +{
> + mkdir -p a
> + $XFS_IO_PROG -c "chattr +x" a
> + mkdir -p a/b/c
> + $XFS_IO_PROG -c "chattr -x" a/b
> + mkdir -p a/b/c/d a/b/e
> + touch a/b/c/d/f a/b/e/g
> +
> + check_xflag a 1
> + check_xflag a/b 0
> + check_xflag a/b/c 1
> + check_xflag a/b/c/d 1
> + check_xflag a/b/c/d/f 1
> + check_xflag a/b/e 0
> + check_xflag a/b/e/g 0
> +
> + rm -rf a
> +}
> +
> +test_xflag_inheritance5()
> +{
> + mkdir -p a b
> + $XFS_IO_PROG -c "chattr +x" a
> + mkdir -p a/c a/d b/e b/f
> + touch a/g b/h
> +
> + cp -r a/c b/
> + cp -r b/e a/
> + cp -r a/g b/
> + mv a/d b/
> + mv b/f a/
> + mv b/h a/
> +
> + check_xflag b/c 0
> + check_xflag b/d 1
> + check_xflag a/e 1
> + check_xflag a/f 0
> + check_xflag b/g 0
> + check_xflag a/h 0
> +
> + rm -rf a b
> +}
> +
> +do_xflag_tests()
> +{
> + local option=$1
> +
> + _scratch_mount "$option"
> + cd $SCRATCH_MNT
> +
> + for i in $(seq 1 5); do
> + test_xflag_inheritance${i}
> + done
> +
> + cd - > /dev/null
> + _scratch_unmount
> +}
> +
> +check_dax_mountopt()
> +{
> + local option=$1
> + local ret=0
> +
> + _try_scratch_mount "-o $option" >> $seqres.full 2>&1 || return 1
> +
> + # Match option name exactly
> + _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
> +
> + _scratch_unmount
> +
> + return $ret
> +}
Should this be a common function?
> +
> +do_tests()
> +{
> + # Mount without dax option
> + do_xflag_tests
> +
> + # Mount with old dax option if fs only supports it.
> + check_dax_mountopt "dax" && do_xflag_tests "-o dax"
I don't understand the order here. If we are on an older kernel and the FS
only supports '-o dax' the do_xflag_tests will fail won't it?
So shouldn't we do this first and bail/'not run' this test if that is the case?
I really don't think there is any point in testing the old XFS behavior because
the FS_XFLAG_DAX had no effect. So even if it is broken it does not matter.
Or perhaps I am missing something here?
Ira
> +
> + # Mount with new dax options if fs supports them.
> + if check_dax_mountopt "dax=always"; then
> + for dax_option in "dax=always" "dax=inode" "dax=never"; do
> + do_xflag_tests "-o $dax_option"
> + done
> + fi
> +}
> +
> +_scratch_mkfs >> $seqres.full 2>&1
> +
> +do_tests
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/605.out b/tests/generic/605.out
> new file mode 100644
> index 00000000..1ae20049
> --- /dev/null
> +++ b/tests/generic/605.out
> @@ -0,0 +1,2 @@
> +QA output created by 605
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index 676e0d2e..a8451862 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -607,3 +607,4 @@
> 602 auto quick encrypt
> 603 auto attr quick dax
> 604 auto attr quick dax
> +605 auto attr quick dax
> --
> 2.21.0
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly
2020-07-15 2:31 ` Ira Weiny
@ 2020-07-15 3:12 ` Xiao Yang
2020-07-15 16:07 ` Darrick J. Wong
0 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-15 3:12 UTC (permalink / raw)
To: Ira Weiny; +Cc: fstests, darrick.wong
On 2020/7/15 10:31, Ira Weiny wrote:
> On Tue, Jul 14, 2020 at 05:40:05PM +0800, Xiao Yang wrote:
>> ext4 can accept the last one if the same mkfs options are passed but xfs cannot
> I'm having trouble parsing this commit message. What does 'last one' refer to?
>
>> accept the same mkfs options and reports "xxx option is respecified" error.
> Ok I think I understand now. Some FS's (XFS) do not accept an option more than
> once. So we can't just blindly add options to the end of the MKFS_OPTIONS. Is
> that correct?
Hi Ira,
Correct.
>> I
>> prefer to override the same mkfs option which is defined in MKFS_OPTION so that
>> we can have a chance to pass other mkfs options to _scratch_mkfs_geom().
> Instead the patch parses the current option string and replaces the value if
> the option is already there. This allows us to specify MKFS_OPTIONS to
> generic/223.
Right. :-)
XFS_MKFS_OPTIONS/MKFS_OPTIONS can be used to specify some custom options by user,
so I don't want to clear it blindly.
Thanks,
Xiao Yang
> I think the code is reasonable although my sed skills are not good enough to
> tell for sure... ;-)
>
> Ira
>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>> common/rc | 14 +++++++++++++-
>> tests/generic/223 | 1 -
>> 2 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/rc b/common/rc
>> index 6c908f2e..567cf83b 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -1051,7 +1051,19 @@ _scratch_mkfs_geom()
>>
>> case $FSTYP in
>> xfs)
>> - MKFS_OPTIONS+=" -b size=$blocksize, -d su=$sunit_bytes,sw=$swidth_mult"
>> + if echo "$MKFS_OPTIONS" | egrep -q "b?size="; then
>> + MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r "s/(b?size=)[0-9]+/\1$blocksize/")
>> + else
>> + MKFS_OPTIONS+=" -b size=$blocksize"
>> + fi
>> +
>> + if echo "$MKFS_OPTIONS" | egrep -q "(su|sunit|sw|swidth)="; then
>> + MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r \
>> + -e "s/(su|sunit)=[0-9kmg]+/su=$sunit_bytes/" \
>> + -e "s/(sw|swidth)=[0-9kmg]+/sw=$swidth_mult/")
>> + else
>> + MKFS_OPTIONS+=" -d su=$sunit_bytes,sw=$swidth_mult"
>> + fi
>> ;;
>> ext4|ext4dev)
>> MKFS_OPTIONS+=" -b $blocksize -E stride=$sunit_blocks,stripe_width=$swidth_blocks"
>> diff --git a/tests/generic/223 b/tests/generic/223
>> index 6cfd00dd..ba7c9a44 100755
>> --- a/tests/generic/223
>> +++ b/tests/generic/223
>> @@ -41,7 +41,6 @@ for SUNIT_K in 8 16 32 64 128; do
>> let SUNIT_BLOCKS=$SUNIT_BYTES/$BLOCKSIZE
>>
>> echo "=== mkfs with su $SUNIT_BLOCKS blocks x 4 ==="
>> - export MKFS_OPTIONS=""
>> _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE>> $seqres.full 2>&1
>> _scratch_mount
>>
>> --
>> 2.21.0
>>
>>
>>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX
2020-07-15 1:59 ` Ira Weiny
@ 2020-07-15 3:19 ` Xiao Yang
2020-07-15 4:15 ` Ira Weiny
0 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-15 3:19 UTC (permalink / raw)
To: Ira Weiny; +Cc: fstests, darrick.wong
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@cn.fujitsu.com>
> Reviewed-by: Ira Weiny<ira.weiny@intel.com>
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"
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
>>
>>
>>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX
2020-07-15 3:19 ` Xiao Yang
@ 2020-07-15 4:15 ` Ira Weiny
2020-07-15 5:55 ` Xiao Yang
0 siblings, 1 reply; 29+ messages in thread
From: Ira Weiny @ 2020-07-15 4:15 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests, darrick.wong
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@cn.fujitsu.com>
> > Reviewed-by: Ira Weiny<ira.weiny@intel.com>
> 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)
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
> > >
> > >
> > >
> >
> > .
> >
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-15 2:48 ` Ira Weiny
@ 2020-07-15 5:39 ` Xiao Yang
2020-07-15 8:10 ` Xiao Yang
2020-07-15 9:44 ` Xiao Yang
0 siblings, 2 replies; 29+ messages in thread
From: Xiao Yang @ 2020-07-15 5:39 UTC (permalink / raw)
To: Ira Weiny; +Cc: fstests, darrick.wong
On 2020/7/15 10:48, Ira Weiny wrote:
> On Tue, Jul 14, 2020 at 05:40:09PM +0800, Xiao Yang wrote:
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>> tests/generic/605 | 199 ++++++++++++++++++++++++++++++++++++++++++
>> tests/generic/605.out | 2 +
>> tests/generic/group | 1 +
>> 3 files changed, 202 insertions(+)
>> create mode 100644 tests/generic/605
>> create mode 100644 tests/generic/605.out
>>
>> diff --git a/tests/generic/605 b/tests/generic/605
>> new file mode 100644
>> index 00000000..6924223a
>> --- /dev/null
>> +++ b/tests/generic/605
>> @@ -0,0 +1,199 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2020 Fujitsu. All Rights Reserved.
>> +#
>> +# FS QA Test 605
>> +#
>> +# Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations.
>> +# 1) New files and directories automatically inherit FS_XFLAG_DAX from their parent directory.
>> +# 2) cp operation make files and directories inherit the FS_XFLAG_DAX from new parent directory.
>> +# 3) mv operation make files and directories preserve the FS_XFLAG_DAX from old parent directory.
>> +# In addition, setting/clearing FS_XFLAG_DAX flag is not impacted by dax mount options.
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1 # failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> + cd /
>> + rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +_supported_fs generic
>> +_supported_os Linux
>> +_require_scratch
>> +_require_dax_iflag
>> +_require_xfs_io_command "lsattr" "-v"
>> +
>> +check_xflag()
>> +{
>> + local target=$1
>> + local exp_xflag=$2
>> +
>> + if [ $exp_xflag -eq 0 ]; then
>> + _test_inode_flag dax $target&& echo "$target has unexpected FS_XFLAG_DAX flag"
>> + else
>> + _test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
>> + fi
>> +}
>> +
>> +test_xflag_inheritance1()
>> +{
>> + mkdir -p a
>> + $XFS_IO_PROG -c "chattr +x" a
>> + mkdir -p a/b/c
>> + touch a/b/c/d
>> +
>> + check_xflag a 1
>> + check_xflag a/b 1
>> + check_xflag a/b/c 1
>> + check_xflag a/b/c/d 1
>> +
>> + rm -rf a
>> +}
>> +
>> +test_xflag_inheritance2()
>> +{
>> + mkdir -p a/b
>> + $XFS_IO_PROG -c "chattr +x" a
>> + mkdir -p a/b/c a/d
>> + touch a/b/c/e a/d/f
>> +
>> + check_xflag a 1
>> + check_xflag a/b 0
>> + check_xflag a/b/c 0
>> + check_xflag a/b/c/e 0
>> + check_xflag a/d 1
>> + check_xflag a/d/f 1
>> +
>> + rm -rf a
>> +}
>> +
>> +test_xflag_inheritance3()
>> +{
>> + mkdir -p a/b
>> + $XFS_IO_PROG -c "chattr +x" a/b
>> + mkdir -p a/b/c a/d
>> + touch a/b/c/e a/d/f
>> +
>> + check_xflag a 0
>> + check_xflag a/b 1
>> + check_xflag a/b/c 1
>> + check_xflag a/b/c/e 1
>> + check_xflag a/d 0
>> + check_xflag a/d/f 0
>> +
>> + rm -rf a
>> +}
> It really seems like 2 and 3 test the same thing?
Hi Ira,
2 constructs the following steps:
1) a is the parent directory of b
2) a doesn't have xflag and b has xflag
3) touch many directories/files in a and b
3 constructs the following steps:
1) a is the parent directory of b and b is the parent directory of c
2) a and c have xflag, and b doesn't have xflag
3) touch many directories/files in b and c
Do you think they are same? I can remove one if you think so.
>> +
>> +test_xflag_inheritance4()
>> +{
>> + mkdir -p a
>> + $XFS_IO_PROG -c "chattr +x" a
>> + mkdir -p a/b/c
>> + $XFS_IO_PROG -c "chattr -x" a/b
>> + mkdir -p a/b/c/d a/b/e
>> + touch a/b/c/d/f a/b/e/g
>> +
>> + check_xflag a 1
>> + check_xflag a/b 0
>> + check_xflag a/b/c 1
>> + check_xflag a/b/c/d 1
>> + check_xflag a/b/c/d/f 1
>> + check_xflag a/b/e 0
>> + check_xflag a/b/e/g 0
>> +
>> + rm -rf a
>> +}
>> +
>> +test_xflag_inheritance5()
>> +{
>> + mkdir -p a b
>> + $XFS_IO_PROG -c "chattr +x" a
>> + mkdir -p a/c a/d b/e b/f
>> + touch a/g b/h
>> +
>> + cp -r a/c b/
>> + cp -r b/e a/
>> + cp -r a/g b/
>> + mv a/d b/
>> + mv b/f a/
>> + mv b/h a/
>> +
>> + check_xflag b/c 0
>> + check_xflag b/d 1
>> + check_xflag a/e 1
>> + check_xflag a/f 0
>> + check_xflag b/g 0
>> + check_xflag a/h 0
>> +
>> + rm -rf a b
>> +}
>> +
>> +do_xflag_tests()
>> +{
>> + local option=$1
>> +
>> + _scratch_mount "$option"
>> + cd $SCRATCH_MNT
>> +
>> + for i in $(seq 1 5); do
>> + test_xflag_inheritance${i}
>> + done
>> +
>> + cd -> /dev/null
>> + _scratch_unmount
>> +}
>> +
>> +check_dax_mountopt()
>> +{
>> + local option=$1
>> + local ret=0
>> +
>> + _try_scratch_mount "-o $option">> $seqres.full 2>&1 || return 1
>> +
>> + # Match option name exactly
>> + _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
>> +
>> + _scratch_unmount
>> +
>> + return $ret
>> +}
> Should this be a common function?
I am not sure if it should be a common function, because it may not be
used by other tests in future.
I also consider to merge the function into _require_scratch_dax_mountopt().
>> +
>> +do_tests()
>> +{
>> + # Mount without dax option
>> + do_xflag_tests
>> +
>> + # Mount with old dax option if fs only supports it.
>> + check_dax_mountopt "dax"&& do_xflag_tests "-o dax"
> I don't understand the order here. If we are on an older kernel and the FS
> only supports '-o dax' the do_xflag_tests will fail won't it?
With both old dax and new dax, the inheritance behavior of FS_XFLAG_DAX
works well.
> So shouldn't we do this first and bail/'not run' this test if that is the case?
>
> I really don't think there is any point in testing the old XFS behavior because
> the FS_XFLAG_DAX had no effect. So even if it is broken it does not matter.
> Or perhaps I am missing something here?
This test is designed to verify the inheritance behavior of
FS_XFLAG_DAX(not related to S_DAX)
so I think it is fine for both old dax and new dax to run the test.
Best Regards,
Xiao Yang
> Ira
>
>> +
>> + # Mount with new dax options if fs supports them.
>> + if check_dax_mountopt "dax=always"; then
>> + for dax_option in "dax=always" "dax=inode" "dax=never"; do
>> + do_xflag_tests "-o $dax_option"
>> + done
>> + fi
>> +}
>> +
>> +_scratch_mkfs>> $seqres.full 2>&1
>> +
>> +do_tests
>> +
>> +# success, all done
>> +echo "Silence is golden"
>> +status=0
>> +exit
>> diff --git a/tests/generic/605.out b/tests/generic/605.out
>> new file mode 100644
>> index 00000000..1ae20049
>> --- /dev/null
>> +++ b/tests/generic/605.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 605
>> +Silence is golden
>> diff --git a/tests/generic/group b/tests/generic/group
>> index 676e0d2e..a8451862 100644
>> --- a/tests/generic/group
>> +++ b/tests/generic/group
>> @@ -607,3 +607,4 @@
>> 602 auto quick encrypt
>> 603 auto attr quick dax
>> 604 auto attr quick dax
>> +605 auto attr quick dax
>> --
>> 2.21.0
>>
>>
>>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX
2020-07-15 4:15 ` Ira Weiny
@ 2020-07-15 5:55 ` Xiao Yang
2020-07-15 15:56 ` Darrick J. Wong
0 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-15 5:55 UTC (permalink / raw)
To: Ira Weiny; +Cc: fstests, darrick.wong
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@cn.fujitsu.com>
>>> Reviewed-by: Ira Weiny<ira.weiny@intel.com>
>> 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
>>>>
>>>>
>>>>
>>> .
>>>
>>
>>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-15 5:39 ` Xiao Yang
@ 2020-07-15 8:10 ` Xiao Yang
2020-07-15 16:43 ` Xiao Yang
2020-07-15 9:44 ` Xiao Yang
1 sibling, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-15 8:10 UTC (permalink / raw)
To: Ira Weiny; +Cc: fstests, darrick.wong
On 2020/7/15 13:39, Xiao Yang wrote:
> On 2020/7/15 10:48, Ira Weiny wrote:
>> On Tue, Jul 14, 2020 at 05:40:09PM +0800, Xiao Yang wrote:
>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>> ---
>>> tests/generic/605 | 199
>>> ++++++++++++++++++++++++++++++++++++++++++
>>> tests/generic/605.out | 2 +
>>> tests/generic/group | 1 +
>>> 3 files changed, 202 insertions(+)
>>> create mode 100644 tests/generic/605
>>> create mode 100644 tests/generic/605.out
>>>
>>> diff --git a/tests/generic/605 b/tests/generic/605
>>> new file mode 100644
>>> index 00000000..6924223a
>>> --- /dev/null
>>> +++ b/tests/generic/605
>>> @@ -0,0 +1,199 @@
>>> +#! /bin/bash
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# Copyright (c) 2020 Fujitsu. All Rights Reserved.
>>> +#
>>> +# FS QA Test 605
>>> +#
>>> +# Verify the inheritance behavior of FS_XFLAG_DAX flag in various
>>> combinations.
>>> +# 1) New files and directories automatically inherit FS_XFLAG_DAX
>>> from their parent directory.
>>> +# 2) cp operation make files and directories inherit the
>>> FS_XFLAG_DAX from new parent directory.
>>> +# 3) mv operation make files and directories preserve the
>>> FS_XFLAG_DAX from old parent directory.
>>> +# In addition, setting/clearing FS_XFLAG_DAX flag is not impacted
>>> by dax mount options.
>>> +
>>> +seq=`basename $0`
>>> +seqres=$RESULT_DIR/$seq
>>> +echo "QA output created by $seq"
>>> +
>>> +here=`pwd`
>>> +tmp=/tmp/$$
>>> +status=1 # failure is the default!
>>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>>> +
>>> +_cleanup()
>>> +{
>>> + cd /
>>> + rm -f $tmp.*
>>> +}
>>> +
>>> +# get standard environment, filters and checks
>>> +. ./common/rc
>>> +. ./common/filter
>>> +
>>> +# remove previous $seqres.full before test
>>> +rm -f $seqres.full
>>> +
>>> +_supported_fs generic
>>> +_supported_os Linux
>>> +_require_scratch
>>> +_require_dax_iflag
>>> +_require_xfs_io_command "lsattr" "-v"
>>> +
>>> +check_xflag()
>>> +{
>>> + local target=$1
>>> + local exp_xflag=$2
>>> +
>>> + if [ $exp_xflag -eq 0 ]; then
>>> + _test_inode_flag dax $target&& echo "$target has
>>> unexpected FS_XFLAG_DAX flag"
>>> + else
>>> + _test_inode_flag dax $target || echo "$target doen't have
>>> expected FS_XFLAG_DAX flag"
>>> + fi
>>> +}
>>> +
>>> +test_xflag_inheritance1()
>>> +{
>>> + mkdir -p a
>>> + $XFS_IO_PROG -c "chattr +x" a
>>> + mkdir -p a/b/c
>>> + touch a/b/c/d
>>> +
>>> + check_xflag a 1
>>> + check_xflag a/b 1
>>> + check_xflag a/b/c 1
>>> + check_xflag a/b/c/d 1
>>> +
>>> + rm -rf a
>>> +}
>>> +
>>> +test_xflag_inheritance2()
>>> +{
>>> + mkdir -p a/b
>>> + $XFS_IO_PROG -c "chattr +x" a
>>> + mkdir -p a/b/c a/d
>>> + touch a/b/c/e a/d/f
>>> +
>>> + check_xflag a 1
>>> + check_xflag a/b 0
>>> + check_xflag a/b/c 0
>>> + check_xflag a/b/c/e 0
>>> + check_xflag a/d 1
>>> + check_xflag a/d/f 1
>>> +
>>> + rm -rf a
>>> +}
>>> +
>>> +test_xflag_inheritance3()
>>> +{
>>> + mkdir -p a/b
>>> + $XFS_IO_PROG -c "chattr +x" a/b
>>> + mkdir -p a/b/c a/d
>>> + touch a/b/c/e a/d/f
>>> +
>>> + check_xflag a 0
>>> + check_xflag a/b 1
>>> + check_xflag a/b/c 1
>>> + check_xflag a/b/c/e 1
>>> + check_xflag a/d 0
>>> + check_xflag a/d/f 0
>>> +
>>> + rm -rf a
>>> +}
>> It really seems like 2 and 3 test the same thing?
> Hi Ira,
>
> 2 constructs the following steps:
> 1) a is the parent directory of b
> 2) a doesn't have xflag and b has xflag
> 3) touch many directories/files in a and b
>
> 3 constructs the following steps:
> 1) a is the parent directory of b and b is the parent directory of c
> 2) a and c have xflag, and b doesn't have xflag
> 3) touch many directories/files in b and c
>
> Do you think they are same? I can remove one if you think so.
>
>>> +
>>> +test_xflag_inheritance4()
>>> +{
>>> + mkdir -p a
>>> + $XFS_IO_PROG -c "chattr +x" a
>>> + mkdir -p a/b/c
>>> + $XFS_IO_PROG -c "chattr -x" a/b
>>> + mkdir -p a/b/c/d a/b/e
>>> + touch a/b/c/d/f a/b/e/g
>>> +
>>> + check_xflag a 1
>>> + check_xflag a/b 0
>>> + check_xflag a/b/c 1
>>> + check_xflag a/b/c/d 1
>>> + check_xflag a/b/c/d/f 1
>>> + check_xflag a/b/e 0
>>> + check_xflag a/b/e/g 0
>>> +
>>> + rm -rf a
>>> +}
>>> +
>>> +test_xflag_inheritance5()
>>> +{
>>> + mkdir -p a b
>>> + $XFS_IO_PROG -c "chattr +x" a
>>> + mkdir -p a/c a/d b/e b/f
>>> + touch a/g b/h
>>> +
>>> + cp -r a/c b/
>>> + cp -r b/e a/
>>> + cp -r a/g b/
>>> + mv a/d b/
>>> + mv b/f a/
>>> + mv b/h a/
>>> +
>>> + check_xflag b/c 0
>>> + check_xflag b/d 1
>>> + check_xflag a/e 1
>>> + check_xflag a/f 0
>>> + check_xflag b/g 0
>>> + check_xflag a/h 0
>>> +
>>> + rm -rf a b
>>> +}
>>> +
>>> +do_xflag_tests()
>>> +{
>>> + local option=$1
>>> +
>>> + _scratch_mount "$option"
>>> + cd $SCRATCH_MNT
>>> +
>>> + for i in $(seq 1 5); do
>>> + test_xflag_inheritance${i}
>>> + done
>>> +
>>> + cd -> /dev/null
>>> + _scratch_unmount
>>> +}
>>> +
>>> +check_dax_mountopt()
>>> +{
>>> + local option=$1
>>> + local ret=0
>>> +
>>> + _try_scratch_mount "-o $option">> $seqres.full 2>&1 || return 1
>>> +
>>> + # Match option name exactly
>>> + _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
>>> +
>>> + _scratch_unmount
>>> +
>>> + return $ret
>>> +}
>> Should this be a common function?
>
> I am not sure if it should be a common function, because it may not be
> used by other tests in future.
> I also consider to merge the function into
> _require_scratch_dax_mountopt().
For this comment, I try to merge the function into
_require_scratch_dax_mountopt(), as below:
----------------------------------------------------------------------------------------------------------------
+# Only accept dax/dax=always mount option becasue dax=always, dax=inode
+# and dax=never are always introduced together.
+# Return 0 if filesystem/device supports the specified dax option.
+# Return 1 if mount fails with the specified dax option.
+# Return 2 if /proc/mounts shows wrong dax option.
+# Check new dax=inode, dax=always or dax=never option by passing
"dax=always".
+# Check old dax or new dax=always by passing "dax".
+_check_scratch_dax_mountopt()
+{
+ local option=$1
+
+ echo "$option" | egrep -q "dax(=always|$)" || \
+ _notrun "invalid $option, only accept dax/dax=always"
+
+ _require_scratch
+ _scratch_mkfs > /dev/null 2>&1
+
+ _try_scratch_mount "-o $option" > /dev/null 2>&1 || return 1
+
+ if _fs_options $SCRATCH_DEV | egrep -q "dax(=always|,|$)"; then
+ _scratch_unmount
+ return 0
+ else
+ _scratch_unmount
+ return 2
+ fi
+}
+
+# Throw notrun if _check_scratch_dax_mountopt() returns a non-zero value.
+_require_scratch_dax_mountopt()
+{
+ local mountopt=$1
+
+ _check_scratch_dax_mountopt "$mountopt"
+ local res=$?
+
+ [ $res -eq 1 ] && _notrun "mount $SCRATCH_DEV with $mountopt failed"
+ [ $res -eq 2 ] && _notrun "$SCRATCH_DEV $FSTYP does not support
-o $mountopt"
+}
-----------------------------------------------------------------------------------------------------------
>
>>> +
>>> +do_tests()
>>> +{
>>> + # Mount without dax option
>>> + do_xflag_tests
>>> +
>>> + # Mount with old dax option if fs only supports it.
>>> + check_dax_mountopt "dax"&& do_xflag_tests "-o dax"
>> I don't understand the order here. If we are on an older kernel and
>> the FS
>> only supports '-o dax' the do_xflag_tests will fail won't it?
>
> With both old dax and new dax, the inheritance behavior of
> FS_XFLAG_DAX works well.
>
>> So shouldn't we do this first and bail/'not run' this test if that is
>> the case?
>>
>> I really don't think there is any point in testing the old XFS
>> behavior because
>> the FS_XFLAG_DAX had no effect. So even if it is broken it does not
>> matter.
>> Or perhaps I am missing something here?
>
> This test is designed to verify the inheritance behavior of
> FS_XFLAG_DAX(not related to S_DAX)
> so I think it is fine for both old dax and new dax to run the test.
For this comment, I try to update it, as below:
-------------------------------------------------------------------------
+do_tests()
+{
+ # Mount without dax option
+ do_xflag_tests
+
+ # Mount with 'dax' or 'dax=always' option if fs supports it.
+ _check_scratch_dax_mountopt "dax" && do_xflag_tests "-o dax"
+
+ # Mount with 'dax=inode' and 'dax=never' options if fs supports
them.
+ if _check_scratch_dax_mountopt "dax=always"; then
+ for dax_option in "dax=inode" "dax=never"; do
+ do_xflag_tests "-o $dax_option"
+ done
+ fi
+}
-------------------------------------------------------------------------
>
> Best Regards,
> Xiao Yang
>> Ira
>>
>>> +
>>> + # Mount with new dax options if fs supports them.
>>> + if check_dax_mountopt "dax=always"; then
>>> + for dax_option in "dax=always" "dax=inode" "dax=never"; do
>>> + do_xflag_tests "-o $dax_option"
>>> + done
>>> + fi
>>> +}
>>> +
>>> +_scratch_mkfs>> $seqres.full 2>&1
>>> +
>>> +do_tests
>>> +
>>> +# success, all done
>>> +echo "Silence is golden"
>>> +status=0
>>> +exit
>>> diff --git a/tests/generic/605.out b/tests/generic/605.out
>>> new file mode 100644
>>> index 00000000..1ae20049
>>> --- /dev/null
>>> +++ b/tests/generic/605.out
>>> @@ -0,0 +1,2 @@
>>> +QA output created by 605
>>> +Silence is golden
>>> diff --git a/tests/generic/group b/tests/generic/group
>>> index 676e0d2e..a8451862 100644
>>> --- a/tests/generic/group
>>> +++ b/tests/generic/group
>>> @@ -607,3 +607,4 @@
>>> 602 auto quick encrypt
>>> 603 auto attr quick dax
>>> 604 auto attr quick dax
>>> +605 auto attr quick dax
>>> --
>>> 2.21.0
>>>
>>>
>>>
>>
>> .
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-15 5:39 ` Xiao Yang
2020-07-15 8:10 ` Xiao Yang
@ 2020-07-15 9:44 ` Xiao Yang
2020-07-15 16:19 ` Darrick J. Wong
1 sibling, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-15 9:44 UTC (permalink / raw)
To: Ira Weiny; +Cc: fstests, darrick.wong
On 2020/7/15 13:39, Xiao Yang wrote:
> On 2020/7/15 10:48, Ira Weiny wrote:
>> On Tue, Jul 14, 2020 at 05:40:09PM +0800, Xiao Yang wrote:
>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>> ---
>>> tests/generic/605 | 199
>>> ++++++++++++++++++++++++++++++++++++++++++
>>> tests/generic/605.out | 2 +
>>> tests/generic/group | 1 +
>>> 3 files changed, 202 insertions(+)
>>> create mode 100644 tests/generic/605
>>> create mode 100644 tests/generic/605.out
>>>
>>> diff --git a/tests/generic/605 b/tests/generic/605
>>> new file mode 100644
>>> index 00000000..6924223a
>>> --- /dev/null
>>> +++ b/tests/generic/605
>>> @@ -0,0 +1,199 @@
>>> +#! /bin/bash
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# Copyright (c) 2020 Fujitsu. All Rights Reserved.
>>> +#
>>> +# FS QA Test 605
>>> +#
>>> +# Verify the inheritance behavior of FS_XFLAG_DAX flag in various
>>> combinations.
>>> +# 1) New files and directories automatically inherit FS_XFLAG_DAX
>>> from their parent directory.
>>> +# 2) cp operation make files and directories inherit the
>>> FS_XFLAG_DAX from new parent directory.
>>> +# 3) mv operation make files and directories preserve the
>>> FS_XFLAG_DAX from old parent directory.
>>> +# In addition, setting/clearing FS_XFLAG_DAX flag is not impacted
>>> by dax mount options.
>>> +
>>> +seq=`basename $0`
>>> +seqres=$RESULT_DIR/$seq
>>> +echo "QA output created by $seq"
>>> +
>>> +here=`pwd`
>>> +tmp=/tmp/$$
>>> +status=1 # failure is the default!
>>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>>> +
>>> +_cleanup()
>>> +{
>>> + cd /
>>> + rm -f $tmp.*
>>> +}
>>> +
>>> +# get standard environment, filters and checks
>>> +. ./common/rc
>>> +. ./common/filter
>>> +
>>> +# remove previous $seqres.full before test
>>> +rm -f $seqres.full
>>> +
>>> +_supported_fs generic
>>> +_supported_os Linux
>>> +_require_scratch
>>> +_require_dax_iflag
>>> +_require_xfs_io_command "lsattr" "-v"
>>> +
>>> +check_xflag()
>>> +{
>>> + local target=$1
>>> + local exp_xflag=$2
>>> +
>>> + if [ $exp_xflag -eq 0 ]; then
>>> + _test_inode_flag dax $target&& echo "$target has
>>> unexpected FS_XFLAG_DAX flag"
>>> + else
>>> + _test_inode_flag dax $target || echo "$target doen't have
>>> expected FS_XFLAG_DAX flag"
>>> + fi
>>> +}
>>> +
>>> +test_xflag_inheritance1()
>>> +{
>>> + mkdir -p a
>>> + $XFS_IO_PROG -c "chattr +x" a
>>> + mkdir -p a/b/c
>>> + touch a/b/c/d
>>> +
>>> + check_xflag a 1
>>> + check_xflag a/b 1
>>> + check_xflag a/b/c 1
>>> + check_xflag a/b/c/d 1
>>> +
>>> + rm -rf a
>>> +}
>>> +
>>> +test_xflag_inheritance2()
>>> +{
>>> + mkdir -p a/b
>>> + $XFS_IO_PROG -c "chattr +x" a
>>> + mkdir -p a/b/c a/d
>>> + touch a/b/c/e a/d/f
>>> +
>>> + check_xflag a 1
>>> + check_xflag a/b 0
>>> + check_xflag a/b/c 0
>>> + check_xflag a/b/c/e 0
>>> + check_xflag a/d 1
>>> + check_xflag a/d/f 1
>>> +
>>> + rm -rf a
>>> +}
>>> +
>>> +test_xflag_inheritance3()
>>> +{
>>> + mkdir -p a/b
>>> + $XFS_IO_PROG -c "chattr +x" a/b
>>> + mkdir -p a/b/c a/d
>>> + touch a/b/c/e a/d/f
>>> +
>>> + check_xflag a 0
>>> + check_xflag a/b 1
>>> + check_xflag a/b/c 1
>>> + check_xflag a/b/c/e 1
>>> + check_xflag a/d 0
>>> + check_xflag a/d/f 0
>>> +
>>> + rm -rf a
>>> +}
>> It really seems like 2 and 3 test the same thing?
> Hi Ira,
>
> 2 constructs the following steps:
> 1) a is the parent directory of b
> 2) a doesn't have xflag and b has xflag
> 3) touch many directories/files in a and b
>
> 3 constructs the following steps:
> 1) a is the parent directory of b and b is the parent directory of c
> 2) a and c have xflag, and b doesn't have xflag
> 3) touch many directories/files in b and c
Hi Ira,
Sorry for misreading your comment, above is the difference between 3 and 4.
The correct one is:
2 constructs the following steps:
1) a is the parent directory of b
2) a has xflag and b doesn't have xflag
3) touch many directories/files in a and b
3 constructs the following steps:
1) a is the parent directory of b
2) a doesn't have xflag and b has xflag
3) touch many directories/files in a and b
Do you think they are same? I can remove one if you think so.
Best Regards,
Xiao Yang
>
> Do you think they are same? I can remove one if you think so.
>
>>> +
>>> +test_xflag_inheritance4()
>>> +{
>>> + mkdir -p a
>>> + $XFS_IO_PROG -c "chattr +x" a
>>> + mkdir -p a/b/c
>>> + $XFS_IO_PROG -c "chattr -x" a/b
>>> + mkdir -p a/b/c/d a/b/e
>>> + touch a/b/c/d/f a/b/e/g
>>> +
>>> + check_xflag a 1
>>> + check_xflag a/b 0
>>> + check_xflag a/b/c 1
>>> + check_xflag a/b/c/d 1
>>> + check_xflag a/b/c/d/f 1
>>> + check_xflag a/b/e 0
>>> + check_xflag a/b/e/g 0
>>> +
>>> + rm -rf a
>>> +}
>>> +
>>> +test_xflag_inheritance5()
>>> +{
>>> + mkdir -p a b
>>> + $XFS_IO_PROG -c "chattr +x" a
>>> + mkdir -p a/c a/d b/e b/f
>>> + touch a/g b/h
>>> +
>>> + cp -r a/c b/
>>> + cp -r b/e a/
>>> + cp -r a/g b/
>>> + mv a/d b/
>>> + mv b/f a/
>>> + mv b/h a/
>>> +
>>> + check_xflag b/c 0
>>> + check_xflag b/d 1
>>> + check_xflag a/e 1
>>> + check_xflag a/f 0
>>> + check_xflag b/g 0
>>> + check_xflag a/h 0
>>> +
>>> + rm -rf a b
>>> +}
>>> +
>>> +do_xflag_tests()
>>> +{
>>> + local option=$1
>>> +
>>> + _scratch_mount "$option"
>>> + cd $SCRATCH_MNT
>>> +
>>> + for i in $(seq 1 5); do
>>> + test_xflag_inheritance${i}
>>> + done
>>> +
>>> + cd -> /dev/null
>>> + _scratch_unmount
>>> +}
>>> +
>>> +check_dax_mountopt()
>>> +{
>>> + local option=$1
>>> + local ret=0
>>> +
>>> + _try_scratch_mount "-o $option">> $seqres.full 2>&1 || return 1
>>> +
>>> + # Match option name exactly
>>> + _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
>>> +
>>> + _scratch_unmount
>>> +
>>> + return $ret
>>> +}
>> Should this be a common function?
>
> I am not sure if it should be a common function, because it may not be
> used by other tests in future.
> I also consider to merge the function into
> _require_scratch_dax_mountopt().
>
>>> +
>>> +do_tests()
>>> +{
>>> + # Mount without dax option
>>> + do_xflag_tests
>>> +
>>> + # Mount with old dax option if fs only supports it.
>>> + check_dax_mountopt "dax"&& do_xflag_tests "-o dax"
>> I don't understand the order here. If we are on an older kernel and
>> the FS
>> only supports '-o dax' the do_xflag_tests will fail won't it?
>
> With both old dax and new dax, the inheritance behavior of
> FS_XFLAG_DAX works well.
>
>> So shouldn't we do this first and bail/'not run' this test if that is
>> the case?
>>
>> I really don't think there is any point in testing the old XFS
>> behavior because
>> the FS_XFLAG_DAX had no effect. So even if it is broken it does not
>> matter.
>> Or perhaps I am missing something here?
>
> This test is designed to verify the inheritance behavior of
> FS_XFLAG_DAX(not related to S_DAX)
> so I think it is fine for both old dax and new dax to run the test.
>
> Best Regards,
> Xiao Yang
>> Ira
>>
>>> +
>>> + # Mount with new dax options if fs supports them.
>>> + if check_dax_mountopt "dax=always"; then
>>> + for dax_option in "dax=always" "dax=inode" "dax=never"; do
>>> + do_xflag_tests "-o $dax_option"
>>> + done
>>> + fi
>>> +}
>>> +
>>> +_scratch_mkfs>> $seqres.full 2>&1
>>> +
>>> +do_tests
>>> +
>>> +# success, all done
>>> +echo "Silence is golden"
>>> +status=0
>>> +exit
>>> diff --git a/tests/generic/605.out b/tests/generic/605.out
>>> new file mode 100644
>>> index 00000000..1ae20049
>>> --- /dev/null
>>> +++ b/tests/generic/605.out
>>> @@ -0,0 +1,2 @@
>>> +QA output created by 605
>>> +Silence is golden
>>> diff --git a/tests/generic/group b/tests/generic/group
>>> index 676e0d2e..a8451862 100644
>>> --- a/tests/generic/group
>>> +++ b/tests/generic/group
>>> @@ -607,3 +607,4 @@
>>> 602 auto quick encrypt
>>> 603 auto attr quick dax
>>> 604 auto attr quick dax
>>> +605 auto attr quick dax
>>> --
>>> 2.21.0
>>>
>>>
>>>
>>
>> .
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX
2020-07-15 5:55 ` Xiao Yang
@ 2020-07-15 15:56 ` Darrick J. Wong
2020-07-15 18:00 ` Ira Weiny
0 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2020-07-15 15:56 UTC (permalink / raw)
To: Xiao Yang; +Cc: Ira Weiny, fstests
On Wed, Jul 15, 2020 at 01:55:14PM +0800, Xiao Yang wrote:
> 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@cn.fujitsu.com>
> > > > Reviewed-by: Ira Weiny<ira.weiny@intel.com>
> > > 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
That does seem like the only way to figure out if a fs/device
combination supports dax.
The valid arguments to the _require_scratch_dax* helpers are worth
mentioning in a comment above the function, but I suspect that adding
validation code is probably overkill.
--D
> 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
> > > > >
> > > > >
> > > > >
> > > > .
> > > >
> > >
> > >
> >
> > .
> >
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly
2020-07-15 3:12 ` Xiao Yang
@ 2020-07-15 16:07 ` Darrick J. Wong
2020-07-16 1:36 ` Xiao Yang
0 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2020-07-15 16:07 UTC (permalink / raw)
To: Xiao Yang; +Cc: Ira Weiny, fstests
On Wed, Jul 15, 2020 at 11:12:36AM +0800, Xiao Yang wrote:
> On 2020/7/15 10:31, Ira Weiny wrote:
> > On Tue, Jul 14, 2020 at 05:40:05PM +0800, Xiao Yang wrote:
> > > ext4 can accept the last one if the same mkfs options are passed but xfs cannot
> > I'm having trouble parsing this commit message. What does 'last one' refer to?
> >
> > > accept the same mkfs options and reports "xxx option is respecified" error.
> > Ok I think I understand now. Some FS's (XFS) do not accept an option more than
> > once. So we can't just blindly add options to the end of the MKFS_OPTIONS. Is
> > that correct?
> Hi Ira,
>
> Correct.
> > > I
> > > prefer to override the same mkfs option which is defined in MKFS_OPTION so that
> > > we can have a chance to pass other mkfs options to _scratch_mkfs_geom().
> > Instead the patch parses the current option string and replaces the value if
> > the option is already there. This allows us to specify MKFS_OPTIONS to
> > generic/223.
> Right. :-)
>
> XFS_MKFS_OPTIONS/MKFS_OPTIONS can be used to specify some custom options by user,
> so I don't want to clear it blindly.
>
> Thanks,
> Xiao Yang
> > I think the code is reasonable although my sed skills are not good enough to
> > tell for sure... ;-)
> >
> > Ira
> >
> > > Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> > > ---
> > > common/rc | 14 +++++++++++++-
> > > tests/generic/223 | 1 -
> > > 2 files changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/common/rc b/common/rc
> > > index 6c908f2e..567cf83b 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -1051,7 +1051,19 @@ _scratch_mkfs_geom()
> > >
> > > case $FSTYP in
> > > xfs)
> > > - MKFS_OPTIONS+=" -b size=$blocksize, -d su=$sunit_bytes,sw=$swidth_mult"
> > > + if echo "$MKFS_OPTIONS" | egrep -q "b?size="; then
> > > + MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r "s/(b?size=)[0-9]+/\1$blocksize/")
> > > + else
> > > + MKFS_OPTIONS+=" -b size=$blocksize"
> > > + fi
> > > +
> > > + if echo "$MKFS_OPTIONS" | egrep -q "(su|sunit|sw|swidth)="; then
> > > + MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r \
> > > + -e "s/(su|sunit)=[0-9kmg]+/su=$sunit_bytes/" \
> > > + -e "s/(sw|swidth)=[0-9kmg]+/sw=$swidth_mult/")
> > > + else
> > > + MKFS_OPTIONS+=" -d su=$sunit_bytes,sw=$swidth_mult"
> > > + fi
...ok, I see, this makes the function smart enough to substitute
geometry parameters instead of dumping them on the end and letting that
blow up. Heh, ok, that's definitely a weird quirk I've noticed.
> > > ;;
> > > ext4|ext4dev)
> > > MKFS_OPTIONS+=" -b $blocksize -E stride=$sunit_blocks,stripe_width=$swidth_blocks"
> > > diff --git a/tests/generic/223 b/tests/generic/223
> > > index 6cfd00dd..ba7c9a44 100755
> > > --- a/tests/generic/223
> > > +++ b/tests/generic/223
> > > @@ -41,7 +41,6 @@ for SUNIT_K in 8 16 32 64 128; do
> > > let SUNIT_BLOCKS=$SUNIT_BYTES/$BLOCKSIZE
> > >
> > > echo "=== mkfs with su $SUNIT_BLOCKS blocks x 4 ==="
> > > - export MKFS_OPTIONS=""
So I guess you're deleting this so that the test runs with whatever
MKFS_OPTIONS the test runner specified, while letting the test edit
blocksize and stripe parameters?
Proving I'm still bad at remembering to read commit messages,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> > > _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE>> $seqres.full 2>&1
> > > _scratch_mount
> > >
> > > --
> > > 2.21.0
> > >
> > >
> > >
> >
> > .
> >
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 2/7] fstests: Use _require_scratch_dax_mountopt() and _require_dax_iflag()
2020-07-14 9:40 ` [PATCH v6 2/7] fstests: Use _require_scratch_dax_mountopt() and _require_dax_iflag() Xiao Yang
@ 2020-07-15 16:08 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2020-07-15 16:08 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests, ira.weiny
On Tue, Jul 14, 2020 at 05:40:04PM +0800, Xiao Yang wrote:
> 1) Make related tests use _require_scratch_dax_mountopt() and _require_dax_iflag()
> 2) Remove unused _require_scratch_dax()
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Looks ok to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
>
> V5->V6:
> Merge the third patch into the second patch.
>
> common/rc | 13 -------------
> tests/ext4/030 | 2 +-
> tests/ext4/031 | 4 ++--
> tests/generic/413 | 2 +-
> tests/generic/462 | 2 +-
> tests/xfs/260 | 4 ++--
> 6 files changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index aeec1f11..6c908f2e 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3175,19 +3175,6 @@ _require_scratch_shutdown()
> }
>
> # Does dax mount option work on this dev/fs?
> -_require_scratch_dax()
> -{
> - _require_scratch
> - _scratch_mkfs > /dev/null 2>&1
> - _try_scratch_mount -o dax || \
> - _notrun "mount $SCRATCH_DEV with dax failed"
> - # Check options to be sure. XFS ignores dax option
> - # and goes on if dev underneath does not support dax.
> - _fs_options $SCRATCH_DEV | grep -qw "dax" || \
> - _notrun "$SCRATCH_DEV $FSTYP does not support -o 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()
> diff --git a/tests/ext4/030 b/tests/ext4/030
> index 93bbca86..fb5cf451 100755
> --- a/tests/ext4/030
> +++ b/tests/ext4/030
> @@ -33,7 +33,7 @@ rm -f $seqres.full
> # Modify as appropriate.
> _supported_os Linux
> _supported_fs ext4
> -_require_scratch_dax
> +_require_scratch_dax_mountopt "dax"
> _require_test_program "t_ext4_dax_journal_corruption"
> _require_command "$CHATTR_PROG" chattr
>
> diff --git a/tests/ext4/031 b/tests/ext4/031
> index dc58214e..20e2fab7 100755
> --- a/tests/ext4/031
> +++ b/tests/ext4/031
> @@ -37,7 +37,7 @@ MOUNT_OPTIONS=""
> # Modify as appropriate.
> _supported_os Linux
> _supported_fs ext4
> -_require_scratch_dax
> +_require_scratch_dax_mountopt "dax"
> _require_test_program "t_ext4_dax_inline_corruption"
> _require_scratch_ext4_feature "inline_data"
>
> @@ -56,7 +56,7 @@ _scratch_unmount >> $seqres.full 2>&1
> _try_scratch_mount "-o dax" >> $seqres.full 2>&1
>
> if [[ $? != 0 ]]; then
> - # _require_scratch_dax already verified that we could mount with DAX.
> + # _require_scratch_dax_mountopt already verified that we could mount with DAX.
> # Failure here is expected because we have inline data.
> echo "Silence is golden"
> status=0
> diff --git a/tests/generic/413 b/tests/generic/413
> index 1ce89aff..19e1b926 100755
> --- a/tests/generic/413
> +++ b/tests/generic/413
> @@ -31,7 +31,7 @@ rm -f $seqres.full
> _supported_fs generic
> _supported_os Linux
> _require_test
> -_require_scratch_dax
> +_require_scratch_dax_mountopt "dax"
> _require_test_program "feature"
> _require_test_program "t_mmap_dio"
> _require_xfs_io_command "falloc"
> diff --git a/tests/generic/462 b/tests/generic/462
> index 1ab6cadc..4a940239 100755
> --- a/tests/generic/462
> +++ b/tests/generic/462
> @@ -37,7 +37,7 @@ rm -f $seqres.full
> _supported_fs generic
> _supported_os Linux
> _require_test
> -_require_scratch_dax
> +_require_scratch_dax_mountopt "dax"
> _require_test_program "t_mmap_write_ro"
> # running by unpriviliged user is not necessary to reproduce
> # this bug, just trying to test more.
> diff --git a/tests/xfs/260 b/tests/xfs/260
> index 3464ffef..bcdc6041 100755
> --- a/tests/xfs/260
> +++ b/tests/xfs/260
> @@ -30,10 +30,10 @@ rm -f $seqres.full
>
> _supported_fs xfs
> _supported_os Linux
> -_require_scratch_dax
> +_require_scratch_dax_mountopt "dax"
> _require_test_program "feature"
> _require_test_program "t_mmap_dio"
> -_require_xfs_io_command "chattr" "x"
> +_require_dax_iflag
> _require_xfs_io_command "falloc"
>
> prep_files()
> --
> 2.21.0
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 4/7] generic/413, xfs/260: Improve format operation for PMD fault testing
2020-07-14 9:40 ` [PATCH v6 4/7] generic/413, xfs/260: Improve format operation for PMD fault testing Xiao Yang
@ 2020-07-15 16:09 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2020-07-15 16:09 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests, ira.weiny
On Tue, Jul 14, 2020 at 05:40:06PM +0800, Xiao Yang wrote:
> 1) Simple code and fix the wrong value of stripe_width by _scratch_mkfs_geom().
> 2) Get hugepage size by _get_hugepagesize() and replace fixed 2M with
> hugepage size because hugepage size/PMD_SIZE is not 2M on some
> arches(e.g. hugepage size/PMD_SIZE is 512M on arm64).
> 3) For debugging, redirect the output of mkfs to $seqres.full.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Thanks for making this change...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> common/rc | 10 ++++++++++
> tests/generic/413 | 10 ++--------
> tests/xfs/260 | 4 ++--
> 3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 567cf83b..b6a48241 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -170,6 +170,16 @@ _get_filesize()
> stat -c %s "$1"
> }
>
> +# Get hugepagesize in bytes
> +_get_hugepagesize()
> +{
> + local hugepgsz=$(awk '/Hugepagesize/ {print $2}' /proc/meminfo)
> + # Call _notrun if $hugepgsz is not a number
> + echo "$hugepgsz" | egrep -q ^[0-9]+$ || \
> + _notrun "Cannot get the value of Hugepagesize"
> + echo $((hugepgsz * 1024))
> +}
> +
> _mount()
> {
> $MOUNT_PROG `_mount_ops_filter $*`
> diff --git a/tests/generic/413 b/tests/generic/413
> index 19e1b926..dfe2912b 100755
> --- a/tests/generic/413
> +++ b/tests/generic/413
> @@ -111,14 +111,8 @@ do_tests()
> t_mmap_dio_dax $((64 * 1024 * 1024))
> }
>
> -# make fs 2Mb aligned for PMD fault testing
> -mkfs_opts=""
> -if [ "$FSTYP" == "ext4" ]; then
> - mkfs_opts="-E stride=512,stripe_width=1"
> -elif [ "$FSTYP" == "xfs" ]; then
> - mkfs_opts="-d su=2m,sw=1"
> -fi
> -_scratch_mkfs "$mkfs_opts" > /dev/null 2>&1
> +# make fs aligned for PMD fault testing
> +_scratch_mkfs_geom $(_get_hugepagesize) 1 >> $seqres.full 2>&1
>
> # mount SCRATCH_DEV with dax option, TEST_DEV not
> export MOUNT_OPTIONS=""
> diff --git a/tests/xfs/260 b/tests/xfs/260
> index bcdc6041..81b42f01 100755
> --- a/tests/xfs/260
> +++ b/tests/xfs/260
> @@ -121,8 +121,8 @@ do_tests()
> t_dax_flag_mmap_dio $((64 * 1024 * 1024))
> }
>
> -# make xfs 2Mb aligned for PMD fault testing
> -_scratch_mkfs "-d su=2m,sw=1" > /dev/null 2>&1
> +# make xfs aligned for PMD fault testing
> +_scratch_mkfs_geom $(_get_hugepagesize) 1 >> $seqres.full 2>&1
>
> # mount with dax option
> _scratch_mount "-o dax"
> --
> 2.21.0
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 5/7] xfs/260: Move and update xfs/260
2020-07-14 9:40 ` [PATCH v6 5/7] xfs/260: Move and update xfs/260 Xiao Yang
@ 2020-07-15 16:10 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2020-07-15 16:10 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests, ira.weiny
On Tue, Jul 14, 2020 at 05:40:07PM +0800, Xiao Yang wrote:
> 1) Both ext4 and xfs have supported FS_XFLAG_DAX so move it to generic.
> 2) Modifying FS_XFLAG_DAX on flies does not take effect immediately so
> make files inherit the DAX state of parent directory.
> 3) Setting/clearing FS_XFLAG_DAX have no chance to change S_DAX flag if
> mount with dax option so remove the related subtest.
> 4) Setting/clearing FS_XFLAG_DAX doesn't change S_DAX flag on older xfs
> due to commit 742d84290739 ("xfs: disable per-inode DAX flag") so
> only do test when fs supports new dax=inode option.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> ---
> tests/{xfs/260 => generic/603} | 68 +++++++++++++++++-----------------
Ooh renaming detection, nice...
Looks fine to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> tests/generic/603.out | 2 +
> tests/generic/group | 1 +
> tests/xfs/260.out | 2 -
> tests/xfs/group | 1 -
> 5 files changed, 36 insertions(+), 38 deletions(-)
> rename tests/{xfs/260 => generic/603} (57%)
> create mode 100644 tests/generic/603.out
> delete mode 100644 tests/xfs/260.out
>
> diff --git a/tests/xfs/260 b/tests/generic/603
> similarity index 57%
> rename from tests/xfs/260
> rename to tests/generic/603
> index 81b42f01..5dabe447 100755
> --- a/tests/xfs/260
> +++ b/tests/generic/603
> @@ -2,7 +2,7 @@
> # SPDX-License-Identifier: GPL-2.0
> # Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
> #
> -# FS QA Test 260
> +# FS QA Test 603
> #
> # Test per-inode DAX flag by mmap direct/buffered IO.
> #
> @@ -28,76 +28,80 @@ _cleanup()
> # remove previous $seqres.full before test
> rm -f $seqres.full
>
> -_supported_fs xfs
> +_supported_fs generic
> _supported_os Linux
> -_require_scratch_dax_mountopt "dax"
> +_require_scratch_dax_mountopt "dax=always"
> _require_test_program "feature"
> _require_test_program "t_mmap_dio"
> _require_dax_iflag
> _require_xfs_io_command "falloc"
>
> -prep_files()
> +SRC_DIR=$SCRATCH_MNT/src
> +SRC_FILE=$SRC_DIR/tf_s
> +DST_DIR=$SCRATCH_MNT/dst
> +DST_FILE=$DST_DIR/tf_d
> +
> +prep_directories()
> {
> - rm -f $SCRATCH_MNT/tf_{s,d}
> + mkdir -p $SRC_DIR $DST_DIR
> +}
>
> +prep_files()
> +{
> + rm -f $SRC_FILE $DST_FILE
> $XFS_IO_PROG -f -c "falloc 0 $tsize" \
> - $SCRATCH_MNT/tf_{s,d} >> $seqres.full 2>&1
> + $SRC_FILE $DST_FILE >> $seqres.full 2>&1
> }
>
> t_both_dax()
> {
> + $XFS_IO_PROG -c "chattr +x" $SRC_DIR $DST_DIR
> prep_files
> - $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_{s,d}
> # with O_DIRECT first
> - $here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} $1 "dio both dax"
> + $here/src/t_mmap_dio $SRC_FILE $DST_FILE \
> + $1 "dio both dax"
>
> prep_files
> - $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_{s,d}
> # again with buffered IO
> - $here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
> + $here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
> $1 "buffered both dax"
> }
>
> t_nondax_to_dax()
> {
> + $XFS_IO_PROG -c "chattr -x" $SRC_DIR
> + $XFS_IO_PROG -c "chattr +x" $DST_DIR
> prep_files
> - $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_s
> - $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_d
> - $here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
> + $here/src/t_mmap_dio $SRC_FILE $DST_FILE \
> $1 "dio nondax to dax"
>
> prep_files
> - $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_s
> - $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_d
> - $here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
> + $here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
> $1 "buffered nondax to dax"
> }
>
> t_dax_to_nondax()
> {
> + $XFS_IO_PROG -c "chattr +x" $SRC_DIR
> + $XFS_IO_PROG -c "chattr -x" $DST_DIR
> prep_files
> - $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_s
> - $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_d
> - $here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
> + $here/src/t_mmap_dio $SRC_FILE $DST_FILE \
> $1 "dio dax to nondax"
>
> prep_files
> - $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_s
> - $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_d
> - $here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
> + $here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
> $1 "buffered dax to nondax"
> }
>
> t_both_nondax()
> {
> + $XFS_IO_PROG -c "chattr -x" $SRC_DIR $DST_DIR
> prep_files
> - $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_{s,d}
> - $here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
> + $here/src/t_mmap_dio $SRC_FILE $DST_FILE \
> $1 "dio both nondax"
>
> prep_files
> - $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_{s,d}
> - $here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
> + $here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
> $1 "buffered both nondax"
> }
>
> @@ -112,6 +116,7 @@ t_dax_flag_mmap_dio()
>
> do_tests()
> {
> + prep_directories
> # less than page size
> t_dax_flag_mmap_dio 1024
> # page size
> @@ -124,17 +129,10 @@ do_tests()
> # make xfs aligned for PMD fault testing
> _scratch_mkfs_geom $(_get_hugepagesize) 1 >> $seqres.full 2>&1
>
> -# mount with dax option
> -_scratch_mount "-o dax"
> -
> tsize=$((128 * 1024 * 1024))
>
> -do_tests
> -_scratch_unmount
> -
> -# mount again without dax option
> -export MOUNT_OPTIONS=""
> -_scratch_mount
> +# mount with dax=inode option
> +_scratch_mount "-o dax=inode"
> do_tests
>
> # success, all done
> diff --git a/tests/generic/603.out b/tests/generic/603.out
> new file mode 100644
> index 00000000..6810da89
> --- /dev/null
> +++ b/tests/generic/603.out
> @@ -0,0 +1,2 @@
> +QA output created by 603
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index d9ab9a31..1d0d5606 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -605,3 +605,4 @@
> 600 auto quick quota
> 601 auto quick quota
> 602 auto quick encrypt
> +603 auto attr quick dax
> diff --git a/tests/xfs/260.out b/tests/xfs/260.out
> deleted file mode 100644
> index 18ca517c..00000000
> --- a/tests/xfs/260.out
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -QA output created by 260
> -Silence is golden
> diff --git a/tests/xfs/group b/tests/xfs/group
> index daf54add..71c30898 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -257,7 +257,6 @@
> 257 auto quick clone
> 258 auto quick clone
> 259 auto quick
> -260 auto attr quick dax
> 261 auto quick quota
> 262 dangerous_fuzzers dangerous_scrub dangerous_online_repair
> 263 auto quick quota
> --
> 2.21.0
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-15 9:44 ` Xiao Yang
@ 2020-07-15 16:19 ` Darrick J. Wong
2020-07-15 16:33 ` Xiao Yang
0 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2020-07-15 16:19 UTC (permalink / raw)
To: Xiao Yang; +Cc: Ira Weiny, fstests
On Wed, Jul 15, 2020 at 05:44:53PM +0800, Xiao Yang wrote:
> On 2020/7/15 13:39, Xiao Yang wrote:
> > On 2020/7/15 10:48, Ira Weiny wrote:
> > > On Tue, Jul 14, 2020 at 05:40:09PM +0800, Xiao Yang wrote:
> > > > Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> > > > ---
> > > > tests/generic/605 | 199
> > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > tests/generic/605.out | 2 +
> > > > tests/generic/group | 1 +
> > > > 3 files changed, 202 insertions(+)
> > > > create mode 100644 tests/generic/605
> > > > create mode 100644 tests/generic/605.out
> > > >
> > > > diff --git a/tests/generic/605 b/tests/generic/605
> > > > new file mode 100644
> > > > index 00000000..6924223a
> > > > --- /dev/null
> > > > +++ b/tests/generic/605
> > > > @@ -0,0 +1,199 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) 2020 Fujitsu. All Rights Reserved.
> > > > +#
> > > > +# FS QA Test 605
> > > > +#
> > > > +# Verify the inheritance behavior of FS_XFLAG_DAX flag in
> > > > various combinations.
> > > > +# 1) New files and directories automatically inherit
> > > > FS_XFLAG_DAX from their parent directory.
> > > > +# 2) cp operation make files and directories inherit the
> > > > FS_XFLAG_DAX from new parent directory.
> > > > +# 3) mv operation make files and directories preserve the
> > > > FS_XFLAG_DAX from old parent directory.
> > > > +# In addition, setting/clearing FS_XFLAG_DAX flag is not
> > > > impacted by dax mount options.
> > > > +
> > > > +seq=`basename $0`
> > > > +seqres=$RESULT_DIR/$seq
> > > > +echo "QA output created by $seq"
> > > > +
> > > > +here=`pwd`
> > > > +tmp=/tmp/$$
> > > > +status=1 # failure is the default!
> > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > > +
> > > > +_cleanup()
> > > > +{
> > > > + cd /
> > > > + rm -f $tmp.*
> > > > +}
> > > > +
> > > > +# get standard environment, filters and checks
> > > > +. ./common/rc
> > > > +. ./common/filter
> > > > +
> > > > +# remove previous $seqres.full before test
> > > > +rm -f $seqres.full
> > > > +
> > > > +_supported_fs generic
> > > > +_supported_os Linux
> > > > +_require_scratch
> > > > +_require_dax_iflag
> > > > +_require_xfs_io_command "lsattr" "-v"
> > > > +
> > > > +check_xflag()
> > > > +{
> > > > + local target=$1
> > > > + local exp_xflag=$2
> > > > +
> > > > + if [ $exp_xflag -eq 0 ]; then
> > > > + _test_inode_flag dax $target&& echo "$target has
> > > > unexpected FS_XFLAG_DAX flag"
> > > > + else
> > > > + _test_inode_flag dax $target || echo "$target doen't
> > > > have expected FS_XFLAG_DAX flag"
> > > > + fi
> > > > +}
> > > > +
> > > > +test_xflag_inheritance1()
> > > > +{
> > > > + mkdir -p a
> > > > + $XFS_IO_PROG -c "chattr +x" a
> > > > + mkdir -p a/b/c
> > > > + touch a/b/c/d
> > > > +
> > > > + check_xflag a 1
> > > > + check_xflag a/b 1
> > > > + check_xflag a/b/c 1
> > > > + check_xflag a/b/c/d 1
> > > > +
> > > > + rm -rf a
> > > > +}
> > > > +
> > > > +test_xflag_inheritance2()
> > > > +{
> > > > + mkdir -p a/b
> > > > + $XFS_IO_PROG -c "chattr +x" a
> > > > + mkdir -p a/b/c a/d
> > > > + touch a/b/c/e a/d/f
> > > > +
> > > > + check_xflag a 1
> > > > + check_xflag a/b 0
> > > > + check_xflag a/b/c 0
> > > > + check_xflag a/b/c/e 0
> > > > + check_xflag a/d 1
> > > > + check_xflag a/d/f 1
> > > > +
> > > > + rm -rf a
> > > > +}
> > > > +
> > > > +test_xflag_inheritance3()
> > > > +{
> > > > + mkdir -p a/b
> > > > + $XFS_IO_PROG -c "chattr +x" a/b
> > > > + mkdir -p a/b/c a/d
> > > > + touch a/b/c/e a/d/f
> > > > +
> > > > + check_xflag a 0
> > > > + check_xflag a/b 1
> > > > + check_xflag a/b/c 1
> > > > + check_xflag a/b/c/e 1
> > > > + check_xflag a/d 0
> > > > + check_xflag a/d/f 0
> > > > +
> > > > + rm -rf a
> > > > +}
> > > It really seems like 2 and 3 test the same thing?
> > Hi Ira,
> >
> > 2 constructs the following steps:
> > 1) a is the parent directory of b
> > 2) a doesn't have xflag and b has xflag
> > 3) touch many directories/files in a and b
> >
> > 3 constructs the following steps:
> > 1) a is the parent directory of b and b is the parent directory of c
> > 2) a and c have xflag, and b doesn't have xflag
> > 3) touch many directories/files in b and c
> Hi Ira,
>
> Sorry for misreading your comment, above is the difference between 3 and 4.
> The correct one is:
> 2 constructs the following steps:
> 1) a is the parent directory of b
> 2) a has xflag and b doesn't have xflag
> 3) touch many directories/files in a and b
>
> 3 constructs the following steps:
> 1) a is the parent directory of b
> 2) a doesn't have xflag and b has xflag
> 3) touch many directories/files in a and b
>
> Do you think they are same? I can remove one if you think so.
For an earlier version of this series I thought about recommending that
each of these functions describe what they aim to test. Then I realized
that such descriptions would probably be nearly as long as the function
body, and said nothing.
But now that Ira's confused, I think that's a stronger argument for each
of the test functions having a short description.
# If a/ is +x and b/ is -x, check that b's new children don't
# inherit +x from a/.
test_xflag_inheritance2() {...}
Put another way, this adds enough redundancy between the comment and the
code that someone else can feel confident that the code still captures
the intent of the author.
FWIW I think 2 and 3 test opposite variations of the same thing (a's
state doesn't somehow override b's), so they're fine. The xfs
implementation uses the same inheritance control code for FS_XFLAG_DAX,
but doesn't mean everyone else will necessarily do that.
--D
> Best Regards,
> Xiao Yang
> >
> > Do you think they are same? I can remove one if you think so.
> >
> > > > +
> > > > +test_xflag_inheritance4()
> > > > +{
> > > > + mkdir -p a
> > > > + $XFS_IO_PROG -c "chattr +x" a
> > > > + mkdir -p a/b/c
> > > > + $XFS_IO_PROG -c "chattr -x" a/b
> > > > + mkdir -p a/b/c/d a/b/e
> > > > + touch a/b/c/d/f a/b/e/g
> > > > +
> > > > + check_xflag a 1
> > > > + check_xflag a/b 0
> > > > + check_xflag a/b/c 1
> > > > + check_xflag a/b/c/d 1
> > > > + check_xflag a/b/c/d/f 1
> > > > + check_xflag a/b/e 0
> > > > + check_xflag a/b/e/g 0
> > > > +
> > > > + rm -rf a
> > > > +}
> > > > +
> > > > +test_xflag_inheritance5()
> > > > +{
> > > > + mkdir -p a b
> > > > + $XFS_IO_PROG -c "chattr +x" a
> > > > + mkdir -p a/c a/d b/e b/f
> > > > + touch a/g b/h
> > > > +
> > > > + cp -r a/c b/
> > > > + cp -r b/e a/
> > > > + cp -r a/g b/
> > > > + mv a/d b/
> > > > + mv b/f a/
> > > > + mv b/h a/
> > > > +
> > > > + check_xflag b/c 0
> > > > + check_xflag b/d 1
> > > > + check_xflag a/e 1
> > > > + check_xflag a/f 0
> > > > + check_xflag b/g 0
> > > > + check_xflag a/h 0
> > > > +
> > > > + rm -rf a b
> > > > +}
> > > > +
> > > > +do_xflag_tests()
> > > > +{
> > > > + local option=$1
> > > > +
> > > > + _scratch_mount "$option"
> > > > + cd $SCRATCH_MNT
> > > > +
> > > > + for i in $(seq 1 5); do
> > > > + test_xflag_inheritance${i}
> > > > + done
> > > > +
> > > > + cd -> /dev/null
> > > > + _scratch_unmount
> > > > +}
> > > > +
> > > > +check_dax_mountopt()
> > > > +{
> > > > + local option=$1
> > > > + local ret=0
> > > > +
> > > > + _try_scratch_mount "-o $option">> $seqres.full 2>&1 || return 1
> > > > +
> > > > + # Match option name exactly
> > > > + _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
> > > > +
> > > > + _scratch_unmount
> > > > +
> > > > + return $ret
> > > > +}
> > > Should this be a common function?
> >
> > I am not sure if it should be a common function, because it may not be
> > used by other tests in future.
> > I also consider to merge the function into
> > _require_scratch_dax_mountopt().
> >
> > > > +
> > > > +do_tests()
> > > > +{
> > > > + # Mount without dax option
> > > > + do_xflag_tests
> > > > +
> > > > + # Mount with old dax option if fs only supports it.
> > > > + check_dax_mountopt "dax"&& do_xflag_tests "-o dax"
> > > I don't understand the order here. If we are on an older kernel and
> > > the FS
> > > only supports '-o dax' the do_xflag_tests will fail won't it?
> >
> > With both old dax and new dax, the inheritance behavior of FS_XFLAG_DAX
> > works well.
> >
> > > So shouldn't we do this first and bail/'not run' this test if that
> > > is the case?
> > >
> > > I really don't think there is any point in testing the old XFS
> > > behavior because
> > > the FS_XFLAG_DAX had no effect. So even if it is broken it does not
> > > matter.
> > > Or perhaps I am missing something here?
> >
> > This test is designed to verify the inheritance behavior of
> > FS_XFLAG_DAX(not related to S_DAX)
> > so I think it is fine for both old dax and new dax to run the test.
> >
> > Best Regards,
> > Xiao Yang
> > > Ira
> > >
> > > > +
> > > > + # Mount with new dax options if fs supports them.
> > > > + if check_dax_mountopt "dax=always"; then
> > > > + for dax_option in "dax=always" "dax=inode" "dax=never"; do
> > > > + do_xflag_tests "-o $dax_option"
> > > > + done
> > > > + fi
> > > > +}
> > > > +
> > > > +_scratch_mkfs>> $seqres.full 2>&1
> > > > +
> > > > +do_tests
> > > > +
> > > > +# success, all done
> > > > +echo "Silence is golden"
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/generic/605.out b/tests/generic/605.out
> > > > new file mode 100644
> > > > index 00000000..1ae20049
> > > > --- /dev/null
> > > > +++ b/tests/generic/605.out
> > > > @@ -0,0 +1,2 @@
> > > > +QA output created by 605
> > > > +Silence is golden
> > > > diff --git a/tests/generic/group b/tests/generic/group
> > > > index 676e0d2e..a8451862 100644
> > > > --- a/tests/generic/group
> > > > +++ b/tests/generic/group
> > > > @@ -607,3 +607,4 @@
> > > > 602 auto quick encrypt
> > > > 603 auto attr quick dax
> > > > 604 auto attr quick dax
> > > > +605 auto attr quick dax
> > > > --
> > > > 2.21.0
> > > >
> > > >
> > > >
> > >
> > > .
> > >
> >
> >
> >
> > .
> >
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-15 16:19 ` Darrick J. Wong
@ 2020-07-15 16:33 ` Xiao Yang
2020-07-15 18:18 ` Ira Weiny
0 siblings, 1 reply; 29+ messages in thread
From: Xiao Yang @ 2020-07-15 16:33 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Ira Weiny, fstests
On 2020/7/16 0:19, Darrick J. Wong wrote:
> On Wed, Jul 15, 2020 at 05:44:53PM +0800, Xiao Yang wrote:
>> On 2020/7/15 13:39, Xiao Yang wrote:
>>> On 2020/7/15 10:48, Ira Weiny wrote:
>>>> On Tue, Jul 14, 2020 at 05:40:09PM +0800, Xiao Yang wrote:
>>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>>> ---
>>>>> tests/generic/605 | 199
>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>> tests/generic/605.out | 2 +
>>>>> tests/generic/group | 1 +
>>>>> 3 files changed, 202 insertions(+)
>>>>> create mode 100644 tests/generic/605
>>>>> create mode 100644 tests/generic/605.out
>>>>>
>>>>> diff --git a/tests/generic/605 b/tests/generic/605
>>>>> new file mode 100644
>>>>> index 00000000..6924223a
>>>>> --- /dev/null
>>>>> +++ b/tests/generic/605
>>>>> @@ -0,0 +1,199 @@
>>>>> +#! /bin/bash
>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>> +# Copyright (c) 2020 Fujitsu. All Rights Reserved.
>>>>> +#
>>>>> +# FS QA Test 605
>>>>> +#
>>>>> +# Verify the inheritance behavior of FS_XFLAG_DAX flag in
>>>>> various combinations.
>>>>> +# 1) New files and directories automatically inherit
>>>>> FS_XFLAG_DAX from their parent directory.
>>>>> +# 2) cp operation make files and directories inherit the
>>>>> FS_XFLAG_DAX from new parent directory.
>>>>> +# 3) mv operation make files and directories preserve the
>>>>> FS_XFLAG_DAX from old parent directory.
>>>>> +# In addition, setting/clearing FS_XFLAG_DAX flag is not
>>>>> impacted by dax mount options.
>>>>> +
>>>>> +seq=`basename $0`
>>>>> +seqres=$RESULT_DIR/$seq
>>>>> +echo "QA output created by $seq"
>>>>> +
>>>>> +here=`pwd`
>>>>> +tmp=/tmp/$$
>>>>> +status=1 # failure is the default!
>>>>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>>>>> +
>>>>> +_cleanup()
>>>>> +{
>>>>> + cd /
>>>>> + rm -f $tmp.*
>>>>> +}
>>>>> +
>>>>> +# get standard environment, filters and checks
>>>>> +. ./common/rc
>>>>> +. ./common/filter
>>>>> +
>>>>> +# remove previous $seqres.full before test
>>>>> +rm -f $seqres.full
>>>>> +
>>>>> +_supported_fs generic
>>>>> +_supported_os Linux
>>>>> +_require_scratch
>>>>> +_require_dax_iflag
>>>>> +_require_xfs_io_command "lsattr" "-v"
>>>>> +
>>>>> +check_xflag()
>>>>> +{
>>>>> + local target=$1
>>>>> + local exp_xflag=$2
>>>>> +
>>>>> + if [ $exp_xflag -eq 0 ]; then
>>>>> + _test_inode_flag dax $target&& echo "$target has
>>>>> unexpected FS_XFLAG_DAX flag"
>>>>> + else
>>>>> + _test_inode_flag dax $target || echo "$target doen't
>>>>> have expected FS_XFLAG_DAX flag"
>>>>> + fi
>>>>> +}
>>>>> +
>>>>> +test_xflag_inheritance1()
>>>>> +{
>>>>> + mkdir -p a
>>>>> + $XFS_IO_PROG -c "chattr +x" a
>>>>> + mkdir -p a/b/c
>>>>> + touch a/b/c/d
>>>>> +
>>>>> + check_xflag a 1
>>>>> + check_xflag a/b 1
>>>>> + check_xflag a/b/c 1
>>>>> + check_xflag a/b/c/d 1
>>>>> +
>>>>> + rm -rf a
>>>>> +}
>>>>> +
>>>>> +test_xflag_inheritance2()
>>>>> +{
>>>>> + mkdir -p a/b
>>>>> + $XFS_IO_PROG -c "chattr +x" a
>>>>> + mkdir -p a/b/c a/d
>>>>> + touch a/b/c/e a/d/f
>>>>> +
>>>>> + check_xflag a 1
>>>>> + check_xflag a/b 0
>>>>> + check_xflag a/b/c 0
>>>>> + check_xflag a/b/c/e 0
>>>>> + check_xflag a/d 1
>>>>> + check_xflag a/d/f 1
>>>>> +
>>>>> + rm -rf a
>>>>> +}
>>>>> +
>>>>> +test_xflag_inheritance3()
>>>>> +{
>>>>> + mkdir -p a/b
>>>>> + $XFS_IO_PROG -c "chattr +x" a/b
>>>>> + mkdir -p a/b/c a/d
>>>>> + touch a/b/c/e a/d/f
>>>>> +
>>>>> + check_xflag a 0
>>>>> + check_xflag a/b 1
>>>>> + check_xflag a/b/c 1
>>>>> + check_xflag a/b/c/e 1
>>>>> + check_xflag a/d 0
>>>>> + check_xflag a/d/f 0
>>>>> +
>>>>> + rm -rf a
>>>>> +}
>>>> It really seems like 2 and 3 test the same thing?
>>> Hi Ira,
>>>
>>> 2 constructs the following steps:
>>> 1) a is the parent directory of b
>>> 2) a doesn't have xflag and b has xflag
>>> 3) touch many directories/files in a and b
>>>
>>> 3 constructs the following steps:
>>> 1) a is the parent directory of b and b is the parent directory of c
>>> 2) a and c have xflag, and b doesn't have xflag
>>> 3) touch many directories/files in b and c
>> Hi Ira,
>>
>> Sorry for misreading your comment, above is the difference between 3 and 4.
>> The correct one is:
>> 2 constructs the following steps:
>> 1) a is the parent directory of b
>> 2) a has xflag and b doesn't have xflag
>> 3) touch many directories/files in a and b
>>
>> 3 constructs the following steps:
>> 1) a is the parent directory of b
>> 2) a doesn't have xflag and b has xflag
>> 3) touch many directories/files in a and b
>>
>> Do you think they are same? I can remove one if you think so.
> For an earlier version of this series I thought about recommending that
> each of these functions describe what they aim to test. Then I realized
> that such descriptions would probably be nearly as long as the function
> body, and said nothing.
>
> But now that Ira's confused, I think that's a stronger argument for each
> of the test functions having a short description.
>
> # If a/ is +x and b/ is -x, check that b's new children don't
> # inherit +x from a/.
> test_xflag_inheritance2() {...}
>
> Put another way, this adds enough redundancy between the comment and the
> code that someone else can feel confident that the code still captures
> the intent of the author.
>
> FWIW I think 2 and 3 test opposite variations of the same thing (a's
> state doesn't somehow override b's), so they're fine. The xfs
> implementation uses the same inheritance control code for FS_XFLAG_DAX,
> but doesn't mean everyone else will necessarily do that.
Hi Darrck,
Do you prefer to keep both 2 and 3? right? :-)
Thanks,
Xiao Yang
> --D
>
>> Best Regards,
>> Xiao Yang
>>> Do you think they are same? I can remove one if you think so.
>>>
>>>>> +
>>>>> +test_xflag_inheritance4()
>>>>> +{
>>>>> + mkdir -p a
>>>>> + $XFS_IO_PROG -c "chattr +x" a
>>>>> + mkdir -p a/b/c
>>>>> + $XFS_IO_PROG -c "chattr -x" a/b
>>>>> + mkdir -p a/b/c/d a/b/e
>>>>> + touch a/b/c/d/f a/b/e/g
>>>>> +
>>>>> + check_xflag a 1
>>>>> + check_xflag a/b 0
>>>>> + check_xflag a/b/c 1
>>>>> + check_xflag a/b/c/d 1
>>>>> + check_xflag a/b/c/d/f 1
>>>>> + check_xflag a/b/e 0
>>>>> + check_xflag a/b/e/g 0
>>>>> +
>>>>> + rm -rf a
>>>>> +}
>>>>> +
>>>>> +test_xflag_inheritance5()
>>>>> +{
>>>>> + mkdir -p a b
>>>>> + $XFS_IO_PROG -c "chattr +x" a
>>>>> + mkdir -p a/c a/d b/e b/f
>>>>> + touch a/g b/h
>>>>> +
>>>>> + cp -r a/c b/
>>>>> + cp -r b/e a/
>>>>> + cp -r a/g b/
>>>>> + mv a/d b/
>>>>> + mv b/f a/
>>>>> + mv b/h a/
>>>>> +
>>>>> + check_xflag b/c 0
>>>>> + check_xflag b/d 1
>>>>> + check_xflag a/e 1
>>>>> + check_xflag a/f 0
>>>>> + check_xflag b/g 0
>>>>> + check_xflag a/h 0
>>>>> +
>>>>> + rm -rf a b
>>>>> +}
>>>>> +
>>>>> +do_xflag_tests()
>>>>> +{
>>>>> + local option=$1
>>>>> +
>>>>> + _scratch_mount "$option"
>>>>> + cd $SCRATCH_MNT
>>>>> +
>>>>> + for i in $(seq 1 5); do
>>>>> + test_xflag_inheritance${i}
>>>>> + done
>>>>> +
>>>>> + cd -> /dev/null
>>>>> + _scratch_unmount
>>>>> +}
>>>>> +
>>>>> +check_dax_mountopt()
>>>>> +{
>>>>> + local option=$1
>>>>> + local ret=0
>>>>> +
>>>>> + _try_scratch_mount "-o $option">> $seqres.full 2>&1 || return 1
>>>>> +
>>>>> + # Match option name exactly
>>>>> + _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
>>>>> +
>>>>> + _scratch_unmount
>>>>> +
>>>>> + return $ret
>>>>> +}
>>>> Should this be a common function?
>>> I am not sure if it should be a common function, because it may not be
>>> used by other tests in future.
>>> I also consider to merge the function into
>>> _require_scratch_dax_mountopt().
>>>
>>>>> +
>>>>> +do_tests()
>>>>> +{
>>>>> + # Mount without dax option
>>>>> + do_xflag_tests
>>>>> +
>>>>> + # Mount with old dax option if fs only supports it.
>>>>> + check_dax_mountopt "dax"&& do_xflag_tests "-o dax"
>>>> I don't understand the order here. If we are on an older kernel and
>>>> the FS
>>>> only supports '-o dax' the do_xflag_tests will fail won't it?
>>> With both old dax and new dax, the inheritance behavior of FS_XFLAG_DAX
>>> works well.
>>>
>>>> So shouldn't we do this first and bail/'not run' this test if that
>>>> is the case?
>>>>
>>>> I really don't think there is any point in testing the old XFS
>>>> behavior because
>>>> the FS_XFLAG_DAX had no effect. So even if it is broken it does not
>>>> matter.
>>>> Or perhaps I am missing something here?
>>> This test is designed to verify the inheritance behavior of
>>> FS_XFLAG_DAX(not related to S_DAX)
>>> so I think it is fine for both old dax and new dax to run the test.
>>>
>>> Best Regards,
>>> Xiao Yang
>>>> Ira
>>>>
>>>>> +
>>>>> + # Mount with new dax options if fs supports them.
>>>>> + if check_dax_mountopt "dax=always"; then
>>>>> + for dax_option in "dax=always" "dax=inode" "dax=never"; do
>>>>> + do_xflag_tests "-o $dax_option"
>>>>> + done
>>>>> + fi
>>>>> +}
>>>>> +
>>>>> +_scratch_mkfs>> $seqres.full 2>&1
>>>>> +
>>>>> +do_tests
>>>>> +
>>>>> +# success, all done
>>>>> +echo "Silence is golden"
>>>>> +status=0
>>>>> +exit
>>>>> diff --git a/tests/generic/605.out b/tests/generic/605.out
>>>>> new file mode 100644
>>>>> index 00000000..1ae20049
>>>>> --- /dev/null
>>>>> +++ b/tests/generic/605.out
>>>>> @@ -0,0 +1,2 @@
>>>>> +QA output created by 605
>>>>> +Silence is golden
>>>>> diff --git a/tests/generic/group b/tests/generic/group
>>>>> index 676e0d2e..a8451862 100644
>>>>> --- a/tests/generic/group
>>>>> +++ b/tests/generic/group
>>>>> @@ -607,3 +607,4 @@
>>>>> 602 auto quick encrypt
>>>>> 603 auto attr quick dax
>>>>> 604 auto attr quick dax
>>>>> +605 auto attr quick dax
>>>>> --
>>>>> 2.21.0
>>>>>
>>>>>
>>>>>
>>>> .
>>>>
>>>
>>>
>>> .
>>>
>>
>>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-15 8:10 ` Xiao Yang
@ 2020-07-15 16:43 ` Xiao Yang
0 siblings, 0 replies; 29+ messages in thread
From: Xiao Yang @ 2020-07-15 16:43 UTC (permalink / raw)
To: darrick.wong; +Cc: Ira Weiny, fstests
于 2020/7/15 16:10, Xiao Yang 写道:
> On 2020/7/15 13:39, Xiao Yang wrote:
>> On 2020/7/15 10:48, Ira Weiny wrote:
>>> On Tue, Jul 14, 2020 at 05:40:09PM +0800, Xiao Yang wrote:
>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>> ---
>>>> tests/generic/605 | 199 ++++++++++++++++++++++++++++++++++++++++++
>>>> tests/generic/605.out | 2 +
>>>> tests/generic/group | 1 +
>>>> 3 files changed, 202 insertions(+)
>>>> create mode 100644 tests/generic/605
>>>> create mode 100644 tests/generic/605.out
>>>>
>>>> diff --git a/tests/generic/605 b/tests/generic/605
>>>> new file mode 100644
>>>> index 00000000..6924223a
>>>> --- /dev/null
>>>> +++ b/tests/generic/605
>>>> @@ -0,0 +1,199 @@
>>>> +#! /bin/bash
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +# Copyright (c) 2020 Fujitsu. All Rights Reserved.
>>>> +#
>>>> +# FS QA Test 605
>>>> +#
>>>> +# Verify the inheritance behavior of FS_XFLAG_DAX flag in various
>>>> combinations.
>>>> +# 1) New files and directories automatically inherit FS_XFLAG_DAX
>>>> from their parent directory.
>>>> +# 2) cp operation make files and directories inherit the
>>>> FS_XFLAG_DAX from new parent directory.
>>>> +# 3) mv operation make files and directories preserve the
>>>> FS_XFLAG_DAX from old parent directory.
>>>> +# In addition, setting/clearing FS_XFLAG_DAX flag is not impacted
>>>> by dax mount options.
>>>> +
>>>> +seq=`basename $0`
>>>> +seqres=$RESULT_DIR/$seq
>>>> +echo "QA output created by $seq"
>>>> +
>>>> +here=`pwd`
>>>> +tmp=/tmp/$$
>>>> +status=1 # failure is the default!
>>>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>>>> +
>>>> +_cleanup()
>>>> +{
>>>> + cd /
>>>> + rm -f $tmp.*
>>>> +}
>>>> +
>>>> +# get standard environment, filters and checks
>>>> +. ./common/rc
>>>> +. ./common/filter
>>>> +
>>>> +# remove previous $seqres.full before test
>>>> +rm -f $seqres.full
>>>> +
>>>> +_supported_fs generic
>>>> +_supported_os Linux
>>>> +_require_scratch
>>>> +_require_dax_iflag
>>>> +_require_xfs_io_command "lsattr" "-v"
>>>> +
>>>> +check_xflag()
>>>> +{
>>>> + local target=$1
>>>> + local exp_xflag=$2
>>>> +
>>>> + if [ $exp_xflag -eq 0 ]; then
>>>> + _test_inode_flag dax $target&& echo "$target has unexpected
>>>> FS_XFLAG_DAX flag"
>>>> + else
>>>> + _test_inode_flag dax $target || echo "$target doen't have
>>>> expected FS_XFLAG_DAX flag"
>>>> + fi
>>>> +}
>>>> +
>>>> +test_xflag_inheritance1()
>>>> +{
>>>> + mkdir -p a
>>>> + $XFS_IO_PROG -c "chattr +x" a
>>>> + mkdir -p a/b/c
>>>> + touch a/b/c/d
>>>> +
>>>> + check_xflag a 1
>>>> + check_xflag a/b 1
>>>> + check_xflag a/b/c 1
>>>> + check_xflag a/b/c/d 1
>>>> +
>>>> + rm -rf a
>>>> +}
>>>> +
>>>> +test_xflag_inheritance2()
>>>> +{
>>>> + mkdir -p a/b
>>>> + $XFS_IO_PROG -c "chattr +x" a
>>>> + mkdir -p a/b/c a/d
>>>> + touch a/b/c/e a/d/f
>>>> +
>>>> + check_xflag a 1
>>>> + check_xflag a/b 0
>>>> + check_xflag a/b/c 0
>>>> + check_xflag a/b/c/e 0
>>>> + check_xflag a/d 1
>>>> + check_xflag a/d/f 1
>>>> +
>>>> + rm -rf a
>>>> +}
>>>> +
>>>> +test_xflag_inheritance3()
>>>> +{
>>>> + mkdir -p a/b
>>>> + $XFS_IO_PROG -c "chattr +x" a/b
>>>> + mkdir -p a/b/c a/d
>>>> + touch a/b/c/e a/d/f
>>>> +
>>>> + check_xflag a 0
>>>> + check_xflag a/b 1
>>>> + check_xflag a/b/c 1
>>>> + check_xflag a/b/c/e 1
>>>> + check_xflag a/d 0
>>>> + check_xflag a/d/f 0
>>>> +
>>>> + rm -rf a
>>>> +}
>>> It really seems like 2 and 3 test the same thing?
>> Hi Ira,
>>
>> 2 constructs the following steps:
>> 1) a is the parent directory of b
>> 2) a doesn't have xflag and b has xflag
>> 3) touch many directories/files in a and b
>>
>> 3 constructs the following steps:
>> 1) a is the parent directory of b and b is the parent directory of c
>> 2) a and c have xflag, and b doesn't have xflag
>> 3) touch many directories/files in b and c
>>
>> Do you think they are same? I can remove one if you think so.
>>
>>>> +
>>>> +test_xflag_inheritance4()
>>>> +{
>>>> + mkdir -p a
>>>> + $XFS_IO_PROG -c "chattr +x" a
>>>> + mkdir -p a/b/c
>>>> + $XFS_IO_PROG -c "chattr -x" a/b
>>>> + mkdir -p a/b/c/d a/b/e
>>>> + touch a/b/c/d/f a/b/e/g
>>>> +
>>>> + check_xflag a 1
>>>> + check_xflag a/b 0
>>>> + check_xflag a/b/c 1
>>>> + check_xflag a/b/c/d 1
>>>> + check_xflag a/b/c/d/f 1
>>>> + check_xflag a/b/e 0
>>>> + check_xflag a/b/e/g 0
>>>> +
>>>> + rm -rf a
>>>> +}
>>>> +
>>>> +test_xflag_inheritance5()
>>>> +{
>>>> + mkdir -p a b
>>>> + $XFS_IO_PROG -c "chattr +x" a
>>>> + mkdir -p a/c a/d b/e b/f
>>>> + touch a/g b/h
>>>> +
>>>> + cp -r a/c b/
>>>> + cp -r b/e a/
>>>> + cp -r a/g b/
>>>> + mv a/d b/
>>>> + mv b/f a/
>>>> + mv b/h a/
>>>> +
>>>> + check_xflag b/c 0
>>>> + check_xflag b/d 1
>>>> + check_xflag a/e 1
>>>> + check_xflag a/f 0
>>>> + check_xflag b/g 0
>>>> + check_xflag a/h 0
>>>> +
>>>> + rm -rf a b
>>>> +}
>>>> +
>>>> +do_xflag_tests()
>>>> +{
>>>> + local option=$1
>>>> +
>>>> + _scratch_mount "$option"
>>>> + cd $SCRATCH_MNT
>>>> +
>>>> + for i in $(seq 1 5); do
>>>> + test_xflag_inheritance${i}
>>>> + done
>>>> +
>>>> + cd -> /dev/null
>>>> + _scratch_unmount
>>>> +}
>>>> +
>>>> +check_dax_mountopt()
>>>> +{
>>>> + local option=$1
>>>> + local ret=0
>>>> +
>>>> + _try_scratch_mount "-o $option">> $seqres.full 2>&1 || return 1
>>>> +
>>>> + # Match option name exactly
>>>> + _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
>>>> +
>>>> + _scratch_unmount
>>>> +
>>>> + return $ret
>>>> +}
>>> Should this be a common function?
>>
>> I am not sure if it should be a common function, because it may not
>> be used by other tests in future.
>> I also consider to merge the function into
>> _require_scratch_dax_mountopt().
> For this comment, I try to merge the function into
> _require_scratch_dax_mountopt(), as below:
> ----------------------------------------------------------------------------------------------------------------
>
> +# Only accept dax/dax=always mount option becasue dax=always, dax=inode
> +# and dax=never are always introduced together.
> +# Return 0 if filesystem/device supports the specified dax option.
> +# Return 1 if mount fails with the specified dax option.
> +# Return 2 if /proc/mounts shows wrong dax option.
> +# Check new dax=inode, dax=always or dax=never option by passing
> "dax=always".
> +# Check old dax or new dax=always by passing "dax".
> +_check_scratch_dax_mountopt()
> +{
> + local option=$1
> +
> + echo "$option" | egrep -q "dax(=always|$)" || \
> + _notrun "invalid $option, only accept dax/dax=always"
> +
> + _require_scratch
> + _scratch_mkfs > /dev/null 2>&1
> +
> + _try_scratch_mount "-o $option" > /dev/null 2>&1 || return 1
> +
> + if _fs_options $SCRATCH_DEV | egrep -q "dax(=always|,|$)"; then
> + _scratch_unmount
> + return 0
> + else
> + _scratch_unmount
> + return 2
> + fi
> +}
> +
> +# Throw notrun if _check_scratch_dax_mountopt() returns a non-zero
> value.
> +_require_scratch_dax_mountopt()
> +{
> + local mountopt=$1
> +
> + _check_scratch_dax_mountopt "$mountopt"
> + local res=$?
> +
> + [ $res -eq 1 ] && _notrun "mount $SCRATCH_DEV with $mountopt failed"
> + [ $res -eq 2 ] && _notrun "$SCRATCH_DEV $FSTYP does not support -o
> $mountopt"
> +}
> -----------------------------------------------------------------------------------------------------------
>
Hi Darrick,
How about this change?
1) Introduce _check_scratch_dax_mountopt() to check dax option.
(return 0 if dax option is supported, return 1 if mount fail or return 2
if /proc/mounts show wrong info)
2) _require_scratch_dax_mountopt() calls Introduce
_check_scratch_dax_mountopt()
and throws notrun if check_scratch_dax_mountopt() returns a non-zero value.
>>
>>>> +
>>>> +do_tests()
>>>> +{
>>>> + # Mount without dax option
>>>> + do_xflag_tests
>>>> +
>>>> + # Mount with old dax option if fs only supports it.
>>>> + check_dax_mountopt "dax"&& do_xflag_tests "-o dax"
>>> I don't understand the order here. If we are on an older kernel and
>>> the FS
>>> only supports '-o dax' the do_xflag_tests will fail won't it?
>>
>> With both old dax and new dax, the inheritance behavior of
>> FS_XFLAG_DAX works well.
>>
>>> So shouldn't we do this first and bail/'not run' this test if that
>>> is the case?
>>>
>>> I really don't think there is any point in testing the old XFS
>>> behavior because
>>> the FS_XFLAG_DAX had no effect. So even if it is broken it does not
>>> matter.
>>> Or perhaps I am missing something here?
>>
>> This test is designed to verify the inheritance behavior of
>> FS_XFLAG_DAX(not related to S_DAX)
>> so I think it is fine for both old dax and new dax to run the test.
> For this comment, I try to update it, as below:
> -------------------------------------------------------------------------
> +do_tests()
> +{
> + # Mount without dax option
> + do_xflag_tests
> +
> + # Mount with 'dax' or 'dax=always' option if fs supports it.
> + _check_scratch_dax_mountopt "dax" && do_xflag_tests "-o dax"
> +
> + # Mount with 'dax=inode' and 'dax=never' options if fs supports them.
> + if _check_scratch_dax_mountopt "dax=always"; then
> + for dax_option in "dax=inode" "dax=never"; do
> + do_xflag_tests "-o $dax_option"
> + done
> + fi
> +}
> -------------------------------------------------------------------------
After the implemention of _check_scratch_dax_mountopt(), we can use it here.
Thanks,
Xiao Yang
>>
>> Best Regards,
>> Xiao Yang
>>> Ira
>>>
>>>> +
>>>> + # Mount with new dax options if fs supports them.
>>>> + if check_dax_mountopt "dax=always"; then
>>>> + for dax_option in "dax=always" "dax=inode" "dax=never"; do
>>>> + do_xflag_tests "-o $dax_option"
>>>> + done
>>>> + fi
>>>> +}
>>>> +
>>>> +_scratch_mkfs>> $seqres.full 2>&1
>>>> +
>>>> +do_tests
>>>> +
>>>> +# success, all done
>>>> +echo "Silence is golden"
>>>> +status=0
>>>> +exit
>>>> diff --git a/tests/generic/605.out b/tests/generic/605.out
>>>> new file mode 100644
>>>> index 00000000..1ae20049
>>>> --- /dev/null
>>>> +++ b/tests/generic/605.out
>>>> @@ -0,0 +1,2 @@
>>>> +QA output created by 605
>>>> +Silence is golden
>>>> diff --git a/tests/generic/group b/tests/generic/group
>>>> index 676e0d2e..a8451862 100644
>>>> --- a/tests/generic/group
>>>> +++ b/tests/generic/group
>>>> @@ -607,3 +607,4 @@
>>>> 602 auto quick encrypt
>>>> 603 auto attr quick dax
>>>> 604 auto attr quick dax
>>>> +605 auto attr quick dax
>>>> --
>>>> 2.21.0
>>>>
>>>>
>>>>
>>>
>>> .
>>>
>>
>>
>>
>> .
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX
2020-07-15 15:56 ` Darrick J. Wong
@ 2020-07-15 18:00 ` Ira Weiny
0 siblings, 0 replies; 29+ messages in thread
From: Ira Weiny @ 2020-07-15 18:00 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Xiao Yang, fstests
On Wed, Jul 15, 2020 at 08:56:31AM -0700, Darrick J. Wong wrote:
> On Wed, Jul 15, 2020 at 01:55:14PM +0800, Xiao Yang wrote:
> > 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@cn.fujitsu.com>
> > > > > Reviewed-by: Ira Weiny<ira.weiny@intel.com>
> > > > 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
>
> That does seem like the only way to figure out if a fs/device
> combination supports dax.
>
> The valid arguments to the _require_scratch_dax* helpers are worth
> mentioning in a comment above the function, but I suspect that adding
> validation code is probably overkill.
Ok yes I see it now.
You are both correct. I think a comment which indicates that one should pads
'-o dax' to check for 'old support' vs '-o dax=always' for 'new support' and
the test chooses which option to pass (based on the support it needs) to
_require_scratch_dax_mountopt()...
Sorry I was not completely following before!
Ira
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations
2020-07-15 16:33 ` Xiao Yang
@ 2020-07-15 18:18 ` Ira Weiny
0 siblings, 0 replies; 29+ messages in thread
From: Ira Weiny @ 2020-07-15 18:18 UTC (permalink / raw)
To: Xiao Yang; +Cc: Darrick J. Wong, fstests
On Thu, Jul 16, 2020 at 12:33:31AM +0800, Xiao Yang wrote:
> On 2020/7/16 0:19, Darrick J. Wong wrote:
> > On Wed, Jul 15, 2020 at 05:44:53PM +0800, Xiao Yang wrote:
> > > On 2020/7/15 13:39, Xiao Yang wrote:
> > > > On 2020/7/15 10:48, Ira Weiny wrote:
> > > > > On Tue, Jul 14, 2020 at 05:40:09PM +0800, Xiao Yang wrote:
> > > > > > Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> > > > > > ---
> > > > > > tests/generic/605 | 199
> > > > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > > > tests/generic/605.out | 2 +
> > > > > > tests/generic/group | 1 +
> > > > > > 3 files changed, 202 insertions(+)
> > > > > > create mode 100644 tests/generic/605
> > > > > > create mode 100644 tests/generic/605.out
> > > > > >
> > > > > > diff --git a/tests/generic/605 b/tests/generic/605
> > > > > > new file mode 100644
> > > > > > index 00000000..6924223a
> > > > > > --- /dev/null
> > > > > > +++ b/tests/generic/605
> > > > > > @@ -0,0 +1,199 @@
> > > > > > +#! /bin/bash
> > > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > > +# Copyright (c) 2020 Fujitsu. All Rights Reserved.
> > > > > > +#
> > > > > > +# FS QA Test 605
> > > > > > +#
> > > > > > +# Verify the inheritance behavior of FS_XFLAG_DAX flag in
> > > > > > various combinations.
> > > > > > +# 1) New files and directories automatically inherit
> > > > > > FS_XFLAG_DAX from their parent directory.
> > > > > > +# 2) cp operation make files and directories inherit the
> > > > > > FS_XFLAG_DAX from new parent directory.
> > > > > > +# 3) mv operation make files and directories preserve the
> > > > > > FS_XFLAG_DAX from old parent directory.
> > > > > > +# In addition, setting/clearing FS_XFLAG_DAX flag is not
> > > > > > impacted by dax mount options.
> > > > > > +
> > > > > > +seq=`basename $0`
> > > > > > +seqres=$RESULT_DIR/$seq
> > > > > > +echo "QA output created by $seq"
> > > > > > +
> > > > > > +here=`pwd`
> > > > > > +tmp=/tmp/$$
> > > > > > +status=1 # failure is the default!
> > > > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > > > > +
> > > > > > +_cleanup()
> > > > > > +{
> > > > > > + cd /
> > > > > > + rm -f $tmp.*
> > > > > > +}
> > > > > > +
> > > > > > +# get standard environment, filters and checks
> > > > > > +. ./common/rc
> > > > > > +. ./common/filter
> > > > > > +
> > > > > > +# remove previous $seqres.full before test
> > > > > > +rm -f $seqres.full
> > > > > > +
> > > > > > +_supported_fs generic
> > > > > > +_supported_os Linux
> > > > > > +_require_scratch
> > > > > > +_require_dax_iflag
> > > > > > +_require_xfs_io_command "lsattr" "-v"
> > > > > > +
> > > > > > +check_xflag()
> > > > > > +{
> > > > > > + local target=$1
> > > > > > + local exp_xflag=$2
> > > > > > +
> > > > > > + if [ $exp_xflag -eq 0 ]; then
> > > > > > + _test_inode_flag dax $target&& echo "$target has
> > > > > > unexpected FS_XFLAG_DAX flag"
> > > > > > + else
> > > > > > + _test_inode_flag dax $target || echo "$target doen't
> > > > > > have expected FS_XFLAG_DAX flag"
> > > > > > + fi
> > > > > > +}
> > > > > > +
> > > > > > +test_xflag_inheritance1()
> > > > > > +{
> > > > > > + mkdir -p a
> > > > > > + $XFS_IO_PROG -c "chattr +x" a
> > > > > > + mkdir -p a/b/c
> > > > > > + touch a/b/c/d
> > > > > > +
> > > > > > + check_xflag a 1
> > > > > > + check_xflag a/b 1
> > > > > > + check_xflag a/b/c 1
> > > > > > + check_xflag a/b/c/d 1
> > > > > > +
> > > > > > + rm -rf a
> > > > > > +}
> > > > > > +
> > > > > > +test_xflag_inheritance2()
> > > > > > +{
> > > > > > + mkdir -p a/b
> > > > > > + $XFS_IO_PROG -c "chattr +x" a
> > > > > > + mkdir -p a/b/c a/d
> > > > > > + touch a/b/c/e a/d/f
> > > > > > +
> > > > > > + check_xflag a 1
> > > > > > + check_xflag a/b 0
> > > > > > + check_xflag a/b/c 0
> > > > > > + check_xflag a/b/c/e 0
> > > > > > + check_xflag a/d 1
> > > > > > + check_xflag a/d/f 1
> > > > > > +
> > > > > > + rm -rf a
> > > > > > +}
> > > > > > +
> > > > > > +test_xflag_inheritance3()
> > > > > > +{
> > > > > > + mkdir -p a/b
> > > > > > + $XFS_IO_PROG -c "chattr +x" a/b
> > > > > > + mkdir -p a/b/c a/d
> > > > > > + touch a/b/c/e a/d/f
> > > > > > +
> > > > > > + check_xflag a 0
> > > > > > + check_xflag a/b 1
> > > > > > + check_xflag a/b/c 1
> > > > > > + check_xflag a/b/c/e 1
> > > > > > + check_xflag a/d 0
> > > > > > + check_xflag a/d/f 0
> > > > > > +
> > > > > > + rm -rf a
> > > > > > +}
> > > > > It really seems like 2 and 3 test the same thing?
> > > > Hi Ira,
> > > >
> > > > 2 constructs the following steps:
> > > > 1) a is the parent directory of b
> > > > 2) a doesn't have xflag and b has xflag
> > > > 3) touch many directories/files in a and b
> > > >
> > > > 3 constructs the following steps:
> > > > 1) a is the parent directory of b and b is the parent directory of c
> > > > 2) a and c have xflag, and b doesn't have xflag
> > > > 3) touch many directories/files in b and c
> > > Hi Ira,
> > >
> > > Sorry for misreading your comment, above is the difference between 3 and 4.
> > > The correct one is:
> > > 2 constructs the following steps:
> > > 1) a is the parent directory of b
> > > 2) a has xflag and b doesn't have xflag
> > > 3) touch many directories/files in a and b
> > >
> > > 3 constructs the following steps:
> > > 1) a is the parent directory of b
> > > 2) a doesn't have xflag and b has xflag
> > > 3) touch many directories/files in a and b
> > >
> > > Do you think they are same? I can remove one if you think so.
> > For an earlier version of this series I thought about recommending that
> > each of these functions describe what they aim to test. Then I realized
> > that such descriptions would probably be nearly as long as the function
> > body, and said nothing.
> >
> > But now that Ira's confused, I think that's a stronger argument for each
> > of the test functions having a short description.
> >
> > # If a/ is +x and b/ is -x, check that b's new children don't
> > # inherit +x from a/.
> > test_xflag_inheritance2() {...}
> >
> > Put another way, this adds enough redundancy between the comment and the
> > code that someone else can feel confident that the code still captures
> > the intent of the author.
> >
> > FWIW I think 2 and 3 test opposite variations of the same thing (a's
> > state doesn't somehow override b's), so they're fine. The xfs
> > implementation uses the same inheritance control code for FS_XFLAG_DAX,
> > but doesn't mean everyone else will necessarily do that.
> Hi Darrck,
>
> Do you prefer to keep both 2 and 3? right? :-)
My point was more about the fact that I don't think 2 and 3 actually exercising
any additional code paths within the kernel.
But looking at it this morning (rather than late last night) I could see where
changes to the kernel logic may introduce some issue in the future so we have
the test and we should leave it!
:-D
Ira
>
> Thanks,
> Xiao Yang
> > --D
> >
> > > Best Regards,
> > > Xiao Yang
> > > > Do you think they are same? I can remove one if you think so.
> > > >
> > > > > > +
> > > > > > +test_xflag_inheritance4()
> > > > > > +{
> > > > > > + mkdir -p a
> > > > > > + $XFS_IO_PROG -c "chattr +x" a
> > > > > > + mkdir -p a/b/c
> > > > > > + $XFS_IO_PROG -c "chattr -x" a/b
> > > > > > + mkdir -p a/b/c/d a/b/e
> > > > > > + touch a/b/c/d/f a/b/e/g
> > > > > > +
> > > > > > + check_xflag a 1
> > > > > > + check_xflag a/b 0
> > > > > > + check_xflag a/b/c 1
> > > > > > + check_xflag a/b/c/d 1
> > > > > > + check_xflag a/b/c/d/f 1
> > > > > > + check_xflag a/b/e 0
> > > > > > + check_xflag a/b/e/g 0
> > > > > > +
> > > > > > + rm -rf a
> > > > > > +}
> > > > > > +
> > > > > > +test_xflag_inheritance5()
> > > > > > +{
> > > > > > + mkdir -p a b
> > > > > > + $XFS_IO_PROG -c "chattr +x" a
> > > > > > + mkdir -p a/c a/d b/e b/f
> > > > > > + touch a/g b/h
> > > > > > +
> > > > > > + cp -r a/c b/
> > > > > > + cp -r b/e a/
> > > > > > + cp -r a/g b/
> > > > > > + mv a/d b/
> > > > > > + mv b/f a/
> > > > > > + mv b/h a/
> > > > > > +
> > > > > > + check_xflag b/c 0
> > > > > > + check_xflag b/d 1
> > > > > > + check_xflag a/e 1
> > > > > > + check_xflag a/f 0
> > > > > > + check_xflag b/g 0
> > > > > > + check_xflag a/h 0
> > > > > > +
> > > > > > + rm -rf a b
> > > > > > +}
> > > > > > +
> > > > > > +do_xflag_tests()
> > > > > > +{
> > > > > > + local option=$1
> > > > > > +
> > > > > > + _scratch_mount "$option"
> > > > > > + cd $SCRATCH_MNT
> > > > > > +
> > > > > > + for i in $(seq 1 5); do
> > > > > > + test_xflag_inheritance${i}
> > > > > > + done
> > > > > > +
> > > > > > + cd -> /dev/null
> > > > > > + _scratch_unmount
> > > > > > +}
> > > > > > +
> > > > > > +check_dax_mountopt()
> > > > > > +{
> > > > > > + local option=$1
> > > > > > + local ret=0
> > > > > > +
> > > > > > + _try_scratch_mount "-o $option">> $seqres.full 2>&1 || return 1
> > > > > > +
> > > > > > + # Match option name exactly
> > > > > > + _fs_options $SCRATCH_DEV | egrep -q "$option(,|$)" || ret=1
> > > > > > +
> > > > > > + _scratch_unmount
> > > > > > +
> > > > > > + return $ret
> > > > > > +}
> > > > > Should this be a common function?
> > > > I am not sure if it should be a common function, because it may not be
> > > > used by other tests in future.
> > > > I also consider to merge the function into
> > > > _require_scratch_dax_mountopt().
> > > >
> > > > > > +
> > > > > > +do_tests()
> > > > > > +{
> > > > > > + # Mount without dax option
> > > > > > + do_xflag_tests
> > > > > > +
> > > > > > + # Mount with old dax option if fs only supports it.
> > > > > > + check_dax_mountopt "dax"&& do_xflag_tests "-o dax"
> > > > > I don't understand the order here. If we are on an older kernel and
> > > > > the FS
> > > > > only supports '-o dax' the do_xflag_tests will fail won't it?
> > > > With both old dax and new dax, the inheritance behavior of FS_XFLAG_DAX
> > > > works well.
> > > >
> > > > > So shouldn't we do this first and bail/'not run' this test if that
> > > > > is the case?
> > > > >
> > > > > I really don't think there is any point in testing the old XFS
> > > > > behavior because
> > > > > the FS_XFLAG_DAX had no effect. So even if it is broken it does not
> > > > > matter.
> > > > > Or perhaps I am missing something here?
> > > > This test is designed to verify the inheritance behavior of
> > > > FS_XFLAG_DAX(not related to S_DAX)
> > > > so I think it is fine for both old dax and new dax to run the test.
> > > >
> > > > Best Regards,
> > > > Xiao Yang
> > > > > Ira
> > > > >
> > > > > > +
> > > > > > + # Mount with new dax options if fs supports them.
> > > > > > + if check_dax_mountopt "dax=always"; then
> > > > > > + for dax_option in "dax=always" "dax=inode" "dax=never"; do
> > > > > > + do_xflag_tests "-o $dax_option"
> > > > > > + done
> > > > > > + fi
> > > > > > +}
> > > > > > +
> > > > > > +_scratch_mkfs>> $seqres.full 2>&1
> > > > > > +
> > > > > > +do_tests
> > > > > > +
> > > > > > +# success, all done
> > > > > > +echo "Silence is golden"
> > > > > > +status=0
> > > > > > +exit
> > > > > > diff --git a/tests/generic/605.out b/tests/generic/605.out
> > > > > > new file mode 100644
> > > > > > index 00000000..1ae20049
> > > > > > --- /dev/null
> > > > > > +++ b/tests/generic/605.out
> > > > > > @@ -0,0 +1,2 @@
> > > > > > +QA output created by 605
> > > > > > +Silence is golden
> > > > > > diff --git a/tests/generic/group b/tests/generic/group
> > > > > > index 676e0d2e..a8451862 100644
> > > > > > --- a/tests/generic/group
> > > > > > +++ b/tests/generic/group
> > > > > > @@ -607,3 +607,4 @@
> > > > > > 602 auto quick encrypt
> > > > > > 603 auto attr quick dax
> > > > > > 604 auto attr quick dax
> > > > > > +605 auto attr quick dax
> > > > > > --
> > > > > > 2.21.0
> > > > > >
> > > > > >
> > > > > >
> > > > > .
> > > > >
> > > >
> > > >
> > > > .
> > > >
> > >
> > >
> >
> > .
> >
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly
2020-07-15 16:07 ` Darrick J. Wong
@ 2020-07-16 1:36 ` Xiao Yang
0 siblings, 0 replies; 29+ messages in thread
From: Xiao Yang @ 2020-07-16 1:36 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Ira Weiny, fstests
于 2020/7/16 0:07, Darrick J. Wong 写道:
> On Wed, Jul 15, 2020 at 11:12:36AM +0800, Xiao Yang wrote:
>> On 2020/7/15 10:31, Ira Weiny wrote:
>>> On Tue, Jul 14, 2020 at 05:40:05PM +0800, Xiao Yang wrote:
>>>> ext4 can accept the last one if the same mkfs options are passed but xfs cannot
>>> I'm having trouble parsing this commit message. What does 'last one' refer to?
>>>
>>>> accept the same mkfs options and reports "xxx option is respecified" error.
>>> Ok I think I understand now. Some FS's (XFS) do not accept an option more than
>>> once. So we can't just blindly add options to the end of the MKFS_OPTIONS. Is
>>> that correct?
>> Hi Ira,
>>
>> Correct.
>>>> I
>>>> prefer to override the same mkfs option which is defined in MKFS_OPTION so that
>>>> we can have a chance to pass other mkfs options to _scratch_mkfs_geom().
>>> Instead the patch parses the current option string and replaces the value if
>>> the option is already there. This allows us to specify MKFS_OPTIONS to
>>> generic/223.
>> Right. :-)
>>
>> XFS_MKFS_OPTIONS/MKFS_OPTIONS can be used to specify some custom options by user,
>> so I don't want to clear it blindly.
>>
>> Thanks,
>> Xiao Yang
>>> I think the code is reasonable although my sed skills are not good enough to
>>> tell for sure... ;-)
>>>
>>> Ira
>>>
>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>> ---
>>>> common/rc | 14 +++++++++++++-
>>>> tests/generic/223 | 1 -
>>>> 2 files changed, 13 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/common/rc b/common/rc
>>>> index 6c908f2e..567cf83b 100644
>>>> --- a/common/rc
>>>> +++ b/common/rc
>>>> @@ -1051,7 +1051,19 @@ _scratch_mkfs_geom()
>>>>
>>>> case $FSTYP in
>>>> xfs)
>>>> - MKFS_OPTIONS+=" -b size=$blocksize, -d su=$sunit_bytes,sw=$swidth_mult"
>>>> + if echo "$MKFS_OPTIONS" | egrep -q "b?size="; then
>>>> + MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r "s/(b?size=)[0-9]+/\1$blocksize/")
>>>> + else
>>>> + MKFS_OPTIONS+=" -b size=$blocksize"
>>>> + fi
>>>> +
>>>> + if echo "$MKFS_OPTIONS" | egrep -q "(su|sunit|sw|swidth)="; then
>>>> + MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r \
>>>> + -e "s/(su|sunit)=[0-9kmg]+/su=$sunit_bytes/" \
>>>> + -e "s/(sw|swidth)=[0-9kmg]+/sw=$swidth_mult/")
>>>> + else
>>>> + MKFS_OPTIONS+=" -d su=$sunit_bytes,sw=$swidth_mult"
>>>> + fi
> ...ok, I see, this makes the function smart enough to substitute
> geometry parameters instead of dumping them on the end and letting that
> blow up. Heh, ok, that's definitely a weird quirk I've noticed.
>
>>>> ;;
>>>> ext4|ext4dev)
>>>> MKFS_OPTIONS+=" -b $blocksize -E stride=$sunit_blocks,stripe_width=$swidth_blocks"
>>>> diff --git a/tests/generic/223 b/tests/generic/223
>>>> index 6cfd00dd..ba7c9a44 100755
>>>> --- a/tests/generic/223
>>>> +++ b/tests/generic/223
>>>> @@ -41,7 +41,6 @@ for SUNIT_K in 8 16 32 64 128; do
>>>> let SUNIT_BLOCKS=$SUNIT_BYTES/$BLOCKSIZE
>>>>
>>>> echo "=== mkfs with su $SUNIT_BLOCKS blocks x 4 ==="
>>>> - export MKFS_OPTIONS=""
> So I guess you're deleting this so that the test runs with whatever
> MKFS_OPTIONS the test runner specified, while letting the test edit
> blocksize and stripe parameters?
Hi Darrick,
Right :-) . My change wants to achieve it.
Thanks,
Xiao Yang
> Proving I'm still bad at remembering to read commit messages,
> Reviewed-by: Darrick J. Wong<darrick.wong@oracle.com>
>
> --D
>
>>>> _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE>> $seqres.full 2>&1
>>>> _scratch_mount
>>>>
>>>> --
>>>> 2.21.0
>>>>
>>>>
>>>>
>>> .
>>>
>>
>>
>
> .
>
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2020-07-16 1:36 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 9:40 [PATCH v6 0/7] Make fstests support new behavior of DAX Xiao Yang
2020-07-14 9:40 ` [PATCH v6 1/7] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX Xiao Yang
2020-07-15 1:59 ` Ira Weiny
2020-07-15 3:19 ` Xiao Yang
2020-07-15 4:15 ` Ira Weiny
2020-07-15 5:55 ` Xiao Yang
2020-07-15 15:56 ` Darrick J. Wong
2020-07-15 18:00 ` Ira Weiny
2020-07-14 9:40 ` [PATCH v6 2/7] fstests: Use _require_scratch_dax_mountopt() and _require_dax_iflag() Xiao Yang
2020-07-15 16:08 ` Darrick J. Wong
2020-07-14 9:40 ` [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly Xiao Yang
2020-07-15 2:31 ` Ira Weiny
2020-07-15 3:12 ` Xiao Yang
2020-07-15 16:07 ` Darrick J. Wong
2020-07-16 1:36 ` Xiao Yang
2020-07-14 9:40 ` [PATCH v6 4/7] generic/413, xfs/260: Improve format operation for PMD fault testing Xiao Yang
2020-07-15 16:09 ` Darrick J. Wong
2020-07-14 9:40 ` [PATCH v6 5/7] xfs/260: Move and update xfs/260 Xiao Yang
2020-07-15 16:10 ` Darrick J. Wong
2020-07-14 9:40 ` [PATCH v6 6/7] generic: Verify if statx() can qurey S_DAX flag on regular file correctly Xiao Yang
2020-07-14 9:40 ` [PATCH v6 7/7] generic: Verify the inheritance behavior of FS_XFLAG_DAX flag in various combinations Xiao Yang
2020-07-15 2:48 ` Ira Weiny
2020-07-15 5:39 ` Xiao Yang
2020-07-15 8:10 ` Xiao Yang
2020-07-15 16:43 ` Xiao Yang
2020-07-15 9:44 ` Xiao Yang
2020-07-15 16:19 ` Darrick J. Wong
2020-07-15 16:33 ` Xiao Yang
2020-07-15 18:18 ` 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).