[fstests PATCH v5 0/2] add test for DAX MAP_SYNC support

[fstests PATCH v5 0/2] add test for DAX MAP_SYNC support

[Ross Zwisler]

This is the fifth revision of my xfstest adding a test for the new
mmap() MAP_SYNC flag.  The previous version can be found here:

https://lists.01.org/pipermail/linux-nvdimm/2017-November/013329.html

Changes since v4:
 - Test for dm-log-writes + DAX support in the spirit of
   _require_scratch_dax() instead of relying on specific module version
   number. (Eryu)

 - Print out the missing mark name in the error output of
   _log_writes_replay_log().  (Eryu)

 - Go back to using the output from "du -sh" to test our file block
   allocations.  The "stat -c %s" I was using in v4 was broken if the
   file existed and only had partial block allocations, and the number
   of blocks reported by "stat -c %b" can vary based on the block size.

 - Move the test number to 999 to avoid rebasing collisions.  Feel free
   to renumber as necessary when merging.

I've updated the links at the bottom of this mail to point to the most
recent version of my xfsprogs changes and to the current version of this
series.

---

The purpose of this series is to exercise the new MAP_SYNC mmap()
functionality [1].  It adds a test which uses dm-log-writes to try and
replay filesystem metadata operations for a file that is being written via
mmap().

If MAP_SYNC is active the dm-log-writes replay will recreate the file's
block allocations and you'll end up with a test file which is a known
size.

If MAP_SYNC is not active the metadata writes will most likely be lost and
the replay will either fail to create the test file at all or it will may
be smaller.  In all of my testing the file simply doesn't exist on the
replay if MAP_SYNC is ommited.

This test relies on a kernel with both the MAP_SYNC mmap() functionality
and the DAX enabling in dm-log-writes.  These both appear in kernel
v4.15-rc*.  For ease of testing I've posted a kernel that is v4.14 plus
just those two patch series [2].

This test also relies on xfsprogs having support for MAP_SYNC and for
dm-log-writes.  I've posed a tree adding that support [3].  This xfsprogs
series is still under review so if the xfs_io interfaces change this
test will of course need to be updated as well.

Lastly, I've also posted a working version of this series [4].

[1]: https://lists.01.org/pipermail/linux-nvdimm/2017-November/013164.html
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/linux.git/log/?h=map_sync_dm_log_writes
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfsprogs-dev.git/log/?h=map_sync_v2
[4]: https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfstests-dev.git/log/?h=map_sync_test_v5

Ross Zwisler (2):
  dm-log-writes: only replay log to marks that exist
  generic: add test for DAX MAP_SYNC support

 common/dmlogwrites    | 24 +++++++++++++++
 tests/generic/999     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  3 ++
 tests/generic/group   |  1 +
 4 files changed, 110 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[fstests PATCH v5 0/2] add test for DAX MAP_SYNC support

[Ross Zwisler]

This is the fifth revision of my xfstest adding a test for the new
mmap() MAP_SYNC flag.  The previous version can be found here:

https://lists.01.org/pipermail/linux-nvdimm/2017-November/013329.html

Changes since v4:
 - Test for dm-log-writes + DAX support in the spirit of
   _require_scratch_dax() instead of relying on specific module version
   number. (Eryu)

 - Print out the missing mark name in the error output of
   _log_writes_replay_log().  (Eryu)

 - Go back to using the output from "du -sh" to test our file block
   allocations.  The "stat -c %s" I was using in v4 was broken if the
   file existed and only had partial block allocations, and the number
   of blocks reported by "stat -c %b" can vary based on the block size.

 - Move the test number to 999 to avoid rebasing collisions.  Feel free
   to renumber as necessary when merging.

I've updated the links at the bottom of this mail to point to the most
recent version of my xfsprogs changes and to the current version of this
series.

---

The purpose of this series is to exercise the new MAP_SYNC mmap()
functionality [1].  It adds a test which uses dm-log-writes to try and
replay filesystem metadata operations for a file that is being written via
mmap().

If MAP_SYNC is active the dm-log-writes replay will recreate the file's
block allocations and you'll end up with a test file which is a known
size.

If MAP_SYNC is not active the metadata writes will most likely be lost and
the replay will either fail to create the test file at all or it will may
be smaller.  In all of my testing the file simply doesn't exist on the
replay if MAP_SYNC is ommited.

This test relies on a kernel with both the MAP_SYNC mmap() functionality
and the DAX enabling in dm-log-writes.  These both appear in kernel
v4.15-rc*.  For ease of testing I've posted a kernel that is v4.14 plus
just those two patch series [2].

This test also relies on xfsprogs having support for MAP_SYNC and for
dm-log-writes.  I've posed a tree adding that support [3].  This xfsprogs
series is still under review so if the xfs_io interfaces change this
test will of course need to be updated as well.

Lastly, I've also posted a working version of this series [4].

[1]: https://lists.01.org/pipermail/linux-nvdimm/2017-November/013164.html
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/linux.git/log/?h=map_sync_dm_log_writes
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfsprogs-dev.git/log/?h=map_sync_v2
[4]: https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfstests-dev.git/log/?h=map_sync_test_v5

Ross Zwisler (2):
  dm-log-writes: only replay log to marks that exist
  generic: add test for DAX MAP_SYNC support

 common/dmlogwrites    | 24 +++++++++++++++
 tests/generic/999     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  3 ++
 tests/generic/group   |  1 +
 4 files changed, 110 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

-- 
2.14.3

[fstests PATCH v5 1/2] dm-log-writes: only replay log to marks that exist

[Ross Zwisler]

The 'replay-log' executable will replay the dm-log-writes log until the
given mark, or until the end of the log if the mark isn't found.

This means that if the mark you're looking for was never inserted in the
log or if you give garbage to _log_writes_replay_log() the entire log will
be replayed.  This can cause unexpected test results.

