* [PATCH 1/3] common: helper to get value from ndctl list by key @ 2017-04-07 7:56 Xiong Zhou 2017-04-07 7:56 ` [PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou ` (2 more replies) 0 siblings, 3 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-07 7:56 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, Xiong Zhou For some nvdimm DAX related tests, it's better to know some details of devices being tested. Adding new prog ndctl to manage nvdimms and jq to parse outputs. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- common/config | 2 ++ common/rc | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/common/config b/common/config index 59041a3..dfdcb8e 100644 --- a/common/config +++ b/common/config @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" export FLOCK_PROG="`set_prog_path flock`" export LDD_PROG="`set_prog_path ldd`" export TIMEOUT_PROG="`set_prog_path timeout`" +export NDCTL_PROG="`set_prog_path ndctl`" +export JQ_PROG="`set_prog_path jq`" # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. # newer systems have udevadm command but older systems like RHEL5 don't. diff --git a/common/rc b/common/rc index e1ab2c6..0540fdc 100644 --- a/common/rc +++ b/common/rc @@ -3148,6 +3148,30 @@ _require_chattr() rm -f $TEST_DIR/syscalltest.out } +_require_ndctl() +{ + ndctl list >> $seqres.full || \ + _notrun "this test requires ndctl" +} + +_require_jq() +{ + jq -V >> $seqres.full || \ + _notrun "this test requires jq" +} + +# $1 SCRATCH_DEV or TEST_DEV or other dev +# $2 key +_ndctl_get_pmem_key_value() +{ + _require_jq + _require_ndctl + + $NDCTL_PROG list | \ + $JQ_PROG -r ".[] | \ + select(.blockdev == \"${1#/dev/}\") | .$2" +} + _get_total_inode() { if [ -z "$1" ]; then -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* [PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported 2017-04-07 7:56 [PATCH 1/3] common: helper to get value from ndctl list by key Xiong Zhou @ 2017-04-07 7:56 ` Xiong Zhou 2017-04-12 3:30 ` Ross Zwisler 2017-04-07 7:56 ` [PATCH 3/3] DAX: mmap write readonly file Xiong Zhou 2017-04-07 17:16 ` [PATCH 1/3] common: helper to get value from ndctl list by key Ross Zwisler 2 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-07 7:56 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, Xiong Zhou By tweaking SCRATCH_DEV by ndctl, only run DAX mapped area DIO to non-DAX area tests when pmem device is in "memory mode". Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/generic/413 | 25 ++++++++++++++++++++++++- tests/xfs/260 | 25 ++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/tests/generic/413 b/tests/generic/413 index a1cc514..55a871f 100755 --- a/tests/generic/413 +++ b/tests/generic/413 @@ -34,6 +34,11 @@ _cleanup() { cd / rm -f $tmp.* + _check_scratch_fs + # umount and restore SCRATCH_DEV to original mode + _scratch_unmount + $NDCTL_PROG create-namespace -f -e $scns -m $scmode \ + > /dev/null 2>&1 } # get standard environment, filters and checks @@ -47,9 +52,13 @@ _supported_fs generic _supported_os Linux _require_test _require_scratch_dax +# fsck manually in cleanup as we will reconfig SCRATCH_DEV +_require_scratch_nocheck _require_test_program "feature" _require_test_program "t_mmap_dio" _require_xfs_io_command "falloc" +_require_ndctl +_require_jq prep_files() { @@ -110,7 +119,7 @@ t_both_nondax() t_mmap_dio_dax() { t_both_dax $1 - t_dax_to_nondax $1 + [ $skipdmd -eq 0 ] && t_dax_to_nondax $1 t_nondax_to_dax $1 t_both_nondax $1 } @@ -126,6 +135,20 @@ do_tests() t_mmap_dio_dax $((64 * 1024 * 1024)) } +skipdmd=0 +# config SCRATCH_DEV to memory mode to support DAX mapped +# area DIO to non-DAX area. + +# save original mode +scmode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) + +# get its namespace +scns=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) + +# skip dax to non-dax dio if config fails +$NDCTL_PROG create-namespace -f -e $scns -m memory \ + > /dev/null 2>&1 || skipdmd=1 + # make fs 2Mb aligned for PMD fault testing mkfs_opts="" if [ "$FSTYP" == "ext4" ]; then diff --git a/tests/xfs/260 b/tests/xfs/260 index e613cc0..d07b273 100755 --- a/tests/xfs/260 +++ b/tests/xfs/260 @@ -34,6 +34,11 @@ _cleanup() { cd / rm -f $tmp.* + _check_scratch_fs + # umount and restore SCRATCH_DEV to original mode + _scratch_unmount + $NDCTL_PROG create-namespace -f -e $scns -m $scmode \ + > /dev/null 2>&1 } # get standard environment, filters and checks @@ -46,10 +51,14 @@ rm -f $seqres.full _supported_fs xfs _supported_os Linux _require_scratch_dax +# fsck manually in cleanup as we will reconfig SCRATCH_DEV +_require_scratch_nocheck _require_test_program "feature" _require_test_program "t_mmap_dio" _require_xfs_io_command "chattr" "+/-x" _require_xfs_io_command "falloc" +_require_ndctl +_require_jq prep_files() { @@ -120,7 +129,7 @@ t_both_nondax() t_dax_flag_mmap_dio() { t_both_dax $1 - t_dax_to_nondax $1 + [ $skipdmd -eq 0 ] && t_dax_to_nondax $1 t_nondax_to_dax $1 t_both_nondax $1 } @@ -136,6 +145,20 @@ do_tests() t_dax_flag_mmap_dio $((64 * 1024 * 1024)) } +skipdmd=0 +# config SCRATCH_DEV to memory mode to support DAX mapped +# area DIO to non-DAX area. + +# save original mode +scmode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) + +# get its namespace +scns=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) + +# skip dax to non-dax dio if config fails +$NDCTL_PROG create-namespace -f -e $scns -m memory \ + > /dev/null 2>&1 || skipdmd=1 + # make xfs 2Mb aligned for PMD fault testing _scratch_mkfs "-d su=2m,sw=1" > /dev/null 2>&1 -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported 2017-04-07 7:56 ` [PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou @ 2017-04-12 3:30 ` Ross Zwisler 0 siblings, 0 replies; 71+ messages in thread From: Ross Zwisler @ 2017-04-12 3:30 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer On Fri, Apr 07, 2017 at 03:56:03PM +0800, Xiong Zhou wrote: > By tweaking SCRATCH_DEV by ndctl, only run DAX mapped > area DIO to non-DAX area tests when pmem device is in > "memory mode". > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > tests/generic/413 | 25 ++++++++++++++++++++++++- > tests/xfs/260 | 25 ++++++++++++++++++++++++- > 2 files changed, 48 insertions(+), 2 deletions(-) > > diff --git a/tests/generic/413 b/tests/generic/413 > index a1cc514..55a871f 100755 > --- a/tests/generic/413 > +++ b/tests/generic/413 > @@ -34,6 +34,11 @@ _cleanup() > { > cd / > rm -f $tmp.* > + _check_scratch_fs > + # umount and restore SCRATCH_DEV to original mode > + _scratch_unmount > + $NDCTL_PROG create-namespace -f -e $scns -m $scmode \ > + > /dev/null 2>&1 If we are running on a system where the PMEM namespace was created with the memmap kernel command line parameter (which is how I usually test), we don't want to do any reconfig (our namespaces are already in "memory" mode), and indeed the NDCTL and JSON commands will fail. Really we can even run the test without requiring these guys to be there. We need to detect this situation, and only do NDCTL & JSON commands if we're running against PMEM that came from an NFIT. Perhaps it's as easy as just grepping for "memmap" in /proc/cmdline? IMO we don't need to worry too much about people running with both an memmap command line *and* a valid NFIT... This comment applies to patches 2 and 3 in this series. ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH 3/3] DAX: mmap write readonly file 2017-04-07 7:56 [PATCH 1/3] common: helper to get value from ndctl list by key Xiong Zhou 2017-04-07 7:56 ` [PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou @ 2017-04-07 7:56 ` Xiong Zhou 2017-04-07 17:16 ` [PATCH 1/3] common: helper to get value from ndctl list by key Ross Zwisler 2 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-07 7:56 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, Xiong Zhou Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- .gitignore | 1 + src/Makefile | 2 +- src/t_mmap_write_ro.c | 55 +++++++++++++++++++++++++++++ tests/generic/422 | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/422.out | 3 ++ tests/generic/group | 1 + 6 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/422 create mode 100644 tests/generic/422.out diff --git a/.gitignore b/.gitignore index 1ed2a92..ee9329f 100644 --- a/.gitignore +++ b/.gitignore @@ -131,6 +131,7 @@ /src/renameat2 /src/t_rename_overwrite /src/t_mmap_dio +/src/t_mmap_write_ro # dmapi/ binaries /dmapi/src/common/cmd/read_invis diff --git a/src/Makefile b/src/Makefile index a7f27f0..2604f52 100644 --- a/src/Makefile +++ b/src/Makefile @@ -12,7 +12,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ godown resvtest writemod makeextents itrash rename \ multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ - holetest t_truncate_self t_mmap_dio + holetest t_truncate_self t_mmap_dio t_mmap_write_ro LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c new file mode 100644 index 0000000..cd13f00 --- /dev/null +++ b/src/t_mmap_write_ro.c @@ -0,0 +1,55 @@ +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <fcntl.h> +#include <unistd.h> +#include <libgen.h> +#include <sys/mman.h> + +int +main(int argc, char **argv) +{ + int fd, pfd, ret; + char *buf, foo; + int pagesize = getpagesize(); + + if (argc < 2) { + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); + exit(0); + } + + fd = open(argv[1], O_RDONLY|O_DIRECT); + if (fd < 0) { + perror("open"); + exit(1); + } + + pfd = open(argv[2], O_RDONLY); + if (pfd < 0) { + perror("pmem open"); + exit(1); + } + + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); + if (buf == MAP_FAILED) { + perror("mmap"); + exit(1); + } + + /* fault in the page */ + foo = *buf; + + ret = read(fd, buf, pagesize); + if (ret != pagesize) { + perror("read"); + exit(1); + } + + ret = msync(buf, pagesize, MS_SYNC); + if (ret != 0) { + perror("msync"); + exit(1); + } + + exit(0); +} diff --git a/tests/generic/422 b/tests/generic/422 new file mode 100755 index 0000000..8c75f10 --- /dev/null +++ b/tests/generic/422 @@ -0,0 +1,98 @@ +#! /bin/bash +# FS QA Test 422 +# +# This is a regression test for kernel commit +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings +# created by Jeffrey Moyer <jmoyer@redhat.com> +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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 -rf $tmp + _check_scratch_fs + # umount and restore SCRATCH_DEV to original mode + _scratch_unmount + $NDCTL_PROG create-namespace -f -e $scns -m $scmode \ + > /dev/null 2>&1 +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# Modify as appropriate. +_supported_fs generic +_supported_os Linux +_require_scratch_dax +# We need reconfig SCRATCH_DEV in cleanup, which will +# corrupt the fs on it. fsck manually in cleanup. +_require_scratch_nocheck +_require_ndctl +_require_jq +_require_test_program "t_mmap_write_ro" +_require_xfs_io_command "w" +_require_user + +# real QA test starts here +mkdir -p $tmp +chown $qa_user $tmp + +# config SCRATCH_DEV to memory mode. + +# save original mode +scmode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) + +# get its namespace +scns=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) + +# notrun if config fails. +$NDCTL_PROG create-namespace -f -e $scns -m memory \ + > /dev/null 2>&1 || _notrun "config $SCRATCH_DEV fail" + +_scratch_mkfs >>$seqres.full 2>&1 +_scratch_mount "-o dax" + +seq -w 0 1023 > $tmp/input 2>&1 +$XFS_IO_PROG -f -c "w -i $tmp/input 0 4096" \ + $SCRATCH_MNT/rofile >> $seqres.full 2>&1 +chmod 0644 $SCRATCH_MNT/rofile + +_user_do "seq -w 9000 100000 > $tmp/lf 2>&1" +# read/write should fail. +_user_do "src/t_mmap_write_ro $tmp/lf $SCRATCH_MNT/rofile" +# read-only file should not get updated. +head -1 $SCRATCH_MNT/rofile + +# success, all done +status=0 +exit diff --git a/tests/generic/422.out b/tests/generic/422.out new file mode 100644 index 0000000..68c6db4 --- /dev/null +++ b/tests/generic/422.out @@ -0,0 +1,3 @@ +QA output created by 422 +read: Bad address +0000 diff --git a/tests/generic/group b/tests/generic/group index 3c7c5e4..d747385 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -424,3 +424,4 @@ 419 auto quick encrypt 420 auto quick punch 421 auto quick encrypt dangerous +422 auto quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH 1/3] common: helper to get value from ndctl list by key 2017-04-07 7:56 [PATCH 1/3] common: helper to get value from ndctl list by key Xiong Zhou 2017-04-07 7:56 ` [PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou 2017-04-07 7:56 ` [PATCH 3/3] DAX: mmap write readonly file Xiong Zhou @ 2017-04-07 17:16 ` Ross Zwisler 2017-04-10 6:05 ` [PATCH v2 " Xiong Zhou 2 siblings, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-04-07 17:16 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer On Fri, Apr 07, 2017 at 03:56:02PM +0800, Xiong Zhou wrote: > For some nvdimm DAX related tests, it's better to know > some details of devices being tested. Adding new prog > ndctl to manage nvdimms and jq to parse outputs. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > common/config | 2 ++ > common/rc | 24 ++++++++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/common/config b/common/config > index 59041a3..dfdcb8e 100644 > --- a/common/config > +++ b/common/config > @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" > export FLOCK_PROG="`set_prog_path flock`" > export LDD_PROG="`set_prog_path ldd`" > export TIMEOUT_PROG="`set_prog_path timeout`" > +export NDCTL_PROG="`set_prog_path ndctl`" > +export JQ_PROG="`set_prog_path jq`" > > # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. > # newer systems have udevadm command but older systems like RHEL5 don't. > diff --git a/common/rc b/common/rc > index e1ab2c6..0540fdc 100644 > --- a/common/rc > +++ b/common/rc > @@ -3148,6 +3148,30 @@ _require_chattr() > rm -f $TEST_DIR/syscalltest.out > } > > +_require_ndctl() > +{ > + ndctl list >> $seqres.full || \ Should this use $NDCTL_PROG instead of running ndctl directly? Ditto with jq in _require_jq(). > + _notrun "this test requires ndctl" > +} > + > +_require_jq() > +{ > + jq -V >> $seqres.full || \ > + _notrun "this test requires jq" > +} > + > +# $1 SCRATCH_DEV or TEST_DEV or other dev > +# $2 key > +_ndctl_get_pmem_key_value() > +{ > + _require_jq > + _require_ndctl > + > + $NDCTL_PROG list | \ > + $JQ_PROG -r ".[] | \ > + select(.blockdev == \"${1#/dev/}\") | .$2" > +} > + > _get_total_inode() > { > if [ -z "$1" ]; then > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v2 1/3] common: helper to get value from ndctl list by key 2017-04-07 17:16 ` [PATCH 1/3] common: helper to get value from ndctl list by key Ross Zwisler @ 2017-04-10 6:05 ` Xiong Zhou 2017-04-10 6:05 ` [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou ` (2 more replies) 0 siblings, 3 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-10 6:05 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou For some nvdimm DAX related tests, it's better to know some details of devices being tested. Adding new prog ndctl to manage nvdimms and jq to parse outputs. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- v2: use _require_command instead of running cmds common/config | 2 ++ common/rc | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/common/config b/common/config index 59041a3..dfdcb8e 100644 --- a/common/config +++ b/common/config @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" export FLOCK_PROG="`set_prog_path flock`" export LDD_PROG="`set_prog_path ldd`" export TIMEOUT_PROG="`set_prog_path timeout`" +export NDCTL_PROG="`set_prog_path ndctl`" +export JQ_PROG="`set_prog_path jq`" # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. # newer systems have udevadm command but older systems like RHEL5 don't. diff --git a/common/rc b/common/rc index e1ab2c6..bc387cf 100644 --- a/common/rc +++ b/common/rc @@ -3148,6 +3148,25 @@ _require_chattr() rm -f $TEST_DIR/syscalltest.out } +_require_ndctl() +{ + _require_command "$NDCTL_PROG" ndctl +} + +_require_jq() +{ + _require_command "$JQ_PROG" jq +} + +# $1 SCRATCH_DEV or TEST_DEV or other dev +# $2 key +_ndctl_get_pmem_key_value() +{ + $NDCTL_PROG list | \ + $JQ_PROG -r ".[] | \ + select(.blockdev == \"${1#/dev/}\") | .$2" +} + _get_total_inode() { if [ -z "$1" ]; then -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported 2017-04-10 6:05 ` [PATCH v2 " Xiong Zhou @ 2017-04-10 6:05 ` Xiong Zhou 2017-04-11 11:44 ` Eryu Guan 2017-04-10 6:05 ` [PATCH v2 3/3] DAX: mmap write readonly file Xiong Zhou 2017-04-11 11:40 ` [PATCH v2 1/3] common: helper to get value from ndctl list by key Eryu Guan 2 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-10 6:05 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou By tweaking SCRATCH_DEV by ndctl, only run DAX mapped area DIO to non-DAX area tests when pmem device is in "memory mode". Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- v2: umount/check scratch fs after test restore SCRATCH_DEV mode in cleanup fix variable names tests/generic/413 | 26 +++++++++++++++++++++++++- tests/xfs/260 | 31 ++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/tests/generic/413 b/tests/generic/413 index a1cc514..fbe8f11 100755 --- a/tests/generic/413 +++ b/tests/generic/413 @@ -34,6 +34,9 @@ _cleanup() { cd / rm -f $tmp.* + # restore SCRATCH_DEV to original mode + $NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ + -m $scratch_dev_mode > /dev/null 2>&1 } # get standard environment, filters and checks @@ -47,9 +50,13 @@ _supported_fs generic _supported_os Linux _require_test _require_scratch_dax +# fsck manually after tests as we will reconfig SCRATCH_DEV +_require_scratch_nocheck _require_test_program "feature" _require_test_program "t_mmap_dio" _require_xfs_io_command "falloc" +_require_ndctl +_require_jq prep_files() { @@ -110,7 +117,7 @@ t_both_nondax() t_mmap_dio_dax() { t_both_dax $1 - t_dax_to_nondax $1 + [ $skip_dax_to_nondax -eq 0 ] && t_dax_to_nondax $1 t_nondax_to_dax $1 t_both_nondax $1 } @@ -126,6 +133,21 @@ do_tests() t_mmap_dio_dax $((64 * 1024 * 1024)) } +skip_dax_to_nondax=0 +# config SCRATCH_DEV to memory mode to support DAX mapped +# area DIO to non-DAX area. + +# save original mode +scratch_dev_mode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) + +# get its namespace +scratch_dev_namespace=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) + +# skip dax to non-dax dio if config fails +$NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ + -m memory > /dev/null 2>&1 \ + || skip_dax_to_nondax=1 + # make fs 2Mb aligned for PMD fault testing mkfs_opts="" if [ "$FSTYP" == "ext4" ]; then @@ -146,6 +168,8 @@ _scratch_mount "-o dax" tsize=$((128 * 1024 * 1024)) do_tests +_scratch_unmount +_check_scratch_fs # success, all done echo "Silence is golden" diff --git a/tests/xfs/260 b/tests/xfs/260 index e613cc0..4e445d3 100755 --- a/tests/xfs/260 +++ b/tests/xfs/260 @@ -34,6 +34,9 @@ _cleanup() { cd / rm -f $tmp.* + # restore SCRATCH_DEV to original mode + $NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ + -m $scratch_dev_mode > /dev/null 2>&1 } # get standard environment, filters and checks @@ -46,10 +49,14 @@ rm -f $seqres.full _supported_fs xfs _supported_os Linux _require_scratch_dax +# fsck manually after tests as we will reconfig SCRATCH_DEV +_require_scratch_nocheck _require_test_program "feature" _require_test_program "t_mmap_dio" _require_xfs_io_command "chattr" "+/-x" _require_xfs_io_command "falloc" +_require_ndctl +_require_jq prep_files() { @@ -120,7 +127,7 @@ t_both_nondax() t_dax_flag_mmap_dio() { t_both_dax $1 - t_dax_to_nondax $1 + [ $skip_dax_to_nondax -eq 0 ] && t_dax_to_nondax $1 t_nondax_to_dax $1 t_both_nondax $1 } @@ -136,6 +143,21 @@ do_tests() t_dax_flag_mmap_dio $((64 * 1024 * 1024)) } +skip_dax_to_nondax=0 +# config SCRATCH_DEV to memory mode to support DAX mapped +# area DIO to non-DAX area. + +# save original mode +scratch_dev_mode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) + +# get its namespace +scratch_dev_namespace=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) + +# skip dax to non-dax dio if config fails +$NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ + -m memory > /dev/null 2>&1 \ + || skip_dax_to_nondax=1 + # make xfs 2Mb aligned for PMD fault testing _scratch_mkfs "-d su=2m,sw=1" > /dev/null 2>&1 @@ -145,12 +167,19 @@ _scratch_mount "-o dax" tsize=$((128 * 1024 * 1024)) do_tests +# we need to reconfig SCRATCH_DEV to its original mode +# in cleanup, which will corrupt the FS. So fsck here +# before cleanup. _scratch_unmount +_check_scratch_fs # mount again without dax option export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" _scratch_mount do_tests +_scratch_unmount +_check_scratch_fs # success, all done echo "Silence is golden" -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported 2017-04-10 6:05 ` [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou @ 2017-04-11 11:44 ` Eryu Guan 2017-04-11 13:54 ` Jeff Moyer 2017-04-11 14:26 ` Xiong Zhou 0 siblings, 2 replies; 71+ messages in thread From: Eryu Guan @ 2017-04-11 11:44 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer On Mon, Apr 10, 2017 at 02:05:52PM +0800, Xiong Zhou wrote: > By tweaking SCRATCH_DEV by ndctl, only run DAX mapped > area DIO to non-DAX area tests when pmem device is in > "memory mode". > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > > v2: > umount/check scratch fs after test > restore SCRATCH_DEV mode in cleanup > fix variable names > > tests/generic/413 | 26 +++++++++++++++++++++++++- > tests/xfs/260 | 31 ++++++++++++++++++++++++++++++- > 2 files changed, 55 insertions(+), 2 deletions(-) > > diff --git a/tests/generic/413 b/tests/generic/413 > index a1cc514..fbe8f11 100755 > --- a/tests/generic/413 > +++ b/tests/generic/413 > @@ -34,6 +34,9 @@ _cleanup() > { > cd / > rm -f $tmp.* > + # restore SCRATCH_DEV to original mode > + $NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ > + -m $scratch_dev_mode > /dev/null 2>&1 > } > > # get standard environment, filters and checks > @@ -47,9 +50,13 @@ _supported_fs generic > _supported_os Linux > _require_test > _require_scratch_dax > +# fsck manually after tests as we will reconfig SCRATCH_DEV > +_require_scratch_nocheck > _require_test_program "feature" > _require_test_program "t_mmap_dio" > _require_xfs_io_command "falloc" > +_require_ndctl > +_require_jq > > prep_files() > { > @@ -110,7 +117,7 @@ t_both_nondax() > t_mmap_dio_dax() > { > t_both_dax $1 > - t_dax_to_nondax $1 > + [ $skip_dax_to_nondax -eq 0 ] && t_dax_to_nondax $1 > t_nondax_to_dax $1 > t_both_nondax $1 > } > @@ -126,6 +133,21 @@ do_tests() > t_mmap_dio_dax $((64 * 1024 * 1024)) > } > > +skip_dax_to_nondax=0 > +# config SCRATCH_DEV to memory mode to support DAX mapped > +# area DIO to non-DAX area. > + > +# save original mode > +scratch_dev_mode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) > + > +# get its namespace > +scratch_dev_namespace=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) > + > +# skip dax to non-dax dio if config fails > +$NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ > + -m memory > /dev/null 2>&1 \ > + || skip_dax_to_nondax=1 > + I'm not sure if it's proper to change the pmem device mode in tests. IMO, fstests just takes use of user specified TEST|SCRATCH_DEV and checks if they meet our requirements. So I think the correct way is just check the mode of pmem device and _notrun if the mode isn't compatible with this test. Thanks, Eryu ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported 2017-04-11 11:44 ` Eryu Guan @ 2017-04-11 13:54 ` Jeff Moyer 2017-04-11 14:26 ` Xiong Zhou 1 sibling, 0 replies; 71+ messages in thread From: Jeff Moyer @ 2017-04-11 13:54 UTC (permalink / raw) To: Eryu Guan; +Cc: Xiong Zhou, fstests, ross.zwisler, dan.j.williams Eryu Guan <eguan@redhat.com> writes: > On Mon, Apr 10, 2017 at 02:05:52PM +0800, Xiong Zhou wrote: >> By tweaking SCRATCH_DEV by ndctl, only run DAX mapped >> area DIO to non-DAX area tests when pmem device is in >> "memory mode". >> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> > I'm not sure if it's proper to change the pmem device mode in tests. > IMO, fstests just takes use of user specified TEST|SCRATCH_DEV and > checks if they meet our requirements. > > So I think the correct way is just check the mode of pmem device and > _notrun if the mode isn't compatible with this test. It's a scratch device, so I see no issue with changing the mode. If you don't allow this, then we'll need to run through the tests several times with different configurations just to get full coverage. -Jeff ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported 2017-04-11 11:44 ` Eryu Guan 2017-04-11 13:54 ` Jeff Moyer @ 2017-04-11 14:26 ` Xiong Zhou 1 sibling, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-11 14:26 UTC (permalink / raw) To: Eryu Guan; +Cc: Xiong Zhou, fstests, ross.zwisler, dan.j.williams, jmoyer On Tue, Apr 11, 2017 at 07:44:10PM +0800, Eryu Guan wrote: > On Mon, Apr 10, 2017 at 02:05:52PM +0800, Xiong Zhou wrote: > > By tweaking SCRATCH_DEV by ndctl, only run DAX mapped > > area DIO to non-DAX area tests when pmem device is in > > "memory mode". > > > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > --- > > > > v2: > > umount/check scratch fs after test > > restore SCRATCH_DEV mode in cleanup > > fix variable names > > > > tests/generic/413 | 26 +++++++++++++++++++++++++- > > tests/xfs/260 | 31 ++++++++++++++++++++++++++++++- > > 2 files changed, 55 insertions(+), 2 deletions(-) > > > > diff --git a/tests/generic/413 b/tests/generic/413 > > index a1cc514..fbe8f11 100755 > > --- a/tests/generic/413 > > +++ b/tests/generic/413 > > @@ -34,6 +34,9 @@ _cleanup() > > { > > cd / > > rm -f $tmp.* > > + # restore SCRATCH_DEV to original mode > > + $NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ > > + -m $scratch_dev_mode > /dev/null 2>&1 > > } > > > > # get standard environment, filters and checks > > @@ -47,9 +50,13 @@ _supported_fs generic > > _supported_os Linux > > _require_test > > _require_scratch_dax > > +# fsck manually after tests as we will reconfig SCRATCH_DEV > > +_require_scratch_nocheck > > _require_test_program "feature" > > _require_test_program "t_mmap_dio" > > _require_xfs_io_command "falloc" > > +_require_ndctl > > +_require_jq > > > > prep_files() > > { > > @@ -110,7 +117,7 @@ t_both_nondax() > > t_mmap_dio_dax() > > { > > t_both_dax $1 > > - t_dax_to_nondax $1 > > + [ $skip_dax_to_nondax -eq 0 ] && t_dax_to_nondax $1 > > t_nondax_to_dax $1 > > t_both_nondax $1 > > } > > @@ -126,6 +133,21 @@ do_tests() > > t_mmap_dio_dax $((64 * 1024 * 1024)) > > } > > > > +skip_dax_to_nondax=0 > > +# config SCRATCH_DEV to memory mode to support DAX mapped > > +# area DIO to non-DAX area. > > + > > +# save original mode > > +scratch_dev_mode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) > > + > > +# get its namespace > > +scratch_dev_namespace=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) > > + > > +# skip dax to non-dax dio if config fails > > +$NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ > > + -m memory > /dev/null 2>&1 \ > > + || skip_dax_to_nondax=1 > > + > > I'm not sure if it's proper to change the pmem device mode in tests. > IMO, fstests just takes use of user specified TEST|SCRATCH_DEV and > checks if they meet our requirements. > > So I think the correct way is just check the mode of pmem device and > _notrun if the mode isn't compatible with this test. I'm working on splitting this case into 2 cases. Running dax_to_nondax solely, _notrun if not compatible. Since only this sub case really need pmem device in specific mode. Thanks, Xiong > > Thanks, > Eryu ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v2 3/3] DAX: mmap write readonly file 2017-04-10 6:05 ` [PATCH v2 " Xiong Zhou 2017-04-10 6:05 ` [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou @ 2017-04-10 6:05 ` Xiong Zhou 2017-04-11 11:46 ` Eryu Guan 2017-04-12 4:03 ` Ross Zwisler 2017-04-11 11:40 ` [PATCH v2 1/3] common: helper to get value from ndctl list by key Eryu Guan 2 siblings, 2 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-10 6:05 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Regression case that one can write to read-only file in a DAX mountpoint. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- v2: compile test programme manually in this test because default cc option -O2 prevents this issue reproduction; use md5sum instead of seq numbers to check file consistence; umount/check scratch fs after test restore SCRATCH_DEV mode in cleanup fix variable names .gitignore | 1 + src/Makefile | 2 +- src/t_mmap_write_ro.c | 56 +++++++++++++++++++++++++ tests/generic/422 | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/422.out | 2 + tests/generic/group | 1 + 6 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/422 create mode 100644 tests/generic/422.out diff --git a/.gitignore b/.gitignore index 1ed2a92..ee9329f 100644 --- a/.gitignore +++ b/.gitignore @@ -131,6 +131,7 @@ /src/renameat2 /src/t_rename_overwrite /src/t_mmap_dio +/src/t_mmap_write_ro # dmapi/ binaries /dmapi/src/common/cmd/read_invis diff --git a/src/Makefile b/src/Makefile index a7f27f0..2604f52 100644 --- a/src/Makefile +++ b/src/Makefile @@ -12,7 +12,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ godown resvtest writemod makeextents itrash rename \ multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ - holetest t_truncate_self t_mmap_dio + holetest t_truncate_self t_mmap_dio t_mmap_write_ro LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c new file mode 100644 index 0000000..cce6e0d --- /dev/null +++ b/src/t_mmap_write_ro.c @@ -0,0 +1,56 @@ +#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <fcntl.h> +#include <unistd.h> +#include <libgen.h> +#include <sys/mman.h> + +int +main(int argc, char **argv) +{ + int fd, pfd, ret; + char *buf, foo; + int pagesize = getpagesize(); + + if (argc < 2) { + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); + exit(0); + } + + fd = open(argv[1], O_RDONLY|O_DIRECT); + if (fd < 0) { + perror("open"); + exit(1); + } + + pfd = open(argv[2], O_RDONLY); + if (pfd < 0) { + perror("pmem open"); + exit(1); + } + + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); + if (buf == MAP_FAILED) { + perror("mmap"); + exit(1); + } + + /* fault in the page */ + foo = *buf; + + ret = read(fd, buf, pagesize); + if (ret != pagesize) { + perror("read"); + exit(1); + } + + ret = msync(buf, pagesize, MS_SYNC); + if (ret != 0) { + perror("msync"); + exit(1); + } + + exit(0); +} diff --git a/tests/generic/422 b/tests/generic/422 new file mode 100755 index 0000000..93e10fe --- /dev/null +++ b/tests/generic/422 @@ -0,0 +1,111 @@ +#! /bin/bash +# FS QA Test 422 +# +# This is a regression test for kernel commit +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings +# created by Jeffrey Moyer <jmoyer@redhat.com> +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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 -rf $tmp.* + rm -f src/t_mmap_write_ro_no_optim + # restore SCRATCH_DEV to original mode + $NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ + -m $scratch_dev_mode > /dev/null 2>&1 +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# Modify as appropriate. +_supported_fs generic +_supported_os Linux +_require_scratch_dax +# We need reconfig SCRATCH_DEV in cleanup, which will +# corrupt the fs on it. fsck manually after tests. +_require_scratch_nocheck +_require_ndctl +_require_jq +_require_test_program "t_mmap_write_ro" +_require_user + +# real QA test starts here + +# config SCRATCH_DEV to memory mode. + +# save original mode +scratch_dev_mode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) + +# get its namespace +scratch_dev_namespace=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) + +# notrun if config fails. +$NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ + -m memory > /dev/null 2>&1 || \ + _notrun "config $SCRATCH_DEV fail" + +_scratch_mkfs >>$seqres.full 2>&1 +_scratch_mount "-o dax" + +# write a 4k read-only file, save its md5sum +$XFS_IO_PROG -f -c "pwrite -S 0xFFFF 0 4096" \ + $SCRATCH_MNT/readonlyfile >> $seqres.full 2>&1 +chmod 0644 $SCRATCH_MNT/readonlyfile +rofile_md5_1="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +# can't reproduce this issue if cc with -O2 option +cc src/t_mmap_write_ro.c -o src/t_mmap_write_ro_no_optim \ + >> $seqres.full 2>&1 + +# run test programme, read another larger file writing into +# the read-only file with mmap. +_user_do "$XFS_IO_PROG -f -c \"pwrite -S 0x0000 0 40960\" \ + ${tmp}.largefile > /dev/null 2>&1" + +# read/write should fail. +_user_do "src/t_mmap_write_ro_no_optim ${tmp}.largefile \ + $SCRATCH_MNT/readonlyfile" + +# read-only file should not get updated, md5sum again. +rofile_md5_2="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +[ $rofile_md5_1 != $rofile_md5_2 ] && echo "read only file changed" + +_scratch_unmount +_check_scratch_fs + +# success, all done +status=0 +exit diff --git a/tests/generic/422.out b/tests/generic/422.out new file mode 100644 index 0000000..dc5bca6 --- /dev/null +++ b/tests/generic/422.out @@ -0,0 +1,2 @@ +QA output created by 422 +read: Bad address diff --git a/tests/generic/group b/tests/generic/group index 3c7c5e4..d747385 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -424,3 +424,4 @@ 419 auto quick encrypt 420 auto quick punch 421 auto quick encrypt dangerous +422 auto quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v2 3/3] DAX: mmap write readonly file 2017-04-10 6:05 ` [PATCH v2 3/3] DAX: mmap write readonly file Xiong Zhou @ 2017-04-11 11:46 ` Eryu Guan 2017-04-11 13:56 ` Jeff Moyer 2017-04-12 4:03 ` Ross Zwisler 1 sibling, 1 reply; 71+ messages in thread From: Eryu Guan @ 2017-04-11 11:46 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer On Mon, Apr 10, 2017 at 02:05:53PM +0800, Xiong Zhou wrote: > Regression case that one can write to read-only > file in a DAX mountpoint. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > > v2: > compile test programme manually in this test because default > cc option -O2 prevents this issue reproduction; Hmm, this looks.. ugly to me :) Better to find out the exact reason that prevents the bug from reproducing and update the c program accordingly. Thanks, Eryu ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v2 3/3] DAX: mmap write readonly file 2017-04-11 11:46 ` Eryu Guan @ 2017-04-11 13:56 ` Jeff Moyer 2017-04-11 18:52 ` Ross Zwisler 0 siblings, 1 reply; 71+ messages in thread From: Jeff Moyer @ 2017-04-11 13:56 UTC (permalink / raw) To: Eryu Guan; +Cc: Xiong Zhou, fstests, ross.zwisler, dan.j.williams Eryu Guan <eguan@redhat.com> writes: > On Mon, Apr 10, 2017 at 02:05:53PM +0800, Xiong Zhou wrote: >> Regression case that one can write to read-only >> file in a DAX mountpoint. >> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> >> --- >> >> v2: >> compile test programme manually in this test because default >> cc option -O2 prevents this issue reproduction; > > Hmm, this looks.. ugly to me :) Better to find out the exact reason that > prevents the bug from reproducing and update the c program accordingly. The compiler probably optimizes this bit out: + /* fault in the page */ + foo = *buf; Cheers, Jeff ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v2 3/3] DAX: mmap write readonly file 2017-04-11 13:56 ` Jeff Moyer @ 2017-04-11 18:52 ` Ross Zwisler 2017-04-11 22:45 ` Ross Zwisler 0 siblings, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-04-11 18:52 UTC (permalink / raw) To: Jeff Moyer; +Cc: Eryu Guan, Xiong Zhou, fstests, ross.zwisler, dan.j.williams On Tue, Apr 11, 2017 at 09:56:14AM -0400, Jeff Moyer wrote: > Eryu Guan <eguan@redhat.com> writes: > > > On Mon, Apr 10, 2017 at 02:05:53PM +0800, Xiong Zhou wrote: > >> Regression case that one can write to read-only > >> file in a DAX mountpoint. > >> > >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> > >> --- > >> > >> v2: > >> compile test programme manually in this test because default > >> cc option -O2 prevents this issue reproduction; > > > > Hmm, this looks.. ugly to me :) Better to find out the exact reason that > > prevents the bug from reproducing and update the c program accordingly. > > The compiler probably optimizes this bit out: > > + /* fault in the page */ > + foo = *buf; Yep, verified this with objdump. You can prevent the compiler from optimizing out this bit by making 'foo' volatile. Patch at the end of this mail, and I also verified that this works with objdump. In my setup at least this test passes both with v4.10.0 (which does not contain the fix we are testing for, and should fail) and with v4.10.3 (which does contain the kernel fix, and should pass). So, I think the test still needs a little love. :) --- diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c index cce6e0d..3960815 100644 --- a/src/t_mmap_write_ro.c +++ b/src/t_mmap_write_ro.c @@ -11,7 +11,8 @@ int main(int argc, char **argv) { int fd, pfd, ret; - char *buf, foo; + char *buf; + volatile char foo; int pagesize = getpagesize(); if (argc < 2) { ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v2 3/3] DAX: mmap write readonly file 2017-04-11 18:52 ` Ross Zwisler @ 2017-04-11 22:45 ` Ross Zwisler 2017-04-12 2:52 ` Xiong Zhou 0 siblings, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-04-11 22:45 UTC (permalink / raw) To: Ross Zwisler; +Cc: Jeff Moyer, Eryu Guan, Xiong Zhou, fstests, dan.j.williams On Tue, Apr 11, 2017 at 12:52:35PM -0600, Ross Zwisler wrote: > On Tue, Apr 11, 2017 at 09:56:14AM -0400, Jeff Moyer wrote: > > Eryu Guan <eguan@redhat.com> writes: > > > > > On Mon, Apr 10, 2017 at 02:05:53PM +0800, Xiong Zhou wrote: > > >> Regression case that one can write to read-only > > >> file in a DAX mountpoint. > > >> > > >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > >> --- > > >> > > >> v2: > > >> compile test programme manually in this test because default > > >> cc option -O2 prevents this issue reproduction; > > > > > > Hmm, this looks.. ugly to me :) Better to find out the exact reason that > > > prevents the bug from reproducing and update the c program accordingly. > > > > The compiler probably optimizes this bit out: > > > > + /* fault in the page */ > > + foo = *buf; > > Yep, verified this with objdump. You can prevent the compiler from > optimizing out this bit by making 'foo' volatile. Patch at the end of this > mail, and I also verified that this works with objdump. > > In my setup at least this test passes both with v4.10.0 (which does not > contain the fix we are testing for, and should fail) and with v4.10.3 (which > does contain the kernel fix, and should pass). > > So, I think the test still needs a little love. :) Umm...never mind, my testing was broken. I was accidentally testing with the upstream generic/422. :( Will retest tomorrow. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v2 3/3] DAX: mmap write readonly file 2017-04-11 22:45 ` Ross Zwisler @ 2017-04-12 2:52 ` Xiong Zhou 0 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-12 2:52 UTC (permalink / raw) To: Ross Zwisler; +Cc: Jeff Moyer, Eryu Guan, Xiong Zhou, fstests, dan.j.williams On Tue, Apr 11, 2017 at 04:45:42PM -0600, Ross Zwisler wrote: > On Tue, Apr 11, 2017 at 12:52:35PM -0600, Ross Zwisler wrote: > > On Tue, Apr 11, 2017 at 09:56:14AM -0400, Jeff Moyer wrote: > > > Eryu Guan <eguan@redhat.com> writes: > > > > > > > On Mon, Apr 10, 2017 at 02:05:53PM +0800, Xiong Zhou wrote: > > > >> Regression case that one can write to read-only > > > >> file in a DAX mountpoint. > > > >> > > > >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > > >> --- > > > >> > > > >> v2: > > > >> compile test programme manually in this test because default > > > >> cc option -O2 prevents this issue reproduction; > > > > > > > > Hmm, this looks.. ugly to me :) Better to find out the exact reason that > > > > prevents the bug from reproducing and update the c program accordingly. > > > > > > The compiler probably optimizes this bit out: > > > > > > + /* fault in the page */ > > > + foo = *buf; > > > > Yep, verified this with objdump. You can prevent the compiler from > > optimizing out this bit by making 'foo' volatile. Patch at the end of this > > mail, and I also verified that this works with objdump. > > > > In my setup at least this test passes both with v4.10.0 (which does not > > contain the fix we are testing for, and should fail) and with v4.10.3 (which > > does contain the kernel fix, and should pass). > > > > So, I think the test still needs a little love. :) > > Umm...never mind, my testing was broken. I was accidentally testing with the > upstream generic/422. :( Will retest tomorrow. Thank you very much Ross, your patch is working fine. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v2 3/3] DAX: mmap write readonly file 2017-04-10 6:05 ` [PATCH v2 3/3] DAX: mmap write readonly file Xiong Zhou 2017-04-11 11:46 ` Eryu Guan @ 2017-04-12 4:03 ` Ross Zwisler 2017-04-12 6:26 ` Xiong Zhou 1 sibling, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-04-12 4:03 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Mon, Apr 10, 2017 at 02:05:53PM +0800, Xiong Zhou wrote: > Regression case that one can write to read-only > file in a DAX mountpoint. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > > v2: > compile test programme manually in this test because default > cc option -O2 prevents this issue reproduction; > use md5sum instead of seq numbers to check file consistence; > umount/check scratch fs after test > restore SCRATCH_DEV mode in cleanup > fix variable names > > .gitignore | 1 + > src/Makefile | 2 +- > src/t_mmap_write_ro.c | 56 +++++++++++++++++++++++++ > tests/generic/422 | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/422.out | 2 + > tests/generic/group | 1 + > 6 files changed, 172 insertions(+), 1 deletion(-) > create mode 100644 src/t_mmap_write_ro.c > create mode 100755 tests/generic/422 > create mode 100644 tests/generic/422.out > > diff --git a/.gitignore b/.gitignore > index 1ed2a92..ee9329f 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -131,6 +131,7 @@ > /src/renameat2 > /src/t_rename_overwrite > /src/t_mmap_dio > +/src/t_mmap_write_ro > > # dmapi/ binaries > /dmapi/src/common/cmd/read_invis > diff --git a/src/Makefile b/src/Makefile > index a7f27f0..2604f52 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -12,7 +12,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ > godown resvtest writemod makeextents itrash rename \ > multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ > t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ > - holetest t_truncate_self t_mmap_dio > + holetest t_truncate_self t_mmap_dio t_mmap_write_ro > > LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ > diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c > new file mode 100644 > index 0000000..cce6e0d > --- /dev/null > +++ b/src/t_mmap_write_ro.c > @@ -0,0 +1,56 @@ > +#define _GNU_SOURCE When you're compiling this with 'make' at the top level of xfstests, I think _GNU_SOURCE is always defined, so you get the following warning: t_mmap_write_ro.c:1:0: warning: "_GNU_SOURCE" redefined #define _GNU_SOURCE Now that we're on track to just use the normal compilation via using 'volatile' for 'foo', we can just drop this #define. > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <fcntl.h> > +#include <unistd.h> > +#include <libgen.h> > +#include <sys/mman.h> > + > +int > +main(int argc, char **argv) > +{ > + int fd, pfd, ret; > + char *buf, foo; > + int pagesize = getpagesize(); > + > + if (argc < 2) { > + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); > + exit(0); > + } > + > + fd = open(argv[1], O_RDONLY|O_DIRECT); > + if (fd < 0) { > + perror("open"); For me at least this open() fails because the file we are trying to open with O_DIRECT is on /tmp, which is tmpfs, and I don't think tmpfs supports O_DIRECT? Running against a copy of the same file that lives on XFS works fine: [ root@alara ~/xfstests ] # ./src/t_mmap_write_ro /tmp/30349.largefile ~/dax/data # no O_DIRECT on tmpfs open: Invalid argument [ root@alara ~/xfstests ] # ./src/t_mmap_write_ro ~/30349.largefile ~/dax/data # the read failure is expected read: Bad address [ root@alara ~/xfstests ] # ls -la ~/30349.largefile /tmp/30349.largefile -rwxrwxrwx. 1 fsgqa fsgqa 40960 Apr 11 21:09 /root/30349.largefile -rwxrwxrwx. 1 fsgqa fsgqa 40960 Apr 11 21:09 /tmp/30349.largefile The open() also succeeds if I get rid of the O_DIRECT argument. So, I think we need our O_DIRECT data file to be on something other than tmpfs? I don't know the limitations we are under in xfstests - we are currently using our scratch device as our memory mode pmem device, and mounting it with DAX. In my setup at least the test device also is often mounted with DAX... How can we get a non-DAX file that isn't on tmpfs that we can use for O_DIRECT? It only needs to be a page in size...can we throw it into the xfstests directory somewhere? Something else? > + exit(1); > + } > + > + pfd = open(argv[2], O_RDONLY); > + if (pfd < 0) { > + perror("pmem open"); > + exit(1); > + } > + > + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); > + if (buf == MAP_FAILED) { > + perror("mmap"); > + exit(1); > + } > + > + /* fault in the page */ > + foo = *buf; > + > + ret = read(fd, buf, pagesize); > + if (ret != pagesize) { > + perror("read"); > + exit(1); > + } > + > + ret = msync(buf, pagesize, MS_SYNC); > + if (ret != 0) { > + perror("msync"); > + exit(1); > + } This msync() isn't needed for the test, and we also get an unused variable warning for 'foo'. I'll throw a patch at the end of this mail which fixes these and adds some comments from my version of this test. > + > + exit(0); > +} > diff --git a/tests/generic/422 b/tests/generic/422 > new file mode 100755 > index 0000000..93e10fe > --- /dev/null > +++ b/tests/generic/422 > @@ -0,0 +1,111 @@ > +#! /bin/bash > +# FS QA Test 422 > +# > +# This is a regression test for kernel commit > +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings > +# created by Jeffrey Moyer <jmoyer@redhat.com> > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2017 Red Hat. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +#----------------------------------------------------------------------- > +# > + > +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 -rf $tmp.* > + rm -f src/t_mmap_write_ro_no_optim Yay, we can get rid of all the t_mmap_write_ro_no_optim stuff. > + # restore SCRATCH_DEV to original mode > + $NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ > + -m $scratch_dev_mode > /dev/null 2>&1 As with the other test we only want to require NDCTL & JSON and do anything with them if we are using PMEM from an NFIT. > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# Modify as appropriate. > +_supported_fs generic > +_supported_os Linux > +_require_scratch_dax > +# We need reconfig SCRATCH_DEV in cleanup, which will > +# corrupt the fs on it. fsck manually after tests. > +_require_scratch_nocheck > +_require_ndctl > +_require_jq > +_require_test_program "t_mmap_write_ro" > +_require_user > + > +# real QA test starts here > + > +# config SCRATCH_DEV to memory mode. > + > +# save original mode > +scratch_dev_mode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) > + > +# get its namespace > +scratch_dev_namespace=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) > + > +# notrun if config fails. > +$NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ > + -m memory > /dev/null 2>&1 || \ > + _notrun "config $SCRATCH_DEV fail" > + > +_scratch_mkfs >>$seqres.full 2>&1 > +_scratch_mount "-o dax" > + > +# write a 4k read-only file, save its md5sum > +$XFS_IO_PROG -f -c "pwrite -S 0xFFFF 0 4096" \ > + $SCRATCH_MNT/readonlyfile >> $seqres.full 2>&1 > +chmod 0644 $SCRATCH_MNT/readonlyfile > +rofile_md5_1="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" > + > +# can't reproduce this issue if cc with -O2 option > +cc src/t_mmap_write_ro.c -o src/t_mmap_write_ro_no_optim \ > + >> $seqres.full 2>&1 > + > +# run test programme, read another larger file writing into > +# the read-only file with mmap. > +_user_do "$XFS_IO_PROG -f -c \"pwrite -S 0x0000 0 40960\" \ > + ${tmp}.largefile > /dev/null 2>&1" > + > +# read/write should fail. > +_user_do "src/t_mmap_write_ro_no_optim ${tmp}.largefile \ > + $SCRATCH_MNT/readonlyfile" > + > +# read-only file should not get updated, md5sum again. > +rofile_md5_2="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" I don't think you need to do anything with the file's md5sum. We know whether we've passed or not based on whether the read() call (which was actually writing to the DAX mapping) succeeded or failed. > + > +[ $rofile_md5_1 != $rofile_md5_2 ] && echo "read only file changed" > + > +_scratch_unmount > +_check_scratch_fs > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/422.out b/tests/generic/422.out > new file mode 100644 > index 0000000..dc5bca6 > --- /dev/null > +++ b/tests/generic/422.out > @@ -0,0 +1,2 @@ > +QA output created by 422 > +read: Bad address > diff --git a/tests/generic/group b/tests/generic/group > index 3c7c5e4..d747385 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -424,3 +424,4 @@ > 419 auto quick encrypt > 420 auto quick punch > 421 auto quick encrypt dangerous > +422 auto quick > -- > 1.8.3.1 > And my patch to clean up a few things and add some comments: --- diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c index cce6e0d..61e6f25 100644 --- a/src/t_mmap_write_ro.c +++ b/src/t_mmap_write_ro.c @@ -1,4 +1,3 @@ -#define _GNU_SOURCE #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -11,7 +10,8 @@ int main(int argc, char **argv) { int fd, pfd, ret; - char *buf, foo; + char *buf; + volatile char foo; int pagesize = getpagesize(); if (argc < 2) { @@ -37,20 +37,24 @@ main(int argc, char **argv) exit(1); } - /* fault in the page */ + /* + * Read from the DAX mmap to populate the first page in the + * address_space with a with a read-only mapping. + */ foo = *buf; + /* quiet the compiler's "unused-but-set-variable" warning */ + foo; + + /* + * Now write to the DAX mmap. This *should* fail, but if the bug is + * present in __get_user_pages_fast(), it will succeed. + */ ret = read(fd, buf, pagesize); if (ret != pagesize) { perror("read"); exit(1); } - ret = msync(buf, pagesize, MS_SYNC); - if (ret != 0) { - perror("msync"); - exit(1); - } - exit(0); } ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v2 3/3] DAX: mmap write readonly file 2017-04-12 4:03 ` Ross Zwisler @ 2017-04-12 6:26 ` Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 0/4] split DAX mmap DIO cases Xiong Zhou 0 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-12 6:26 UTC (permalink / raw) To: Ross Zwisler; +Cc: Xiong Zhou, fstests, dan.j.williams, jmoyer, eguan On Tue, Apr 11, 2017 at 10:03:13PM -0600, Ross Zwisler wrote: > On Mon, Apr 10, 2017 at 02:05:53PM +0800, Xiong Zhou wrote: > > Regression case that one can write to read-only > > file in a DAX mountpoint. > > > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > --- > > > > v2: > > compile test programme manually in this test because default > > cc option -O2 prevents this issue reproduction; > > use md5sum instead of seq numbers to check file consistence; > > umount/check scratch fs after test > > restore SCRATCH_DEV mode in cleanup > > fix variable names > > > > .gitignore | 1 + > > src/Makefile | 2 +- > > src/t_mmap_write_ro.c | 56 +++++++++++++++++++++++++ > > tests/generic/422 | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/422.out | 2 + > > tests/generic/group | 1 + > > 6 files changed, 172 insertions(+), 1 deletion(-) > > create mode 100644 src/t_mmap_write_ro.c > > create mode 100755 tests/generic/422 > > create mode 100644 tests/generic/422.out > > > > diff --git a/.gitignore b/.gitignore > > index 1ed2a92..ee9329f 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -131,6 +131,7 @@ > > /src/renameat2 > > /src/t_rename_overwrite > > /src/t_mmap_dio > > +/src/t_mmap_write_ro > > > > # dmapi/ binaries > > /dmapi/src/common/cmd/read_invis > > diff --git a/src/Makefile b/src/Makefile > > index a7f27f0..2604f52 100644 > > --- a/src/Makefile > > +++ b/src/Makefile > > @@ -12,7 +12,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ > > godown resvtest writemod makeextents itrash rename \ > > multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ > > t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ > > - holetest t_truncate_self t_mmap_dio > > + holetest t_truncate_self t_mmap_dio t_mmap_write_ro > > > > LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > > preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ > > diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c > > new file mode 100644 > > index 0000000..cce6e0d > > --- /dev/null > > +++ b/src/t_mmap_write_ro.c > > @@ -0,0 +1,56 @@ > > +#define _GNU_SOURCE > > When you're compiling this with 'make' at the top level of xfstests, I think > _GNU_SOURCE is always defined, so you get the following warning: > > t_mmap_write_ro.c:1:0: warning: "_GNU_SOURCE" redefined > #define _GNU_SOURCE > > Now that we're on track to just use the normal compilation via using > 'volatile' for 'foo', we can just drop this #define. Okay. > > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <fcntl.h> > > +#include <unistd.h> > > +#include <libgen.h> > > +#include <sys/mman.h> > > + > > +int > > +main(int argc, char **argv) > > +{ > > + int fd, pfd, ret; > > + char *buf, foo; > > + int pagesize = getpagesize(); > > + > > + if (argc < 2) { > > + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); > > + exit(0); > > + } > > + > > + fd = open(argv[1], O_RDONLY|O_DIRECT); > > + if (fd < 0) { > > + perror("open"); > > For me at least this open() fails because the file we are trying to open with > O_DIRECT is on /tmp, which is tmpfs, and I don't think tmpfs supports > O_DIRECT? Running against a copy of the same file that lives on XFS works > fine: > > [ root@alara ~/xfstests ] > # ./src/t_mmap_write_ro /tmp/30349.largefile ~/dax/data # no O_DIRECT on tmpfs > open: Invalid argument > [ root@alara ~/xfstests ] > # ./src/t_mmap_write_ro ~/30349.largefile ~/dax/data # the read failure is expected > read: Bad address > [ root@alara ~/xfstests ] > # ls -la ~/30349.largefile /tmp/30349.largefile > -rwxrwxrwx. 1 fsgqa fsgqa 40960 Apr 11 21:09 /root/30349.largefile > -rwxrwxrwx. 1 fsgqa fsgqa 40960 Apr 11 21:09 /tmp/30349.largefile > > The open() also succeeds if I get rid of the O_DIRECT argument. > > So, I think we need our O_DIRECT data file to be on something other than > tmpfs? I don't know the limitations we are under in xfstests - we are > currently using our scratch device as our memory mode pmem device, and > mounting it with DAX. In my setup at least the test device also is often > mounted with DAX... > > How can we get a non-DAX file that isn't on tmpfs that we can use for > O_DIRECT? It only needs to be a page in size...can we throw it into the > xfstests directory somewhere? Something else? Remount TEST_DEV wo/ dax for this case, and throw it there. > > > + exit(1); > > + } > > + > > + pfd = open(argv[2], O_RDONLY); > > + if (pfd < 0) { > > + perror("pmem open"); > > + exit(1); > > + } > > + > > + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); > > + if (buf == MAP_FAILED) { > > + perror("mmap"); > > + exit(1); > > + } > > + > > + /* fault in the page */ > > + foo = *buf; > > + > > + ret = read(fd, buf, pagesize); > > + if (ret != pagesize) { > > + perror("read"); > > + exit(1); > > + } > > + > > + ret = msync(buf, pagesize, MS_SYNC); > > + if (ret != 0) { > > + perror("msync"); > > + exit(1); > > + } > > This msync() isn't needed for the test, and we also get an unused variable > warning for 'foo'. I'll throw a patch at the end of this mail which fixes > these and adds some comments from my version of this test. Aha, I'd prefer keep this msync, and add munmap and close. It doesn't hurt to test more. :) > > > + > > + exit(0); > > +} > > diff --git a/tests/generic/422 b/tests/generic/422 > > new file mode 100755 > > index 0000000..93e10fe > > --- /dev/null > > +++ b/tests/generic/422 > > @@ -0,0 +1,111 @@ > > +#! /bin/bash > > +# FS QA Test 422 > > +# > > +# This is a regression test for kernel commit > > +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings > > +# created by Jeffrey Moyer <jmoyer@redhat.com> > > +# > > +#----------------------------------------------------------------------- > > +# Copyright (c) 2017 Red Hat. All Rights Reserved. > > +# > > +# This program is free software; you can redistribute it and/or > > +# modify it under the terms of the GNU General Public License as > > +# published by the Free Software Foundation. > > +# > > +# This program is distributed in the hope that it would be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program; if not, write the Free Software Foundation, > > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > > +#----------------------------------------------------------------------- > > +# > > + > > +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 -rf $tmp.* > > + rm -f src/t_mmap_write_ro_no_optim > > Yay, we can get rid of all the t_mmap_write_ro_no_optim stuff. > > > + # restore SCRATCH_DEV to original mode > > + $NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ > > + -m $scratch_dev_mode > /dev/null 2>&1 > > As with the other test we only want to require NDCTL & JSON and do anything > with them if we are using PMEM from an NFIT. I agree with you. Will try. > > > +} > > + > > +# get standard environment, filters and checks > > +. ./common/rc > > +. ./common/filter > > + > > +# remove previous $seqres.full before test > > +rm -f $seqres.full > > + > > +# Modify as appropriate. > > +_supported_fs generic > > +_supported_os Linux > > +_require_scratch_dax > > +# We need reconfig SCRATCH_DEV in cleanup, which will > > +# corrupt the fs on it. fsck manually after tests. > > +_require_scratch_nocheck > > +_require_ndctl > > +_require_jq > > +_require_test_program "t_mmap_write_ro" > > +_require_user > > + > > +# real QA test starts here > > + > > +# config SCRATCH_DEV to memory mode. > > + > > +# save original mode > > +scratch_dev_mode=$(_ndctl_get_pmem_key_value $SCRATCH_DEV mode) > > + > > +# get its namespace > > +scratch_dev_namespace=$(_ndctl_get_pmem_key_value $SCRATCH_DEV dev) > > + > > +# notrun if config fails. > > +$NDCTL_PROG create-namespace -f -e $scratch_dev_namespace \ > > + -m memory > /dev/null 2>&1 || \ > > + _notrun "config $SCRATCH_DEV fail" > > + > > +_scratch_mkfs >>$seqres.full 2>&1 > > +_scratch_mount "-o dax" > > + > > +# write a 4k read-only file, save its md5sum > > +$XFS_IO_PROG -f -c "pwrite -S 0xFFFF 0 4096" \ > > + $SCRATCH_MNT/readonlyfile >> $seqres.full 2>&1 > > +chmod 0644 $SCRATCH_MNT/readonlyfile > > +rofile_md5_1="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" > > + > > +# can't reproduce this issue if cc with -O2 option > > +cc src/t_mmap_write_ro.c -o src/t_mmap_write_ro_no_optim \ > > + >> $seqres.full 2>&1 > > + > > +# run test programme, read another larger file writing into > > +# the read-only file with mmap. > > +_user_do "$XFS_IO_PROG -f -c \"pwrite -S 0x0000 0 40960\" \ > > + ${tmp}.largefile > /dev/null 2>&1" > > + > > +# read/write should fail. > > +_user_do "src/t_mmap_write_ro_no_optim ${tmp}.largefile \ > > + $SCRATCH_MNT/readonlyfile" > > + > > +# read-only file should not get updated, md5sum again. > > +rofile_md5_2="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" > > I don't think you need to do anything with the file's md5sum. We know whether > we've passed or not based on whether the read() call (which was actually > writing to the DAX mapping) succeeded or failed. For now, it is true. We can keep this check to win jackpot that read does fail but file gets changed. > > > + > > +[ $rofile_md5_1 != $rofile_md5_2 ] && echo "read only file changed" > > + > > +_scratch_unmount > > +_check_scratch_fs > > + > > +# success, all done > > +status=0 > > +exit > > diff --git a/tests/generic/422.out b/tests/generic/422.out > > new file mode 100644 > > index 0000000..dc5bca6 > > --- /dev/null > > +++ b/tests/generic/422.out > > @@ -0,0 +1,2 @@ > > +QA output created by 422 > > +read: Bad address > > diff --git a/tests/generic/group b/tests/generic/group > > index 3c7c5e4..d747385 100644 > > --- a/tests/generic/group > > +++ b/tests/generic/group > > @@ -424,3 +424,4 @@ > > 419 auto quick encrypt > > 420 auto quick punch > > 421 auto quick encrypt dangerous > > +422 auto quick > > -- > > 1.8.3.1 > > > > And my patch to clean up a few things and add some comments: > > --- > diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c > index cce6e0d..61e6f25 100644 > --- a/src/t_mmap_write_ro.c > +++ b/src/t_mmap_write_ro.c > @@ -1,4 +1,3 @@ > -#define _GNU_SOURCE > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > @@ -11,7 +10,8 @@ int > main(int argc, char **argv) > { > int fd, pfd, ret; > - char *buf, foo; > + char *buf; > + volatile char foo; > int pagesize = getpagesize(); > > if (argc < 2) { > @@ -37,20 +37,24 @@ main(int argc, char **argv) > exit(1); > } > > - /* fault in the page */ > + /* > + * Read from the DAX mmap to populate the first page in the > + * address_space with a with a read-only mapping. > + */ > foo = *buf; > > + /* quiet the compiler's "unused-but-set-variable" warning */ > + foo; > + > + /* > + * Now write to the DAX mmap. This *should* fail, but if the bug is > + * present in __get_user_pages_fast(), it will succeed. > + */ Thanks very much! -- Xiong > ret = read(fd, buf, pagesize); > if (ret != pagesize) { > perror("read"); > exit(1); > } > > - ret = msync(buf, pagesize, MS_SYNC); > - if (ret != 0) { > - perror("msync"); > - exit(1); > - } > - > exit(0); > } ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v3 0/4] split DAX mmap DIO cases 2017-04-12 6:26 ` Xiong Zhou @ 2017-04-12 14:46 ` Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou ` (4 more replies) 0 siblings, 5 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-12 14:46 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou In generic/413 and xfs/260, we run mmap DIO tests between DAX and non-DAX mountpoints: dax_to_dax, dax_to_nondax, nondax_to_dax, nondax_to_nondax. Now tests from DAX to non-DAX fails as: read(Bad address) len 1024 dio dax to nondax ... It is expected: https://lists.01.org/pipermail/linux-nvdimm/2017-February/008959.html So, this patchset run dax_to_nondax in separated cases, in which checking devices underneath, _notrun if not compatible. The checking helper may be ugly, though it stops confusing failures and tests as much as possible. Comments are welcome. The 4/4 write read only file test is not related to above changes, just sending it together. v3: split dax_to_nondax to separated cases; adding helper _require_pmem_key_value to require *_DEV in specific status; adding Ross's fix to t_mmap_write_ro.c; adding munmap and close to t_mmap_write_ro.c; do not require devs' status in read-only file case, just adding comment that it requires pmem memory mode device to reproduce; throw large testfile in TEST_DIR instead of $tmp. Xiong Zhou (4): DAX-DIO: make dax_to_non_dax dio test solo generic: test mmap io fom DAX to non-DAX xfs: test per-inode DAX flag DAX to non-DAX generic: mmap write readonly DAX file .gitignore | 1 + common/config | 2 + common/rc | 41 ++++++++++++++++++ src/Makefile | 2 +- src/t_mmap_write_ro.c | 76 +++++++++++++++++++++++++++++++++ tests/generic/413 | 13 +----- tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/423.out | 2 + tests/generic/424 | 92 ++++++++++++++++++++++++++++++++++++++++ tests/generic/424.out | 2 + tests/generic/group | 2 + tests/xfs/260 | 19 ++------- tests/xfs/288 | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/288.out | 2 + tests/xfs/group | 1 + 15 files changed, 450 insertions(+), 29 deletions(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/423 create mode 100644 tests/generic/423.out create mode 100755 tests/generic/424 create mode 100644 tests/generic/424.out create mode 100755 tests/xfs/288 create mode 100644 tests/xfs/288.out -- 1.8.3.1 ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v3 1/4] DAX-DIO: make dax_to_non_dax dio test solo 2017-04-12 14:46 ` [PATCH v3 0/4] split DAX mmap DIO cases Xiong Zhou @ 2017-04-12 14:46 ` Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou ` (3 subsequent siblings) 4 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-12 14:46 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Split it from old testsets in generic/413 and xfs/260, since this test need pmem device underneath has memory(struct page) backend. We will _notrun the solo if the device is not compatible for the test, preveting its confusing failure. The other dax/nondax dio tests don't have this limitation. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/generic/413 | 13 +------------ tests/xfs/260 | 19 +++---------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/tests/generic/413 b/tests/generic/413 index a1cc514..32843d4 100755 --- a/tests/generic/413 +++ b/tests/generic/413 @@ -85,17 +85,6 @@ t_nondax_to_dax() $SCRATCH_MNT/tf_d $1 "buffered nondax to dax" } -t_dax_to_nondax() -{ - prep_files - src/t_mmap_dio $SCRATCH_MNT/tf_s \ - $TEST_DIR/tf_d $1 "dio dax to nondax" - - prep_files - src/t_mmap_dio -b $SCRATCH_MNT/tf_s \ - $TEST_DIR/tf_d $1 "buffered dax to nondax" -} - t_both_nondax() { prep_files @@ -109,8 +98,8 @@ t_both_nondax() # $1 mmap read/write size t_mmap_dio_dax() { + # split t_dax_to_nondax to another case t_both_dax $1 - t_dax_to_nondax $1 t_nondax_to_dax $1 t_both_nondax $1 } diff --git a/tests/xfs/260 b/tests/xfs/260 index e613cc0..3066cb9 100755 --- a/tests/xfs/260 +++ b/tests/xfs/260 @@ -88,21 +88,6 @@ t_nondax_to_dax() $1 "buffered nondax to dax" } -t_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 - src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \ - $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 - src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \ - $1 "buffered dax to nondax" -} - t_both_nondax() { prep_files @@ -119,8 +104,8 @@ t_both_nondax() # $1 mmap read/write size t_dax_flag_mmap_dio() { + # split t_dax_to_nondax to another case t_both_dax $1 - t_dax_to_nondax $1 t_nondax_to_dax $1 t_both_nondax $1 } @@ -146,9 +131,11 @@ tsize=$((128 * 1024 * 1024)) do_tests _scratch_unmount +_check_scratch_fs # mount again without dax option export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" _scratch_mount do_tests -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-12 14:46 ` [PATCH v3 0/4] split DAX mmap DIO cases Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou @ 2017-04-12 14:46 ` Xiong Zhou 2017-04-13 4:11 ` Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 3/4] xfs: test per-inode DAX flag " Xiong Zhou ` (2 subsequent siblings) 4 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-12 14:46 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then do mmap DIO from DAX to non-DAX. This test is split from generic/413. Since DIO from DAX to non-DAX is only supported/doable when device underneath has memory(struct page) backend. By ndctl looking at SCRATCH_DEV, skip this test if it is not in "memory mode". Adding helper to check pmem device status, which requires new PROGs ndctl to tweaking pmem devices and jq to parse ndctl's JSON format outputs. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- common/config | 2 + common/rc | 41 ++++++++++++++++++ tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/423.out | 2 + tests/generic/group | 1 + 5 files changed, 159 insertions(+) create mode 100755 tests/generic/423 create mode 100644 tests/generic/423.out diff --git a/common/config b/common/config index 59041a3..dfdcb8e 100644 --- a/common/config +++ b/common/config @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" export FLOCK_PROG="`set_prog_path flock`" export LDD_PROG="`set_prog_path ldd`" export TIMEOUT_PROG="`set_prog_path timeout`" +export NDCTL_PROG="`set_prog_path ndctl`" +export JQ_PROG="`set_prog_path jq`" # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. # newer systems have udevadm command but older systems like RHEL5 don't. diff --git a/common/rc b/common/rc index 78a2101..35b8f6a 100644 --- a/common/rc +++ b/common/rc @@ -3151,6 +3151,47 @@ _require_chattr() rm -f $TEST_DIR/syscalltest.out } +# Looking up pmem devices' arttributes in +# `ndctl list` output like this: +# { +# "dev":"namespace2.0", +# "mode":"raw", +# "size":8589934592, +# "blockdev":"pmem2" +# }, +_ndctl_get_pmem_key_value() +{ + local dev=$1 + local key=$2 + + $NDCTL_PROG list | \ + $JQ_PROG -r ".[] | \ + select(.blockdev == \"${dev#/dev/}\") | .$key" +} + +# Require pmem device having specific arttibute key/value +# we need. +_require_pmem_key_value() +{ + local dev=$1 + local key=$2 + local value=$3 + + [[ ! "${dev#/dev/}" =~ pmem ]] && \ + _notrun "this test requires pmem device" + + # if pmem devices are setup by memmap, just run + grep -q memmap /proc/cmdline && return + + # pmem devices from NFIT + _require_command "$NDCTL_PROG" "ndctl" + _require_command "$JQ_PROG" "jq" + # get dev specific attr + dev_value=$(_ndctl_get_pmem_key_value $dev $key) + [ "$dev_value" != "$value" ] && \ + _notrun "this test requires $dev $value $key" +} + _get_total_inode() { if [ -z "$1" ]; then diff --git a/tests/generic/423 b/tests/generic/423 new file mode 100755 index 0000000..73c0b53 --- /dev/null +++ b/tests/generic/423 @@ -0,0 +1,113 @@ +#! /bin/bash +# FS QA Test 423 +# +# mmap direct/buffered io from DAX to non-DAX mountpoints. +# Split from generic/413, only do the DAX to non-DAX part. +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat Inc. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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_test +_require_scratch_dax +_require_test_program "feature" +_require_test_program "t_mmap_dio" +_require_xfs_io_command "falloc" +_require_pmem_key_value $SCRATCH_DEV "mode" "memory" + +prep_files() +{ + rm -f $SCRATCH_MNT/tf_{s,d} + rm -f $TEST_DIR/tf_{s,d} + + $XFS_IO_PROG -f -c "falloc 0 $tsize" \ + $SCRATCH_MNT/tf_{s,d} >> $seqres.full 2>&1 + $XFS_IO_PROG -f -c "falloc 0 $tsize" \ + $TEST_DIR/tf_{s,d} >> $seqres.full 2>&1 +} + +t_dax_to_nondax() +{ + prep_files + src/t_mmap_dio $SCRATCH_MNT/tf_s \ + $TEST_DIR/tf_d $1 "dio dax to nondax" + + prep_files + src/t_mmap_dio -b $SCRATCH_MNT/tf_s \ + $TEST_DIR/tf_d $1 "buffered dax to nondax" +} + +do_tests() +{ + # less than page size + t_dax_to_nondax 1024 + # page size + t_dax_to_nondax `src/feature -s` + # bigger sizes, for PMD faults + t_dax_to_nondax $((16 * 1024 * 1024)) + t_dax_to_nondax $((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 + +# mount SCRATCH_DEV with dax option, TEST_DEV not +export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" +_test_cycle_mount +_fs_options $TEST_DEV | grep -qw "dax" && \ + _notrun "we need $TEST_DEV mount without dax" +_scratch_mount "-o dax" + +tsize=$((128 * 1024 * 1024)) + +do_tests + +# success, all done +echo "Silence is golden" +status=0 +exit diff --git a/tests/generic/423.out b/tests/generic/423.out new file mode 100644 index 0000000..22c4029 --- /dev/null +++ b/tests/generic/423.out @@ -0,0 +1,2 @@ +QA output created by 423 +Silence is golden diff --git a/tests/generic/group b/tests/generic/group index d747385..52553fa 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -425,3 +425,4 @@ 420 auto quick punch 421 auto quick encrypt dangerous 422 auto quick +423 auto quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-12 14:46 ` [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou @ 2017-04-13 4:11 ` Xiong Zhou 2017-04-13 13:36 ` Dan Williams 0 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-13 4:11 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Wed, Apr 12, 2017 at 10:46:18PM +0800, Xiong Zhou wrote: > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > do mmap DIO from DAX to non-DAX. > > This test is split from generic/413. Since DIO from DAX > to non-DAX is only supported/doable when device underneath > has memory(struct page) backend. > > By ndctl looking at SCRATCH_DEV, skip this test if it is > not in "memory mode". > > Adding helper to check pmem device status, which requires new > PROGs ndctl to tweaking pmem devices and jq to parse ndctl's > JSON format outputs. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > common/config | 2 + > common/rc | 41 ++++++++++++++++++ > tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/423.out | 2 + > tests/generic/group | 1 + > 5 files changed, 159 insertions(+) > create mode 100755 tests/generic/423 > create mode 100644 tests/generic/423.out > > diff --git a/common/config b/common/config > index 59041a3..dfdcb8e 100644 > --- a/common/config > +++ b/common/config > @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" > export FLOCK_PROG="`set_prog_path flock`" > export LDD_PROG="`set_prog_path ldd`" > export TIMEOUT_PROG="`set_prog_path timeout`" > +export NDCTL_PROG="`set_prog_path ndctl`" > +export JQ_PROG="`set_prog_path jq`" > > # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. > # newer systems have udevadm command but older systems like RHEL5 don't. > diff --git a/common/rc b/common/rc > index 78a2101..35b8f6a 100644 > --- a/common/rc > +++ b/common/rc > @@ -3151,6 +3151,47 @@ _require_chattr() > rm -f $TEST_DIR/syscalltest.out > } > > +# Looking up pmem devices' arttributes in > +# `ndctl list` output like this: > +# { > +# "dev":"namespace2.0", > +# "mode":"raw", > +# "size":8589934592, > +# "blockdev":"pmem2" > +# }, > +_ndctl_get_pmem_key_value() > +{ > + local dev=$1 > + local key=$2 > + > + $NDCTL_PROG list | \ > + $JQ_PROG -r ".[] | \ > + select(.blockdev == \"${dev#/dev/}\") | .$key" > +} > + > +# Require pmem device having specific arttibute key/value > +# we need. > +_require_pmem_key_value() > +{ > + local dev=$1 > + local key=$2 > + local value=$3 > + > + [[ ! "${dev#/dev/}" =~ pmem ]] && \ > + _notrun "this test requires pmem device" > + > + # if pmem devices are setup by memmap, just run > + grep -q memmap /proc/cmdline && return > + > + # pmem devices from NFIT > + _require_command "$NDCTL_PROG" "ndctl" > + _require_command "$JQ_PROG" "jq" > + # get dev specific attr > + dev_value=$(_ndctl_get_pmem_key_value $dev $key) > + [ "$dev_value" != "$value" ] && \ > + _notrun "this test requires $dev $value $key" This is too ugly. I'm looking for another way by searching sysfs for required info of *_DEV, then we don't need these commands and ugly device name matching. Thanks Eryu for the review! -- Xiong > +} > + > _get_total_inode() > { > if [ -z "$1" ]; then > diff --git a/tests/generic/423 b/tests/generic/423 > new file mode 100755 > index 0000000..73c0b53 > --- /dev/null > +++ b/tests/generic/423 > @@ -0,0 +1,113 @@ > +#! /bin/bash > +# FS QA Test 423 > +# > +# mmap direct/buffered io from DAX to non-DAX mountpoints. > +# Split from generic/413, only do the DAX to non-DAX part. > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2017 Red Hat Inc. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +#----------------------------------------------------------------------- > +# > + > +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_test > +_require_scratch_dax > +_require_test_program "feature" > +_require_test_program "t_mmap_dio" > +_require_xfs_io_command "falloc" > +_require_pmem_key_value $SCRATCH_DEV "mode" "memory" > + > +prep_files() > +{ > + rm -f $SCRATCH_MNT/tf_{s,d} > + rm -f $TEST_DIR/tf_{s,d} > + > + $XFS_IO_PROG -f -c "falloc 0 $tsize" \ > + $SCRATCH_MNT/tf_{s,d} >> $seqres.full 2>&1 > + $XFS_IO_PROG -f -c "falloc 0 $tsize" \ > + $TEST_DIR/tf_{s,d} >> $seqres.full 2>&1 > +} > + > +t_dax_to_nondax() > +{ > + prep_files > + src/t_mmap_dio $SCRATCH_MNT/tf_s \ > + $TEST_DIR/tf_d $1 "dio dax to nondax" > + > + prep_files > + src/t_mmap_dio -b $SCRATCH_MNT/tf_s \ > + $TEST_DIR/tf_d $1 "buffered dax to nondax" > +} > + > +do_tests() > +{ > + # less than page size > + t_dax_to_nondax 1024 > + # page size > + t_dax_to_nondax `src/feature -s` > + # bigger sizes, for PMD faults > + t_dax_to_nondax $((16 * 1024 * 1024)) > + t_dax_to_nondax $((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 > + > +# mount SCRATCH_DEV with dax option, TEST_DEV not > +export MOUNT_OPTIONS="" > +export TEST_FS_MOUNT_OPTS="" > +_test_cycle_mount > +_fs_options $TEST_DEV | grep -qw "dax" && \ > + _notrun "we need $TEST_DEV mount without dax" > +_scratch_mount "-o dax" > + > +tsize=$((128 * 1024 * 1024)) > + > +do_tests > + > +# success, all done > +echo "Silence is golden" > +status=0 > +exit > diff --git a/tests/generic/423.out b/tests/generic/423.out > new file mode 100644 > index 0000000..22c4029 > --- /dev/null > +++ b/tests/generic/423.out > @@ -0,0 +1,2 @@ > +QA output created by 423 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index d747385..52553fa 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -425,3 +425,4 @@ > 420 auto quick punch > 421 auto quick encrypt dangerous > 422 auto quick > +423 auto quick > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-13 4:11 ` Xiong Zhou @ 2017-04-13 13:36 ` Dan Williams 2017-04-14 10:01 ` Xiong Zhou 0 siblings, 1 reply; 71+ messages in thread From: Dan Williams @ 2017-04-13 13:36 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, Ross Zwisler, jmoyer, Eryu Guan On Wed, Apr 12, 2017 at 9:11 PM, Xiong Zhou <xzhou@redhat.com> wrote: > On Wed, Apr 12, 2017 at 10:46:18PM +0800, Xiong Zhou wrote: >> Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then >> do mmap DIO from DAX to non-DAX. >> >> This test is split from generic/413. Since DIO from DAX >> to non-DAX is only supported/doable when device underneath >> has memory(struct page) backend. >> >> By ndctl looking at SCRATCH_DEV, skip this test if it is >> not in "memory mode". >> >> Adding helper to check pmem device status, which requires new >> PROGs ndctl to tweaking pmem devices and jq to parse ndctl's >> JSON format outputs. >> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> >> --- >> common/config | 2 + >> common/rc | 41 ++++++++++++++++++ >> tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/423.out | 2 + >> tests/generic/group | 1 + >> 5 files changed, 159 insertions(+) >> create mode 100755 tests/generic/423 >> create mode 100644 tests/generic/423.out >> >> diff --git a/common/config b/common/config >> index 59041a3..dfdcb8e 100644 >> --- a/common/config >> +++ b/common/config >> @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" >> export FLOCK_PROG="`set_prog_path flock`" >> export LDD_PROG="`set_prog_path ldd`" >> export TIMEOUT_PROG="`set_prog_path timeout`" >> +export NDCTL_PROG="`set_prog_path ndctl`" >> +export JQ_PROG="`set_prog_path jq`" >> >> # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. >> # newer systems have udevadm command but older systems like RHEL5 don't. >> diff --git a/common/rc b/common/rc >> index 78a2101..35b8f6a 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -3151,6 +3151,47 @@ _require_chattr() >> rm -f $TEST_DIR/syscalltest.out >> } >> >> +# Looking up pmem devices' arttributes in >> +# `ndctl list` output like this: >> +# { >> +# "dev":"namespace2.0", >> +# "mode":"raw", >> +# "size":8589934592, >> +# "blockdev":"pmem2" >> +# }, >> +_ndctl_get_pmem_key_value() >> +{ >> + local dev=$1 >> + local key=$2 >> + >> + $NDCTL_PROG list | \ >> + $JQ_PROG -r ".[] | \ >> + select(.blockdev == \"${dev#/dev/}\") | .$key" >> +} >> + >> +# Require pmem device having specific arttibute key/value >> +# we need. >> +_require_pmem_key_value() >> +{ >> + local dev=$1 >> + local key=$2 >> + local value=$3 >> + >> + [[ ! "${dev#/dev/}" =~ pmem ]] && \ >> + _notrun "this test requires pmem device" >> + >> + # if pmem devices are setup by memmap, just run >> + grep -q memmap /proc/cmdline && return >> + >> + # pmem devices from NFIT >> + _require_command "$NDCTL_PROG" "ndctl" >> + _require_command "$JQ_PROG" "jq" >> + # get dev specific attr >> + dev_value=$(_ndctl_get_pmem_key_value $dev $key) >> + [ "$dev_value" != "$value" ] && \ >> + _notrun "this test requires $dev $value $key" > > This is too ugly. I'm looking for another way by searching sysfs for > required info of *_DEV, then we don't need these commands and ugly > device name matching. > > Thanks Eryu for the review! Why not just let jq do this key matching validation for you? I don't think the word NFIT should appear anywhere in xfstests. That's an internal detail of an nvdimm implementation and going forward makes no sense when we have NVDIMMs on powerpc for example. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-13 13:36 ` Dan Williams @ 2017-04-14 10:01 ` Xiong Zhou 2017-04-14 14:49 ` Dan Williams 0 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-14 10:01 UTC (permalink / raw) To: Dan Williams; +Cc: Xiong Zhou, fstests, Ross Zwisler, jmoyer, Eryu Guan On Thu, Apr 13, 2017 at 06:36:34AM -0700, Dan Williams wrote: > On Wed, Apr 12, 2017 at 9:11 PM, Xiong Zhou <xzhou@redhat.com> wrote: > > On Wed, Apr 12, 2017 at 10:46:18PM +0800, Xiong Zhou wrote: > >> Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > >> do mmap DIO from DAX to non-DAX. > >> > >> This test is split from generic/413. Since DIO from DAX > >> to non-DAX is only supported/doable when device underneath > >> has memory(struct page) backend. > >> > >> By ndctl looking at SCRATCH_DEV, skip this test if it is > >> not in "memory mode". > >> > >> Adding helper to check pmem device status, which requires new > >> PROGs ndctl to tweaking pmem devices and jq to parse ndctl's > >> JSON format outputs. > >> > >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> > >> --- > >> common/config | 2 + > >> common/rc | 41 ++++++++++++++++++ > >> tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >> tests/generic/423.out | 2 + > >> tests/generic/group | 1 + > >> 5 files changed, 159 insertions(+) > >> create mode 100755 tests/generic/423 > >> create mode 100644 tests/generic/423.out > >> > >> diff --git a/common/config b/common/config > >> index 59041a3..dfdcb8e 100644 > >> --- a/common/config > >> +++ b/common/config > >> @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" > >> export FLOCK_PROG="`set_prog_path flock`" > >> export LDD_PROG="`set_prog_path ldd`" > >> export TIMEOUT_PROG="`set_prog_path timeout`" > >> +export NDCTL_PROG="`set_prog_path ndctl`" > >> +export JQ_PROG="`set_prog_path jq`" > >> > >> # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. > >> # newer systems have udevadm command but older systems like RHEL5 don't. > >> diff --git a/common/rc b/common/rc > >> index 78a2101..35b8f6a 100644 > >> --- a/common/rc > >> +++ b/common/rc > >> @@ -3151,6 +3151,47 @@ _require_chattr() > >> rm -f $TEST_DIR/syscalltest.out > >> } > >> > >> +# Looking up pmem devices' arttributes in > >> +# `ndctl list` output like this: > >> +# { > >> +# "dev":"namespace2.0", > >> +# "mode":"raw", > >> +# "size":8589934592, > >> +# "blockdev":"pmem2" > >> +# }, > >> +_ndctl_get_pmem_key_value() > >> +{ > >> + local dev=$1 > >> + local key=$2 > >> + > >> + $NDCTL_PROG list | \ > >> + $JQ_PROG -r ".[] | \ > >> + select(.blockdev == \"${dev#/dev/}\") | .$key" > >> +} > >> + > >> +# Require pmem device having specific arttibute key/value > >> +# we need. > >> +_require_pmem_key_value() > >> +{ > >> + local dev=$1 > >> + local key=$2 > >> + local value=$3 > >> + > >> + [[ ! "${dev#/dev/}" =~ pmem ]] && \ > >> + _notrun "this test requires pmem device" > >> + > >> + # if pmem devices are setup by memmap, just run > >> + grep -q memmap /proc/cmdline && return > >> + > >> + # pmem devices from NFIT > >> + _require_command "$NDCTL_PROG" "ndctl" > >> + _require_command "$JQ_PROG" "jq" > >> + # get dev specific attr > >> + dev_value=$(_ndctl_get_pmem_key_value $dev $key) > >> + [ "$dev_value" != "$value" ] && \ > >> + _notrun "this test requires $dev $value $key" > > > > This is too ugly. I'm looking for another way by searching sysfs for > > required info of *_DEV, then we don't need these commands and ugly > > device name matching. > > > > Thanks Eryu for the review! > > Why not just let jq do this key matching validation for you? I don't I agree with Ross that we don't even need ndctl and jq, if we run tests on memmap setup. > think the word NFIT should appear anywhere in xfstests. That's an > internal detail of an nvdimm implementation and going forward makes no > sense when we have NVDIMMs on powerpc for example. Fair enough. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-14 10:01 ` Xiong Zhou @ 2017-04-14 14:49 ` Dan Williams 2017-04-14 15:22 ` Ross Zwisler 0 siblings, 1 reply; 71+ messages in thread From: Dan Williams @ 2017-04-14 14:49 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, Ross Zwisler, jmoyer, Eryu Guan On Fri, Apr 14, 2017 at 3:01 AM, Xiong Zhou <xzhou@redhat.com> wrote: > On Thu, Apr 13, 2017 at 06:36:34AM -0700, Dan Williams wrote: >> On Wed, Apr 12, 2017 at 9:11 PM, Xiong Zhou <xzhou@redhat.com> wrote: >> > On Wed, Apr 12, 2017 at 10:46:18PM +0800, Xiong Zhou wrote: >> >> Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then >> >> do mmap DIO from DAX to non-DAX. >> >> >> >> This test is split from generic/413. Since DIO from DAX >> >> to non-DAX is only supported/doable when device underneath >> >> has memory(struct page) backend. >> >> >> >> By ndctl looking at SCRATCH_DEV, skip this test if it is >> >> not in "memory mode". >> >> >> >> Adding helper to check pmem device status, which requires new >> >> PROGs ndctl to tweaking pmem devices and jq to parse ndctl's >> >> JSON format outputs. >> >> >> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> >> >> --- >> >> common/config | 2 + >> >> common/rc | 41 ++++++++++++++++++ >> >> tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> >> tests/generic/423.out | 2 + >> >> tests/generic/group | 1 + >> >> 5 files changed, 159 insertions(+) >> >> create mode 100755 tests/generic/423 >> >> create mode 100644 tests/generic/423.out >> >> >> >> diff --git a/common/config b/common/config >> >> index 59041a3..dfdcb8e 100644 >> >> --- a/common/config >> >> +++ b/common/config >> >> @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" >> >> export FLOCK_PROG="`set_prog_path flock`" >> >> export LDD_PROG="`set_prog_path ldd`" >> >> export TIMEOUT_PROG="`set_prog_path timeout`" >> >> +export NDCTL_PROG="`set_prog_path ndctl`" >> >> +export JQ_PROG="`set_prog_path jq`" >> >> >> >> # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. >> >> # newer systems have udevadm command but older systems like RHEL5 don't. >> >> diff --git a/common/rc b/common/rc >> >> index 78a2101..35b8f6a 100644 >> >> --- a/common/rc >> >> +++ b/common/rc >> >> @@ -3151,6 +3151,47 @@ _require_chattr() >> >> rm -f $TEST_DIR/syscalltest.out >> >> } >> >> >> >> +# Looking up pmem devices' arttributes in >> >> +# `ndctl list` output like this: >> >> +# { >> >> +# "dev":"namespace2.0", >> >> +# "mode":"raw", >> >> +# "size":8589934592, >> >> +# "blockdev":"pmem2" >> >> +# }, >> >> +_ndctl_get_pmem_key_value() >> >> +{ >> >> + local dev=$1 >> >> + local key=$2 >> >> + >> >> + $NDCTL_PROG list | \ >> >> + $JQ_PROG -r ".[] | \ >> >> + select(.blockdev == \"${dev#/dev/}\") | .$key" >> >> +} >> >> + >> >> +# Require pmem device having specific arttibute key/value >> >> +# we need. >> >> +_require_pmem_key_value() >> >> +{ >> >> + local dev=$1 >> >> + local key=$2 >> >> + local value=$3 >> >> + >> >> + [[ ! "${dev#/dev/}" =~ pmem ]] && \ >> >> + _notrun "this test requires pmem device" >> >> + >> >> + # if pmem devices are setup by memmap, just run >> >> + grep -q memmap /proc/cmdline && return >> >> + >> >> + # pmem devices from NFIT >> >> + _require_command "$NDCTL_PROG" "ndctl" >> >> + _require_command "$JQ_PROG" "jq" >> >> + # get dev specific attr >> >> + dev_value=$(_ndctl_get_pmem_key_value $dev $key) >> >> + [ "$dev_value" != "$value" ] && \ >> >> + _notrun "this test requires $dev $value $key" >> > >> > This is too ugly. I'm looking for another way by searching sysfs for >> > required info of *_DEV, then we don't need these commands and ugly >> > device name matching. >> > >> > Thanks Eryu for the review! >> >> Why not just let jq do this key matching validation for you? I don't > > I agree with Ross that we don't even need ndctl and jq, if we run tests > on memmap setup. I don't understand this comment. How is the test determining if it is on an memmap defined namespace without querying sysfs? ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-14 14:49 ` Dan Williams @ 2017-04-14 15:22 ` Ross Zwisler 2017-04-14 15:33 ` Dan Williams 0 siblings, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-04-14 15:22 UTC (permalink / raw) To: Dan Williams; +Cc: Xiong Zhou, fstests, Ross Zwisler, jmoyer, Eryu Guan On Fri, Apr 14, 2017 at 07:49:48AM -0700, Dan Williams wrote: > On Fri, Apr 14, 2017 at 3:01 AM, Xiong Zhou <xzhou@redhat.com> wrote: > > I agree with Ross that we don't even need ndctl and jq, if we run tests > > on memmap setup. > > I don't understand this comment. How is the test determining if it is > on an memmap defined namespace without querying sysfs? If we we assume that a setup is *either* memmap based or NFIT based, we can choose whether to use jq & ndctl by grepping for "memmap" with a trailing "!" in /proc/cmdline. If you think we need to worry about the case where users have both a valid NFIT and a memmap kernel command line param making them a PMEM region, this shortcut won't work. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-14 15:22 ` Ross Zwisler @ 2017-04-14 15:33 ` Dan Williams 2017-04-14 15:51 ` Ross Zwisler 0 siblings, 1 reply; 71+ messages in thread From: Dan Williams @ 2017-04-14 15:33 UTC (permalink / raw) To: Ross Zwisler; +Cc: Xiong Zhou, fstests, jmoyer, Eryu Guan On Fri, Apr 14, 2017 at 8:22 AM, Ross Zwisler <ross.zwisler@linux.intel.com> wrote: > On Fri, Apr 14, 2017 at 07:49:48AM -0700, Dan Williams wrote: >> On Fri, Apr 14, 2017 at 3:01 AM, Xiong Zhou <xzhou@redhat.com> wrote: >> > I agree with Ross that we don't even need ndctl and jq, if we run tests >> > on memmap setup. >> >> I don't understand this comment. How is the test determining if it is >> on an memmap defined namespace without querying sysfs? > > If we we assume that a setup is *either* memmap based or NFIT based, we can > choose whether to use jq & ndctl by grepping for "memmap" with a trailing "!" > in /proc/cmdline. > > If you think we need to worry about the case where users have both a valid > NFIT and a memmap kernel command line param making them a PMEM region, this > shortcut won't work. Since the user is picking the pmem device the test will gave the wrong answer if they pick the wrong one. Especially since the kernel is growing new nvdimm bus types, like the powerpc one, this test should check the namespace mode. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-14 15:33 ` Dan Williams @ 2017-04-14 15:51 ` Ross Zwisler 0 siblings, 0 replies; 71+ messages in thread From: Ross Zwisler @ 2017-04-14 15:51 UTC (permalink / raw) To: Dan Williams; +Cc: Ross Zwisler, Xiong Zhou, fstests, jmoyer, Eryu Guan On Fri, Apr 14, 2017 at 08:33:44AM -0700, Dan Williams wrote: > On Fri, Apr 14, 2017 at 8:22 AM, Ross Zwisler > <ross.zwisler@linux.intel.com> wrote: > > On Fri, Apr 14, 2017 at 07:49:48AM -0700, Dan Williams wrote: > >> On Fri, Apr 14, 2017 at 3:01 AM, Xiong Zhou <xzhou@redhat.com> wrote: > >> > I agree with Ross that we don't even need ndctl and jq, if we run tests > >> > on memmap setup. > >> > >> I don't understand this comment. How is the test determining if it is > >> on an memmap defined namespace without querying sysfs? > > > > If we we assume that a setup is *either* memmap based or NFIT based, we can > > choose whether to use jq & ndctl by grepping for "memmap" with a trailing "!" > > in /proc/cmdline. > > > > If you think we need to worry about the case where users have both a valid > > NFIT and a memmap kernel command line param making them a PMEM region, this > > shortcut won't work. > > Since the user is picking the pmem device the test will gave the wrong > answer if they pick the wrong one. Especially since the kernel is > growing new nvdimm bus types, like the powerpc one, this test should > check the namespace mode. Fair enough. ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v3 3/4] xfs: test per-inode DAX flag DAX to non-DAX 2017-04-12 14:46 ` [PATCH v3 0/4] split DAX mmap DIO cases Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou @ 2017-04-12 14:46 ` Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 4/4] generic: mmap write readonly DAX file Xiong Zhou 2017-04-17 7:14 ` [PATCH v4 0/4] split DAX mmap DIO cases Xiong Zhou 4 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-12 14:46 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou This test is split from xfs/260, only do IO from DAX to non-DAX. Since DIO from DAX to non-DAX is only supported/doable when device underneath has memory(struct page) backend. By ndctl looking at SCRATCH_DEV, skip this test if it is not in "memory mode". Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/xfs/288 | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/288.out | 2 + tests/xfs/group | 1 + 3 files changed, 114 insertions(+) create mode 100755 tests/xfs/288 create mode 100644 tests/xfs/288.out diff --git a/tests/xfs/288 b/tests/xfs/288 new file mode 100755 index 0000000..d296a36 --- /dev/null +++ b/tests/xfs/288 @@ -0,0 +1,111 @@ +#! /bin/bash +# FS QA Test 288 +# +# Test per-inode DAX flag by mmap direct/buffered IO. +# Split from xfs/260, only do the DAX to non-DAX part. +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat Inc. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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 xfs +_supported_os Linux +_require_scratch_dax +_require_test_program "feature" +_require_test_program "t_mmap_dio" +_require_xfs_io_command "chattr" "+/-x" +_require_xfs_io_command "falloc" +_require_pmem_key_value $SCRATCH_DEV "mode" "memory" + +prep_files() +{ + rm -f $SCRATCH_MNT/tf_{s,d} + + $XFS_IO_PROG -f -c "falloc 0 $tsize" \ + $SCRATCH_MNT/tf_{s,d} >> $seqres.full 2>&1 +} + +t_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 + src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \ + $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 + src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \ + $1 "buffered dax to nondax" +} + +do_tests() +{ + # less than page size + t_dax_to_nondax 1024 + # page size + t_dax_to_nondax `src/feature -s` + # bigger sizes, for PMD faults + t_dax_to_nondax $((16 * 1024 * 1024)) + t_dax_to_nondax $((64 * 1024 * 1024)) +} + +# make xfs 2Mb aligned for PMD fault testing +_scratch_mkfs "-d su=2m,sw=1" > /dev/null 2>&1 + +# mount with dax option +_scratch_mount "-o dax" + +tsize=$((128 * 1024 * 1024)) + +do_tests +_scratch_unmount +_check_scratch_fs + +# mount again without dax option +export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" +_scratch_mount +do_tests + +# success, all done +echo "Silence is golden" +status=0 +exit diff --git a/tests/xfs/288.out b/tests/xfs/288.out new file mode 100644 index 0000000..2958a5c --- /dev/null +++ b/tests/xfs/288.out @@ -0,0 +1,2 @@ +QA output created by 288 +Silence is golden diff --git a/tests/xfs/group b/tests/xfs/group index 75769f9..736de3c 100644 --- a/tests/xfs/group +++ b/tests/xfs/group @@ -285,6 +285,7 @@ 285 dangerous_fuzzers dangerous_scrub 286 dangerous_fuzzers dangerous_scrub dangerous_online_repair 287 auto dump quota quick +288 auto quick 290 auto rw prealloc quick ioctl zero 291 auto repair 292 auto mkfs quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* [PATCH v3 4/4] generic: mmap write readonly DAX file 2017-04-12 14:46 ` [PATCH v3 0/4] split DAX mmap DIO cases Xiong Zhou ` (2 preceding siblings ...) 2017-04-12 14:46 ` [PATCH v3 3/4] xfs: test per-inode DAX flag " Xiong Zhou @ 2017-04-12 14:46 ` Xiong Zhou 2017-04-17 7:14 ` [PATCH v4 0/4] split DAX mmap DIO cases Xiong Zhou 4 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-12 14:46 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Regression case that one can write to read-only file in a DAX mountpoint. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- .gitignore | 1 + src/Makefile | 2 +- src/t_mmap_write_ro.c | 76 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/424 | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/424.out | 2 ++ tests/generic/group | 1 + 6 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/424 create mode 100644 tests/generic/424.out diff --git a/.gitignore b/.gitignore index 1ed2a92..ee9329f 100644 --- a/.gitignore +++ b/.gitignore @@ -131,6 +131,7 @@ /src/renameat2 /src/t_rename_overwrite /src/t_mmap_dio +/src/t_mmap_write_ro # dmapi/ binaries /dmapi/src/common/cmd/read_invis diff --git a/src/Makefile b/src/Makefile index a7f27f0..2604f52 100644 --- a/src/Makefile +++ b/src/Makefile @@ -12,7 +12,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ godown resvtest writemod makeextents itrash rename \ multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ - holetest t_truncate_self t_mmap_dio + holetest t_truncate_self t_mmap_dio t_mmap_write_ro LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c new file mode 100644 index 0000000..b6bf2a7 --- /dev/null +++ b/src/t_mmap_write_ro.c @@ -0,0 +1,76 @@ +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <fcntl.h> +#include <unistd.h> +#include <libgen.h> +#include <errno.h> +#include <sys/mman.h> + +void +err_exit(char *op) +{ + fprintf(stderr, "%s: %s\n", op, strerror(errno)); + exit(1); +} + +int +main(int argc, char **argv) +{ + int fd, pfd, ret; + char *buf; + volatile char foo; + int pagesize = getpagesize(); + + if (argc < 2) { + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); + exit(0); + } + + fd = open(argv[1], O_RDONLY|O_DIRECT); + if (fd < 0) + err_exit("open"); + + pfd = open(argv[2], O_RDONLY); + if (pfd < 0) + err_exit("pmem open"); + + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); + if (buf == MAP_FAILED) + err_exit("mmap"); + + /* + * Read from the DAX mmap to populate the first page in the + * address_space with a read-only mapping. + */ + foo = *buf; + + /* quiet the compiler's "unused-but-set-variable" warning */ + foo; + + /* + * Now write to the DAX mmap. This *should* fail, but if the bug is + * present in __get_user_pages_fast(), it will succeed. + */ + ret = read(fd, buf, pagesize); + if (ret != pagesize) + err_exit("read"); + + ret = msync(buf, pagesize, MS_SYNC); + if (ret != 0) + err_exit("msync"); + + ret = munmap(buf, pagesize); + if (ret < 0) + err_exit("munmap"); + + ret = close(fd); + if (ret < 0) + err_exit("clsoe fd"); + + ret = close(pfd); + if (ret < 0) + err_exit("close pfd"); + + exit(0); +} diff --git a/tests/generic/424 b/tests/generic/424 new file mode 100755 index 0000000..f73e08a --- /dev/null +++ b/tests/generic/424 @@ -0,0 +1,92 @@ +#! /bin/bash +# FS QA Test 424 +# +# This is a regression test for kernel commit +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings +# created by Jeffrey Moyer <jmoyer@redhat.com> +# +# This is reproducible only when testing on pmem device +# which is configured in "memory mode", not in "raw mode". +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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 -rf $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# Modify as appropriate. +_supported_fs generic +_supported_os Linux +_require_scratch_dax +_require_test_program "t_mmap_write_ro" +_require_user + +# real QA test starts here + +_scratch_mkfs >>$seqres.full 2>&1 +_scratch_mount "-o dax" + +# remount TEST_DEV wo/ dax +export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" +_test_cycle_mount + +# prepare a 4k read-only DAX file, save its md5sum +$XFS_IO_PROG -f -c "pwrite -S 0xFFFF 0 4096" \ + $SCRATCH_MNT/readonlyfile >> $seqres.full 2>&1 +chmod 0644 $SCRATCH_MNT/readonlyfile +md5_1="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +# prepare another larger non-DAX file +$XFS_IO_PROG -f -c "pwrite -S 0x0000 0 40960" \ + $TEST_DIR/${seq}.largefile >> $seqres.full 2>&1 +# allow qa_user access +chown $qa_user $TEST_DIR/${seq}.largefile + +# run test programme, read another larger file writing into +# the read-only file with mmap, which should fail. +_user_do "src/t_mmap_write_ro $TEST_DIR/${seq}.largefile \ + $SCRATCH_MNT/readonlyfile" + +# read-only file should not get updated, md5sum again. +md5_2="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +[ "$md5_1" != "$md5_2" ] && echo "read only file changed" + +# success, all done +status=0 +exit diff --git a/tests/generic/424.out b/tests/generic/424.out new file mode 100644 index 0000000..c836ec8 --- /dev/null +++ b/tests/generic/424.out @@ -0,0 +1,2 @@ +QA output created by 424 +read: Bad address diff --git a/tests/generic/group b/tests/generic/group index 52553fa..81db660 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -426,3 +426,4 @@ 421 auto quick encrypt dangerous 422 auto quick 423 auto quick +424 auto quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* [PATCH v4 0/4] split DAX mmap DIO cases 2017-04-12 14:46 ` [PATCH v3 0/4] split DAX mmap DIO cases Xiong Zhou ` (3 preceding siblings ...) 2017-04-12 14:46 ` [PATCH v3 4/4] generic: mmap write readonly DAX file Xiong Zhou @ 2017-04-17 7:14 ` Xiong Zhou 2017-04-17 7:14 ` [PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou ` (4 more replies) 4 siblings, 5 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-17 7:14 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou v4: simpler helper to require *_DEV nvdimm device with specific attr, by searching sysfs: find region by device name then find namespace mode by region id. Don't need ndctl and jq. use __attribute__((__unused__)) to mute the warning instead. Xiong Zhou (4): DAX-DIO: make dax_to_non_dax dio test solo generic: test mmap io fom DAX to non-DAX xfs: test per-inode DAX flag DAX to non-DAX generic: mmap write readonly DAX file .gitignore | 1 + common/rc | 45 ++++++++++++++++++++ src/Makefile | 2 +- src/t_mmap_write_ro.c | 77 ++++++++++++++++++++++++++++++++++ tests/generic/413 | 13 +----- tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/423.out | 2 + tests/generic/424 | 92 ++++++++++++++++++++++++++++++++++++++++ tests/generic/424.out | 2 + tests/generic/group | 2 + tests/xfs/260 | 17 +------- tests/xfs/288 | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/288.out | 2 + tests/xfs/group | 1 + 14 files changed, 451 insertions(+), 29 deletions(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/423 create mode 100644 tests/generic/423.out create mode 100755 tests/generic/424 create mode 100644 tests/generic/424.out create mode 100755 tests/xfs/288 create mode 100644 tests/xfs/288.out -- 1.8.3.1 ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo 2017-04-17 7:14 ` [PATCH v4 0/4] split DAX mmap DIO cases Xiong Zhou @ 2017-04-17 7:14 ` Xiong Zhou 2017-04-18 16:31 ` Ross Zwisler 2017-04-17 7:14 ` [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou ` (3 subsequent siblings) 4 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-17 7:14 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Split it from old testsets in generic/413 and xfs/260, since this test need pmem device underneath has memory(struct page) backend. We will _notrun the solo if the device is not compatible for the test, preveting its confusing failure. The other dax/nondax dio tests don't have this limitation. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/generic/413 | 13 +------------ tests/xfs/260 | 17 +---------------- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/tests/generic/413 b/tests/generic/413 index a1cc514..32e0966 100755 --- a/tests/generic/413 +++ b/tests/generic/413 @@ -85,17 +85,6 @@ t_nondax_to_dax() $SCRATCH_MNT/tf_d $1 "buffered nondax to dax" } -t_dax_to_nondax() -{ - prep_files - src/t_mmap_dio $SCRATCH_MNT/tf_s \ - $TEST_DIR/tf_d $1 "dio dax to nondax" - - prep_files - src/t_mmap_dio -b $SCRATCH_MNT/tf_s \ - $TEST_DIR/tf_d $1 "buffered dax to nondax" -} - t_both_nondax() { prep_files @@ -109,8 +98,8 @@ t_both_nondax() # $1 mmap read/write size t_mmap_dio_dax() { + # t_dax_to_nondax run in separated case t_both_dax $1 - t_dax_to_nondax $1 t_nondax_to_dax $1 t_both_nondax $1 } diff --git a/tests/xfs/260 b/tests/xfs/260 index e613cc0..0763450 100755 --- a/tests/xfs/260 +++ b/tests/xfs/260 @@ -88,21 +88,6 @@ t_nondax_to_dax() $1 "buffered nondax to dax" } -t_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 - src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \ - $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 - src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \ - $1 "buffered dax to nondax" -} - t_both_nondax() { prep_files @@ -119,8 +104,8 @@ t_both_nondax() # $1 mmap read/write size t_dax_flag_mmap_dio() { + # t_dax_to_nondax run in separated case t_both_dax $1 - t_dax_to_nondax $1 t_nondax_to_dax $1 t_both_nondax $1 } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo 2017-04-17 7:14 ` [PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou @ 2017-04-18 16:31 ` Ross Zwisler 0 siblings, 0 replies; 71+ messages in thread From: Ross Zwisler @ 2017-04-18 16:31 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Mon, Apr 17, 2017 at 03:14:12PM +0800, Xiong Zhou wrote: > Split it from old testsets in generic/413 and xfs/260, since this > test need pmem device underneath has memory(struct page) backend. > We will _notrun the solo if the device is not compatible for the > test, preveting its confusing failure. > > The other dax/nondax dio tests don't have this limitation. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> Sure, this seems like a good solution to me. Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com> ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-17 7:14 ` [PATCH v4 0/4] split DAX mmap DIO cases Xiong Zhou 2017-04-17 7:14 ` [PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou @ 2017-04-17 7:14 ` Xiong Zhou 2017-04-17 14:14 ` Dan Williams ` (2 more replies) 2017-04-17 7:14 ` [PATCH v4 3/4] xfs: test per-inode DAX flag " Xiong Zhou ` (2 subsequent siblings) 4 siblings, 3 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-17 7:14 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then do mmap DIO from DAX to non-DAX. This test is split from generic/413. Since DIO from DAX to non-DAX is only supported/doable when device underneath has memory(struct page) backend. By ndctl looking at SCRATCH_DEV, skip this test if it is not in "memory mode". Adding helper to check pmem device status, which requires new PROGs ndctl to tweaking pmem devices and jq to parse ndctl's JSON format outputs. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- common/rc | 45 ++++++++++++++++++++ tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/423.out | 2 + tests/generic/group | 1 + 4 files changed, 161 insertions(+) create mode 100755 tests/generic/423 create mode 100644 tests/generic/423.out diff --git a/common/rc b/common/rc index 78a2101..73ac79c 100644 --- a/common/rc +++ b/common/rc @@ -3151,6 +3151,51 @@ _require_chattr() rm -f $TEST_DIR/syscalltest.out } +# Require test/scratch device nvdimm and having specific +# arttibute key/value we need. +# +# This is designed to get attr values of nvdimm persistent +# memory device, by searching sysfs. +# +# Other non-nvdimm or non-persistent-memory devices would +# fail this helper anyway. +# +# So, ONLY use this helper when you REALLY need nvdimm and +# specific attr on it. +# +_require_pmem_key_value() +{ + local dev=${1#/dev/} + local key=$2 + local value=$3 + local region index keyfile dev_value + + # find a filename string contains the region of dev, eg: + # /sys/devices/platform/e820_pmem/ndbus0/region1/\ + # namespace1.0/block/pmem1 + # + region=$(find /sys/ | grep $dev | grep region | head -1) + [ -z "$region" ] && \ + _notrun "requires persistent memory $dev $key $value" + + # get region number index + index=$(expr $region : .*region) + + # find the file for the key, eg: + # /sys/devices/platform/e820_pmem/ndbus0/region1/\ + # namespace1.0/mode + # + keyfile=$(find /sys/ | grep region${region:$index:2} \ + | grep namespace | grep $key) + [ -z "$keyfile" ] && \ + _notrun "requires persistent memory $dev $key $value" + + # get the value and test + dev_value=$(cat $keyfile) + [ "$dev_value" != "$value" ] && \ + _notrun "requires $dev $key $value" +} + _get_total_inode() { if [ -z "$1" ]; then diff --git a/tests/generic/423 b/tests/generic/423 new file mode 100755 index 0000000..73c0b53 --- /dev/null +++ b/tests/generic/423 @@ -0,0 +1,113 @@ +#! /bin/bash +# FS QA Test 423 +# +# mmap direct/buffered io from DAX to non-DAX mountpoints. +# Split from generic/413, only do the DAX to non-DAX part. +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat Inc. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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_test +_require_scratch_dax +_require_test_program "feature" +_require_test_program "t_mmap_dio" +_require_xfs_io_command "falloc" +_require_pmem_key_value $SCRATCH_DEV "mode" "memory" + +prep_files() +{ + rm -f $SCRATCH_MNT/tf_{s,d} + rm -f $TEST_DIR/tf_{s,d} + + $XFS_IO_PROG -f -c "falloc 0 $tsize" \ + $SCRATCH_MNT/tf_{s,d} >> $seqres.full 2>&1 + $XFS_IO_PROG -f -c "falloc 0 $tsize" \ + $TEST_DIR/tf_{s,d} >> $seqres.full 2>&1 +} + +t_dax_to_nondax() +{ + prep_files + src/t_mmap_dio $SCRATCH_MNT/tf_s \ + $TEST_DIR/tf_d $1 "dio dax to nondax" + + prep_files + src/t_mmap_dio -b $SCRATCH_MNT/tf_s \ + $TEST_DIR/tf_d $1 "buffered dax to nondax" +} + +do_tests() +{ + # less than page size + t_dax_to_nondax 1024 + # page size + t_dax_to_nondax `src/feature -s` + # bigger sizes, for PMD faults + t_dax_to_nondax $((16 * 1024 * 1024)) + t_dax_to_nondax $((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 + +# mount SCRATCH_DEV with dax option, TEST_DEV not +export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" +_test_cycle_mount +_fs_options $TEST_DEV | grep -qw "dax" && \ + _notrun "we need $TEST_DEV mount without dax" +_scratch_mount "-o dax" + +tsize=$((128 * 1024 * 1024)) + +do_tests + +# success, all done +echo "Silence is golden" +status=0 +exit diff --git a/tests/generic/423.out b/tests/generic/423.out new file mode 100644 index 0000000..22c4029 --- /dev/null +++ b/tests/generic/423.out @@ -0,0 +1,2 @@ +QA output created by 423 +Silence is golden diff --git a/tests/generic/group b/tests/generic/group index d747385..52553fa 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -425,3 +425,4 @@ 420 auto quick punch 421 auto quick encrypt dangerous 422 auto quick +423 auto quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-17 7:14 ` [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou @ 2017-04-17 14:14 ` Dan Williams 2017-04-17 22:54 ` Dan Williams 2017-04-19 8:40 ` Xiong Zhou 2017-04-18 10:12 ` Christoph Hellwig 2017-04-18 16:32 ` Ross Zwisler 2 siblings, 2 replies; 71+ messages in thread From: Dan Williams @ 2017-04-17 14:14 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, Ross Zwisler, jmoyer, Eryu Guan On Mon, Apr 17, 2017 at 12:14 AM, Xiong Zhou <xzhou@redhat.com> wrote: > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > do mmap DIO from DAX to non-DAX. > > This test is split from generic/413. Since DIO from DAX > to non-DAX is only supported/doable when device underneath > has memory(struct page) backend. > > By ndctl looking at SCRATCH_DEV, skip this test if it is > not in "memory mode". > > Adding helper to check pmem device status, which requires new > PROGs ndctl to tweaking pmem devices and jq to parse ndctl's > JSON format outputs. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > common/rc | 45 ++++++++++++++++++++ > tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/423.out | 2 + > tests/generic/group | 1 + > 4 files changed, 161 insertions(+) > create mode 100755 tests/generic/423 > create mode 100644 tests/generic/423.out > > diff --git a/common/rc b/common/rc > index 78a2101..73ac79c 100644 > --- a/common/rc > +++ b/common/rc > @@ -3151,6 +3151,51 @@ _require_chattr() > rm -f $TEST_DIR/syscalltest.out > } > > +# Require test/scratch device nvdimm and having specific > +# arttibute key/value we need. > +# > +# This is designed to get attr values of nvdimm persistent > +# memory device, by searching sysfs. > +# > +# Other non-nvdimm or non-persistent-memory devices would > +# fail this helper anyway. > +# > +# So, ONLY use this helper when you REALLY need nvdimm and > +# specific attr on it. > +# > +_require_pmem_key_value() > +{ > + local dev=${1#/dev/} > + local key=$2 > + local value=$3 > + local region index keyfile dev_value > + > + # find a filename string contains the region of dev, eg: > + # /sys/devices/platform/e820_pmem/ndbus0/region1/\ > + # namespace1.0/block/pmem1 > + # > + region=$(find /sys/ | grep $dev | grep region | head -1) > + [ -z "$region" ] && \ > + _notrun "requires persistent memory $dev $key $value" Running 'find' in sysfs is overkill. You can go directly to the sysfs path for a given block device by stat(1) on the block device special file to get the device major and minor numbers. Then go directly to the sysfs path for that device by following this link /sys/dev/block/$major:$minor. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-17 14:14 ` Dan Williams @ 2017-04-17 22:54 ` Dan Williams 2017-04-17 23:39 ` Ross Zwisler 2017-04-19 8:40 ` Xiong Zhou 1 sibling, 1 reply; 71+ messages in thread From: Dan Williams @ 2017-04-17 22:54 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, Ross Zwisler, jmoyer, Eryu Guan On Mon, Apr 17, 2017 at 7:14 AM, Dan Williams <dan.j.williams@intel.com> wrote: > On Mon, Apr 17, 2017 at 12:14 AM, Xiong Zhou <xzhou@redhat.com> wrote: >> Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then >> do mmap DIO from DAX to non-DAX. >> >> This test is split from generic/413. Since DIO from DAX >> to non-DAX is only supported/doable when device underneath >> has memory(struct page) backend. >> >> By ndctl looking at SCRATCH_DEV, skip this test if it is >> not in "memory mode". >> >> Adding helper to check pmem device status, which requires new >> PROGs ndctl to tweaking pmem devices and jq to parse ndctl's >> JSON format outputs. >> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> >> --- >> common/rc | 45 ++++++++++++++++++++ >> tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/423.out | 2 + >> tests/generic/group | 1 + >> 4 files changed, 161 insertions(+) >> create mode 100755 tests/generic/423 >> create mode 100644 tests/generic/423.out >> >> diff --git a/common/rc b/common/rc >> index 78a2101..73ac79c 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -3151,6 +3151,51 @@ _require_chattr() >> rm -f $TEST_DIR/syscalltest.out >> } >> >> +# Require test/scratch device nvdimm and having specific >> +# arttibute key/value we need. >> +# >> +# This is designed to get attr values of nvdimm persistent >> +# memory device, by searching sysfs. >> +# >> +# Other non-nvdimm or non-persistent-memory devices would >> +# fail this helper anyway. >> +# >> +# So, ONLY use this helper when you REALLY need nvdimm and >> +# specific attr on it. >> +# >> +_require_pmem_key_value() >> +{ >> + local dev=${1#/dev/} >> + local key=$2 >> + local value=$3 >> + local region index keyfile dev_value >> + >> + # find a filename string contains the region of dev, eg: >> + # /sys/devices/platform/e820_pmem/ndbus0/region1/\ >> + # namespace1.0/block/pmem1 >> + # >> + region=$(find /sys/ | grep $dev | grep region | head -1) >> + [ -z "$region" ] && \ >> + _notrun "requires persistent memory $dev $key $value" > > Running 'find' in sysfs is overkill. You can go directly to the sysfs > path for a given block device by stat(1) on the block device special > file to get the device major and minor numbers. Then go directly to > the sysfs path for that device by following this link > /sys/dev/block/$major:$minor. Of course, this is all handled for you if you just use ndctl, so I'm not sure what the motivation is to bypass those tools? They are widely available. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-17 22:54 ` Dan Williams @ 2017-04-17 23:39 ` Ross Zwisler 2017-04-17 23:47 ` Dan Williams 0 siblings, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-04-17 23:39 UTC (permalink / raw) To: Dan Williams; +Cc: Xiong Zhou, fstests, Ross Zwisler, jmoyer, Eryu Guan On Mon, Apr 17, 2017 at 03:54:30PM -0700, Dan Williams wrote: > On Mon, Apr 17, 2017 at 7:14 AM, Dan Williams <dan.j.williams@intel.com> wrote: > > On Mon, Apr 17, 2017 at 12:14 AM, Xiong Zhou <xzhou@redhat.com> wrote: > >> Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > >> do mmap DIO from DAX to non-DAX. > >> > >> This test is split from generic/413. Since DIO from DAX > >> to non-DAX is only supported/doable when device underneath > >> has memory(struct page) backend. > >> > >> By ndctl looking at SCRATCH_DEV, skip this test if it is > >> not in "memory mode". > >> > >> Adding helper to check pmem device status, which requires new > >> PROGs ndctl to tweaking pmem devices and jq to parse ndctl's > >> JSON format outputs. > >> > >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> > >> --- > >> common/rc | 45 ++++++++++++++++++++ > >> tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >> tests/generic/423.out | 2 + > >> tests/generic/group | 1 + > >> 4 files changed, 161 insertions(+) > >> create mode 100755 tests/generic/423 > >> create mode 100644 tests/generic/423.out > >> > >> diff --git a/common/rc b/common/rc > >> index 78a2101..73ac79c 100644 > >> --- a/common/rc > >> +++ b/common/rc > >> @@ -3151,6 +3151,51 @@ _require_chattr() > >> rm -f $TEST_DIR/syscalltest.out > >> } > >> > >> +# Require test/scratch device nvdimm and having specific > >> +# arttibute key/value we need. > >> +# > >> +# This is designed to get attr values of nvdimm persistent > >> +# memory device, by searching sysfs. > >> +# > >> +# Other non-nvdimm or non-persistent-memory devices would > >> +# fail this helper anyway. > >> +# > >> +# So, ONLY use this helper when you REALLY need nvdimm and > >> +# specific attr on it. > >> +# > >> +_require_pmem_key_value() > >> +{ > >> + local dev=${1#/dev/} > >> + local key=$2 > >> + local value=$3 > >> + local region index keyfile dev_value > >> + > >> + # find a filename string contains the region of dev, eg: > >> + # /sys/devices/platform/e820_pmem/ndbus0/region1/\ > >> + # namespace1.0/block/pmem1 > >> + # > >> + region=$(find /sys/ | grep $dev | grep region | head -1) > >> + [ -z "$region" ] && \ > >> + _notrun "requires persistent memory $dev $key $value" > > > > Running 'find' in sysfs is overkill. You can go directly to the sysfs > > path for a given block device by stat(1) on the block device special > > file to get the device major and minor numbers. Then go directly to > > the sysfs path for that device by following this link > > /sys/dev/block/$major:$minor. > > Of course, this is all handled for you if you just use ndctl, so I'm > not sure what the motivation is to bypass those tools? They are widely > available. Yea, I think I may have lead Xiong in the wrong direction. :-/ I thought that we didn't need to worry about the case where we had both NFIT generated PMEM devices and memmap generated PMEM devices, and if that was true we could basically have a shortcut in the memmap case and not add the extra dependencies of ndctl and jq. In thinking about it a bit more, though, I think Dan's right about that basically being a premature optimization. We probably do need to consider that case, and future-proof ourselves against other non-NFIT PMEM regions on other architectures. The sysfs interface isn't meant to be stable, though, and ndctl is stable, so I think we're better off going back to using ndctl + jq. Sorry for the thrash. I think your v3 changes in common/rc do exactly what you need, and you just need to pull out the lines that special case the memmap case and you're done. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-17 23:39 ` Ross Zwisler @ 2017-04-17 23:47 ` Dan Williams 0 siblings, 0 replies; 71+ messages in thread From: Dan Williams @ 2017-04-17 23:47 UTC (permalink / raw) To: Ross Zwisler; +Cc: Xiong Zhou, fstests, jmoyer, Eryu Guan On Mon, Apr 17, 2017 at 4:39 PM, Ross Zwisler <ross.zwisler@linux.intel.com> wrote: > On Mon, Apr 17, 2017 at 03:54:30PM -0700, Dan Williams wrote: >> On Mon, Apr 17, 2017 at 7:14 AM, Dan Williams <dan.j.williams@intel.com> wrote: >> > On Mon, Apr 17, 2017 at 12:14 AM, Xiong Zhou <xzhou@redhat.com> wrote: >> >> Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then >> >> do mmap DIO from DAX to non-DAX. >> >> >> >> This test is split from generic/413. Since DIO from DAX >> >> to non-DAX is only supported/doable when device underneath >> >> has memory(struct page) backend. >> >> >> >> By ndctl looking at SCRATCH_DEV, skip this test if it is >> >> not in "memory mode". >> >> >> >> Adding helper to check pmem device status, which requires new >> >> PROGs ndctl to tweaking pmem devices and jq to parse ndctl's >> >> JSON format outputs. >> >> >> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> >> >> --- >> >> common/rc | 45 ++++++++++++++++++++ >> >> tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> >> tests/generic/423.out | 2 + >> >> tests/generic/group | 1 + >> >> 4 files changed, 161 insertions(+) >> >> create mode 100755 tests/generic/423 >> >> create mode 100644 tests/generic/423.out >> >> >> >> diff --git a/common/rc b/common/rc >> >> index 78a2101..73ac79c 100644 >> >> --- a/common/rc >> >> +++ b/common/rc >> >> @@ -3151,6 +3151,51 @@ _require_chattr() >> >> rm -f $TEST_DIR/syscalltest.out >> >> } >> >> >> >> +# Require test/scratch device nvdimm and having specific >> >> +# arttibute key/value we need. >> >> +# >> >> +# This is designed to get attr values of nvdimm persistent >> >> +# memory device, by searching sysfs. >> >> +# >> >> +# Other non-nvdimm or non-persistent-memory devices would >> >> +# fail this helper anyway. >> >> +# >> >> +# So, ONLY use this helper when you REALLY need nvdimm and >> >> +# specific attr on it. >> >> +# >> >> +_require_pmem_key_value() >> >> +{ >> >> + local dev=${1#/dev/} >> >> + local key=$2 >> >> + local value=$3 >> >> + local region index keyfile dev_value >> >> + >> >> + # find a filename string contains the region of dev, eg: >> >> + # /sys/devices/platform/e820_pmem/ndbus0/region1/\ >> >> + # namespace1.0/block/pmem1 >> >> + # >> >> + region=$(find /sys/ | grep $dev | grep region | head -1) >> >> + [ -z "$region" ] && \ >> >> + _notrun "requires persistent memory $dev $key $value" >> > >> > Running 'find' in sysfs is overkill. You can go directly to the sysfs >> > path for a given block device by stat(1) on the block device special >> > file to get the device major and minor numbers. Then go directly to >> > the sysfs path for that device by following this link >> > /sys/dev/block/$major:$minor. >> >> Of course, this is all handled for you if you just use ndctl, so I'm >> not sure what the motivation is to bypass those tools? They are widely >> available. > > Yea, I think I may have lead Xiong in the wrong direction. :-/ I thought > that we didn't need to worry about the case where we had both NFIT generated > PMEM devices and memmap generated PMEM devices, and if that was true we could > basically have a shortcut in the memmap case and not add the extra > dependencies of ndctl and jq. > > In thinking about it a bit more, though, I think Dan's right about that > basically being a premature optimization. We probably do need to consider > that case, and future-proof ourselves against other non-NFIT PMEM regions on > other architectures. The sysfs interface isn't meant to be stable, though, > and ndctl is stable, so I think we're better off going back to using ndctl + > jq. Sorry for the thrash. One additional clarification, the sysfs interface is stable from an ABI perspective. What is fluid is the arrival of certain attributes relative to others due to asynchronous probing. The ndctl tool handles this asynchronous behavior. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-17 14:14 ` Dan Williams 2017-04-17 22:54 ` Dan Williams @ 2017-04-19 8:40 ` Xiong Zhou 2017-04-19 15:53 ` Dan Williams 1 sibling, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-19 8:40 UTC (permalink / raw) To: Dan Williams; +Cc: Xiong Zhou, fstests, Ross Zwisler, jmoyer, Eryu Guan On Mon, Apr 17, 2017 at 07:14:32AM -0700, Dan Williams wrote: > On Mon, Apr 17, 2017 at 12:14 AM, Xiong Zhou <xzhou@redhat.com> wrote: > > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > > do mmap DIO from DAX to non-DAX. > > > > This test is split from generic/413. Since DIO from DAX > > to non-DAX is only supported/doable when device underneath > > has memory(struct page) backend. > > > > By ndctl looking at SCRATCH_DEV, skip this test if it is > > not in "memory mode". > > > > Adding helper to check pmem device status, which requires new > > PROGs ndctl to tweaking pmem devices and jq to parse ndctl's > > JSON format outputs. > > > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > --- > > common/rc | 45 ++++++++++++++++++++ > > tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/423.out | 2 + > > tests/generic/group | 1 + > > 4 files changed, 161 insertions(+) > > create mode 100755 tests/generic/423 > > create mode 100644 tests/generic/423.out > > > > diff --git a/common/rc b/common/rc > > index 78a2101..73ac79c 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -3151,6 +3151,51 @@ _require_chattr() > > rm -f $TEST_DIR/syscalltest.out > > } > > > > +# Require test/scratch device nvdimm and having specific > > +# arttibute key/value we need. > > +# > > +# This is designed to get attr values of nvdimm persistent > > +# memory device, by searching sysfs. > > +# > > +# Other non-nvdimm or non-persistent-memory devices would > > +# fail this helper anyway. > > +# > > +# So, ONLY use this helper when you REALLY need nvdimm and > > +# specific attr on it. > > +# > > +_require_pmem_key_value() > > +{ > > + local dev=${1#/dev/} > > + local key=$2 > > + local value=$3 > > + local region index keyfile dev_value > > + > > + # find a filename string contains the region of dev, eg: > > + # /sys/devices/platform/e820_pmem/ndbus0/region1/\ > > + # namespace1.0/block/pmem1 > > + # > > + region=$(find /sys/ | grep $dev | grep region | head -1) > > + [ -z "$region" ] && \ > > + _notrun "requires persistent memory $dev $key $value" > > Running 'find' in sysfs is overkill. You can go directly to the sysfs > path for a given block device by stat(1) on the block device special > file to get the device major and minor numbers. Then go directly to > the sysfs path for that device by following this link > /sys/dev/block/$major:$minor. This sounds better. While I have trouble with getting the right region mode through this sysfs path. sh-4.2# ndctl list -r 2 -N { "dev":"namespace2.0", "mode":"raw", "size":8589934592, "blockdev":"pmem2" } sh-4.2# cat /sys/devices/LNXSYSTM:00/device:00/ACPI0012:00/ndbus0/region2/namespace2.0/mode raw sh-4.2# stat -c %t:%T /dev/pmem2 103:2 sh-4.2# cat /sys/dev/block/259\:2/device/mode raw sh-4.2# ndctl create-namespace -f -e namespace2.0 -m memory { "dev":"namespace2.0", "mode":"memory", "size":8453619712, "uuid":"66d5afc1-6876-4e8b-a0ba-0082af84bf82", "blockdev":"pmem2" } sh-4.2# stat -c %t:%T /dev/pmem2 103:2 sh-4.2# cat /sys/dev/block/259\:2/device/mode # Here pmem sh-4.2# cat /sys/devices/LNXSYSTM:00/device:00/ACPI0012:00/ndbus0/region2/namespace2.0/mode memory sh-4.2# ndctl list -r 2 -N { "dev":"namespace2.0", "mode":"memory", "size":8453619712, "uuid":"66d5afc1-6876-4e8b-a0ba-0082af84bf82", "blockdev":"pmem2" } sh-4.2# ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-19 8:40 ` Xiong Zhou @ 2017-04-19 15:53 ` Dan Williams 0 siblings, 0 replies; 71+ messages in thread From: Dan Williams @ 2017-04-19 15:53 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, Ross Zwisler, jmoyer, Eryu Guan On Wed, Apr 19, 2017 at 1:40 AM, Xiong Zhou <xzhou@redhat.com> wrote: > On Mon, Apr 17, 2017 at 07:14:32AM -0700, Dan Williams wrote: >> On Mon, Apr 17, 2017 at 12:14 AM, Xiong Zhou <xzhou@redhat.com> wrote: >> > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then >> > do mmap DIO from DAX to non-DAX. >> > >> > This test is split from generic/413. Since DIO from DAX >> > to non-DAX is only supported/doable when device underneath >> > has memory(struct page) backend. >> > >> > By ndctl looking at SCRATCH_DEV, skip this test if it is >> > not in "memory mode". >> > >> > Adding helper to check pmem device status, which requires new >> > PROGs ndctl to tweaking pmem devices and jq to parse ndctl's >> > JSON format outputs. >> > >> > Signed-off-by: Xiong Zhou <xzhou@redhat.com> >> > --- >> > common/rc | 45 ++++++++++++++++++++ >> > tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> > tests/generic/423.out | 2 + >> > tests/generic/group | 1 + >> > 4 files changed, 161 insertions(+) >> > create mode 100755 tests/generic/423 >> > create mode 100644 tests/generic/423.out >> > >> > diff --git a/common/rc b/common/rc >> > index 78a2101..73ac79c 100644 >> > --- a/common/rc >> > +++ b/common/rc >> > @@ -3151,6 +3151,51 @@ _require_chattr() >> > rm -f $TEST_DIR/syscalltest.out >> > } >> > >> > +# Require test/scratch device nvdimm and having specific >> > +# arttibute key/value we need. >> > +# >> > +# This is designed to get attr values of nvdimm persistent >> > +# memory device, by searching sysfs. >> > +# >> > +# Other non-nvdimm or non-persistent-memory devices would >> > +# fail this helper anyway. >> > +# >> > +# So, ONLY use this helper when you REALLY need nvdimm and >> > +# specific attr on it. >> > +# >> > +_require_pmem_key_value() >> > +{ >> > + local dev=${1#/dev/} >> > + local key=$2 >> > + local value=$3 >> > + local region index keyfile dev_value >> > + >> > + # find a filename string contains the region of dev, eg: >> > + # /sys/devices/platform/e820_pmem/ndbus0/region1/\ >> > + # namespace1.0/block/pmem1 >> > + # >> > + region=$(find /sys/ | grep $dev | grep region | head -1) >> > + [ -z "$region" ] && \ >> > + _notrun "requires persistent memory $dev $key $value" >> >> Running 'find' in sysfs is overkill. You can go directly to the sysfs >> path for a given block device by stat(1) on the block device special >> file to get the device major and minor numbers. Then go directly to >> the sysfs path for that device by following this link > >> /sys/dev/block/$major:$minor. > > This sounds better. While I have trouble with getting the right region > mode through this sysfs path. > > sh-4.2# ndctl list -r 2 -N > { > "dev":"namespace2.0", > "mode":"raw", > "size":8589934592, > "blockdev":"pmem2" > } > sh-4.2# cat /sys/devices/LNXSYSTM:00/device:00/ACPI0012:00/ndbus0/region2/namespace2.0/mode > raw > sh-4.2# stat -c %t:%T /dev/pmem2 > 103:2 > sh-4.2# cat /sys/dev/block/259\:2/device/mode > raw > sh-4.2# ndctl create-namespace -f -e namespace2.0 -m memory > { > "dev":"namespace2.0", > "mode":"memory", > "size":8453619712, > "uuid":"66d5afc1-6876-4e8b-a0ba-0082af84bf82", > "blockdev":"pmem2" > } > sh-4.2# stat -c %t:%T /dev/pmem2 > 103:2 > sh-4.2# cat /sys/dev/block/259\:2/device/mode # Here > pmem This is another reason to use ndctl and not sysfs directly. After the create-namespace command the namespace is "claimed" by a pfn-device instance which writes metadata to the device to indicate where the struct page memmap is located. In this case the sysfs path for the parent device is a pfn-device. So the "device/mode" link is pointing to: /sys/devices/LNXSYSTM:00/device:00/ACPI0012:00/ndbus0/region2/pfn2.0/mode ...and in that case "pmem" means that the struct page memmap is being allocated on the pmem range directly. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-17 7:14 ` [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou 2017-04-17 14:14 ` Dan Williams @ 2017-04-18 10:12 ` Christoph Hellwig 2017-04-18 14:49 ` Xiong Zhou 2017-07-28 14:55 ` Jeff Moyer 2017-04-18 16:32 ` Ross Zwisler 2 siblings, 2 replies; 71+ messages in thread From: Christoph Hellwig @ 2017-04-18 10:12 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Mon, Apr 17, 2017 at 03:14:13PM +0800, Xiong Zhou wrote: > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > do mmap DIO from DAX to non-DAX. > > This test is split from generic/413. Since DIO from DAX > to non-DAX is only supported/doable when device underneath > has memory(struct page) backend. > > By ndctl looking at SCRATCH_DEV, skip this test if it is > not in "memory mode". DAX devices don't need to be something using NFIT, so I don't think this method is correct. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-18 10:12 ` Christoph Hellwig @ 2017-04-18 14:49 ` Xiong Zhou 2017-07-28 14:55 ` Jeff Moyer 1 sibling, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-18 14:49 UTC (permalink / raw) To: Christoph Hellwig Cc: Xiong Zhou, fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Tue, Apr 18, 2017 at 03:12:26AM -0700, Christoph Hellwig wrote: > On Mon, Apr 17, 2017 at 03:14:13PM +0800, Xiong Zhou wrote: > > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > > do mmap DIO from DAX to non-DAX. > > > > This test is split from generic/413. Since DIO from DAX > > to non-DAX is only supported/doable when device underneath > > has memory(struct page) backend. > > > > By ndctl looking at SCRATCH_DEV, skip this test if it is > > not in "memory mode". > > DAX devices don't need to be something using NFIT, so I don't think this > method is correct. Ya. This commit message needs update. I'm trying another method, not being that nvdimm specific. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-18 10:12 ` Christoph Hellwig 2017-04-18 14:49 ` Xiong Zhou @ 2017-07-28 14:55 ` Jeff Moyer 1 sibling, 0 replies; 71+ messages in thread From: Jeff Moyer @ 2017-07-28 14:55 UTC (permalink / raw) To: Christoph Hellwig Cc: Xiong Zhou, fstests, ross.zwisler, dan.j.williams, eguan Christoph Hellwig <hch@infradead.org> writes: > On Mon, Apr 17, 2017 at 03:14:13PM +0800, Xiong Zhou wrote: >> Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then >> do mmap DIO from DAX to non-DAX. >> >> This test is split from generic/413. Since DIO from DAX >> to non-DAX is only supported/doable when device underneath >> has memory(struct page) backend. >> >> By ndctl looking at SCRATCH_DEV, skip this test if it is >> not in "memory mode". > > DAX devices don't need to be something using NFIT, so I don't think this > method is correct. Memory mode does not depend on an NFIT. There's a superblock stored in the persistent memory that tracks its mode when labels are not present. -Jeff ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX 2017-04-17 7:14 ` [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou 2017-04-17 14:14 ` Dan Williams 2017-04-18 10:12 ` Christoph Hellwig @ 2017-04-18 16:32 ` Ross Zwisler 2 siblings, 0 replies; 71+ messages in thread From: Ross Zwisler @ 2017-04-18 16:32 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Mon, Apr 17, 2017 at 03:14:13PM +0800, Xiong Zhou wrote: > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > do mmap DIO from DAX to non-DAX. > > This test is split from generic/413. Since DIO from DAX > to non-DAX is only supported/doable when device underneath > has memory(struct page) backend. > > By ndctl looking at SCRATCH_DEV, skip this test if it is > not in "memory mode". > > Adding helper to check pmem device status, which requires new > PROGs ndctl to tweaking pmem devices and jq to parse ndctl's > JSON format outputs. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > common/rc | 45 ++++++++++++++++++++ > tests/generic/423 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ generic/423 looks good to me, just need to change the memory mode detection. ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v4 3/4] xfs: test per-inode DAX flag DAX to non-DAX 2017-04-17 7:14 ` [PATCH v4 0/4] split DAX mmap DIO cases Xiong Zhou 2017-04-17 7:14 ` [PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou 2017-04-17 7:14 ` [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou @ 2017-04-17 7:14 ` Xiong Zhou 2017-04-18 16:36 ` Ross Zwisler 2017-04-17 7:14 ` [PATCH v4 4/4] generic: mmap write readonly DAX file Xiong Zhou 2017-09-25 8:40 ` [PATCH v5 0/3] fix dax to nondax dio fake failures Xiong Zhou 4 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-04-17 7:14 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou This test is split from xfs/260, only do IO from DAX to non-DAX. Since DIO from DAX to non-DAX is only supported/doable when device underneath has memory(struct page) backend. By ndctl looking at SCRATCH_DEV, skip this test if it is not in "memory mode". Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/xfs/288 | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/288.out | 2 + tests/xfs/group | 1 + 3 files changed, 114 insertions(+) create mode 100755 tests/xfs/288 create mode 100644 tests/xfs/288.out diff --git a/tests/xfs/288 b/tests/xfs/288 new file mode 100755 index 0000000..d296a36 --- /dev/null +++ b/tests/xfs/288 @@ -0,0 +1,111 @@ +#! /bin/bash +# FS QA Test 288 +# +# Test per-inode DAX flag by mmap direct/buffered IO. +# Split from xfs/260, only do the DAX to non-DAX part. +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat Inc. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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 xfs +_supported_os Linux +_require_scratch_dax +_require_test_program "feature" +_require_test_program "t_mmap_dio" +_require_xfs_io_command "chattr" "+/-x" +_require_xfs_io_command "falloc" +_require_pmem_key_value $SCRATCH_DEV "mode" "memory" + +prep_files() +{ + rm -f $SCRATCH_MNT/tf_{s,d} + + $XFS_IO_PROG -f -c "falloc 0 $tsize" \ + $SCRATCH_MNT/tf_{s,d} >> $seqres.full 2>&1 +} + +t_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 + src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \ + $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 + src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \ + $1 "buffered dax to nondax" +} + +do_tests() +{ + # less than page size + t_dax_to_nondax 1024 + # page size + t_dax_to_nondax `src/feature -s` + # bigger sizes, for PMD faults + t_dax_to_nondax $((16 * 1024 * 1024)) + t_dax_to_nondax $((64 * 1024 * 1024)) +} + +# make xfs 2Mb aligned for PMD fault testing +_scratch_mkfs "-d su=2m,sw=1" > /dev/null 2>&1 + +# mount with dax option +_scratch_mount "-o dax" + +tsize=$((128 * 1024 * 1024)) + +do_tests +_scratch_unmount +_check_scratch_fs + +# mount again without dax option +export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" +_scratch_mount +do_tests + +# success, all done +echo "Silence is golden" +status=0 +exit diff --git a/tests/xfs/288.out b/tests/xfs/288.out new file mode 100644 index 0000000..2958a5c --- /dev/null +++ b/tests/xfs/288.out @@ -0,0 +1,2 @@ +QA output created by 288 +Silence is golden diff --git a/tests/xfs/group b/tests/xfs/group index 75769f9..736de3c 100644 --- a/tests/xfs/group +++ b/tests/xfs/group @@ -285,6 +285,7 @@ 285 dangerous_fuzzers dangerous_scrub 286 dangerous_fuzzers dangerous_scrub dangerous_online_repair 287 auto dump quota quick +288 auto quick 290 auto rw prealloc quick ioctl zero 291 auto repair 292 auto mkfs quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v4 3/4] xfs: test per-inode DAX flag DAX to non-DAX 2017-04-17 7:14 ` [PATCH v4 3/4] xfs: test per-inode DAX flag " Xiong Zhou @ 2017-04-18 16:36 ` Ross Zwisler 0 siblings, 0 replies; 71+ messages in thread From: Ross Zwisler @ 2017-04-18 16:36 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Mon, Apr 17, 2017 at 03:14:14PM +0800, Xiong Zhou wrote: > This test is split from xfs/260, only do IO from DAX > to non-DAX. > > Since DIO from DAX to non-DAX is only supported/doable > when device underneath has memory(struct page) backend. > > By ndctl looking at SCRATCH_DEV, skip this test if it is > not in "memory mode". > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> Looks good. Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com> ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v4 4/4] generic: mmap write readonly DAX file 2017-04-17 7:14 ` [PATCH v4 0/4] split DAX mmap DIO cases Xiong Zhou ` (2 preceding siblings ...) 2017-04-17 7:14 ` [PATCH v4 3/4] xfs: test per-inode DAX flag " Xiong Zhou @ 2017-04-17 7:14 ` Xiong Zhou 2017-04-18 17:05 ` Ross Zwisler 2017-04-24 16:34 ` Ross Zwisler 2017-09-25 8:40 ` [PATCH v5 0/3] fix dax to nondax dio fake failures Xiong Zhou 4 siblings, 2 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-17 7:14 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Regression case that one can write to read-only file in a DAX mountpoint. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- .gitignore | 1 + src/Makefile | 2 +- src/t_mmap_write_ro.c | 77 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/424 | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/424.out | 2 ++ tests/generic/group | 1 + 6 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/424 create mode 100644 tests/generic/424.out diff --git a/.gitignore b/.gitignore index 1ed2a92..ee9329f 100644 --- a/.gitignore +++ b/.gitignore @@ -131,6 +131,7 @@ /src/renameat2 /src/t_rename_overwrite /src/t_mmap_dio +/src/t_mmap_write_ro # dmapi/ binaries /dmapi/src/common/cmd/read_invis diff --git a/src/Makefile b/src/Makefile index a7f27f0..2604f52 100644 --- a/src/Makefile +++ b/src/Makefile @@ -12,7 +12,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ godown resvtest writemod makeextents itrash rename \ multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ - holetest t_truncate_self t_mmap_dio + holetest t_truncate_self t_mmap_dio t_mmap_write_ro LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c new file mode 100644 index 0000000..08ec1d0 --- /dev/null +++ b/src/t_mmap_write_ro.c @@ -0,0 +1,77 @@ +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <fcntl.h> +#include <unistd.h> +#include <libgen.h> +#include <errno.h> +#include <sys/mman.h> + +void +err_exit(char *op) +{ + fprintf(stderr, "%s: %s\n", op, strerror(errno)); + exit(1); +} + +int +main(int argc, char **argv) +{ + int fd, pfd, ret; + char *buf; + /* gcc -O2 will optimize foo's storage, preventing + * reproduce this issue. + * foo is never actually used after fault in value stored. + */ + volatile char foo __attribute__((__unused__)); + int pagesize = getpagesize(); + + if (argc < 2) { + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); + exit(0); + } + + fd = open(argv[1], O_RDONLY|O_DIRECT); + if (fd < 0) + err_exit("open"); + + pfd = open(argv[2], O_RDONLY); + if (pfd < 0) + err_exit("pmem open"); + + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); + if (buf == MAP_FAILED) + err_exit("mmap"); + + /* + * Read from the DAX mmap to populate the first page in the + * address_space with a read-only mapping. + */ + foo = *buf; + + /* + * Now write to the DAX mmap. This *should* fail, but if the bug is + * present in __get_user_pages_fast(), it will succeed. + */ + ret = read(fd, buf, pagesize); + if (ret != pagesize) + err_exit("read"); + + ret = msync(buf, pagesize, MS_SYNC); + if (ret != 0) + err_exit("msync"); + + ret = munmap(buf, pagesize); + if (ret < 0) + err_exit("munmap"); + + ret = close(fd); + if (ret < 0) + err_exit("clsoe fd"); + + ret = close(pfd); + if (ret < 0) + err_exit("close pfd"); + + exit(0); +} diff --git a/tests/generic/424 b/tests/generic/424 new file mode 100755 index 0000000..f73e08a --- /dev/null +++ b/tests/generic/424 @@ -0,0 +1,92 @@ +#! /bin/bash +# FS QA Test 424 +# +# This is a regression test for kernel commit +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings +# created by Jeffrey Moyer <jmoyer@redhat.com> +# +# This is reproducible only when testing on pmem device +# which is configured in "memory mode", not in "raw mode". +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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 -rf $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# Modify as appropriate. +_supported_fs generic +_supported_os Linux +_require_scratch_dax +_require_test_program "t_mmap_write_ro" +_require_user + +# real QA test starts here + +_scratch_mkfs >>$seqres.full 2>&1 +_scratch_mount "-o dax" + +# remount TEST_DEV wo/ dax +export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" +_test_cycle_mount + +# prepare a 4k read-only DAX file, save its md5sum +$XFS_IO_PROG -f -c "pwrite -S 0xFFFF 0 4096" \ + $SCRATCH_MNT/readonlyfile >> $seqres.full 2>&1 +chmod 0644 $SCRATCH_MNT/readonlyfile +md5_1="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +# prepare another larger non-DAX file +$XFS_IO_PROG -f -c "pwrite -S 0x0000 0 40960" \ + $TEST_DIR/${seq}.largefile >> $seqres.full 2>&1 +# allow qa_user access +chown $qa_user $TEST_DIR/${seq}.largefile + +# run test programme, read another larger file writing into +# the read-only file with mmap, which should fail. +_user_do "src/t_mmap_write_ro $TEST_DIR/${seq}.largefile \ + $SCRATCH_MNT/readonlyfile" + +# read-only file should not get updated, md5sum again. +md5_2="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +[ "$md5_1" != "$md5_2" ] && echo "read only file changed" + +# success, all done +status=0 +exit diff --git a/tests/generic/424.out b/tests/generic/424.out new file mode 100644 index 0000000..c836ec8 --- /dev/null +++ b/tests/generic/424.out @@ -0,0 +1,2 @@ +QA output created by 424 +read: Bad address diff --git a/tests/generic/group b/tests/generic/group index 52553fa..81db660 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -426,3 +426,4 @@ 421 auto quick encrypt dangerous 422 auto quick 423 auto quick +424 auto quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v4 4/4] generic: mmap write readonly DAX file 2017-04-17 7:14 ` [PATCH v4 4/4] generic: mmap write readonly DAX file Xiong Zhou @ 2017-04-18 17:05 ` Ross Zwisler 2017-04-19 2:54 ` Xiong Zhou 2017-04-24 16:34 ` Ross Zwisler 1 sibling, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-04-18 17:05 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Mon, Apr 17, 2017 at 03:14:15PM +0800, Xiong Zhou wrote: > Regression case that one can write to read-only > file in a DAX mountpoint. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> Yep, this test works fine in my setup. It fails for me, as expected, with v4.10.0, and passes as expected with v4.10.3. I've added a few small comments below, but with those addressed you can add: Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com> > +int > +main(int argc, char **argv) > +{ > + int fd, pfd, ret; > + char *buf; > + /* gcc -O2 will optimize foo's storage, preventing > + * reproduce this issue. > + * foo is never actually used after fault in value stored. > + */ > + volatile char foo __attribute__((__unused__)); > + int pagesize = getpagesize(); > + > + if (argc < 2) { > + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); > + exit(0); > + } > + > + fd = open(argv[1], O_RDONLY|O_DIRECT); > + if (fd < 0) > + err_exit("open"); > + > + pfd = open(argv[2], O_RDONLY); > + if (pfd < 0) > + err_exit("pmem open"); > + > + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); > + if (buf == MAP_FAILED) > + err_exit("mmap"); > + > + /* > + * Read from the DAX mmap to populate the first page in the > + * address_space with a read-only mapping. > + */ > + foo = *buf; > + > + /* > + * Now write to the DAX mmap. This *should* fail, but if the bug is > + * present in __get_user_pages_fast(), it will succeed. > + */ > + ret = read(fd, buf, pagesize); > + if (ret != pagesize) > + err_exit("read"); > + > + ret = msync(buf, pagesize, MS_SYNC); > + if (ret != 0) > + err_exit("msync"); > + > + ret = munmap(buf, pagesize); > + if (ret < 0) > + err_exit("munmap"); > + > + ret = close(fd); > + if (ret < 0) > + err_exit("clsoe fd"); close > diff --git a/tests/generic/424 b/tests/generic/424 > new file mode 100755 > index 0000000..f73e08a > --- /dev/null > +++ b/tests/generic/424 > @@ -0,0 +1,92 @@ > +#! /bin/bash > +# FS QA Test 424 > +# > +# This is a regression test for kernel commit > +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings > +# created by Jeffrey Moyer <jmoyer@redhat.com> > +# > +# This is reproducible only when testing on pmem device > +# which is configured in "memory mode", not in "raw mode". > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2017 Red Hat. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +#----------------------------------------------------------------------- > +# > + > +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 -rf $tmp.* > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# Modify as appropriate. > +_supported_fs generic > +_supported_os Linux > +_require_scratch_dax > +_require_test_program "t_mmap_write_ro" > +_require_user Could also maybe benefit from "_require_test", which makes sure your TEST_DEV is a block device? I don't think you get this explicitly tested with _test_cycle_mount? I think you're also missing a: _require_pmem_key_value $SCRATCH_DEV "mode" "memory" ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 4/4] generic: mmap write readonly DAX file 2017-04-18 17:05 ` Ross Zwisler @ 2017-04-19 2:54 ` Xiong Zhou 0 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-04-19 2:54 UTC (permalink / raw) To: Ross Zwisler; +Cc: Xiong Zhou, fstests, dan.j.williams, jmoyer, eguan On Tue, Apr 18, 2017 at 11:05:06AM -0600, Ross Zwisler wrote: > On Mon, Apr 17, 2017 at 03:14:15PM +0800, Xiong Zhou wrote: > > Regression case that one can write to read-only > > file in a DAX mountpoint. > > > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > Yep, this test works fine in my setup. It fails for me, as expected, with > v4.10.0, and passes as expected with v4.10.3. I've added a few small comments > below, but with those addressed you can add: > > Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > > +int > > +main(int argc, char **argv) > > +{ > > + int fd, pfd, ret; > > + char *buf; > > + /* gcc -O2 will optimize foo's storage, preventing > > + * reproduce this issue. > > + * foo is never actually used after fault in value stored. > > + */ > > + volatile char foo __attribute__((__unused__)); > > + int pagesize = getpagesize(); > > + > > + if (argc < 2) { > > + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); > > + exit(0); > > + } > > + > > + fd = open(argv[1], O_RDONLY|O_DIRECT); > > + if (fd < 0) > > + err_exit("open"); > > + > > + pfd = open(argv[2], O_RDONLY); > > + if (pfd < 0) > > + err_exit("pmem open"); > > + > > + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); > > + if (buf == MAP_FAILED) > > + err_exit("mmap"); > > + > > + /* > > + * Read from the DAX mmap to populate the first page in the > > + * address_space with a read-only mapping. > > + */ > > + foo = *buf; > > + > > + /* > > + * Now write to the DAX mmap. This *should* fail, but if the bug is > > + * present in __get_user_pages_fast(), it will succeed. > > + */ > > + ret = read(fd, buf, pagesize); > > + if (ret != pagesize) > > + err_exit("read"); > > + > > + ret = msync(buf, pagesize, MS_SYNC); > > + if (ret != 0) > > + err_exit("msync"); > > + > > + ret = munmap(buf, pagesize); > > + if (ret < 0) > > + err_exit("munmap"); > > + > > + ret = close(fd); > > + if (ret < 0) > > + err_exit("clsoe fd"); > close > > > diff --git a/tests/generic/424 b/tests/generic/424 > > new file mode 100755 > > index 0000000..f73e08a > > --- /dev/null > > +++ b/tests/generic/424 > > @@ -0,0 +1,92 @@ > > +#! /bin/bash > > +# FS QA Test 424 > > +# > > +# This is a regression test for kernel commit > > +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings > > +# created by Jeffrey Moyer <jmoyer@redhat.com> > > +# > > +# This is reproducible only when testing on pmem device > > +# which is configured in "memory mode", not in "raw mode". > > +# > > +#----------------------------------------------------------------------- > > +# Copyright (c) 2017 Red Hat. All Rights Reserved. > > +# > > +# This program is free software; you can redistribute it and/or > > +# modify it under the terms of the GNU General Public License as > > +# published by the Free Software Foundation. > > +# > > +# This program is distributed in the hope that it would be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program; if not, write the Free Software Foundation, > > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > > +#----------------------------------------------------------------------- > > +# > > + > > +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 -rf $tmp.* > > +} > > + > > +# get standard environment, filters and checks > > +. ./common/rc > > +. ./common/filter > > + > > +# remove previous $seqres.full before test > > +rm -f $seqres.full > > + > > +# Modify as appropriate. > > +_supported_fs generic > > +_supported_os Linux > > +_require_scratch_dax > > +_require_test_program "t_mmap_write_ro" > > +_require_user > > Could also maybe benefit from "_require_test", which makes sure your TEST_DEV Yes! Good catch! > is a block device? I don't think you get this explicitly tested with > _test_cycle_mount? > > I think you're also missing a: > _require_pmem_key_value $SCRATCH_DEV "mode" "memory" No. I don't wanna limit this test running only on "memory mode" pmem devices because it CAN run on "raw mode" devices. More testing will not hurt, :) ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v4 4/4] generic: mmap write readonly DAX file 2017-04-17 7:14 ` [PATCH v4 4/4] generic: mmap write readonly DAX file Xiong Zhou 2017-04-18 17:05 ` Ross Zwisler @ 2017-04-24 16:34 ` Ross Zwisler 1 sibling, 0 replies; 71+ messages in thread From: Ross Zwisler @ 2017-04-24 16:34 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Mon, Apr 17, 2017 at 03:14:15PM +0800, Xiong Zhou wrote: > Regression case that one can write to read-only > file in a DAX mountpoint. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- <> > diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c > new file mode 100644 > index 0000000..08ec1d0 > --- /dev/null > +++ b/src/t_mmap_write_ro.c > @@ -0,0 +1,77 @@ > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <fcntl.h> > +#include <unistd.h> > +#include <libgen.h> > +#include <errno.h> > +#include <sys/mman.h> > + > +void > +err_exit(char *op) > +{ > + fprintf(stderr, "%s: %s\n", op, strerror(errno)); > + exit(1); > +} > + > +int > +main(int argc, char **argv) > +{ > + int fd, pfd, ret; > + char *buf; > + /* gcc -O2 will optimize foo's storage, preventing > + * reproduce this issue. > + * foo is never actually used after fault in value stored. > + */ > + volatile char foo __attribute__((__unused__)); > + int pagesize = getpagesize(); > + > + if (argc < 2) { > + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); This comparison should be: + if (argc < 3) { $ ./src/t_mmap_write_ro Usage: t_mmap_write_ro <file> <pmem file> $ ./src/t_mmap_write_ro a open: No such file or directory ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v5 0/3] fix dax to nondax dio fake failures 2017-04-17 7:14 ` [PATCH v4 0/4] split DAX mmap DIO cases Xiong Zhou ` (3 preceding siblings ...) 2017-04-17 7:14 ` [PATCH v4 4/4] generic: mmap write readonly DAX file Xiong Zhou @ 2017-09-25 8:40 ` Xiong Zhou 2017-09-25 8:40 ` [PATCH v5 1/3] generic: mmap write readonly DAX file Xiong Zhou ` (3 more replies) 4 siblings, 4 replies; 71+ messages in thread From: Xiong Zhou @ 2017-09-25 8:40 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou v5: Whatever the device is, skip this subtest if it failed a silent test run. Remove xfs/260 since per-inode flag has been disabled in kernel. This test now passes on 4.14-rc1+, however it's not what it intends to test. Minor fix on write_ro test. This one can be merged solely since it's not related to the other 2 patches. Xiong Zhou (3): generic: mmap write readonly DAX file generic/413: skip dax to nondax dio test if needed xfs/260: remove per-inode DAX flag test .gitignore | 1 + src/Makefile | 2 +- src/t_mmap_write_ro.c | 77 ++++++++++++++++++++++++ tests/generic/413 | 8 +++ tests/generic/461 | 93 +++++++++++++++++++++++++++++ tests/generic/461.out | 2 + tests/generic/group | 1 + tests/xfs/260 | 158 -------------------------------------------------- tests/xfs/260.out | 2 - tests/xfs/group | 1 - 10 files changed, 183 insertions(+), 162 deletions(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/461 create mode 100644 tests/generic/461.out delete mode 100755 tests/xfs/260 delete mode 100644 tests/xfs/260.out -- 1.8.3.1 ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v5 1/3] generic: mmap write readonly DAX file 2017-09-25 8:40 ` [PATCH v5 0/3] fix dax to nondax dio fake failures Xiong Zhou @ 2017-09-25 8:40 ` Xiong Zhou 2017-09-27 9:57 ` Eryu Guan 2017-09-25 8:40 ` [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed Xiong Zhou ` (2 subsequent siblings) 3 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-09-25 8:40 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Regression case that one can write to read-only file in a DAX mountpoint. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- .gitignore | 1 + src/Makefile | 2 +- src/t_mmap_write_ro.c | 77 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/461 | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/461.out | 2 ++ tests/generic/group | 1 + 6 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/461 create mode 100644 tests/generic/461.out diff --git a/.gitignore b/.gitignore index 63928b4..864d899 100644 --- a/.gitignore +++ b/.gitignore @@ -155,6 +155,7 @@ /src/t_mmap_cow_race /src/t_mmap_fallocate /src/log-writes/replay-log +/src/t_mmap_write_ro # dmapi/ binaries /dmapi/src/common/cmd/read_invis diff --git a/src/Makefile b/src/Makefile index 7d1306b..3eb25b1 100644 --- a/src/Makefile +++ b/src/Makefile @@ -13,7 +13,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd \ - t_mmap_cow_race t_mmap_fallocate fsync-err + t_mmap_cow_race t_mmap_fallocate fsync-err t_mmap_write_ro LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c new file mode 100644 index 0000000..55cb3d1 --- /dev/null +++ b/src/t_mmap_write_ro.c @@ -0,0 +1,77 @@ +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <fcntl.h> +#include <unistd.h> +#include <libgen.h> +#include <errno.h> +#include <sys/mman.h> + +void +err_exit(char *op) +{ + fprintf(stderr, "%s: %s\n", op, strerror(errno)); + exit(1); +} + +int +main(int argc, char **argv) +{ + int fd, pfd, ret; + char *buf; + /* gcc -O2 will optimize foo's storage, preventing + * reproduce this issue. + * foo is never actually used after fault in value stored. + */ + volatile char foo __attribute__((__unused__)); + int pagesize = getpagesize(); + + if (argc < 3) { + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); + exit(0); + } + + fd = open(argv[1], O_RDONLY|O_DIRECT); + if (fd < 0) + err_exit("open"); + + pfd = open(argv[2], O_RDONLY); + if (pfd < 0) + err_exit("pmem open"); + + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); + if (buf == MAP_FAILED) + err_exit("mmap"); + + /* + * Read from the DAX mmap to populate the first page in the + * address_space with a read-only mapping. + */ + foo = *buf; + + /* + * Now write to the DAX mmap. This *should* fail, but if the bug is + * present in __get_user_pages_fast(), it will succeed. + */ + ret = read(fd, buf, pagesize); + if (ret != pagesize) + err_exit("read"); + + ret = msync(buf, pagesize, MS_SYNC); + if (ret != 0) + err_exit("msync"); + + ret = munmap(buf, pagesize); + if (ret < 0) + err_exit("munmap"); + + ret = close(fd); + if (ret < 0) + err_exit("clsoe fd"); + + ret = close(pfd); + if (ret < 0) + err_exit("close pfd"); + + exit(0); +} diff --git a/tests/generic/461 b/tests/generic/461 new file mode 100755 index 0000000..a6df2b6 --- /dev/null +++ b/tests/generic/461 @@ -0,0 +1,93 @@ +#! /bin/bash +# FS QA Test 461 +# +# This is a regression test for kernel commit +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings +# created by Jeffrey Moyer <jmoyer@redhat.com> +# +# This is reproducible only when testing on pmem device +# which is configured in "memory mode", not in "raw mode". +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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 -rf $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# Modify as appropriate. +_supported_fs generic +_supported_os Linux +_require_test +_require_scratch_dax +_require_test_program "t_mmap_write_ro" +_require_user + +# real QA test starts here + +_scratch_mkfs >>$seqres.full 2>&1 +_scratch_mount "-o dax" + +# remount TEST_DEV wo/ dax +export MOUNT_OPTIONS="" +export TEST_FS_MOUNT_OPTS="" +_test_cycle_mount + +# prepare a 4k read-only DAX file, save its md5sum +$XFS_IO_PROG -f -c "pwrite -S 0xFFFF 0 4096" \ + $SCRATCH_MNT/readonlyfile >> $seqres.full 2>&1 +chmod 0644 $SCRATCH_MNT/readonlyfile +md5_1="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +# prepare another larger non-DAX file +$XFS_IO_PROG -f -c "pwrite -S 0x0000 0 40960" \ + $TEST_DIR/${seq}.largefile >> $seqres.full 2>&1 +# allow qa_user access +chown $qa_user $TEST_DIR/${seq}.largefile + +# run test programme, read another larger file writing into +# the read-only file with mmap, which should fail. +_user_do "src/t_mmap_write_ro $TEST_DIR/${seq}.largefile \ + $SCRATCH_MNT/readonlyfile" + +# read-only file should not get updated, md5sum again. +md5_2="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +[ "$md5_1" != "$md5_2" ] && echo "read only file changed" + +# success, all done +status=0 +exit diff --git a/tests/generic/461.out b/tests/generic/461.out new file mode 100644 index 0000000..8b1e29c --- /dev/null +++ b/tests/generic/461.out @@ -0,0 +1,2 @@ +QA output created by 461 +read: Bad address diff --git a/tests/generic/group b/tests/generic/group index f922b49..1575a2d 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -463,3 +463,4 @@ 458 auto quick clone 459 auto dangerous 460 auto quick rw +461 auto quick -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v5 1/3] generic: mmap write readonly DAX file 2017-09-25 8:40 ` [PATCH v5 1/3] generic: mmap write readonly DAX file Xiong Zhou @ 2017-09-27 9:57 ` Eryu Guan 0 siblings, 0 replies; 71+ messages in thread From: Eryu Guan @ 2017-09-27 9:57 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer On Mon, Sep 25, 2017 at 04:40:45PM +0800, Xiong Zhou wrote: > Regression case that one can write to read-only > file in a DAX mountpoint. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > .gitignore | 1 + > src/Makefile | 2 +- > src/t_mmap_write_ro.c | 77 ++++++++++++++++++++++++++++++++++++++++++ > tests/generic/461 | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/461.out | 2 ++ > tests/generic/group | 1 + > 6 files changed, 175 insertions(+), 1 deletion(-) > create mode 100644 src/t_mmap_write_ro.c > create mode 100755 tests/generic/461 > create mode 100644 tests/generic/461.out > > diff --git a/.gitignore b/.gitignore > index 63928b4..864d899 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -155,6 +155,7 @@ > /src/t_mmap_cow_race > /src/t_mmap_fallocate > /src/log-writes/replay-log > +/src/t_mmap_write_ro Better to make these entries sorted, perhaps in a separate patch. > > # dmapi/ binaries > /dmapi/src/common/cmd/read_invis > diff --git a/src/Makefile b/src/Makefile > index 7d1306b..3eb25b1 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -13,7 +13,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ > multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ > t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ > holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd \ > - t_mmap_cow_race t_mmap_fallocate fsync-err > + t_mmap_cow_race t_mmap_fallocate fsync-err t_mmap_write_ro > > LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ > diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c > new file mode 100644 > index 0000000..55cb3d1 > --- /dev/null > +++ b/src/t_mmap_write_ro.c > @@ -0,0 +1,77 @@ > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <fcntl.h> > +#include <unistd.h> > +#include <libgen.h> > +#include <errno.h> > +#include <sys/mman.h> > + > +void > +err_exit(char *op) > +{ > + fprintf(stderr, "%s: %s\n", op, strerror(errno)); > + exit(1); > +} > + > +int > +main(int argc, char **argv) > +{ > + int fd, pfd, ret; > + char *buf; > + /* gcc -O2 will optimize foo's storage, preventing Nitpick, let "/*" stays in the line alone for multi-line comments. /* * gcc -O2... * ... */ > + * reproduce this issue. > + * foo is never actually used after fault in value stored. > + */ > + volatile char foo __attribute__((__unused__)); > + int pagesize = getpagesize(); > + > + if (argc < 3) { > + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); > + exit(0); > + } > + > + fd = open(argv[1], O_RDONLY|O_DIRECT); Is O_DIRECT really required? If so, some comments would be good. > + if (fd < 0) > + err_exit("open"); > + > + pfd = open(argv[2], O_RDONLY); > + if (pfd < 0) > + err_exit("pmem open"); > + > + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); > + if (buf == MAP_FAILED) > + err_exit("mmap"); > + > + /* > + * Read from the DAX mmap to populate the first page in the > + * address_space with a read-only mapping. > + */ > + foo = *buf; > + > + /* > + * Now write to the DAX mmap. This *should* fail, but if the bug is > + * present in __get_user_pages_fast(), it will succeed. > + */ > + ret = read(fd, buf, pagesize); > + if (ret != pagesize) > + err_exit("read"); > + > + ret = msync(buf, pagesize, MS_SYNC); > + if (ret != 0) > + err_exit("msync"); > + > + ret = munmap(buf, pagesize); > + if (ret < 0) > + err_exit("munmap"); > + > + ret = close(fd); > + if (ret < 0) > + err_exit("clsoe fd"); > + > + ret = close(pfd); > + if (ret < 0) > + err_exit("close pfd"); > + > + exit(0); > +} > diff --git a/tests/generic/461 b/tests/generic/461 > new file mode 100755 > index 0000000..a6df2b6 > --- /dev/null > +++ b/tests/generic/461 > @@ -0,0 +1,93 @@ > +#! /bin/bash > +# FS QA Test 461 > +# > +# This is a regression test for kernel commit > +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings > +# created by Jeffrey Moyer <jmoyer@redhat.com> > +# > +# This is reproducible only when testing on pmem device > +# which is configured in "memory mode", not in "raw mode". > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2017 Red Hat. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +#----------------------------------------------------------------------- > +# > + > +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 -rf $tmp.* > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# Modify as appropriate. > +_supported_fs generic > +_supported_os Linux > +_require_test > +_require_scratch_dax > +_require_test_program "t_mmap_write_ro" > +_require_user Hmm, not sure why we need the test run by unprivileged user just from the test. > + > +# real QA test starts here > + > +_scratch_mkfs >>$seqres.full 2>&1 > +_scratch_mount "-o dax" > + > +# remount TEST_DEV wo/ dax > +export MOUNT_OPTIONS="" This is not needed. > +export TEST_FS_MOUNT_OPTS="" > +_test_cycle_mount > + > +# prepare a 4k read-only DAX file, save its md5sum > +$XFS_IO_PROG -f -c "pwrite -S 0xFFFF 0 4096" \ "pwrite -S 0xFF" should be sufficient. > + $SCRATCH_MNT/readonlyfile >> $seqres.full 2>&1 > +chmod 0644 $SCRATCH_MNT/readonlyfile > +md5_1="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" > + > +# prepare another larger non-DAX file > +$XFS_IO_PROG -f -c "pwrite -S 0x0000 0 40960" \ > + $TEST_DIR/${seq}.largefile >> $seqres.full 2>&1 > +# allow qa_user access > +chown $qa_user $TEST_DIR/${seq}.largefile > + > +# run test programme, read another larger file writing into > +# the read-only file with mmap, which should fail. > +_user_do "src/t_mmap_write_ro $TEST_DIR/${seq}.largefile \ > + $SCRATCH_MNT/readonlyfile" > + > +# read-only file should not get updated, md5sum again. > +md5_2="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" > + > +[ "$md5_1" != "$md5_2" ] && echo "read only file changed" > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/461.out b/tests/generic/461.out > new file mode 100644 > index 0000000..8b1e29c > --- /dev/null > +++ b/tests/generic/461.out > @@ -0,0 +1,2 @@ > +QA output created by 461 > +read: Bad address > diff --git a/tests/generic/group b/tests/generic/group > index f922b49..1575a2d 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -463,3 +463,4 @@ > 458 auto quick clone > 459 auto dangerous > 460 auto quick rw > +461 auto quick Time for a new 'dax' group? Perhaps add 'dax' group to other existing tests too in a separate patch. Thanks, Eryu ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed 2017-09-25 8:40 ` [PATCH v5 0/3] fix dax to nondax dio fake failures Xiong Zhou 2017-09-25 8:40 ` [PATCH v5 1/3] generic: mmap write readonly DAX file Xiong Zhou @ 2017-09-25 8:40 ` Xiong Zhou 2017-09-27 9:50 ` Eryu Guan 2017-09-25 8:40 ` [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 0/4] fix dax to nondax dio fake failures Xiong Zhou 3 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-09-25 8:40 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Since not all devices support dax has struct page backend, which will not support this test. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/generic/413 | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/generic/413 b/tests/generic/413 index a1cc514..b86c10f 100755 --- a/tests/generic/413 +++ b/tests/generic/413 @@ -88,6 +88,14 @@ t_nondax_to_dax() t_dax_to_nondax() { prep_files + # dax to nondax dio needs struct page backend, which is + # not always avaiable among various devices. Skip this + # subtest if not compatible. + if ! src/t_mmap_dio $SCRATCH_MNT/tf_s \ + $TEST_DIR/tf_d $1 "test" > /dev/null 2>&1 ; then + return + fi + src/t_mmap_dio $SCRATCH_MNT/tf_s \ $TEST_DIR/tf_d $1 "dio dax to nondax" -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed 2017-09-25 8:40 ` [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed Xiong Zhou @ 2017-09-27 9:50 ` Eryu Guan 2017-09-27 15:01 ` Jeff Moyer 0 siblings, 1 reply; 71+ messages in thread From: Eryu Guan @ 2017-09-27 9:50 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer On Mon, Sep 25, 2017 at 04:40:46PM +0800, Xiong Zhou wrote: > Since not all devices support dax has struct page backend, > which will not support this test. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > tests/generic/413 | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tests/generic/413 b/tests/generic/413 > index a1cc514..b86c10f 100755 > --- a/tests/generic/413 > +++ b/tests/generic/413 > @@ -88,6 +88,14 @@ t_nondax_to_dax() > t_dax_to_nondax() > { > prep_files > + # dax to nondax dio needs struct page backend, which is > + # not always avaiable among various devices. Skip this > + # subtest if not compatible. > + if ! src/t_mmap_dio $SCRATCH_MNT/tf_s \ > + $TEST_DIR/tf_d $1 "test" > /dev/null 2>&1 ; then > + return > + fi > + Then we will never get a failure from this case, even if it's a real bug.. We need better way to tell if there's struct page present :) Thanks, Eryu > src/t_mmap_dio $SCRATCH_MNT/tf_s \ > $TEST_DIR/tf_d $1 "dio dax to nondax" > > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed 2017-09-27 9:50 ` Eryu Guan @ 2017-09-27 15:01 ` Jeff Moyer 0 siblings, 0 replies; 71+ messages in thread From: Jeff Moyer @ 2017-09-27 15:01 UTC (permalink / raw) To: Eryu Guan; +Cc: Xiong Zhou, fstests, ross.zwisler, dan.j.williams Eryu Guan <eguan@redhat.com> writes: > On Mon, Sep 25, 2017 at 04:40:46PM +0800, Xiong Zhou wrote: >> Since not all devices support dax has struct page backend, >> which will not support this test. >> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> >> --- >> tests/generic/413 | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/tests/generic/413 b/tests/generic/413 >> index a1cc514..b86c10f 100755 >> --- a/tests/generic/413 >> +++ b/tests/generic/413 >> @@ -88,6 +88,14 @@ t_nondax_to_dax() >> t_dax_to_nondax() >> { >> prep_files >> + # dax to nondax dio needs struct page backend, which is >> + # not always avaiable among various devices. Skip this >> + # subtest if not compatible. >> + if ! src/t_mmap_dio $SCRATCH_MNT/tf_s \ >> + $TEST_DIR/tf_d $1 "test" > /dev/null 2>&1 ; then >> + return >> + fi >> + > > Then we will never get a failure from this case, even if it's a real > bug.. We need better way to tell if there's struct page present :) ndctl list will tell you the mode of the namespace. If it's 'raw', then it doesn't have struct page backing. If it's 'memory', it should work fine. -Jeff ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test 2017-09-25 8:40 ` [PATCH v5 0/3] fix dax to nondax dio fake failures Xiong Zhou 2017-09-25 8:40 ` [PATCH v5 1/3] generic: mmap write readonly DAX file Xiong Zhou 2017-09-25 8:40 ` [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed Xiong Zhou @ 2017-09-25 8:40 ` Xiong Zhou 2017-09-25 16:59 ` Ross Zwisler 2017-09-28 7:41 ` [PATCH v6 0/4] fix dax to nondax dio fake failures Xiong Zhou 3 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-09-25 8:40 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou This flag has been disabled upstream kernel. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/xfs/260 | 158 ------------------------------------------------------ tests/xfs/260.out | 2 - tests/xfs/group | 1 - 3 files changed, 161 deletions(-) delete mode 100755 tests/xfs/260 delete mode 100644 tests/xfs/260.out diff --git a/tests/xfs/260 b/tests/xfs/260 deleted file mode 100755 index e613cc0..0000000 --- a/tests/xfs/260 +++ /dev/null @@ -1,158 +0,0 @@ -#! /bin/bash -# FS QA Test 260 -# -# Test per-inode DAX flag by mmap direct/buffered IO. -# -#----------------------------------------------------------------------- -# Copyright (c) 2017 Red Hat Inc. All Rights Reserved. -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it would be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write the Free Software Foundation, -# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA -#----------------------------------------------------------------------- -# - -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 xfs -_supported_os Linux -_require_scratch_dax -_require_test_program "feature" -_require_test_program "t_mmap_dio" -_require_xfs_io_command "chattr" "+/-x" -_require_xfs_io_command "falloc" - -prep_files() -{ - rm -f $SCRATCH_MNT/tf_{s,d} - - $XFS_IO_PROG -f -c "falloc 0 $tsize" \ - $SCRATCH_MNT/tf_{s,d} >> $seqres.full 2>&1 -} - -t_both_dax() -{ - prep_files - $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_{s,d} - # with O_DIRECT first - src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} $1 "dio both dax" - - prep_files - $XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_{s,d} - # again with buffered IO - src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \ - $1 "buffered both dax" -} - -t_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 - src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \ - $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 - src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \ - $1 "buffered nondax to dax" -} - -t_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 - src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \ - $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 - src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \ - $1 "buffered dax to nondax" -} - -t_both_nondax() -{ - prep_files - $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_{s,d} - src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \ - $1 "dio both nondax" - - prep_files - $XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_{s,d} - src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \ - $1 "buffered both nondax" -} - -# $1 mmap read/write size -t_dax_flag_mmap_dio() -{ - t_both_dax $1 - t_dax_to_nondax $1 - t_nondax_to_dax $1 - t_both_nondax $1 -} - -do_tests() -{ - # less than page size - t_dax_flag_mmap_dio 1024 - # page size - t_dax_flag_mmap_dio `src/feature -s` - # bigger sizes, for PMD faults - t_dax_flag_mmap_dio $((16 * 1024 * 1024)) - 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 - -# 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 -do_tests - -# success, all done -echo "Silence is golden" -status=0 -exit diff --git a/tests/xfs/260.out b/tests/xfs/260.out deleted file mode 100644 index 18ca517..0000000 --- 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 0a449b9..2b89f0e 100644 --- a/tests/xfs/group +++ b/tests/xfs/group @@ -256,7 +256,6 @@ 257 auto quick clone 258 auto quick clone 259 auto quick -260 auto attr quick 261 auto quick quota 262 dangerous_fuzzers dangerous_scrub dangerous_online_repair 263 auto quick quota -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test 2017-09-25 8:40 ` [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test Xiong Zhou @ 2017-09-25 16:59 ` Ross Zwisler 2017-09-26 0:51 ` Xiong Zhou 0 siblings, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-09-25 16:59 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Mon, Sep 25, 2017 at 04:40:47PM +0800, Xiong Zhou wrote: > This flag has been disabled upstream kernel. I am working on patches to re-enable the DAX inode flag in the upstream kernel, so I'd prefer that we keep this test. It currently passes even with the per-inode DAX flag in XFS disables, so I don't see the harm in keeping it around. ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test 2017-09-25 16:59 ` Ross Zwisler @ 2017-09-26 0:51 ` Xiong Zhou 0 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-09-26 0:51 UTC (permalink / raw) To: Ross Zwisler; +Cc: Xiong Zhou, fstests, dan.j.williams, jmoyer, eguan On Mon, Sep 25, 2017 at 10:59:28AM -0600, Ross Zwisler wrote: > On Mon, Sep 25, 2017 at 04:40:47PM +0800, Xiong Zhou wrote: > > This flag has been disabled upstream kernel. > > I am working on patches to re-enable the DAX inode flag in the upstream > kernel, so I'd prefer that we keep this test. It currently passes even with > the per-inode DAX flag in XFS disables, so I don't see the harm in keeping it > around. Okay for me. Thanks! ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v6 0/4] fix dax to nondax dio fake failures 2017-09-25 8:40 ` [PATCH v5 0/3] fix dax to nondax dio fake failures Xiong Zhou ` (2 preceding siblings ...) 2017-09-25 8:40 ` [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test Xiong Zhou @ 2017-09-28 7:41 ` Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 1/4] tests: add new group dax Xiong Zhou ` (4 more replies) 3 siblings, 5 replies; 71+ messages in thread From: Xiong Zhou @ 2017-09-28 7:41 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou v6: add new group dax. sort src/ binaries name. add comment about O_DIRECT in t_mmap_write_ro.c add comment about _require_user in write_ro case. fix test file size and write pattern in write_ro case. check errno in dax to nondax dio testrun, then skip if fit. Xiong Zhou (4): tests: add new group dax gitignore: sort src/ binaries name generic: mmap write readonly DAX file generic/413: skip dax to nondax dio test if needed .gitignore | 51 +++++++++++++-------------- src/Makefile | 2 +- src/t_mmap_dio.c | 2 +- src/t_mmap_write_ro.c | 81 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/413 | 10 ++++++ tests/generic/461 | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/461.out | 2 ++ tests/generic/group | 9 ++--- tests/xfs/group | 2 +- 9 files changed, 222 insertions(+), 32 deletions(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/461 create mode 100644 tests/generic/461.out -- 1.8.3.1 ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v6 1/4] tests: add new group dax 2017-09-28 7:41 ` [PATCH v6 0/4] fix dax to nondax dio fake failures Xiong Zhou @ 2017-09-28 7:41 ` Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 2/4] gitignore: sort src/ binaries name Xiong Zhou ` (3 subsequent siblings) 4 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-09-28 7:41 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/generic/group | 8 ++++---- tests/xfs/group | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/generic/group b/tests/generic/group index f922b49..0d42d54 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -415,7 +415,7 @@ 410 auto quick mount 411 auto quick mount 412 auto quick metadata -413 auto quick +413 auto quick dax 414 auto quick clone 415 auto clone 416 auto enospc @@ -430,7 +430,7 @@ 425 auto quick attr 426 auto quick exportfs 427 auto quick aio rw -428 auto quick +428 auto quick dax 429 auto encrypt 430 auto quick copy 431 auto quick copy @@ -439,7 +439,7 @@ 434 auto quick copy 435 auto encrypt 436 auto quick rw -437 auto quick +437 auto quick dax 438 auto 439 auto quick punch 440 auto quick encrypt @@ -454,7 +454,7 @@ 449 auto quick acl enospc 450 auto quick rw 451 auto quick rw aio -452 auto quick +452 auto quick dax 453 auto quick dir 454 auto quick attr 455 auto log replay diff --git a/tests/xfs/group b/tests/xfs/group index d352fe0..42e928b 100644 --- a/tests/xfs/group +++ b/tests/xfs/group @@ -257,7 +257,7 @@ 257 auto quick clone 258 auto quick clone 259 auto quick -260 auto attr quick +260 auto attr quick dax 261 auto quick quota 262 dangerous_fuzzers dangerous_scrub dangerous_online_repair 263 auto quick quota -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* [PATCH v6 2/4] gitignore: sort src/ binaries name 2017-09-28 7:41 ` [PATCH v6 0/4] fix dax to nondax dio fake failures Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 1/4] tests: add new group dax Xiong Zhou @ 2017-09-28 7:41 ` Xiong Zhou 2017-09-28 11:10 ` Dave Chinner 2017-09-28 7:41 ` [PATCH v6 3/4] generic: mmap write readonly DAX file Xiong Zhou ` (2 subsequent siblings) 4 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-09-28 7:41 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- .gitignore | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index 63928b4..73ab533 100644 --- a/.gitignore +++ b/.gitignore @@ -49,6 +49,19 @@ # src/ binaries /src/af_unix +/src/aio-dio-regress/aio-dio-cycle-write +/src/aio-dio-regress/aio-dio-eof-race +/src/aio-dio-regress/aio-dio-extend-stat +/src/aio-dio-regress/aio-dio-fcntl-race +/src/aio-dio-regress/aio-dio-hole-filling-race +/src/aio-dio-regress/aio-dio-invalidate-failure +/src/aio-dio-regress/aio-dio-invalidate-readahead +/src/aio-dio-regress/aio-dio-subblock-eof-read +/src/aio-dio-regress/aio-free-ring-with-bogus-nr-pages +/src/aio-dio-regress/aio-io-setup-with-nonwritable-context-pointer +/src/aio-dio-regress/aio-last-ref-held-by-io +/src/aio-dio-regress/aiocp +/src/aio-dio-regress/aiodio_sparse2 /src/alloc /src/append_reader /src/append_writer @@ -56,10 +69,12 @@ /src/bstat /src/bulkstat_unlink_test /src/bulkstat_unlink_test_modified +/src/cloner /src/dbtest /src/devzero /src/dio-interleaved /src/dio-invalidate-cache +/src/dirhash_collide /src/dirperf /src/dirstress /src/dmiperf @@ -76,7 +91,6 @@ /src/fsync-tester /src/ftrunc /src/genhashnames -/src/dirhash_collide /src/getdevicesize /src/getpagesize /src/godown @@ -85,6 +99,7 @@ /src/itrash /src/listxattr /src/locktest +/src/log-writes/replay-log /src/loggen /src/looptest /src/lstat64 @@ -98,11 +113,11 @@ /src/permname /src/preallo_rw_pattern_reader /src/preallo_rw_pattern_writer +/src/punch-alternating /src/pwrite_mmap_blocked /src/randholes -/src/t_readdir_1 -/src/t_readdir_2 /src/rename +/src/renameat2 /src/resvtest /src/runas /src/seek_copy_test @@ -118,8 +133,15 @@ /src/t_getcwd /src/t_holes /src/t_immutable +/src/t_mmap_cow_race +/src/t_mmap_dio +/src/t_mmap_fallocate +/src/t_mmap_stale_pmd /src/t_mmap_writev /src/t_mtab +/src/t_readdir_1 +/src/t_readdir_2 +/src/t_rename_overwrite /src/t_stripealign /src/t_truncate_cmtime /src/t_truncate_self @@ -133,28 +155,6 @@ /src/writemod /src/writev_on_pagefault /src/xfsctl -/src/aio-dio-regress/aio-dio-cycle-write -/src/aio-dio-regress/aio-dio-extend-stat -/src/aio-dio-regress/aio-dio-fcntl-race -/src/aio-dio-regress/aio-dio-hole-filling-race -/src/aio-dio-regress/aio-dio-invalidate-failure -/src/aio-dio-regress/aio-dio-invalidate-readahead -/src/aio-dio-regress/aio-dio-subblock-eof-read -/src/aio-dio-regress/aio-free-ring-with-bogus-nr-pages -/src/aio-dio-regress/aio-io-setup-with-nonwritable-context-pointer -/src/aio-dio-regress/aio-last-ref-held-by-io -/src/aio-dio-regress/aiocp -/src/aio-dio-regress/aiodio_sparse2 -/src/aio-dio-regress/aio-dio-eof-race -/src/punch-alternating -/src/cloner -/src/renameat2 -/src/t_rename_overwrite -/src/t_mmap_dio -/src/t_mmap_stale_pmd -/src/t_mmap_cow_race -/src/t_mmap_fallocate -/src/log-writes/replay-log # dmapi/ binaries /dmapi/src/common/cmd/read_invis -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v6 2/4] gitignore: sort src/ binaries name 2017-09-28 7:41 ` [PATCH v6 2/4] gitignore: sort src/ binaries name Xiong Zhou @ 2017-09-28 11:10 ` Dave Chinner 2017-09-28 12:28 ` Xiong Zhou 0 siblings, 1 reply; 71+ messages in thread From: Dave Chinner @ 2017-09-28 11:10 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Thu, Sep 28, 2017 at 03:41:38PM +0800, Xiong Zhou wrote: > Signed-off-by: Xiong Zhou <xzhou@redhat.com> Why? you're not fixing any problem here, and it's completely unrelated to fixing the issue the series is about.... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v6 2/4] gitignore: sort src/ binaries name 2017-09-28 11:10 ` Dave Chinner @ 2017-09-28 12:28 ` Xiong Zhou 2017-09-28 21:43 ` Dave Chinner 0 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-09-28 12:28 UTC (permalink / raw) To: Dave Chinner Cc: Xiong Zhou, fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Thu, Sep 28, 2017 at 09:10:08PM +1000, Dave Chinner wrote: > On Thu, Sep 28, 2017 at 03:41:38PM +0800, Xiong Zhou wrote: > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > Why? you're not fixing any problem here, and it's completely > unrelated to fixing the issue the series is about.... Yes. It's not related and a little weird. Because I'm too lazy to split them. This is a "by the way" patch. If you strongly recommend, I could resend these into 2 series: one with the fix of fake failures, the other is the write_ro case with the name/group fixes. Thanks, Xiong > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v6 2/4] gitignore: sort src/ binaries name 2017-09-28 12:28 ` Xiong Zhou @ 2017-09-28 21:43 ` Dave Chinner 0 siblings, 0 replies; 71+ messages in thread From: Dave Chinner @ 2017-09-28 21:43 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Thu, Sep 28, 2017 at 08:28:32PM +0800, Xiong Zhou wrote: > On Thu, Sep 28, 2017 at 09:10:08PM +1000, Dave Chinner wrote: > > On Thu, Sep 28, 2017 at 03:41:38PM +0800, Xiong Zhou wrote: > > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > > > Why? you're not fixing any problem here, and it's completely > > unrelated to fixing the issue the series is about.... > > Yes. It's not related and a little weird. Because I'm too lazy > to split them. This is a "by the way" patch. > > If you strongly recommend, I could resend these into 2 series: > one with the fix of fake failures, the other is the write_ro > case with the name/group fixes. Yes, please send them in a separate series - patch series are supposed to contain related patches. i.e. random changes should not be added to a set of fixes for a specific problem. Thanks! Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 71+ messages in thread
* [PATCH v6 3/4] generic: mmap write readonly DAX file 2017-09-28 7:41 ` [PATCH v6 0/4] fix dax to nondax dio fake failures Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 1/4] tests: add new group dax Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 2/4] gitignore: sort src/ binaries name Xiong Zhou @ 2017-09-28 7:41 ` Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 4/4] generic/413: skip dax to nondax dio test if needed Xiong Zhou 2017-09-29 2:16 ` [PATCH v7] " Xiong Zhou 4 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-09-28 7:41 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou Regression case that one can write to read-only file in a DAX mountpoint. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- .gitignore | 1 + src/Makefile | 2 +- src/t_mmap_write_ro.c | 81 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/461 | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/461.out | 2 ++ tests/generic/group | 1 + 6 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 src/t_mmap_write_ro.c create mode 100755 tests/generic/461 create mode 100644 tests/generic/461.out diff --git a/.gitignore b/.gitignore index 73ab533..cf5d91e 100644 --- a/.gitignore +++ b/.gitignore @@ -137,6 +137,7 @@ /src/t_mmap_dio /src/t_mmap_fallocate /src/t_mmap_stale_pmd +/src/t_mmap_write_ro /src/t_mmap_writev /src/t_mtab /src/t_readdir_1 diff --git a/src/Makefile b/src/Makefile index 7d1306b..3eb25b1 100644 --- a/src/Makefile +++ b/src/Makefile @@ -13,7 +13,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \ multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \ t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \ holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd \ - t_mmap_cow_race t_mmap_fallocate fsync-err + t_mmap_cow_race t_mmap_fallocate fsync-err t_mmap_write_ro LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \ diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c new file mode 100644 index 0000000..1a0ee71 --- /dev/null +++ b/src/t_mmap_write_ro.c @@ -0,0 +1,81 @@ +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <fcntl.h> +#include <unistd.h> +#include <libgen.h> +#include <errno.h> +#include <sys/mman.h> + +void +err_exit(char *op) +{ + fprintf(stderr, "%s: %s\n", op, strerror(errno)); + exit(1); +} + +int +main(int argc, char **argv) +{ + int fd, pfd, ret; + char *buf; + /* + * gcc -O2 will optimize foo's storage, prevent reproducing + * this issue. + * foo is never actually used after fault in value stored. + */ + volatile char foo __attribute__((__unused__)); + int pagesize = getpagesize(); + + if (argc < 3) { + printf("Usage: %s <file> <pmem file>\n", basename(argv[0])); + exit(0); + } + + /* + * This O_DIRECT is necessary for reproduce this bug. + */ + fd = open(argv[1], O_RDONLY|O_DIRECT); + if (fd < 0) + err_exit("open"); + + pfd = open(argv[2], O_RDONLY); + if (pfd < 0) + err_exit("pmem open"); + + buf = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, pfd, 0); + if (buf == MAP_FAILED) + err_exit("mmap"); + + /* + * Read from the DAX mmap to populate the first page in the + * address_space with a read-only mapping. + */ + foo = *buf; + + /* + * Now write to the DAX mmap. This *should* fail, but if the bug is + * present in __get_user_pages_fast(), it will succeed. + */ + ret = read(fd, buf, pagesize); + if (ret != pagesize) + err_exit("read"); + + ret = msync(buf, pagesize, MS_SYNC); + if (ret != 0) + err_exit("msync"); + + ret = munmap(buf, pagesize); + if (ret < 0) + err_exit("munmap"); + + ret = close(fd); + if (ret < 0) + err_exit("clsoe fd"); + + ret = close(pfd); + if (ret < 0) + err_exit("close pfd"); + + exit(0); +} diff --git a/tests/generic/461 b/tests/generic/461 new file mode 100755 index 0000000..b988135 --- /dev/null +++ b/tests/generic/461 @@ -0,0 +1,95 @@ +#! /bin/bash +# FS QA Test 461 +# +# This is a regression test for kernel commit +# ef947b2 x86, mm: fix gup_pte_range() vs DAX mappings +# created by Jeffrey Moyer <jmoyer@redhat.com> +# +# This is reproducible only when testing on pmem device +# which is configured in "memory mode", not in "raw mode". +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Red Hat. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +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 -rf $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# Modify as appropriate. +_supported_fs generic +_supported_os Linux +_require_test +_require_scratch_dax +_require_test_program "t_mmap_write_ro" +# running by unpriviliged user is not necessary to reproduce +# this bug, just trying to test more. +_require_user + +# real QA test starts here + +_scratch_mkfs >>$seqres.full 2>&1 +_scratch_mount "-o dax" + +# remount TEST_DEV wo/ dax +export TEST_FS_MOUNT_OPTS="" +_test_cycle_mount + +# prepare a 4k read-only DAX file, save its md5sum +$XFS_IO_PROG -f -c "pwrite -S 0xFF 0 4096" \ + $SCRATCH_MNT/readonlyfile >> $seqres.full 2>&1 +chmod 0644 $SCRATCH_MNT/readonlyfile +md5_1="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +# prepare another 4k non-DAX file +$XFS_IO_PROG -f -c "pwrite -S 0x00 0 4096" \ + $TEST_DIR/${seq}.file >> $seqres.full 2>&1 + +# allow qa_user access +chown $qa_user $TEST_DIR/${seq}.file + +# run test programme, read another file writing into +# the read-only file with mmap, which should fail. +_user_do "src/t_mmap_write_ro $TEST_DIR/${seq}.file \ + $SCRATCH_MNT/readonlyfile" + +# read-only file should not get updated, md5sum again. +md5_2="$(_md5_checksum $SCRATCH_MNT/readonlyfile)" + +[ "$md5_1" != "$md5_2" ] && echo "read only file changed" + +# success, all done +status=0 +exit diff --git a/tests/generic/461.out b/tests/generic/461.out new file mode 100644 index 0000000..8b1e29c --- /dev/null +++ b/tests/generic/461.out @@ -0,0 +1,2 @@ +QA output created by 461 +read: Bad address diff --git a/tests/generic/group b/tests/generic/group index 0d42d54..28df131 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -463,3 +463,4 @@ 458 auto quick clone 459 auto dangerous 460 auto quick rw +461 auto quick dax -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* [PATCH v6 4/4] generic/413: skip dax to nondax dio test if needed 2017-09-28 7:41 ` [PATCH v6 0/4] fix dax to nondax dio fake failures Xiong Zhou ` (2 preceding siblings ...) 2017-09-28 7:41 ` [PATCH v6 3/4] generic: mmap write readonly DAX file Xiong Zhou @ 2017-09-28 7:41 ` Xiong Zhou 2017-09-29 2:16 ` [PATCH v7] " Xiong Zhou 4 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-09-28 7:41 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou This test requires there is struct page backend for the testing dax device. But not all devices which support dax have that. So we give it a try, if it fails with EFAULT, which is the same errno with the wrong device situation, we skip this subtest. This is not perfect, but it's efficient. Many devices support dax, and there are more coming. It's nearly impossible to maintain an uniq way to detect struct page present for all kinds of devices modes. >From testing perspectice, a testrun could cover this code path as a sanity check and avoid more unnecessary failires. If the device is compatible with the test, one more testrun will not hurt much. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- src/t_mmap_dio.c | 2 +- tests/generic/413 | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/t_mmap_dio.c b/src/t_mmap_dio.c index 6c8ca1a..73e7648 100644 --- a/src/t_mmap_dio.c +++ b/src/t_mmap_dio.c @@ -27,7 +27,7 @@ void err_exit(char *op, unsigned long len, char *s) { fprintf(stderr, "%s(%s) len %lu %s\n", op, strerror(errno), len, s); - exit(1); + exit(errno); } int main(int argc, char **argv) diff --git a/tests/generic/413 b/tests/generic/413 index a1cc514..d86747f 100755 --- a/tests/generic/413 +++ b/tests/generic/413 @@ -88,6 +88,16 @@ t_nondax_to_dax() t_dax_to_nondax() { prep_files + # dax to nondax dio needs struct page backend, which is + # not always available among various devices. Skip this + # subtest if EFAULT(14 Bad address) returned, which means + # probably the device is not compatible with this test. + src/t_mmap_dio $SCRATCH_MNT/tf_s $TEST_DIR/tf_d \ + $1 "test" > /dev/null 2>&1 + if [ $? -eq 14 ] ; then + return + fi + src/t_mmap_dio $SCRATCH_MNT/tf_s \ $TEST_DIR/tf_d $1 "dio dax to nondax" -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* [PATCH v7] generic/413: skip dax to nondax dio test if needed 2017-09-28 7:41 ` [PATCH v6 0/4] fix dax to nondax dio fake failures Xiong Zhou ` (3 preceding siblings ...) 2017-09-28 7:41 ` [PATCH v6 4/4] generic/413: skip dax to nondax dio test if needed Xiong Zhou @ 2017-09-29 2:16 ` Xiong Zhou 2017-09-29 16:24 ` Ross Zwisler 4 siblings, 1 reply; 71+ messages in thread From: Xiong Zhou @ 2017-09-29 2:16 UTC (permalink / raw) To: fstests; +Cc: ross.zwisler, dan.j.williams, jmoyer, eguan, Xiong Zhou This test requires there is struct page backend for the testing dax device. But not all devices which support dax have that. So we give it a try, if it fails with EFAULT, which is the same errno with the wrong device situation, we skip this subtest. This is not perfect, but it's efficient. Many devices support dax, and there are more coming. It's nearly impossible to maintain an uniq way to detect struct page present for all kinds of devices modes. >From testing perspectice, a testrun could cover this code path as a sanity check and avoid more unnecessary failires. If the device is compatible with the test, one more testrun will not hurt much. Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- v7: split this patch from that unrelated series. minor comment fix. src/t_mmap_dio.c | 2 +- tests/generic/413 | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/t_mmap_dio.c b/src/t_mmap_dio.c index 6c8ca1a..73e7648 100644 --- a/src/t_mmap_dio.c +++ b/src/t_mmap_dio.c @@ -27,7 +27,7 @@ void err_exit(char *op, unsigned long len, char *s) { fprintf(stderr, "%s(%s) len %lu %s\n", op, strerror(errno), len, s); - exit(1); + exit(errno); } int main(int argc, char **argv) diff --git a/tests/generic/413 b/tests/generic/413 index a1cc514..311bdc2 100755 --- a/tests/generic/413 +++ b/tests/generic/413 @@ -88,6 +88,18 @@ t_nondax_to_dax() t_dax_to_nondax() { prep_files + + # dax to nondax dio needs struct page backend, which is + # not always available among various devices. Skip this + # subtest if EFAULT(14 Bad address) returned, which means + # probably the device is not compatible with this test. + # + src/t_mmap_dio $SCRATCH_MNT/tf_s $TEST_DIR/tf_d \ + $1 "test" > /dev/null 2>&1 + if [ $? -eq 14 ] ; then + return + fi + src/t_mmap_dio $SCRATCH_MNT/tf_s \ $TEST_DIR/tf_d $1 "dio dax to nondax" -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [PATCH v7] generic/413: skip dax to nondax dio test if needed 2017-09-29 2:16 ` [PATCH v7] " Xiong Zhou @ 2017-09-29 16:24 ` Ross Zwisler 2017-09-30 2:30 ` Xiong Zhou 0 siblings, 1 reply; 71+ messages in thread From: Ross Zwisler @ 2017-09-29 16:24 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer, eguan On Fri, Sep 29, 2017 at 10:16:11AM +0800, Xiong Zhou wrote: > This test requires there is struct page backend for the > testing dax device. But not all devices which support dax > have that. So we give it a try, if it fails with EFAULT, > which is the same errno with the wrong device situation, > we skip this subtest. > > This is not perfect, but it's efficient. Many devices > support dax, and there are more coming. It's nearly > impossible to maintain an uniq way to detect struct page > present for all kinds of devices modes. > > From testing perspectice, a testrun could cover this > code path as a sanity check and avoid more unnecessary > failires. If the device is compatible with the test, one > more testrun will not hurt much. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > > v7: > split this patch from that unrelated series. > minor comment fix. > > src/t_mmap_dio.c | 2 +- > tests/generic/413 | 12 ++++++++++++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/src/t_mmap_dio.c b/src/t_mmap_dio.c > index 6c8ca1a..73e7648 100644 > --- a/src/t_mmap_dio.c > +++ b/src/t_mmap_dio.c > @@ -27,7 +27,7 @@ void err_exit(char *op, unsigned long len, char *s) > { > fprintf(stderr, "%s(%s) len %lu %s\n", > op, strerror(errno), len, s); > - exit(1); > + exit(errno); > } > > int main(int argc, char **argv) > diff --git a/tests/generic/413 b/tests/generic/413 > index a1cc514..311bdc2 100755 > --- a/tests/generic/413 > +++ b/tests/generic/413 > @@ -88,6 +88,18 @@ t_nondax_to_dax() > t_dax_to_nondax() > { > prep_files > + > + # dax to nondax dio needs struct page backend, which is > + # not always available among various devices. Skip this > + # subtest if EFAULT(14 Bad address) returned, which means > + # probably the device is not compatible with this test. > + # > + src/t_mmap_dio $SCRATCH_MNT/tf_s $TEST_DIR/tf_d \ > + $1 "test" > /dev/null 2>&1 > + if [ $? -eq 14 ] ; then > + return > + fi > + > src/t_mmap_dio $SCRATCH_MNT/tf_s \ > $TEST_DIR/tf_d $1 "dio dax to nondax" > > -- > 1.8.3.1 I think you may have missed this feedback from Jeff & Eryu? (I can't find an online archive of fstests, so just copy-n-pasting here: Eryu Guan <eguan@redhat.com> writes: > On Mon, Sep 25, 2017 at 04:40:46PM +0800, Xiong Zhou wrote: >> Since not all devices support dax has struct page backend, >> which will not support this test. >> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> >> --- >> tests/generic/413 | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/tests/generic/413 b/tests/generic/413 >> index a1cc514..b86c10f 100755 >> --- a/tests/generic/413 >> +++ b/tests/generic/413 >> @@ -88,6 +88,14 @@ t_nondax_to_dax() >> t_dax_to_nondax() >> { >> prep_files >> + # dax to nondax dio needs struct page backend, which is >> + # not always avaiable among various devices. Skip this >> + # subtest if not compatible. >> + if ! src/t_mmap_dio $SCRATCH_MNT/tf_s \ >> + $TEST_DIR/tf_d $1 "test" > /dev/null 2>&1 ; then >> + return >> + fi >> + > > Then we will never get a failure from this case, even if it's a real > bug.. We need better way to tell if there's struct page present :) ndctl list will tell you the mode of the namespace. If it's 'raw', then it doesn't have struct page backing. If it's 'memory', it should work fine. -Jeff ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v7] generic/413: skip dax to nondax dio test if needed 2017-09-29 16:24 ` Ross Zwisler @ 2017-09-30 2:30 ` Xiong Zhou 0 siblings, 0 replies; 71+ messages in thread From: Xiong Zhou @ 2017-09-30 2:30 UTC (permalink / raw) To: Ross Zwisler; +Cc: Xiong Zhou, fstests, dan.j.williams, jmoyer, eguan On Fri, Sep 29, 2017 at 10:24:46AM -0600, Ross Zwisler wrote: > On Fri, Sep 29, 2017 at 10:16:11AM +0800, Xiong Zhou wrote: > > This test requires there is struct page backend for the > > testing dax device. But not all devices which support dax > > have that. So we give it a try, if it fails with EFAULT, > > which is the same errno with the wrong device situation, > > we skip this subtest. > > > > This is not perfect, but it's efficient. Many devices > > support dax, and there are more coming. It's nearly > > impossible to maintain an uniq way to detect struct page > > present for all kinds of devices modes. > > > > From testing perspectice, a testrun could cover this > > code path as a sanity check and avoid more unnecessary > > failires. If the device is compatible with the test, one > > more testrun will not hurt much. > > > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > > --- > > > > v7: > > split this patch from that unrelated series. > > minor comment fix. > > > > src/t_mmap_dio.c | 2 +- > > tests/generic/413 | 12 ++++++++++++ > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/src/t_mmap_dio.c b/src/t_mmap_dio.c > > index 6c8ca1a..73e7648 100644 > > --- a/src/t_mmap_dio.c > > +++ b/src/t_mmap_dio.c > > @@ -27,7 +27,7 @@ void err_exit(char *op, unsigned long len, char *s) > > { > > fprintf(stderr, "%s(%s) len %lu %s\n", > > op, strerror(errno), len, s); > > - exit(1); > > + exit(errno); > > } > > > > int main(int argc, char **argv) > > diff --git a/tests/generic/413 b/tests/generic/413 > > index a1cc514..311bdc2 100755 > > --- a/tests/generic/413 > > +++ b/tests/generic/413 > > @@ -88,6 +88,18 @@ t_nondax_to_dax() > > t_dax_to_nondax() > > { > > prep_files > > + > > + # dax to nondax dio needs struct page backend, which is > > + # not always available among various devices. Skip this > > + # subtest if EFAULT(14 Bad address) returned, which means > > + # probably the device is not compatible with this test. > > + # > > + src/t_mmap_dio $SCRATCH_MNT/tf_s $TEST_DIR/tf_d \ > > + $1 "test" > /dev/null 2>&1 > > + if [ $? -eq 14 ] ; then > > + return > > + fi > > + > > src/t_mmap_dio $SCRATCH_MNT/tf_s \ > > $TEST_DIR/tf_d $1 "dio dax to nondax" > > > > -- > > 1.8.3.1 > > I think you may have missed this feedback from Jeff & Eryu? (I can't find an > online archive of fstests, so just copy-n-pasting here: > > Eryu Guan <eguan@redhat.com> writes: > > > On Mon, Sep 25, 2017 at 04:40:46PM +0800, Xiong Zhou wrote: > >> Since not all devices support dax has struct page backend, > >> which will not support this test. > >> > >> Signed-off-by: Xiong Zhou <xzhou@redhat.com> > >> --- > >> tests/generic/413 | 8 ++++++++ > >> 1 file changed, 8 insertions(+) > >> > >> diff --git a/tests/generic/413 b/tests/generic/413 > >> index a1cc514..b86c10f 100755 > >> --- a/tests/generic/413 > >> +++ b/tests/generic/413 > >> @@ -88,6 +88,14 @@ t_nondax_to_dax() > >> t_dax_to_nondax() > >> { > >> prep_files > >> + # dax to nondax dio needs struct page backend, which is > >> + # not always avaiable among various devices. Skip this > >> + # subtest if not compatible. > >> + if ! src/t_mmap_dio $SCRATCH_MNT/tf_s \ > >> + $TEST_DIR/tf_d $1 "test" > /dev/null 2>&1 ; then > >> + return > >> + fi > >> + > > > > Then we will never get a failure from this case, even if it's a real > > bug.. We need better way to tell if there's struct page present :) I was adding errno based verdict in v7 per Eryu's advice. > > ndctl list will tell you the mode of the namespace. If it's 'raw', then > it doesn't have struct page backing. If it's 'memory', it should work > fine. > > -Jeff My original solution was also ndctl. However ndctl can't do all the job. brd ramdisk and tmpfs support dax but they have nothing to do with NFIT. Quoting HCH: On Tue, Apr 18, 2017 at 03:12:26AM -0700, Christoph Hellwig wrote: > On Mon, Apr 17, 2017 at 03:14:13PM +0800, Xiong Zhou wrote: > > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then > > do mmap DIO from DAX to non-DAX. > > > > This test is split from generic/413. Since DIO from DAX > > to non-DAX is only supported/doable when device underneath > > has memory(struct page) backend. > > > > By ndctl looking at SCRATCH_DEV, skip this test if it is > > not in "memory mode". > > DAX devices don't need to be something using NFIT, so I don't think this > method is correct. Thanks, Xiong ^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [PATCH v2 1/3] common: helper to get value from ndctl list by key 2017-04-10 6:05 ` [PATCH v2 " Xiong Zhou 2017-04-10 6:05 ` [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou 2017-04-10 6:05 ` [PATCH v2 3/3] DAX: mmap write readonly file Xiong Zhou @ 2017-04-11 11:40 ` Eryu Guan 2 siblings, 0 replies; 71+ messages in thread From: Eryu Guan @ 2017-04-11 11:40 UTC (permalink / raw) To: Xiong Zhou; +Cc: fstests, ross.zwisler, dan.j.williams, jmoyer On Mon, Apr 10, 2017 at 02:05:51PM +0800, Xiong Zhou wrote: > For some nvdimm DAX related tests, it's better to know > some details of devices being tested. Adding new prog > ndctl to manage nvdimms and jq to parse outputs. > > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > > v2: > use _require_command instead of running cmds > > common/config | 2 ++ > common/rc | 19 +++++++++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/common/config b/common/config > index 59041a3..dfdcb8e 100644 > --- a/common/config > +++ b/common/config > @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`" > export FLOCK_PROG="`set_prog_path flock`" > export LDD_PROG="`set_prog_path ldd`" > export TIMEOUT_PROG="`set_prog_path timeout`" > +export NDCTL_PROG="`set_prog_path ndctl`" > +export JQ_PROG="`set_prog_path jq`" > > # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. > # newer systems have udevadm command but older systems like RHEL5 don't. > diff --git a/common/rc b/common/rc > index e1ab2c6..bc387cf 100644 > --- a/common/rc > +++ b/common/rc > @@ -3148,6 +3148,25 @@ _require_chattr() > rm -f $TEST_DIR/syscalltest.out > } > > +_require_ndctl() > +{ > + _require_command "$NDCTL_PROG" ndctl > +} > + > +_require_jq() > +{ > + _require_command "$JQ_PROG" jq > +} Don't need new wrappers for one line requirement, you could require the command in tests directly. > + > +# $1 SCRATCH_DEV or TEST_DEV or other dev > +# $2 key > +_ndctl_get_pmem_key_value() > +{ > + $NDCTL_PROG list | \ > + $JQ_PROG -r ".[] | \ > + select(.blockdev == \"${1#/dev/}\") | .$2" > +} > + And I think this patch could be merged with patch 2. Thanks, Eryu > _get_total_inode() > { > if [ -z "$1" ]; then > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 71+ messages in thread
end of thread, other threads:[~2017-09-30 2:30 UTC | newest] Thread overview: 71+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-07 7:56 [PATCH 1/3] common: helper to get value from ndctl list by key Xiong Zhou 2017-04-07 7:56 ` [PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou 2017-04-12 3:30 ` Ross Zwisler 2017-04-07 7:56 ` [PATCH 3/3] DAX: mmap write readonly file Xiong Zhou 2017-04-07 17:16 ` [PATCH 1/3] common: helper to get value from ndctl list by key Ross Zwisler 2017-04-10 6:05 ` [PATCH v2 " Xiong Zhou 2017-04-10 6:05 ` [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported Xiong Zhou 2017-04-11 11:44 ` Eryu Guan 2017-04-11 13:54 ` Jeff Moyer 2017-04-11 14:26 ` Xiong Zhou 2017-04-10 6:05 ` [PATCH v2 3/3] DAX: mmap write readonly file Xiong Zhou 2017-04-11 11:46 ` Eryu Guan 2017-04-11 13:56 ` Jeff Moyer 2017-04-11 18:52 ` Ross Zwisler 2017-04-11 22:45 ` Ross Zwisler 2017-04-12 2:52 ` Xiong Zhou 2017-04-12 4:03 ` Ross Zwisler 2017-04-12 6:26 ` Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 0/4] split DAX mmap DIO cases Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou 2017-04-13 4:11 ` Xiong Zhou 2017-04-13 13:36 ` Dan Williams 2017-04-14 10:01 ` Xiong Zhou 2017-04-14 14:49 ` Dan Williams 2017-04-14 15:22 ` Ross Zwisler 2017-04-14 15:33 ` Dan Williams 2017-04-14 15:51 ` Ross Zwisler 2017-04-12 14:46 ` [PATCH v3 3/4] xfs: test per-inode DAX flag " Xiong Zhou 2017-04-12 14:46 ` [PATCH v3 4/4] generic: mmap write readonly DAX file Xiong Zhou 2017-04-17 7:14 ` [PATCH v4 0/4] split DAX mmap DIO cases Xiong Zhou 2017-04-17 7:14 ` [PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo Xiong Zhou 2017-04-18 16:31 ` Ross Zwisler 2017-04-17 7:14 ` [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX Xiong Zhou 2017-04-17 14:14 ` Dan Williams 2017-04-17 22:54 ` Dan Williams 2017-04-17 23:39 ` Ross Zwisler 2017-04-17 23:47 ` Dan Williams 2017-04-19 8:40 ` Xiong Zhou 2017-04-19 15:53 ` Dan Williams 2017-04-18 10:12 ` Christoph Hellwig 2017-04-18 14:49 ` Xiong Zhou 2017-07-28 14:55 ` Jeff Moyer 2017-04-18 16:32 ` Ross Zwisler 2017-04-17 7:14 ` [PATCH v4 3/4] xfs: test per-inode DAX flag " Xiong Zhou 2017-04-18 16:36 ` Ross Zwisler 2017-04-17 7:14 ` [PATCH v4 4/4] generic: mmap write readonly DAX file Xiong Zhou 2017-04-18 17:05 ` Ross Zwisler 2017-04-19 2:54 ` Xiong Zhou 2017-04-24 16:34 ` Ross Zwisler 2017-09-25 8:40 ` [PATCH v5 0/3] fix dax to nondax dio fake failures Xiong Zhou 2017-09-25 8:40 ` [PATCH v5 1/3] generic: mmap write readonly DAX file Xiong Zhou 2017-09-27 9:57 ` Eryu Guan 2017-09-25 8:40 ` [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed Xiong Zhou 2017-09-27 9:50 ` Eryu Guan 2017-09-27 15:01 ` Jeff Moyer 2017-09-25 8:40 ` [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test Xiong Zhou 2017-09-25 16:59 ` Ross Zwisler 2017-09-26 0:51 ` Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 0/4] fix dax to nondax dio fake failures Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 1/4] tests: add new group dax Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 2/4] gitignore: sort src/ binaries name Xiong Zhou 2017-09-28 11:10 ` Dave Chinner 2017-09-28 12:28 ` Xiong Zhou 2017-09-28 21:43 ` Dave Chinner 2017-09-28 7:41 ` [PATCH v6 3/4] generic: mmap write readonly DAX file Xiong Zhou 2017-09-28 7:41 ` [PATCH v6 4/4] generic/413: skip dax to nondax dio test if needed Xiong Zhou 2017-09-29 2:16 ` [PATCH v7] " Xiong Zhou 2017-09-29 16:24 ` Ross Zwisler 2017-09-30 2:30 ` Xiong Zhou 2017-04-11 11:40 ` [PATCH v2 1/3] common: helper to get value from ndctl list by key Eryu Guan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.