[PATCH 1/3] common: helper to get value from ndctl list by key

[PATCH 1/3] common: helper to get value from ndctl list by key

[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

[PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported

[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

Re: [PATCH 2/3] DAX-DIO: skip DAX to non-DAX if unsupported

[Ross Zwisler]

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.

[PATCH 3/3] DAX: mmap write readonly file

[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

Re: [PATCH 1/3] common: helper to get value from ndctl list by key

[Ross Zwisler]

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
> 

[PATCH v2 1/3] common: helper to get value from ndctl list by key

[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

[PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported

[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

Re: [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported

[Eryu Guan]

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

Re: [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported

[Jeff Moyer]

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

Re: [PATCH v2 2/3] DAX-DIO: skip DAX to non-DAX if unsupported

[Xiong Zhou]

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

[PATCH v2 3/3] DAX: mmap write readonly file

[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

Re: [PATCH v2 3/3] DAX: mmap write readonly file

[Eryu Guan]

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

Re: [PATCH v2 3/3] DAX: mmap write readonly file

[Jeff Moyer]

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

Re: [PATCH v2 3/3] DAX: mmap write readonly file

[Ross Zwisler]

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) {

Re: [PATCH v2 3/3] DAX: mmap write readonly file

[Ross Zwisler]

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.

Re: [PATCH v2 3/3] DAX: mmap write readonly file

[Xiong Zhou]

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.

Re: [PATCH v2 3/3] DAX: mmap write readonly file

[Ross Zwisler]

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);
 }

Re: [PATCH v2 3/3] DAX: mmap write readonly file

[Xiong Zhou]

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);
>  }

[PATCH v3 0/4] split DAX mmap DIO cases

[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

[PATCH v3 1/4] DAX-DIO: make dax_to_non_dax dio test solo

[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

[PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[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

Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[Xiong Zhou]

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
> 

Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[Dan Williams]

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.

Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[Xiong Zhou]

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.

Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[Dan Williams]

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?

Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[Ross Zwisler]

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.

Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[Dan Williams]

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.

Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[Ross Zwisler]

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.

[PATCH v3 3/4] xfs: test per-inode DAX flag DAX to non-DAX

[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

[PATCH v3 4/4] generic: mmap write readonly DAX file

[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

[PATCH v4 0/4] split DAX mmap DIO cases

[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

[PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo

[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

Re: [PATCH v4 1/4] DAX-DIO: make dax_to_non_dax dio test solo

[Ross Zwisler]

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>

[PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[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

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Dan Williams]

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.

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Dan Williams]

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.

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Ross Zwisler]

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.

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Dan Williams]

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.

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Xiong Zhou]

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# 

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Dan Williams]

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.

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Christoph Hellwig]

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.

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Xiong Zhou]

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.

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Jeff Moyer]

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

Re: [PATCH v4 2/4] generic: test mmap io fom DAX to non-DAX

[Ross Zwisler]

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.

[PATCH v4 3/4] xfs: test per-inode DAX flag DAX to non-DAX

[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

Re: [PATCH v4 3/4] xfs: test per-inode DAX flag DAX to non-DAX

[Ross Zwisler]

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>

[PATCH v4 4/4] generic: mmap write readonly DAX file

[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

Re: [PATCH v4 4/4] generic: mmap write readonly DAX file

[Ross Zwisler]

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"

Re: [PATCH v4 4/4] generic: mmap write readonly DAX file

[Xiong Zhou]

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, :)

Re: [PATCH v4 4/4] generic: mmap write readonly DAX file

[Ross Zwisler]

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

[PATCH v5 0/3] fix dax to nondax dio fake failures

[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

[PATCH v5 1/3] generic: mmap write readonly DAX file

[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

Re: [PATCH v5 1/3] generic: mmap write readonly DAX file

[Eryu Guan]

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

[PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed

[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

Re: [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed

[Eryu Guan]

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
> 

Re: [PATCH v5 2/3] generic/413: skip dax to nondax dio test if needed

[Jeff Moyer]

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

[PATCH v5 3/3] xfs/260: remove per-inode DAX flag test

[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

Re: [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test

[Ross Zwisler]

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.

Re: [PATCH v5 3/3] xfs/260: remove per-inode DAX flag test

[Xiong Zhou]

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!

[PATCH v6 0/4] fix dax to nondax dio fake failures

[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

[PATCH v6 1/4] tests: add new group dax

[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

[PATCH v6 2/4] gitignore: sort src/ binaries name

[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

Re: [PATCH v6 2/4] gitignore: sort src/ binaries name

[Dave Chinner]

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

Re: [PATCH v6 2/4] gitignore: sort src/ binaries name

[Xiong Zhou]

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

Re: [PATCH v6 2/4] gitignore: sort src/ binaries name

[Dave Chinner]

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

[PATCH v6 3/4] generic: mmap write readonly DAX file

[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

[PATCH v6 4/4] generic/413: skip dax to nondax dio test if needed

[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

[PATCH v7] generic/413: skip dax to nondax dio test if needed

[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

Re: [PATCH v7] generic/413: skip dax to nondax dio test if needed

[Ross Zwisler]

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

Re: [PATCH v7] generic/413: skip dax to nondax dio test if needed

[Xiong Zhou]

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

Re: [PATCH v2 1/3] common: helper to get value from ndctl list by key

[Eryu Guan]

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
>