Fix this by making sure that the mark we're given actually exists in the
log before we allow the replay.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 common/dmlogwrites | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/dmlogwrites b/common/dmlogwrites
index 247c7442..05829dbc 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -72,6 +72,10 @@ _log_writes_replay_log()
 {
 	_mark=$1
 
+	$here/src/log-writes/replay-log --log $LOGWRITES_DEV --find \
+		--end-mark $_mark >> $seqres.full 2>&1
+	[ $? -ne 0 ] && _fail "mark '$_mark' does not exist"
+
 	$here/src/log-writes/replay-log --log $LOGWRITES_DEV --replay $SCRATCH_DEV \
 		--end-mark $_mark >> $seqres.full 2>&1
 	[ $? -ne 0 ] && _fail "replay failed"
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[fstests PATCH v5 1/2] dm-log-writes: only replay log to marks that exist

[Ross Zwisler]

The 'replay-log' executable will replay the dm-log-writes log until the
given mark, or until the end of the log if the mark isn't found.

This means that if the mark you're looking for was never inserted in the
log or if you give garbage to _log_writes_replay_log() the entire log will
be replayed.  This can cause unexpected test results.

Fix this by making sure that the mark we're given actually exists in the
log before we allow the replay.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 common/dmlogwrites | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/dmlogwrites b/common/dmlogwrites
index 247c7442..05829dbc 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -72,6 +72,10 @@ _log_writes_replay_log()
 {
 	_mark=$1
 
+	$here/src/log-writes/replay-log --log $LOGWRITES_DEV --find \
+		--end-mark $_mark >> $seqres.full 2>&1
+	[ $? -ne 0 ] && _fail "mark '$_mark' does not exist"
+
 	$here/src/log-writes/replay-log --log $LOGWRITES_DEV --replay $SCRATCH_DEV \
 		--end-mark $_mark >> $seqres.full 2>&1
 	[ $? -ne 0 ] && _fail "replay failed"
-- 
2.14.3

[fstests PATCH v5 2/2] generic: add test for DAX MAP_SYNC support

[Ross Zwisler]

This test creates a file and writes to it via an mmap(), but never syncs
via fsync/msync.  This process is tracked via dm-log-writes, then replayed.

If MAP_SYNC is working the dm-log-writes replay will show the test file
with 1 MiB of on-media block allocations.  This is because each allocating
page fault included an implicit metadata sync.  If MAP_SYNC isn't working
(which you can test by removing the "-S" flag to xfs_io mmap) the file
will be smaller or missing entirely.

Note that dm-log-writes doesn't track the data that we write via the
mmap(), so we can't do any data integrity checking.  We can only verify
that the metadata writes for the page faults happened.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 common/dmlogwrites    | 20 +++++++++++++
 tests/generic/999     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  3 ++
 tests/generic/group   |  1 +
 4 files changed, 106 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

diff --git a/common/dmlogwrites b/common/dmlogwrites
index 05829dbc..2b697bec 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -28,6 +28,26 @@ _require_log_writes()
 	_require_test_program "log-writes/replay-log"
 }
 
+_require_log_writes_dax()
+{
+	[ -z "$LOGWRITES_DEV" -o ! -b "$LOGWRITES_DEV" ] && \
+		_notrun "This test requires a valid \$LOGWRITES_DEV"
+
+	_require_dm_target log-writes
+	_require_test_program "log-writes/replay-log"
+
+	_scratch_unmount
+	_log_writes_init
+	_log_writes_mkfs > /dev/null 2>&1
+	_log_writes_mount -o dax
+	# Check options to be sure. XFS ignores dax option
+	# and goes on if dev underneath does not support dax.
+	_fs_options $LOGWRITES_DMDEV | grep -qw "dax" || \
+		_notrun "$LOGWRITES_DMDEV $FSTYP does not support -o dax"
+	_log_writes_unmount
+	_log_writes_remove
+}
+
 _log_writes_init()
 {
 	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
diff --git a/tests/generic/999 b/tests/generic/999
new file mode 100755
index 00000000..ca5772da
--- /dev/null
+++ b/tests/generic/999
@@ -0,0 +1,82 @@
+#! /bin/bash
+# FS QA Test No. 999
+#
+# Use dm-log-writes to verify that MAP_SYNC actually syncs metadata during
+# page faults.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Intel Corporation.  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`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_log_writes_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmlogwrites
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_log_writes_dax
+_require_xfs_io_command "log_writes"
+
+_log_writes_init
+_log_writes_mkfs >> $seqres.full 2>&1
+_log_writes_mount -o dax
+
+LEN=$((1024 * 1024)) # 1 MiB
+
+xfs_io -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
+	-c "log_writes -d $LOGWRITES_NAME -m preunmap" \
+	-f $SCRATCH_MNT/test
+
+# Unmount the scratch dir and tear down the log writes target
+_log_writes_unmount
+_log_writes_remove
+_check_scratch_fs
+
+# destroy previous filesystem so we can be sure our rebuild works
+_scratch_mkfs >> $seqres.full 2>&1
+
+# check pre-unmap state
+_log_writes_replay_log preunmap
+_scratch_mount
+
+# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
+du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
+
+_scratch_unmount
+_check_scratch_fs
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/999.out b/tests/generic/999.out
new file mode 100644
index 00000000..c7b8f8a2
--- /dev/null
+++ b/tests/generic/999.out
@@ -0,0 +1,3 @@
+QA output created by 999
+1.0M SCRATCH_MNT/test
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 6c3bb03a..ae88aa03 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -472,3 +472,4 @@
 467 auto quick exportfs
 468 shutdown auto quick metadata
 469 auto quick
+999 auto quick dax
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[fstests PATCH v5 2/2] generic: add test for DAX MAP_SYNC support

[Ross Zwisler]

This test creates a file and writes to it via an mmap(), but never syncs
via fsync/msync.  This process is tracked via dm-log-writes, then replayed.

If MAP_SYNC is working the dm-log-writes replay will show the test file
with 1 MiB of on-media block allocations.  This is because each allocating
page fault included an implicit metadata sync.  If MAP_SYNC isn't working
(which you can test by removing the "-S" flag to xfs_io mmap) the file
will be smaller or missing entirely.

Note that dm-log-writes doesn't track the data that we write via the
mmap(), so we can't do any data integrity checking.  We can only verify
that the metadata writes for the page faults happened.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 common/dmlogwrites    | 20 +++++++++++++
 tests/generic/999     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  3 ++
 tests/generic/group   |  1 +
 4 files changed, 106 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

diff --git a/common/dmlogwrites b/common/dmlogwrites
index 05829dbc..2b697bec 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -28,6 +28,26 @@ _require_log_writes()
 	_require_test_program "log-writes/replay-log"
 }
 
+_require_log_writes_dax()
+{
+	[ -z "$LOGWRITES_DEV" -o ! -b "$LOGWRITES_DEV" ] && \
+		_notrun "This test requires a valid \$LOGWRITES_DEV"
+
+	_require_dm_target log-writes
+	_require_test_program "log-writes/replay-log"
+
+	_scratch_unmount
+	_log_writes_init
+	_log_writes_mkfs > /dev/null 2>&1
+	_log_writes_mount -o dax
+	# Check options to be sure. XFS ignores dax option
+	# and goes on if dev underneath does not support dax.
+	_fs_options $LOGWRITES_DMDEV | grep -qw "dax" || \
+		_notrun "$LOGWRITES_DMDEV $FSTYP does not support -o dax"
+	_log_writes_unmount
+	_log_writes_remove
+}
+
 _log_writes_init()
 {
 	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
diff --git a/tests/generic/999 b/tests/generic/999
new file mode 100755
index 00000000..ca5772da
--- /dev/null
+++ b/tests/generic/999
@@ -0,0 +1,82 @@
+#! /bin/bash
+# FS QA Test No. 999
+#
+# Use dm-log-writes to verify that MAP_SYNC actually syncs metadata during
+# page faults.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Intel Corporation.  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`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_log_writes_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmlogwrites
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_log_writes_dax
+_require_xfs_io_command "log_writes"
+
+_log_writes_init
+_log_writes_mkfs >> $seqres.full 2>&1
+_log_writes_mount -o dax
+
+LEN=$((1024 * 1024)) # 1 MiB
+
+xfs_io -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
+	-c "log_writes -d $LOGWRITES_NAME -m preunmap" \
+	-f $SCRATCH_MNT/test
+
+# Unmount the scratch dir and tear down the log writes target
+_log_writes_unmount
+_log_writes_remove
+_check_scratch_fs
+
+# destroy previous filesystem so we can be sure our rebuild works
+_scratch_mkfs >> $seqres.full 2>&1
+
+# check pre-unmap state
+_log_writes_replay_log preunmap
+_scratch_mount
+
+# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
+du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
+
+_scratch_unmount
+_check_scratch_fs
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/999.out b/tests/generic/999.out
new file mode 100644
index 00000000..c7b8f8a2
--- /dev/null
+++ b/tests/generic/999.out
@@ -0,0 +1,3 @@
+QA output created by 999
+1.0M SCRATCH_MNT/test
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 6c3bb03a..ae88aa03 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -472,3 +472,4 @@
 467 auto quick exportfs
 468 shutdown auto quick metadata
 469 auto quick
+999 auto quick dax
-- 
2.14.3

Re: [fstests PATCH v5 1/2] dm-log-writes: only replay log to marks that exist

[Amir Goldstein]

On Wed, Dec 6, 2017 at 2:37 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> The 'replay-log' executable will replay the dm-log-writes log until the
> given mark, or until the end of the log if the mark isn't found.
>
> This means that if the mark you're looking for was never inserted in the
> log or if you give garbage to _log_writes_replay_log() the entire log will
> be replayed.  This can cause unexpected test results.
>
> Fix this by making sure that the mark we're given actually exists in the
> log before we allow the replay.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
Looks good.

Amir.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [fstests PATCH v5 1/2] dm-log-writes: only replay log to marks that exist

[Amir Goldstein]

On Wed, Dec 6, 2017 at 2:37 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> The 'replay-log' executable will replay the dm-log-writes log until the
> given mark, or until the end of the log if the mark isn't found.
>
> This means that if the mark you're looking for was never inserted in the
> log or if you give garbage to _log_writes_replay_log() the entire log will
> be replayed.  This can cause unexpected test results.
>
> Fix this by making sure that the mark we're given actually exists in the
> log before we allow the replay.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
Looks good.

Amir.

Re: [fstests PATCH v5 2/2] generic: add test for DAX MAP_SYNC support

[Amir Goldstein]

On Wed, Dec 6, 2017 at 2:37 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> This test creates a file and writes to it via an mmap(), but never syncs
> via fsync/msync.  This process is tracked via dm-log-writes, then replayed.
>
> If MAP_SYNC is working the dm-log-writes replay will show the test file
> with 1 MiB of on-media block allocations.  This is because each allocating
> page fault included an implicit metadata sync.  If MAP_SYNC isn't working
> (which you can test by removing the "-S" flag to xfs_io mmap) the file
> will be smaller or missing entirely.
>
> Note that dm-log-writes doesn't track the data that we write via the
> mmap(), so we can't do any data integrity checking.  We can only verify
> that the metadata writes for the page faults happened.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
>  common/dmlogwrites    | 20 +++++++++++++
>  tests/generic/999     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  3 ++
>  tests/generic/group   |  1 +
>  4 files changed, 106 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
>
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index 05829dbc..2b697bec 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -28,6 +28,26 @@ _require_log_writes()
>         _require_test_program "log-writes/replay-log"
>  }
>
> +_require_log_writes_dax()
> +{
> +       [ -z "$LOGWRITES_DEV" -o ! -b "$LOGWRITES_DEV" ] && \
> +               _notrun "This test requires a valid \$LOGWRITES_DEV"
> +
> +       _require_dm_target log-writes
> +       _require_test_program "log-writes/replay-log"
> +
> +       _scratch_unmount
> +       _log_writes_init
> +       _log_writes_mkfs > /dev/null 2>&1
> +       _log_writes_mount -o dax
> +       # Check options to be sure. XFS ignores dax option
> +       # and goes on if dev underneath does not support dax.
> +       _fs_options $LOGWRITES_DMDEV | grep -qw "dax" || \
> +               _notrun "$LOGWRITES_DMDEV $FSTYP does not support -o dax"
> +       _log_writes_unmount
> +       _log_writes_remove
> +}
> +
>  _log_writes_init()
>  {
>         local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..ca5772da
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,82 @@
> +#! /bin/bash
> +# FS QA Test No. 999
> +#
> +# Use dm-log-writes to verify that MAP_SYNC actually syncs metadata during
> +# page faults.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Intel Corporation.  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`
> +status=1       # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +       _log_writes_cleanup
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmlogwrites
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_log_writes_dax
> +_require_xfs_io_command "log_writes"
> +
> +_log_writes_init
> +_log_writes_mkfs >> $seqres.full 2>&1
> +_log_writes_mount -o dax
> +
> +LEN=$((1024 * 1024)) # 1 MiB
> +
> +xfs_io -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
> +       -c "log_writes -d $LOGWRITES_NAME -m preunmap" \
> +       -f $SCRATCH_MNT/test
> +
> +# Unmount the scratch dir and tear down the log writes target
> +_log_writes_unmount
> +_log_writes_remove
> +_check_scratch_fs
> +
> +# destroy previous filesystem so we can be sure our rebuild works
> +_scratch_mkfs >> $seqres.full 2>&1
> +
> +# check pre-unmap state
> +_log_writes_replay_log preunmap
> +_scratch_mount
> +
> +# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
> +du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
> +
> +_scratch_unmount
> +_check_scratch_fs
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..c7b8f8a2
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,3 @@
> +QA output created by 999
> +1.0M SCRATCH_MNT/test
> +Silence is golden

You test is not silent ;)

Otherwise looks good

Amir.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [fstests PATCH v5 2/2] generic: add test for DAX MAP_SYNC support

[Amir Goldstein]

On Wed, Dec 6, 2017 at 2:37 AM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> This test creates a file and writes to it via an mmap(), but never syncs
> via fsync/msync.  This process is tracked via dm-log-writes, then replayed.
>
> If MAP_SYNC is working the dm-log-writes replay will show the test file
> with 1 MiB of on-media block allocations.  This is because each allocating
> page fault included an implicit metadata sync.  If MAP_SYNC isn't working
> (which you can test by removing the "-S" flag to xfs_io mmap) the file
> will be smaller or missing entirely.
>
> Note that dm-log-writes doesn't track the data that we write via the
> mmap(), so we can't do any data integrity checking.  We can only verify
> that the metadata writes for the page faults happened.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
>  common/dmlogwrites    | 20 +++++++++++++
>  tests/generic/999     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  3 ++
>  tests/generic/group   |  1 +
>  4 files changed, 106 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
>
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index 05829dbc..2b697bec 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -28,6 +28,26 @@ _require_log_writes()
>         _require_test_program "log-writes/replay-log"
>  }
>
> +_require_log_writes_dax()
> +{
> +       [ -z "$LOGWRITES_DEV" -o ! -b "$LOGWRITES_DEV" ] && \
> +               _notrun "This test requires a valid \$LOGWRITES_DEV"
> +
> +       _require_dm_target log-writes
> +       _require_test_program "log-writes/replay-log"
> +
> +       _scratch_unmount
> +       _log_writes_init
> +       _log_writes_mkfs > /dev/null 2>&1
> +       _log_writes_mount -o dax
> +       # Check options to be sure. XFS ignores dax option
> +       # and goes on if dev underneath does not support dax.
> +       _fs_options $LOGWRITES_DMDEV | grep -qw "dax" || \
> +               _notrun "$LOGWRITES_DMDEV $FSTYP does not support -o dax"
> +       _log_writes_unmount
> +       _log_writes_remove
> +}
> +
>  _log_writes_init()
>  {
>         local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..ca5772da
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,82 @@
> +#! /bin/bash
> +# FS QA Test No. 999
> +#
> +# Use dm-log-writes to verify that MAP_SYNC actually syncs metadata during
> +# page faults.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Intel Corporation.  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`
> +status=1       # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +       _log_writes_cleanup
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmlogwrites
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_log_writes_dax
> +_require_xfs_io_command "log_writes"
> +
> +_log_writes_init
> +_log_writes_mkfs >> $seqres.full 2>&1
> +_log_writes_mount -o dax
> +
> +LEN=$((1024 * 1024)) # 1 MiB
> +
> +xfs_io -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
> +       -c "log_writes -d $LOGWRITES_NAME -m preunmap" \
> +       -f $SCRATCH_MNT/test
> +
> +# Unmount the scratch dir and tear down the log writes target
> +_log_writes_unmount
> +_log_writes_remove
> +_check_scratch_fs
> +
> +# destroy previous filesystem so we can be sure our rebuild works
> +_scratch_mkfs >> $seqres.full 2>&1
> +
> +# check pre-unmap state
> +_log_writes_replay_log preunmap
> +_scratch_mount
> +
> +# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
> +du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
> +
> +_scratch_unmount
> +_check_scratch_fs
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..c7b8f8a2
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,3 @@
> +QA output created by 999
> +1.0M SCRATCH_MNT/test
> +Silence is golden

You test is not silent ;)

Otherwise looks good

Amir.

Re: [fstests PATCH v5 2/2] generic: add test for DAX MAP_SYNC support

[Eryu Guan]

On Tue, Dec 05, 2017 at 05:37:44PM -0700, Ross Zwisler wrote:
> This test creates a file and writes to it via an mmap(), but never syncs
> via fsync/msync.  This process is tracked via dm-log-writes, then replayed.
> 
> If MAP_SYNC is working the dm-log-writes replay will show the test file
> with 1 MiB of on-media block allocations.  This is because each allocating
> page fault included an implicit metadata sync.  If MAP_SYNC isn't working
> (which you can test by removing the "-S" flag to xfs_io mmap) the file
> will be smaller or missing entirely.
> 
> Note that dm-log-writes doesn't track the data that we write via the
> mmap(), so we can't do any data integrity checking.  We can only verify
> that the metadata writes for the page faults happened.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
>  common/dmlogwrites    | 20 +++++++++++++
>  tests/generic/999     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  3 ++
>  tests/generic/group   |  1 +
>  4 files changed, 106 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
> 
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index 05829dbc..2b697bec 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -28,6 +28,26 @@ _require_log_writes()
>  	_require_test_program "log-writes/replay-log"
>  }
>  
> +_require_log_writes_dax()
> +{
> +	[ -z "$LOGWRITES_DEV" -o ! -b "$LOGWRITES_DEV" ] && \
> +		_notrun "This test requires a valid \$LOGWRITES_DEV"
> +
> +	_require_dm_target log-writes
> +	_require_test_program "log-writes/replay-log"
> +
> +	_scratch_unmount
> +	_log_writes_init
> +	_log_writes_mkfs > /dev/null 2>&1
> +	_log_writes_mount -o dax
> +	# Check options to be sure. XFS ignores dax option
> +	# and goes on if dev underneath does not support dax.
> +	_fs_options $LOGWRITES_DMDEV | grep -qw "dax" || \
> +		_notrun "$LOGWRITES_DMDEV $FSTYP does not support -o dax"
> +	_log_writes_unmount
> +	_log_writes_remove
> +}

Now we have two _require rules to test log_writes dm target:
_require_log_writes 	# _notrun explicitly when MOUNT_OPTIONS contains dax
_require_log_writes_dax	# _notrun if log-writes target doesn't support dax

I think we can merge the two into one, i.e. extend _require_log_writes
to check dax support status only when
- MOUNT_OPTIONS contains dax, or
- dax is given as a param explicitly, e.g. _require_log_writes dax

So old kernels that don't support dax log-writes still _notrun, and new
kernels that have dax log-writes support could run all log-writes tests,
like generic/455 and generic/457, in dax environment.

(I did a quick test with generic/45[57] on v4.15-rc2 kernel, 455 always
fails due to md5sum mismatch, not sure where the problem is yet; 457 is
_notrun, perhaps due to there's no dax support on reflink XFS.)

> +
>  _log_writes_init()
>  {
>  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..ca5772da
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,82 @@
> +#! /bin/bash
> +# FS QA Test No. 999
> +#
> +# Use dm-log-writes to verify that MAP_SYNC actually syncs metadata during
> +# page faults.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Intel Corporation.  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`
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	_log_writes_cleanup
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmlogwrites
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux

Need _require_scratch before _require_log_writes

> +_require_log_writes_dax
> +_require_xfs_io_command "log_writes"

Also need to check "-S" option of mmap xfs_io command.

> +
> +_log_writes_init
> +_log_writes_mkfs >> $seqres.full 2>&1
> +_log_writes_mount -o dax
> +
> +LEN=$((1024 * 1024)) # 1 MiB
> +
> +xfs_io -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
> +	-c "log_writes -d $LOGWRITES_NAME -m preunmap" \
> +	-f $SCRATCH_MNT/test

$XFS_IO_PROG

> +
> +# Unmount the scratch dir and tear down the log writes target
> +_log_writes_unmount
> +_log_writes_remove
> +_check_scratch_fs
> +
> +# destroy previous filesystem so we can be sure our rebuild works
> +_scratch_mkfs >> $seqres.full 2>&1
> +
> +# check pre-unmap state
> +_log_writes_replay_log preunmap
> +_scratch_mount
> +
> +# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
> +du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
> +
> +_scratch_unmount
> +_check_scratch_fs

No need to umount & check scratch fs, 'check' will do it after test, as
long as we called _require_scratch

> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..c7b8f8a2
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,3 @@
> +QA output created by 999
> +1.0M SCRATCH_MNT/test
> +Silence is golden

Yeah, as Amir mentioned, test is not silence :)

Thanks,
Eryu

> diff --git a/tests/generic/group b/tests/generic/group
> index 6c3bb03a..ae88aa03 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -472,3 +472,4 @@
>  467 auto quick exportfs
>  468 shutdown auto quick metadata
>  469 auto quick
> +999 auto quick dax
> -- 
> 2.14.3
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [fstests PATCH v5 2/2] generic: add test for DAX MAP_SYNC support

[Eryu Guan]

On Tue, Dec 05, 2017 at 05:37:44PM -0700, Ross Zwisler wrote:
> This test creates a file and writes to it via an mmap(), but never syncs
> via fsync/msync.  This process is tracked via dm-log-writes, then replayed.
> 
> If MAP_SYNC is working the dm-log-writes replay will show the test file
> with 1 MiB of on-media block allocations.  This is because each allocating
> page fault included an implicit metadata sync.  If MAP_SYNC isn't working
> (which you can test by removing the "-S" flag to xfs_io mmap) the file
> will be smaller or missing entirely.
> 
> Note that dm-log-writes doesn't track the data that we write via the
> mmap(), so we can't do any data integrity checking.  We can only verify
> that the metadata writes for the page faults happened.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
>  common/dmlogwrites    | 20 +++++++++++++
>  tests/generic/999     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  3 ++
>  tests/generic/group   |  1 +
>  4 files changed, 106 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
> 
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index 05829dbc..2b697bec 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -28,6 +28,26 @@ _require_log_writes()
>  	_require_test_program "log-writes/replay-log"
>  }
>  
> +_require_log_writes_dax()
> +{
> +	[ -z "$LOGWRITES_DEV" -o ! -b "$LOGWRITES_DEV" ] && \
> +		_notrun "This test requires a valid \$LOGWRITES_DEV"
> +
> +	_require_dm_target log-writes
> +	_require_test_program "log-writes/replay-log"
> +
> +	_scratch_unmount
> +	_log_writes_init
> +	_log_writes_mkfs > /dev/null 2>&1
> +	_log_writes_mount -o dax
> +	# Check options to be sure. XFS ignores dax option
> +	# and goes on if dev underneath does not support dax.
> +	_fs_options $LOGWRITES_DMDEV | grep -qw "dax" || \
> +		_notrun "$LOGWRITES_DMDEV $FSTYP does not support -o dax"
> +	_log_writes_unmount
> +	_log_writes_remove
> +}

Now we have two _require rules to test log_writes dm target:
_require_log_writes 	# _notrun explicitly when MOUNT_OPTIONS contains dax
_require_log_writes_dax	# _notrun if log-writes target doesn't support dax

I think we can merge the two into one, i.e. extend _require_log_writes
to check dax support status only when
- MOUNT_OPTIONS contains dax, or
- dax is given as a param explicitly, e.g. _require_log_writes dax

So old kernels that don't support dax log-writes still _notrun, and new
kernels that have dax log-writes support could run all log-writes tests,
like generic/455 and generic/457, in dax environment.

(I did a quick test with generic/45[57] on v4.15-rc2 kernel, 455 always
fails due to md5sum mismatch, not sure where the problem is yet; 457 is
_notrun, perhaps due to there's no dax support on reflink XFS.)

> +
>  _log_writes_init()
>  {
>  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..ca5772da
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,82 @@
> +#! /bin/bash
> +# FS QA Test No. 999
> +#
> +# Use dm-log-writes to verify that MAP_SYNC actually syncs metadata during
> +# page faults.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Intel Corporation.  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`
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	_log_writes_cleanup
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmlogwrites
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux

Need _require_scratch before _require_log_writes

> +_require_log_writes_dax
> +_require_xfs_io_command "log_writes"

Also need to check "-S" option of mmap xfs_io command.

> +
> +_log_writes_init
> +_log_writes_mkfs >> $seqres.full 2>&1
> +_log_writes_mount -o dax
> +
> +LEN=$((1024 * 1024)) # 1 MiB
> +
> +xfs_io -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
> +	-c "log_writes -d $LOGWRITES_NAME -m preunmap" \
> +	-f $SCRATCH_MNT/test

$XFS_IO_PROG

> +
> +# Unmount the scratch dir and tear down the log writes target
> +_log_writes_unmount
> +_log_writes_remove
> +_check_scratch_fs
> +
> +# destroy previous filesystem so we can be sure our rebuild works
> +_scratch_mkfs >> $seqres.full 2>&1
> +
> +# check pre-unmap state
> +_log_writes_replay_log preunmap
> +_scratch_mount
> +
> +# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
> +du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
> +
> +_scratch_unmount
> +_check_scratch_fs

No need to umount & check scratch fs, 'check' will do it after test, as
long as we called _require_scratch

> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..c7b8f8a2
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,3 @@
> +QA output created by 999
> +1.0M SCRATCH_MNT/test
> +Silence is golden

Yeah, as Amir mentioned, test is not silence :)

Thanks,
Eryu

> diff --git a/tests/generic/group b/tests/generic/group
> index 6c3bb03a..ae88aa03 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -472,3 +472,4 @@
>  467 auto quick exportfs
>  468 shutdown auto quick metadata
>  469 auto quick
> +999 auto quick dax
> -- 
> 2.14.3
> 

Re: [fstests PATCH v5 2/2] generic: add test for DAX MAP_SYNC support

[Ross Zwisler]

On Thu, Dec 07, 2017 at 06:36:57PM +0800, Eryu Guan wrote:

> Now we have two _require rules to test log_writes dm target:
> _require_log_writes 	# _notrun explicitly when MOUNT_OPTIONS contains dax
> _require_log_writes_dax	# _notrun if log-writes target doesn't support dax
> 
> I think we can merge the two into one, i.e. extend _require_log_writes
> to check dax support status only when
> - MOUNT_OPTIONS contains dax, or
> - dax is given as a param explicitly, e.g. _require_log_writes dax
> 
> So old kernels that don't support dax log-writes still _notrun, and new
> kernels that have dax log-writes support could run all log-writes tests,
> like generic/455 and generic/457, in dax environment.
> 
> (I did a quick test with generic/45[57] on v4.15-rc2 kernel, 455 always
> fails due to md5sum mismatch, not sure where the problem is yet; 457 is
> _notrun, perhaps due to there's no dax support on reflink XFS.)

I looked in to generic/455 md5sum mismatch, and it is expected with the
current DAX support found in dm-log-writes.  The issue is that we snoop I/O,
but we *don't* snoop the data written by mmap().

For DAX mmaps, the sync calls msync()/fsync() don't cause writes to the block
driver of flushed page cache pages as they would in the page cache case.
Instead the user writes directly into the persistent memory, and all we see is
a flush call.  For us to properly handle mmap() writes we'll need to catch the
flush happening in dm-log-writes, iterate through the flushed data and copy it
into the dm-log-writes log.

I actually had this implemented in my initial version of the DAX support for
dm-log-writes, but by the time I was ready to merge the DAX flush path had
been removed from DM.  See
commit c3ca015fab6d ("dax: remove the pmem_dax_ops->flush abstraction")
for more info.

I can look at adding some level of flush support back into DM so we can handle
this case.  Until/unless that happens, though, I think we should continue to
make users specifically request DAX+dm-log-writes support that lacks mmap()
data verfication capabilities via _require_log_writes_dax.

Thank you for the feedback.  I'll post an updated patch that takes care of the
rest of your comments.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [fstests PATCH v5 2/2] generic: add test for DAX MAP_SYNC support

[Ross Zwisler]

On Thu, Dec 07, 2017 at 06:36:57PM +0800, Eryu Guan wrote:

> Now we have two _require rules to test log_writes dm target:
> _require_log_writes 	# _notrun explicitly when MOUNT_OPTIONS contains dax
> _require_log_writes_dax	# _notrun if log-writes target doesn't support dax
> 
> I think we can merge the two into one, i.e. extend _require_log_writes
> to check dax support status only when
> - MOUNT_OPTIONS contains dax, or
> - dax is given as a param explicitly, e.g. _require_log_writes dax
> 
> So old kernels that don't support dax log-writes still _notrun, and new
> kernels that have dax log-writes support could run all log-writes tests,
> like generic/455 and generic/457, in dax environment.
> 
> (I did a quick test with generic/45[57] on v4.15-rc2 kernel, 455 always
> fails due to md5sum mismatch, not sure where the problem is yet; 457 is
> _notrun, perhaps due to there's no dax support on reflink XFS.)

I looked in to generic/455 md5sum mismatch, and it is expected with the
current DAX support found in dm-log-writes.  The issue is that we snoop I/O,
but we *don't* snoop the data written by mmap().

For DAX mmaps, the sync calls msync()/fsync() don't cause writes to the block
driver of flushed page cache pages as they would in the page cache case.
Instead the user writes directly into the persistent memory, and all we see is
a flush call.  For us to properly handle mmap() writes we'll need to catch the
flush happening in dm-log-writes, iterate through the flushed data and copy it
into the dm-log-writes log.

I actually had this implemented in my initial version of the DAX support for
dm-log-writes, but by the time I was ready to merge the DAX flush path had
been removed from DM.  See
commit c3ca015fab6d ("dax: remove the pmem_dax_ops->flush abstraction")
for more info.

I can look at adding some level of flush support back into DM so we can handle
this case.  Until/unless that happens, though, I think we should continue to
make users specifically request DAX+dm-log-writes support that lacks mmap()
data verfication capabilities via _require_log_writes_dax.

Thank you for the feedback.  I'll post an updated patch that takes care of the
rest of your comments.

[fstests PATCH v6 2/2] generic: add test for DAX MAP_SYNC support

[Ross Zwisler]

This test creates a file and writes to it via an mmap(), but never syncs
via fsync/msync.  This process is tracked via dm-log-writes, then replayed.

If MAP_SYNC is working the dm-log-writes replay will show the test file
with 1 MiB of on-media block allocations.  This is because each allocating
page fault included an implicit metadata sync.  If MAP_SYNC isn't working
(which you can test by removing the "-S" flag to xfs_io mmap) the file
will be smaller or missing entirely.

Note that dm-log-writes doesn't track the data that we write via the
mmap(), so we can't do any data integrity checking.  We can only verify
that the metadata writes for the page faults happened.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>

---

Changes since v5:
 - Addressed Eryu's comments.  Thanks for the review.

---
 common/dmlogwrites    | 19 ++++++++++++
 tests/generic/999     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  2 ++
 tests/generic/group   |  1 +
 4 files changed, 102 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

diff --git a/common/dmlogwrites b/common/dmlogwrites
index 05829dbc..c235ee49 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -28,6 +28,25 @@ _require_log_writes()
 	_require_test_program "log-writes/replay-log"
 }
 
+_require_log_writes_dax()
+{
+	[ -z "$LOGWRITES_DEV" -o ! -b "$LOGWRITES_DEV" ] && \
+		_notrun "This test requires a valid \$LOGWRITES_DEV"
+
+	_require_dm_target log-writes
+	_require_test_program "log-writes/replay-log"
+
+	_log_writes_init
+	_log_writes_mkfs > /dev/null 2>&1
+	_log_writes_mount -o dax
+	# Check options to be sure. XFS ignores dax option
+	# and goes on if dev underneath does not support dax.
+	_fs_options $LOGWRITES_DMDEV | grep -qw "dax" || \
+		_notrun "$LOGWRITES_DMDEV $FSTYP does not support -o dax"
+	_log_writes_unmount
+	_log_writes_remove
+}
+
 _log_writes_init()
 {
 	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
diff --git a/tests/generic/999 b/tests/generic/999
new file mode 100755
index 00000000..e20faa8f
--- /dev/null
+++ b/tests/generic/999
@@ -0,0 +1,80 @@
+#! /bin/bash
+# FS QA Test No. 999
+#
+# Use dm-log-writes to verify that MAP_SYNC actually syncs metadata during
+# page faults.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Intel Corporation.  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`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_log_writes_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmlogwrites
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_log_writes_dax
+_require_xfs_io_command "mmap" "-S"
+_require_xfs_io_command "log_writes"
+
+_log_writes_init
+_log_writes_mkfs >> $seqres.full 2>&1
+_log_writes_mount -o dax
+
+LEN=$((1024 * 1024)) # 1 MiB
+
+$XFS_IO_PROG -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
+	-c "log_writes -d $LOGWRITES_NAME -m preunmap" \
+	-f $SCRATCH_MNT/test
+
+# Unmount the scratch dir and tear down the log writes target
+_log_writes_unmount
+_log_writes_remove
+_check_scratch_fs
+
+# destroy previous filesystem so we can be sure our rebuild works
+_scratch_mkfs >> $seqres.full 2>&1
+
+# check pre-unmap state
+_log_writes_replay_log preunmap
+_scratch_mount
+
+# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
+du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
+
+status=0
+exit
diff --git a/tests/generic/999.out b/tests/generic/999.out
new file mode 100644
index 00000000..611289b6
--- /dev/null
+++ b/tests/generic/999.out
@@ -0,0 +1,2 @@
+QA output created by 999
+1.0M SCRATCH_MNT/test
diff --git a/tests/generic/group b/tests/generic/group
index 6c3bb03a..ae88aa03 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -472,3 +472,4 @@
 467 auto quick exportfs
 468 shutdown auto quick metadata
 469 auto quick
+999 auto quick dax
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[fstests PATCH v6 2/2] generic: add test for DAX MAP_SYNC support

[Ross Zwisler]

This test creates a file and writes to it via an mmap(), but never syncs
via fsync/msync.  This process is tracked via dm-log-writes, then replayed.

If MAP_SYNC is working the dm-log-writes replay will show the test file
with 1 MiB of on-media block allocations.  This is because each allocating
page fault included an implicit metadata sync.  If MAP_SYNC isn't working
(which you can test by removing the "-S" flag to xfs_io mmap) the file
will be smaller or missing entirely.

Note that dm-log-writes doesn't track the data that we write via the
mmap(), so we can't do any data integrity checking.  We can only verify
that the metadata writes for the page faults happened.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>

---

Changes since v5:
 - Addressed Eryu's comments.  Thanks for the review.

---
 common/dmlogwrites    | 19 ++++++++++++
 tests/generic/999     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  2 ++
 tests/generic/group   |  1 +
 4 files changed, 102 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

diff --git a/common/dmlogwrites b/common/dmlogwrites
index 05829dbc..c235ee49 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -28,6 +28,25 @@ _require_log_writes()
 	_require_test_program "log-writes/replay-log"
 }
 
+_require_log_writes_dax()
+{
+	[ -z "$LOGWRITES_DEV" -o ! -b "$LOGWRITES_DEV" ] && \
+		_notrun "This test requires a valid \$LOGWRITES_DEV"
+
+	_require_dm_target log-writes
+	_require_test_program "log-writes/replay-log"
+
+	_log_writes_init
+	_log_writes_mkfs > /dev/null 2>&1
+	_log_writes_mount -o dax
+	# Check options to be sure. XFS ignores dax option
+	# and goes on if dev underneath does not support dax.
+	_fs_options $LOGWRITES_DMDEV | grep -qw "dax" || \
+		_notrun "$LOGWRITES_DMDEV $FSTYP does not support -o dax"
+	_log_writes_unmount
+	_log_writes_remove
+}
+
 _log_writes_init()
 {
 	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
diff --git a/tests/generic/999 b/tests/generic/999
new file mode 100755
index 00000000..e20faa8f
--- /dev/null
+++ b/tests/generic/999
@@ -0,0 +1,80 @@
+#! /bin/bash
+# FS QA Test No. 999
+#
+# Use dm-log-writes to verify that MAP_SYNC actually syncs metadata during
+# page faults.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Intel Corporation.  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`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_log_writes_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmlogwrites
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_log_writes_dax
+_require_xfs_io_command "mmap" "-S"
+_require_xfs_io_command "log_writes"
+
+_log_writes_init
+_log_writes_mkfs >> $seqres.full 2>&1
+_log_writes_mount -o dax
+
+LEN=$((1024 * 1024)) # 1 MiB
+
+$XFS_IO_PROG -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
+	-c "log_writes -d $LOGWRITES_NAME -m preunmap" \
+	-f $SCRATCH_MNT/test
+
+# Unmount the scratch dir and tear down the log writes target
+_log_writes_unmount
+_log_writes_remove
+_check_scratch_fs
+
+# destroy previous filesystem so we can be sure our rebuild works
+_scratch_mkfs >> $seqres.full 2>&1
+
+# check pre-unmap state
+_log_writes_replay_log preunmap
+_scratch_mount
+
+# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
+du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
+
+status=0
+exit
diff --git a/tests/generic/999.out b/tests/generic/999.out
new file mode 100644
index 00000000..611289b6
--- /dev/null
+++ b/tests/generic/999.out
@@ -0,0 +1,2 @@
+QA output created by 999
+1.0M SCRATCH_MNT/test
diff --git a/tests/generic/group b/tests/generic/group
index 6c3bb03a..ae88aa03 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -472,3 +472,4 @@
 467 auto quick exportfs
 468 shutdown auto quick metadata
 469 auto quick
+999 auto quick dax
-- 
2.14.3

Re: [fstests PATCH v6 2/2] generic: add test for DAX MAP_SYNC support

[Eryu Guan]

On Thu, Dec 07, 2017 at 04:19:50PM -0700, Ross Zwisler wrote:
> This test creates a file and writes to it via an mmap(), but never syncs
> via fsync/msync.  This process is tracked via dm-log-writes, then replayed.
> 
> If MAP_SYNC is working the dm-log-writes replay will show the test file
> with 1 MiB of on-media block allocations.  This is because each allocating
> page fault included an implicit metadata sync.  If MAP_SYNC isn't working
> (which you can test by removing the "-S" flag to xfs_io mmap) the file
> will be smaller or missing entirely.
> 
> Note that dm-log-writes doesn't track the data that we write via the
> mmap(), so we can't do any data integrity checking.  We can only verify
> that the metadata writes for the page faults happened.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> 
> ---
> 
> Changes since v5:
>  - Addressed Eryu's comments.  Thanks for the review.

Thanks for the explanation on the dm-log-writes dax support! I agree
that we should keep _require_log_writes_dax for now till dm-log-writes
gains full dax support. I added some comments about this above the
helper definition and queued the patchset for next fstests update. (Test
was re-numbered as generic/470, BTW.)

Thanks,
Eryu
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [fstests PATCH v6 2/2] generic: add test for DAX MAP_SYNC support

[Eryu Guan]

On Thu, Dec 07, 2017 at 04:19:50PM -0700, Ross Zwisler wrote:
> This test creates a file and writes to it via an mmap(), but never syncs
> via fsync/msync.  This process is tracked via dm-log-writes, then replayed.
> 
> If MAP_SYNC is working the dm-log-writes replay will show the test file
> with 1 MiB of on-media block allocations.  This is because each allocating
> page fault included an implicit metadata sync.  If MAP_SYNC isn't working
> (which you can test by removing the "-S" flag to xfs_io mmap) the file
> will be smaller or missing entirely.
> 
> Note that dm-log-writes doesn't track the data that we write via the
> mmap(), so we can't do any data integrity checking.  We can only verify
> that the metadata writes for the page faults happened.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> 
> ---
> 
> Changes since v5:
>  - Addressed Eryu's comments.  Thanks for the review.

Thanks for the explanation on the dm-log-writes dax support! I agree
that we should keep _require_log_writes_dax for now till dm-log-writes
gains full dax support. I added some comments about this above the
helper definition and queued the patchset for next fstests update. (Test
was re-numbered as generic/470, BTW.)

Thanks,
Eryu

Re: [fstests PATCH v6 2/2] generic: add test for DAX MAP_SYNC support

[Ross Zwisler]

On Fri, Dec 08, 2017 at 02:36:10PM +0800, Eryu Guan wrote:

> (Test was re-numbered as generic/470, BTW.)

Thanks!  For future reference, does the pattern of us submitting tests with
high numbers (generic/999) to avoid merge conflicts and asking you to renumber
them when you merge work for you?  Or would you prefer that we number our
tests to the next available, which may change from submission to submission?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [fstests PATCH v6 2/2] generic: add test for DAX MAP_SYNC support

[Ross Zwisler]

On Fri, Dec 08, 2017 at 02:36:10PM +0800, Eryu Guan wrote:

> (Test was re-numbered as generic/470, BTW.)

Thanks!  For future reference, does the pattern of us submitting tests with
high numbers (generic/999) to avoid merge conflicts and asking you to renumber
them when you merge work for you?  Or would you prefer that we number our
tests to the next available, which may change from submission to submission?

Re: [fstests PATCH v6 2/2] generic: add test for DAX MAP_SYNC support

[Eryu Guan]

On Fri, Dec 08, 2017 at 10:47:47AM -0700, Ross Zwisler wrote:
> On Fri, Dec 08, 2017 at 02:36:10PM +0800, Eryu Guan wrote:
> 
> > (Test was re-numbered as generic/470, BTW.)
> 
> Thanks!  For future reference, does the pattern of us submitting tests with
> high numbers (generic/999) to avoid merge conflicts and asking you to renumber
> them when you merge work for you?  Or would you prefer that we number our
> tests to the next available, which may change from submission to submission?

For patch that adds a single test, either way is fine, I need to edit
the patch anyway on conflicts, as the group file conflicts. For patch or
patchset that adds multiple tests, starting with a high test seq number
would be better, I only need to edit the group file by hand, not the seq
numbers in each test (that can be done by ./tools/mvtest script).

But overall, the starting seq number doesn't matter that much. OTOH,
basing new tests on top of latest master as much as possible would be
perfered, that reduces the possibility of conflicts, as I only need to
resolve conflicts within all the new tests.

Thanks,
Eryu
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [fstests PATCH v6 2/2] generic: add test for DAX MAP_SYNC support

[Eryu Guan]

On Fri, Dec 08, 2017 at 10:47:47AM -0700, Ross Zwisler wrote:
> On Fri, Dec 08, 2017 at 02:36:10PM +0800, Eryu Guan wrote:
> 
> > (Test was re-numbered as generic/470, BTW.)
> 
> Thanks!  For future reference, does the pattern of us submitting tests with
> high numbers (generic/999) to avoid merge conflicts and asking you to renumber
> them when you merge work for you?  Or would you prefer that we number our
> tests to the next available, which may change from submission to submission?

For patch that adds a single test, either way is fine, I need to edit
the patch anyway on conflicts, as the group file conflicts. For patch or
patchset that adds multiple tests, starting with a high test seq number
would be better, I only need to edit the group file by hand, not the seq
numbers in each test (that can be done by ./tools/mvtest script).

But overall, the starting seq number doesn't matter that much. OTOH,
basing new tests on top of latest master as much as possible would be
perfered, that reduces the possibility of conflicts, as I only need to
resolve conflicts within all the new tests.

Thanks,
Eryu