All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] generic/290: Add test for fallocate zero range
@ 2014-02-25 19:15 ` Lukas Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: xfs, linux-fsdevel, Lukas Czerner

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 common/rc             |  14 +++++++
 tests/generic/290     |  92 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/290.out | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/group   |   1 +
 4 files changed, 216 insertions(+)
 create mode 100755 tests/generic/290
 create mode 100644 tests/generic/290.out

diff --git a/common/rc b/common/rc
index f2c3c3a..d3ec4db 100644
--- a/common/rc
+++ b/common/rc
@@ -1359,6 +1359,20 @@ _require_xfs_io_fiemap()
 		_notrun "xfs_io fiemap command failed (no fs support?)"
 }
 
+# check that xfs_io, kernel and filesystem all support fallocate with zero
+# range
+_require_xfs_io_falloc_zero()
+{
+	testfile=$TEST_DIR/$$.falloc
+	testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
+		-c "fzero 4k 8k" $testfile 2>&1`
+	rm -f $testfile 2>&1 > /dev/null
+	echo $testio | grep -q "not found" && \
+		_notrun "xfs_io fallocate zero range support is missing"
+	echo $testio | grep -q "Operation not supported" && \
+		_notrun "xfs_io fallocate fzero command failed (no fs support?)"
+}
+
 # Check that a fs has enough free space (in 1024b blocks)
 #
 _require_fs_space()
diff --git a/tests/generic/290 b/tests/generic/290
new file mode 100755
index 0000000..90f560a
--- /dev/null
+++ b/tests/generic/290
@@ -0,0 +1,92 @@
+#! /bin/bash
+# FS QA Test No. 290
+#
+# Makes calls to fallocate zero range and checks tossed ranges
+#
+# Nothing should be tossed unless the range includes a page boundry
+#
+# Primarily tests page boundries and boundries that are
+#  off-by-one to ensure we're only tossing what's expected
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2012 SGI.  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 "exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_os Linux
+
+_require_xfs_io_falloc_zero
+
+testfile=$TEST_DIR/290.$$
+
+test_zero()
+{
+	zero_start=$1
+	zero_len=$2
+
+	$XFS_IO_PROG -f -t -c "pwrite -S 0x41 0 4096" \
+                      -c "pwrite -S 0x42 4096 4096" \
+                      -c "fzero $zero_start $zero_len" \
+                      -c "pread -v 0 8192" \
+                      $testfile | _filter_xfs_io_unique
+}
+
+echo "[0,1] -- Shouldn't toss anything"
+test_zero    0    1
+
+echo "[0,4095] -- Shouldn't toss anything"
+test_zero    0 4095
+
+echo "[0,4096] -- Should toss first page"
+test_zero    0 4096
+
+echo "[0,4097] -- Should toss first page"
+test_zero    0 4097
+
+echo "[4095,8191] -- Should toss last byte of first page"
+test_zero 4095 4096
+
+echo "[4095,8192] -- Should toss second page & last byte of first page"
+test_zero 4095 4097
+
+echo "[4095,8193] -- Should toss second page & last byte of first page"
+test_zero 4095 4098
+
+echo "[4096,8192] -- Should toss second page"
+test_zero 4096 4096
+
+echo "[1024,5120] -- Should toss from 1024 to end of first page"
+test_zero 1024 4096
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/290.out b/tests/generic/290.out
new file mode 100644
index 0000000..4614540
--- /dev/null
+++ b/tests/generic/290.out
@@ -0,0 +1,109 @@
+QA output created by 290
+[0,1] -- Shouldn't toss anything
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  .AAAAAAAAAAAAAAA
+00000010:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[0,4095] -- Shouldn't toss anything
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00000ff0:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 41  ...............A
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[0,4096] -- Should toss first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[0,4097] -- Should toss first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001000:  00 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  .BBBBBBBBBBBBBBB
+00001010:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[4095,8191] -- Should toss last byte of first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00  AAAAAAAAAAAAAAA.
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001ff0:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 42  ...............B
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[4095,8192] -- Should toss second page & last byte of first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00  AAAAAAAAAAAAAAA.
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[4095,8193] -- Should toss second page & last byte of first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00  AAAAAAAAAAAAAAA.
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[4096,8192] -- Should toss second page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[1024,5120] -- Should toss from 1024 to end of first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000400:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001400:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/generic/group b/tests/generic/group
index 34bd118..f3b2dc7 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -106,6 +106,7 @@
 285 auto rw
 286 auto quick other
 288 auto quick ioctl trim
+290 auto quick prealloc
 294 auto quick
 299 auto aio enospc rw stress
 300 auto aio enospc preallocrw stress
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 1/6] generic/290: Add test for fallocate zero range
@ 2014-02-25 19:15 ` Lukas Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, Lukas Czerner, xfs

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 common/rc             |  14 +++++++
 tests/generic/290     |  92 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/290.out | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/group   |   1 +
 4 files changed, 216 insertions(+)
 create mode 100755 tests/generic/290
 create mode 100644 tests/generic/290.out

diff --git a/common/rc b/common/rc
index f2c3c3a..d3ec4db 100644
--- a/common/rc
+++ b/common/rc
@@ -1359,6 +1359,20 @@ _require_xfs_io_fiemap()
 		_notrun "xfs_io fiemap command failed (no fs support?)"
 }
 
+# check that xfs_io, kernel and filesystem all support fallocate with zero
+# range
+_require_xfs_io_falloc_zero()
+{
+	testfile=$TEST_DIR/$$.falloc
+	testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
+		-c "fzero 4k 8k" $testfile 2>&1`
+	rm -f $testfile 2>&1 > /dev/null
+	echo $testio | grep -q "not found" && \
+		_notrun "xfs_io fallocate zero range support is missing"
+	echo $testio | grep -q "Operation not supported" && \
+		_notrun "xfs_io fallocate fzero command failed (no fs support?)"
+}
+
 # Check that a fs has enough free space (in 1024b blocks)
 #
 _require_fs_space()
diff --git a/tests/generic/290 b/tests/generic/290
new file mode 100755
index 0000000..90f560a
--- /dev/null
+++ b/tests/generic/290
@@ -0,0 +1,92 @@
+#! /bin/bash
+# FS QA Test No. 290
+#
+# Makes calls to fallocate zero range and checks tossed ranges
+#
+# Nothing should be tossed unless the range includes a page boundry
+#
+# Primarily tests page boundries and boundries that are
+#  off-by-one to ensure we're only tossing what's expected
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2012 SGI.  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 "exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_os Linux
+
+_require_xfs_io_falloc_zero
+
+testfile=$TEST_DIR/290.$$
+
+test_zero()
+{
+	zero_start=$1
+	zero_len=$2
+
+	$XFS_IO_PROG -f -t -c "pwrite -S 0x41 0 4096" \
+                      -c "pwrite -S 0x42 4096 4096" \
+                      -c "fzero $zero_start $zero_len" \
+                      -c "pread -v 0 8192" \
+                      $testfile | _filter_xfs_io_unique
+}
+
+echo "[0,1] -- Shouldn't toss anything"
+test_zero    0    1
+
+echo "[0,4095] -- Shouldn't toss anything"
+test_zero    0 4095
+
+echo "[0,4096] -- Should toss first page"
+test_zero    0 4096
+
+echo "[0,4097] -- Should toss first page"
+test_zero    0 4097
+
+echo "[4095,8191] -- Should toss last byte of first page"
+test_zero 4095 4096
+
+echo "[4095,8192] -- Should toss second page & last byte of first page"
+test_zero 4095 4097
+
+echo "[4095,8193] -- Should toss second page & last byte of first page"
+test_zero 4095 4098
+
+echo "[4096,8192] -- Should toss second page"
+test_zero 4096 4096
+
+echo "[1024,5120] -- Should toss from 1024 to end of first page"
+test_zero 1024 4096
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/290.out b/tests/generic/290.out
new file mode 100644
index 0000000..4614540
--- /dev/null
+++ b/tests/generic/290.out
@@ -0,0 +1,109 @@
+QA output created by 290
+[0,1] -- Shouldn't toss anything
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  .AAAAAAAAAAAAAAA
+00000010:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[0,4095] -- Shouldn't toss anything
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00000ff0:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 41  ...............A
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[0,4096] -- Should toss first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001000:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[0,4097] -- Should toss first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001000:  00 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  .BBBBBBBBBBBBBBB
+00001010:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[4095,8191] -- Should toss last byte of first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00  AAAAAAAAAAAAAAA.
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001ff0:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 42  ...............B
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[4095,8192] -- Should toss second page & last byte of first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00  AAAAAAAAAAAAAAA.
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[4095,8193] -- Should toss second page & last byte of first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000ff0:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 00  AAAAAAAAAAAAAAA.
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[4096,8192] -- Should toss second page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00001000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[1024,5120] -- Should toss from 1024 to end of first page
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+00000000:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
+*
+00000400:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
+*
+00001400:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+*
+read 8192/8192 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/generic/group b/tests/generic/group
index 34bd118..f3b2dc7 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -106,6 +106,7 @@
 285 auto rw
 286 auto quick other
 288 auto quick ioctl trim
+290 auto quick prealloc
 294 auto quick
 299 auto aio enospc rw stress
 300 auto aio enospc preallocrw stress
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 2/6] xfstests: Add fallocate zero range operation to fsstress
  2014-02-25 19:15 ` Lukas Czerner
@ 2014-02-25 19:15   ` Lukas Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: xfs, linux-fsdevel, Lukas Czerner

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 ltp/fsstress.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index c56f168..4c3368f 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -33,6 +33,9 @@
 /* Copy-paste from linux/falloc.h */
 #define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
 #endif
+#ifndef FALLOC_FL_ZERO_RANGE
+#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
+#endif
 #endif
 #ifndef HAVE_ATTR_LIST
 #define attr_list(path, buf, size, flags, cursor) (errno = -ENOSYS, -1)
@@ -77,6 +80,7 @@ typedef enum {
 	OP_MKDIR,
 	OP_MKNOD,
 	OP_PUNCH,
+	OP_ZERO,
 	OP_READ,
 	OP_READLINK,
 	OP_RENAME,
@@ -162,6 +166,7 @@ void	link_f(int, long);
 void	mkdir_f(int, long);
 void	mknod_f(int, long);
 void	punch_f(int, long);
+void	zero_f(int, long);
 void	read_f(int, long);
 void	readlink_f(int, long);
 void	rename_f(int, long);
@@ -199,6 +204,7 @@ opdesc_t	ops[] = {
 	{ OP_MKDIR, "mkdir", mkdir_f, 2, 1 },
 	{ OP_MKNOD, "mknod", mknod_f, 2, 1 },
 	{ OP_PUNCH, "punch", punch_f, 1, 1 },
+	{ OP_ZERO, "zero", zero_f, 1, 1 },
 	{ OP_READ, "read", read_f, 1, 0 },
 	{ OP_READLINK, "readlink", readlink_f, 1, 0 },
 	{ OP_RENAME, "rename", rename_f, 2, 1 },
@@ -2561,6 +2567,62 @@ punch_f(int opno, long r)
 }
 
 void
+zero_f(int opno, long r)
+{
+#ifdef FALLOCATE
+	int		e;
+	pathname_t	f;
+	int		fd;
+	__int64_t	lr;
+	off64_t		off;
+	off64_t		len;
+	struct stat64	stb;
+	int		v;
+	char		st[1024];
+	int mode = FALLOC_FL_ZERO_RANGE;
+
+	init_pathname(&f);
+	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
+		if (v)
+			printf("%d/%d: zero range - no filename\n", procid, opno);
+		free_pathname(&f);
+		return;
+	}
+	fd = open_path(&f, O_RDWR);
+	e = fd < 0 ? errno : 0;
+	check_cwd();
+	if (fd < 0) {
+		if (v)
+			printf("%d/%d: zero range - open %s failed %d\n",
+				procid, opno, f.path, e);
+		free_pathname(&f);
+		return;
+	}
+	if (fstat64(fd, &stb) < 0) {
+		if (v)
+			printf("%d/%d: zero range - fstat64 %s failed %d\n",
+				procid, opno, f.path, errno);
+		free_pathname(&f);
+		close(fd);
+		return;
+	}
+	inode_info(st, sizeof(st), &stb, v);
+	lr = ((__int64_t)random() << 32) + random();
+	off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE));
+	off %= maxfsize;
+	len = (off64_t)(random() % (1024 * 1024));
+	mode |= FALLOC_FL_KEEP_SIZE & random();
+	e = fallocate(fd, mode, (loff_t)off, (loff_t)len) < 0 ? errno : 0;
+	if (v)
+		printf("%d/%d: zero range(%d) %s %s %lld %lld %d\n",
+		       procid, opno, mode,
+		       f.path, st, (long long)off, (long long)len, e);
+	free_pathname(&f);
+	close(fd);
+#endif
+}
+
+void
 read_f(int opno, long r)
 {
 	char		*buf;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 2/6] xfstests: Add fallocate zero range operation to fsstress
@ 2014-02-25 19:15   ` Lukas Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, Lukas Czerner, xfs

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 ltp/fsstress.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index c56f168..4c3368f 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -33,6 +33,9 @@
 /* Copy-paste from linux/falloc.h */
 #define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
 #endif
+#ifndef FALLOC_FL_ZERO_RANGE
+#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
+#endif
 #endif
 #ifndef HAVE_ATTR_LIST
 #define attr_list(path, buf, size, flags, cursor) (errno = -ENOSYS, -1)
@@ -77,6 +80,7 @@ typedef enum {
 	OP_MKDIR,
 	OP_MKNOD,
 	OP_PUNCH,
+	OP_ZERO,
 	OP_READ,
 	OP_READLINK,
 	OP_RENAME,
@@ -162,6 +166,7 @@ void	link_f(int, long);
 void	mkdir_f(int, long);
 void	mknod_f(int, long);
 void	punch_f(int, long);
+void	zero_f(int, long);
 void	read_f(int, long);
 void	readlink_f(int, long);
 void	rename_f(int, long);
@@ -199,6 +204,7 @@ opdesc_t	ops[] = {
 	{ OP_MKDIR, "mkdir", mkdir_f, 2, 1 },
 	{ OP_MKNOD, "mknod", mknod_f, 2, 1 },
 	{ OP_PUNCH, "punch", punch_f, 1, 1 },
+	{ OP_ZERO, "zero", zero_f, 1, 1 },
 	{ OP_READ, "read", read_f, 1, 0 },
 	{ OP_READLINK, "readlink", readlink_f, 1, 0 },
 	{ OP_RENAME, "rename", rename_f, 2, 1 },
@@ -2561,6 +2567,62 @@ punch_f(int opno, long r)
 }
 
 void
+zero_f(int opno, long r)
+{
+#ifdef FALLOCATE
+	int		e;
+	pathname_t	f;
+	int		fd;
+	__int64_t	lr;
+	off64_t		off;
+	off64_t		len;
+	struct stat64	stb;
+	int		v;
+	char		st[1024];
+	int mode = FALLOC_FL_ZERO_RANGE;
+
+	init_pathname(&f);
+	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
+		if (v)
+			printf("%d/%d: zero range - no filename\n", procid, opno);
+		free_pathname(&f);
+		return;
+	}
+	fd = open_path(&f, O_RDWR);
+	e = fd < 0 ? errno : 0;
+	check_cwd();
+	if (fd < 0) {
+		if (v)
+			printf("%d/%d: zero range - open %s failed %d\n",
+				procid, opno, f.path, e);
+		free_pathname(&f);
+		return;
+	}
+	if (fstat64(fd, &stb) < 0) {
+		if (v)
+			printf("%d/%d: zero range - fstat64 %s failed %d\n",
+				procid, opno, f.path, errno);
+		free_pathname(&f);
+		close(fd);
+		return;
+	}
+	inode_info(st, sizeof(st), &stb, v);
+	lr = ((__int64_t)random() << 32) + random();
+	off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE));
+	off %= maxfsize;
+	len = (off64_t)(random() % (1024 * 1024));
+	mode |= FALLOC_FL_KEEP_SIZE & random();
+	e = fallocate(fd, mode, (loff_t)off, (loff_t)len) < 0 ? errno : 0;
+	if (v)
+		printf("%d/%d: zero range(%d) %s %s %lld %lld %d\n",
+		       procid, opno, mode,
+		       f.path, st, (long long)off, (long long)len, e);
+	free_pathname(&f);
+	close(fd);
+#endif
+}
+
+void
 read_f(int opno, long r)
 {
 	char		*buf;
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 3/6] xfstests: fsstress punch should always have FALLOC_FL_KEEP_SIZE set
  2014-02-25 19:15 ` Lukas Czerner
@ 2014-02-25 19:15   ` Lukas Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: xfs, linux-fsdevel, Lukas Czerner

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 ltp/fsstress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index 4c3368f..42c8a5a 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -2555,8 +2555,8 @@ punch_f(int opno, long r)
 	off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE));
 	off %= maxfsize;
 	len = (off64_t)(random() % (1024 * 1024));
-	mode |= FALLOC_FL_KEEP_SIZE & random();
 	e = fallocate(fd, mode, (loff_t)off, (loff_t)len) < 0 ? errno : 0;
+	mode |= FALLOC_FL_KEEP_SIZE;
 	if (v)
 		printf("%d/%d: punch hole(%d) %s %s %lld %lld %d\n",
 		       procid, opno, mode,
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 3/6] xfstests: fsstress punch should always have FALLOC_FL_KEEP_SIZE set
@ 2014-02-25 19:15   ` Lukas Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, Lukas Czerner, xfs

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 ltp/fsstress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index 4c3368f..42c8a5a 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -2555,8 +2555,8 @@ punch_f(int opno, long r)
 	off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE));
 	off %= maxfsize;
 	len = (off64_t)(random() % (1024 * 1024));
-	mode |= FALLOC_FL_KEEP_SIZE & random();
 	e = fallocate(fd, mode, (loff_t)off, (loff_t)len) < 0 ? errno : 0;
+	mode |= FALLOC_FL_KEEP_SIZE;
 	if (v)
 		printf("%d/%d: punch hole(%d) %s %s %lld %lld %d\n",
 		       procid, opno, mode,
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 4/6] xfstests: Add fallocate zero range operation to fsx
  2014-02-25 19:15 ` Lukas Czerner
@ 2014-02-25 19:15   ` Lukas Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: xfs, linux-fsdevel, Lukas Czerner

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 ltp/fsx.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 107 insertions(+), 3 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 2f1e3e8..b3c30db 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -72,6 +72,7 @@ int			logcount = 0;	/* total ops */
  * TRUNCATE:	-	4
  * FALLOCATE:	-	5
  * PUNCH HOLE:	-	6
+ * ZERO RANGE:	-	7
  *
  * When mapped read/writes are disabled, they are simply converted to normal
  * reads and writes. When fallocate/fpunch calls are disabled, they are
@@ -95,7 +96,8 @@ int			logcount = 0;	/* total ops */
 #define OP_TRUNCATE	4
 #define OP_FALLOCATE	5
 #define OP_PUNCH_HOLE	6
-#define OP_MAX_FULL	7
+#define OP_ZERO_RANGE	7
+#define OP_MAX_FULL	8
 
 /* operation modifiers */
 #define OP_CLOSEOPEN	100
@@ -142,6 +144,7 @@ int	seed = 1;			/* -S flag */
 int     mapped_writes = 1;              /* -W flag disables */
 int     fallocate_calls = 1;            /* -F flag disables */
 int     punch_hole_calls = 1;           /* -H flag disables */
+int     zero_range_calls = 1;           /* -E flag disables */
 int 	mapped_reads = 1;		/* -R flag disables it */
 int	fsxgoodfd = 0;
 int	o_direct;			/* -Z */
@@ -320,6 +323,14 @@ logdump(void)
 						     lp->args[0] + lp->args[1])
 				prt("\t******PPPP");
 			break;
+		case OP_ZERO_RANGE:
+			prt("ZERO    0x%x thru 0x%x\t(0x%x bytes)",
+			    lp->args[0], lp->args[0] + lp->args[1] - 1,
+			    lp->args[1]);
+			if (badoff >= lp->args[0] && badoff <
+						     lp->args[0] + lp->args[1])
+				prt("\t******ZZZZ");
+			break;
 		case OP_SKIPPED:
 			prt("SKIPPED (no operation)");
 			break;
@@ -882,6 +893,64 @@ do_punch_hole(unsigned offset, unsigned length)
 }
 #endif
 
+#ifdef FALLOC_FL_ZERO_RANGE
+void
+do_zero_range(unsigned offset, unsigned length)
+{
+	unsigned end_offset;
+	int max_offset = 0;
+	int max_len = 0;
+	int mode = FALLOC_FL_ZERO_RANGE;
+
+	if (length == 0) {
+		if (!quiet && testcalls > simulatedopcount)
+			prt("skipping zero length zero range\n");
+			log4(OP_SKIPPED, OP_ZERO_RANGE, offset, length);
+		return;
+	}
+
+	if (file_size <= (loff_t)offset) {
+		if (!quiet && testcalls > simulatedopcount)
+			prt("skipping zero range off the end of the file\n");
+			log4(OP_SKIPPED, OP_ZERO_RANGE, offset, length);
+		return;
+	}
+
+	end_offset = offset + length;
+
+	log4(OP_ZERO_RANGE, offset, length, 0);
+
+	if (testcalls <= simulatedopcount)
+		return;
+
+	if ((progressinterval && testcalls % progressinterval == 0) ||
+	    (debug && (monitorstart == -1 || monitorend == -1 ||
+		      end_offset <= monitorend))) {
+		prt("%lu zero\tfrom 0x%x to 0x%x, (0x%x bytes)\n", testcalls,
+			offset, offset+length, length);
+	}
+	if (fallocate(fd, mode, (loff_t)offset, (loff_t)length) == -1) {
+		prt("%pzero range: %x to %x\n", offset, length);
+		prterr("do_zero_range: fallocate");
+		report_failure(161);
+	}
+
+
+	max_offset = offset < file_size ? offset : file_size;
+	max_len = max_offset + length <= file_size ? length :
+			file_size - max_offset;
+	memset(good_buf + max_offset, '\0', max_len);
+}
+
+#else
+void
+do_zero_range(unsigned offset, unsigned length)
+{
+	return;
+}
+#endif
+
+
 #ifdef FALLOCATE
 /* fallocate is basically a no-op unless extending, then a lot like a truncate */
 void
@@ -1050,6 +1119,12 @@ test(void)
 			goto out;
 		}
 		break;
+	case OP_ZERO_RANGE:
+		if (!zero_range_calls) {
+			log4(OP_SKIPPED, OP_ZERO_RANGE, offset, size);
+			goto out;
+		}
+		break;
 	}
 
 	switch (op) {
@@ -1088,6 +1163,10 @@ test(void)
 		TRIM_OFF_LEN(offset, size, file_size);
 		do_punch_hole(offset, size);
 		break;
+	case OP_ZERO_RANGE:
+		TRIM_OFF_LEN(offset, size, file_size);
+		do_zero_range(offset, size);
+		break;
 	default:
 		prterr("test: unknown operation");
 		report_failure(42);
@@ -1143,7 +1222,10 @@ usage(void)
 "	-F: Do not use fallocate (preallocation) calls\n"
 #endif
 #ifdef FALLOC_FL_PUNCH_HOLE
-"       -H: Do not use punch hole calls\n"
+"	-H: Do not use punch hole calls\n"
+#endif
+#ifdef FALLOC_FL_ZERO_RANGE
+"	-E: Do not use zero range calls\n"
 #endif
 "	-L: fsxLite - no file creations & no file size changes\n\
 	-N numops: total # operations to do (default infinity)\n\
@@ -1330,6 +1412,24 @@ test_punch_hole()
 #endif
 }
 
+void
+test_zero_range()
+{
+#ifdef FALLOC_FL_ZERO_RANGE
+	if (!lite && zero_range_calls) {
+		if (fallocate(fd, FALLOC_FL_ZERO_RANGE,
+				0, 1) && errno == EOPNOTSUPP) {
+			if(!quiet)
+				warn("main: filesystem does not support fallocate zero range, disabling");
+			zero_range_calls = 0;
+		} else
+			ftruncate(fd, 0);
+	}
+#else /* ! ZERO RANGE */
+	zero_range_calls = 0;
+#endif
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1348,7 +1448,7 @@ main(int argc, char **argv)
 
 	setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */
 
-	while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHLN:OP:RS:WZ"))
+	while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHELN:OP:RS:WZ"))
 	       != EOF)
 		switch (ch) {
 		case 'b':
@@ -1448,6 +1548,9 @@ main(int argc, char **argv)
 		case 'H':
 			punch_hole_calls = 0;
 			break;
+		case 'E':
+			zero_range_calls = 0;
+			break;
 		case 'L':
 		        lite = 1;
 			break;
@@ -1595,6 +1698,7 @@ main(int argc, char **argv)
 
 	test_fallocate();
 	test_punch_hole();
+	test_zero_range();
 
 	while (numops == -1 || numops--)
 		test();
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 4/6] xfstests: Add fallocate zero range operation to fsx
@ 2014-02-25 19:15   ` Lukas Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, Lukas Czerner, xfs

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 ltp/fsx.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 107 insertions(+), 3 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 2f1e3e8..b3c30db 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -72,6 +72,7 @@ int			logcount = 0;	/* total ops */
  * TRUNCATE:	-	4
  * FALLOCATE:	-	5
  * PUNCH HOLE:	-	6
+ * ZERO RANGE:	-	7
  *
  * When mapped read/writes are disabled, they are simply converted to normal
  * reads and writes. When fallocate/fpunch calls are disabled, they are
@@ -95,7 +96,8 @@ int			logcount = 0;	/* total ops */
 #define OP_TRUNCATE	4
 #define OP_FALLOCATE	5
 #define OP_PUNCH_HOLE	6
-#define OP_MAX_FULL	7
+#define OP_ZERO_RANGE	7
+#define OP_MAX_FULL	8
 
 /* operation modifiers */
 #define OP_CLOSEOPEN	100
@@ -142,6 +144,7 @@ int	seed = 1;			/* -S flag */
 int     mapped_writes = 1;              /* -W flag disables */
 int     fallocate_calls = 1;            /* -F flag disables */
 int     punch_hole_calls = 1;           /* -H flag disables */
+int     zero_range_calls = 1;           /* -E flag disables */
 int 	mapped_reads = 1;		/* -R flag disables it */
 int	fsxgoodfd = 0;
 int	o_direct;			/* -Z */
@@ -320,6 +323,14 @@ logdump(void)
 						     lp->args[0] + lp->args[1])
 				prt("\t******PPPP");
 			break;
+		case OP_ZERO_RANGE:
+			prt("ZERO    0x%x thru 0x%x\t(0x%x bytes)",
+			    lp->args[0], lp->args[0] + lp->args[1] - 1,
+			    lp->args[1]);
+			if (badoff >= lp->args[0] && badoff <
+						     lp->args[0] + lp->args[1])
+				prt("\t******ZZZZ");
+			break;
 		case OP_SKIPPED:
 			prt("SKIPPED (no operation)");
 			break;
@@ -882,6 +893,64 @@ do_punch_hole(unsigned offset, unsigned length)
 }
 #endif
 
+#ifdef FALLOC_FL_ZERO_RANGE
+void
+do_zero_range(unsigned offset, unsigned length)
+{
+	unsigned end_offset;
+	int max_offset = 0;
+	int max_len = 0;
+	int mode = FALLOC_FL_ZERO_RANGE;
+
+	if (length == 0) {
+		if (!quiet && testcalls > simulatedopcount)
+			prt("skipping zero length zero range\n");
+			log4(OP_SKIPPED, OP_ZERO_RANGE, offset, length);
+		return;
+	}
+
+	if (file_size <= (loff_t)offset) {
+		if (!quiet && testcalls > simulatedopcount)
+			prt("skipping zero range off the end of the file\n");
+			log4(OP_SKIPPED, OP_ZERO_RANGE, offset, length);
+		return;
+	}
+
+	end_offset = offset + length;
+
+	log4(OP_ZERO_RANGE, offset, length, 0);
+
+	if (testcalls <= simulatedopcount)
+		return;
+
+	if ((progressinterval && testcalls % progressinterval == 0) ||
+	    (debug && (monitorstart == -1 || monitorend == -1 ||
+		      end_offset <= monitorend))) {
+		prt("%lu zero\tfrom 0x%x to 0x%x, (0x%x bytes)\n", testcalls,
+			offset, offset+length, length);
+	}
+	if (fallocate(fd, mode, (loff_t)offset, (loff_t)length) == -1) {
+		prt("%pzero range: %x to %x\n", offset, length);
+		prterr("do_zero_range: fallocate");
+		report_failure(161);
+	}
+
+
+	max_offset = offset < file_size ? offset : file_size;
+	max_len = max_offset + length <= file_size ? length :
+			file_size - max_offset;
+	memset(good_buf + max_offset, '\0', max_len);
+}
+
+#else
+void
+do_zero_range(unsigned offset, unsigned length)
+{
+	return;
+}
+#endif
+
+
 #ifdef FALLOCATE
 /* fallocate is basically a no-op unless extending, then a lot like a truncate */
 void
@@ -1050,6 +1119,12 @@ test(void)
 			goto out;
 		}
 		break;
+	case OP_ZERO_RANGE:
+		if (!zero_range_calls) {
+			log4(OP_SKIPPED, OP_ZERO_RANGE, offset, size);
+			goto out;
+		}
+		break;
 	}
 
 	switch (op) {
@@ -1088,6 +1163,10 @@ test(void)
 		TRIM_OFF_LEN(offset, size, file_size);
 		do_punch_hole(offset, size);
 		break;
+	case OP_ZERO_RANGE:
+		TRIM_OFF_LEN(offset, size, file_size);
+		do_zero_range(offset, size);
+		break;
 	default:
 		prterr("test: unknown operation");
 		report_failure(42);
@@ -1143,7 +1222,10 @@ usage(void)
 "	-F: Do not use fallocate (preallocation) calls\n"
 #endif
 #ifdef FALLOC_FL_PUNCH_HOLE
-"       -H: Do not use punch hole calls\n"
+"	-H: Do not use punch hole calls\n"
+#endif
+#ifdef FALLOC_FL_ZERO_RANGE
+"	-E: Do not use zero range calls\n"
 #endif
 "	-L: fsxLite - no file creations & no file size changes\n\
 	-N numops: total # operations to do (default infinity)\n\
@@ -1330,6 +1412,24 @@ test_punch_hole()
 #endif
 }
 
+void
+test_zero_range()
+{
+#ifdef FALLOC_FL_ZERO_RANGE
+	if (!lite && zero_range_calls) {
+		if (fallocate(fd, FALLOC_FL_ZERO_RANGE,
+				0, 1) && errno == EOPNOTSUPP) {
+			if(!quiet)
+				warn("main: filesystem does not support fallocate zero range, disabling");
+			zero_range_calls = 0;
+		} else
+			ftruncate(fd, 0);
+	}
+#else /* ! ZERO RANGE */
+	zero_range_calls = 0;
+#endif
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1348,7 +1448,7 @@ main(int argc, char **argv)
 
 	setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */
 
-	while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHLN:OP:RS:WZ"))
+	while ((ch = getopt(argc, argv, "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHELN:OP:RS:WZ"))
 	       != EOF)
 		switch (ch) {
 		case 'b':
@@ -1448,6 +1548,9 @@ main(int argc, char **argv)
 		case 'H':
 			punch_hole_calls = 0;
 			break;
+		case 'E':
+			zero_range_calls = 0;
+			break;
 		case 'L':
 		        lite = 1;
 			break;
@@ -1595,6 +1698,7 @@ main(int argc, char **argv)
 
 	test_fallocate();
 	test_punch_hole();
+	test_zero_range();
 
 	while (numops == -1 || numops--)
 		test();
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 5/6] xfstests: Define fallocate flags locally in fsx
  2014-02-25 19:15 ` Lukas Czerner
@ 2014-02-25 19:15   ` Lukas Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: xfs, linux-fsdevel, Lukas Czerner

Define fallocate flags such as FALLOC_FL_PUNCH_HOLE and
FALLOC_FL_ZERO_RANGE locally if they do not exist.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 ltp/fsx.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index b3c30db..331257e 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -35,8 +35,13 @@
 #endif
 #ifdef FALLOCATE
 #include <linux/falloc.h>
+#ifndef FALLOC_FL_PUNCH_HOLE
+#define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
+#endif
+#ifndef FALLOC_FL_ZERO_RANGE
+#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
+#endif
 #endif
-
 #ifndef MAP_FILE
 # define MAP_FILE 0
 #endif
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 5/6] xfstests: Define fallocate flags locally in fsx
@ 2014-02-25 19:15   ` Lukas Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, Lukas Czerner, xfs

Define fallocate flags such as FALLOC_FL_PUNCH_HOLE and
FALLOC_FL_ZERO_RANGE locally if they do not exist.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 ltp/fsx.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index b3c30db..331257e 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -35,8 +35,13 @@
 #endif
 #ifdef FALLOCATE
 #include <linux/falloc.h>
+#ifndef FALLOC_FL_PUNCH_HOLE
+#define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
+#endif
+#ifndef FALLOC_FL_ZERO_RANGE
+#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
+#endif
 #endif
-
 #ifndef MAP_FILE
 # define MAP_FILE 0
 #endif
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-25 19:15 ` Lukas Czerner
@ 2014-02-25 19:15   ` Lukas Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: xfs, linux-fsdevel, Lukas Czerner

This is copy of xfs/242. However it's better to make it file system
specific because the range can be zeroes either directly by writing
zeroes, or converting to unwritten extent, so the actual result might
differ from file system to file system.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 tests/ext4/242     | 63 ++++++++++++++++++++++++++++++++++++++++
 tests/ext4/242.out | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/group   |  1 +
 3 files changed, 148 insertions(+)
 create mode 100755 tests/ext4/242
 create mode 100644 tests/ext4/242.out

diff --git a/tests/ext4/242 b/tests/ext4/242
new file mode 100755
index 0000000..f34036c
--- /dev/null
+++ b/tests/ext4/242
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 242
+#
+# Test fallocate FALLOC_FL_ZERO_RANGE
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2010 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!
+
+_cleanup()
+{
+    rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/punch
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+_require_xfs_io_falloc_zero
+
+_test_io_zero()
+{
+	$XFS_IO_PROG -c "fzero help" 2>&1 | \
+			grep 'command "zero" not found' > /dev/null
+	echo $?
+}
+
+[ $(_test_io_zero) -eq 0 ] && _notrun "fzero command not supported"
+
+testfile=$TEST_DIR/242.$$
+
+_test_generic_punch falloc fzero fzero fiemap _filter_fiemap $testfile
+
+status=0 ; exit
diff --git a/tests/ext4/242.out b/tests/ext4/242.out
new file mode 100644
index 0000000..3b80b4d
--- /dev/null
+++ b/tests/ext4/242.out
@@ -0,0 +1,84 @@
+QA output created by 242
+	1. into a hole
+0: [0..7]: hole
+1: [8..23]: unwritten
+2: [24..39]: hole
+daa100df6e6711906b61c9ab5aa16032
+	2. into allocated space
+0: [0..7]: data
+1: [8..23]: unwritten
+2: [24..39]: data
+cc58a7417c2d7763adc45b6fcd3fa024
+	3. into unwritten space
+0: [0..39]: unwritten
+daa100df6e6711906b61c9ab5aa16032
+	4. hole -> data
+0: [0..7]: hole
+1: [8..23]: unwritten
+2: [24..31]: data
+3: [32..39]: hole
+cc63069677939f69a6e8f68cae6a6dac
+	5. hole -> unwritten
+0: [0..7]: hole
+1: [8..31]: unwritten
+2: [32..39]: hole
+daa100df6e6711906b61c9ab5aa16032
+	6. data -> hole
+0: [0..7]: data
+1: [8..23]: unwritten
+2: [24..39]: hole
+1b3779878366498b28c702ef88c4a773
+	7. data -> unwritten
+0: [0..7]: data
+1: [8..31]: unwritten
+2: [32..39]: hole
+1b3779878366498b28c702ef88c4a773
+	8. unwritten -> hole
+0: [0..23]: unwritten
+1: [24..39]: hole
+daa100df6e6711906b61c9ab5aa16032
+	9. unwritten -> data
+0: [0..23]: unwritten
+1: [24..31]: data
+2: [32..39]: hole
+cc63069677939f69a6e8f68cae6a6dac
+	10. hole -> data -> hole
+0: [0..7]: hole
+1: [8..31]: unwritten
+2: [32..39]: hole
+daa100df6e6711906b61c9ab5aa16032
+	11. data -> hole -> data
+0: [0..7]: data
+1: [8..31]: unwritten
+2: [32..39]: data
+f6aeca13ec49e5b266cd1c913cd726e3
+	12. unwritten -> data -> unwritten
+0: [0..7]: data
+1: [8..31]: unwritten
+2: [32..39]: data
+daa100df6e6711906b61c9ab5aa16032
+	13. data -> unwritten -> data
+0: [0..7]: data
+1: [8..31]: unwritten
+2: [32..39]: data
+f6aeca13ec49e5b266cd1c913cd726e3
+	14. data -> hole @ EOF
+0: [0..23]: data
+1: [24..39]: unwritten
+e1f024eedd27ea6b1c3e9b841c850404
+	15. data -> hole @ 0
+0: [0..15]: unwritten
+1: [16..39]: data
+eecb7aa303d121835de05028751d301c
+	16. data -> cache cold ->hole
+0: [0..15]: unwritten
+1: [16..39]: data
+eecb7aa303d121835de05028751d301c
+	17. data -> hole in single block file
+0: [0..7]: data
+0000000 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
+*
+0000200 0000 0000 0000 0000 0000 0000 0000 0000
+*
+0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
+*
diff --git a/tests/ext4/group b/tests/ext4/group
index 7e1a68b..c8b47cc 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -3,6 +3,7 @@
 # - do not start group names with a digit
 # - comment line before each group is "new" description
 #
+242 auto prealloc quick
 271 auto rw quick
 301 aio dangerous ioctl rw stress
 302 aio dangerous ioctl rw stress
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-25 19:15   ` Lukas Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukas Czerner @ 2014-02-25 19:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, Lukas Czerner, xfs

This is copy of xfs/242. However it's better to make it file system
specific because the range can be zeroes either directly by writing
zeroes, or converting to unwritten extent, so the actual result might
differ from file system to file system.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 tests/ext4/242     | 63 ++++++++++++++++++++++++++++++++++++++++
 tests/ext4/242.out | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/group   |  1 +
 3 files changed, 148 insertions(+)
 create mode 100755 tests/ext4/242
 create mode 100644 tests/ext4/242.out

diff --git a/tests/ext4/242 b/tests/ext4/242
new file mode 100755
index 0000000..f34036c
--- /dev/null
+++ b/tests/ext4/242
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 242
+#
+# Test fallocate FALLOC_FL_ZERO_RANGE
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2010 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!
+
+_cleanup()
+{
+    rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/punch
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+_require_xfs_io_falloc_zero
+
+_test_io_zero()
+{
+	$XFS_IO_PROG -c "fzero help" 2>&1 | \
+			grep 'command "zero" not found' > /dev/null
+	echo $?
+}
+
+[ $(_test_io_zero) -eq 0 ] && _notrun "fzero command not supported"
+
+testfile=$TEST_DIR/242.$$
+
+_test_generic_punch falloc fzero fzero fiemap _filter_fiemap $testfile
+
+status=0 ; exit
diff --git a/tests/ext4/242.out b/tests/ext4/242.out
new file mode 100644
index 0000000..3b80b4d
--- /dev/null
+++ b/tests/ext4/242.out
@@ -0,0 +1,84 @@
+QA output created by 242
+	1. into a hole
+0: [0..7]: hole
+1: [8..23]: unwritten
+2: [24..39]: hole
+daa100df6e6711906b61c9ab5aa16032
+	2. into allocated space
+0: [0..7]: data
+1: [8..23]: unwritten
+2: [24..39]: data
+cc58a7417c2d7763adc45b6fcd3fa024
+	3. into unwritten space
+0: [0..39]: unwritten
+daa100df6e6711906b61c9ab5aa16032
+	4. hole -> data
+0: [0..7]: hole
+1: [8..23]: unwritten
+2: [24..31]: data
+3: [32..39]: hole
+cc63069677939f69a6e8f68cae6a6dac
+	5. hole -> unwritten
+0: [0..7]: hole
+1: [8..31]: unwritten
+2: [32..39]: hole
+daa100df6e6711906b61c9ab5aa16032
+	6. data -> hole
+0: [0..7]: data
+1: [8..23]: unwritten
+2: [24..39]: hole
+1b3779878366498b28c702ef88c4a773
+	7. data -> unwritten
+0: [0..7]: data
+1: [8..31]: unwritten
+2: [32..39]: hole
+1b3779878366498b28c702ef88c4a773
+	8. unwritten -> hole
+0: [0..23]: unwritten
+1: [24..39]: hole
+daa100df6e6711906b61c9ab5aa16032
+	9. unwritten -> data
+0: [0..23]: unwritten
+1: [24..31]: data
+2: [32..39]: hole
+cc63069677939f69a6e8f68cae6a6dac
+	10. hole -> data -> hole
+0: [0..7]: hole
+1: [8..31]: unwritten
+2: [32..39]: hole
+daa100df6e6711906b61c9ab5aa16032
+	11. data -> hole -> data
+0: [0..7]: data
+1: [8..31]: unwritten
+2: [32..39]: data
+f6aeca13ec49e5b266cd1c913cd726e3
+	12. unwritten -> data -> unwritten
+0: [0..7]: data
+1: [8..31]: unwritten
+2: [32..39]: data
+daa100df6e6711906b61c9ab5aa16032
+	13. data -> unwritten -> data
+0: [0..7]: data
+1: [8..31]: unwritten
+2: [32..39]: data
+f6aeca13ec49e5b266cd1c913cd726e3
+	14. data -> hole @ EOF
+0: [0..23]: data
+1: [24..39]: unwritten
+e1f024eedd27ea6b1c3e9b841c850404
+	15. data -> hole @ 0
+0: [0..15]: unwritten
+1: [16..39]: data
+eecb7aa303d121835de05028751d301c
+	16. data -> cache cold ->hole
+0: [0..15]: unwritten
+1: [16..39]: data
+eecb7aa303d121835de05028751d301c
+	17. data -> hole in single block file
+0: [0..7]: data
+0000000 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
+*
+0000200 0000 0000 0000 0000 0000 0000 0000 0000
+*
+0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
+*
diff --git a/tests/ext4/group b/tests/ext4/group
index 7e1a68b..c8b47cc 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -3,6 +3,7 @@
 # - do not start group names with a digit
 # - comment line before each group is "new" description
 #
+242 auto prealloc quick
 271 auto rw quick
 301 aio dangerous ioctl rw stress
 302 aio dangerous ioctl rw stress
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* Re: [PATCH 1/6] generic/290: Add test for fallocate zero range
  2014-02-25 19:15 ` Lukas Czerner
@ 2014-02-25 20:01   ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:01 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-ext4, linux-fsdevel, xfs

On Tue, Feb 25, 2014 at 08:15:23PM +0100, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

You need a commit message that describes the test....

> ---
>  common/rc             |  14 +++++++
>  tests/generic/290     |  92 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/290.out | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/group   |   1 +
>  4 files changed, 216 insertions(+)
>  create mode 100755 tests/generic/290
>  create mode 100644 tests/generic/290.out
> 
> diff --git a/common/rc b/common/rc
> index f2c3c3a..d3ec4db 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1359,6 +1359,20 @@ _require_xfs_io_fiemap()
>  		_notrun "xfs_io fiemap command failed (no fs support?)"
>  }
>  
> +# check that xfs_io, kernel and filesystem all support fallocate with zero
> +# range
> +_require_xfs_io_falloc_zero()
> +{
> +	testfile=$TEST_DIR/$$.falloc
> +	testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> +		-c "fzero 4k 8k" $testfile 2>&1`
> +	rm -f $testfile 2>&1 > /dev/null
> +	echo $testio | grep -q "not found" && \
> +		_notrun "xfs_io fallocate zero range support is missing"
> +	echo $testio | grep -q "Operation not supported" && \
> +		_notrun "xfs_io fallocate fzero command failed (no fs support?)"
> +}
> +

Ok, we've now got 4 or 5 copies of this same set of tests for
different fallocate commands. Please factor.

>  # Check that a fs has enough free space (in 1024b blocks)
>  #
>  _require_fs_space()
> diff --git a/tests/generic/290 b/tests/generic/290
> new file mode 100755
> index 0000000..90f560a
> --- /dev/null
> +++ b/tests/generic/290
> @@ -0,0 +1,92 @@
> +#! /bin/bash
> +# FS QA Test No. 290
> +#
> +# Makes calls to fallocate zero range and checks tossed ranges
> +#
> +# Nothing should be tossed unless the range includes a page boundry
> +#
> +# Primarily tests page boundries and boundries that are
> +#  off-by-one to ensure we're only tossing what's expected
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2012 SGI.  All Rights Reserved.

Really?

Oh, it's a direct copy-n-paste of the XFS_IOC_ZERO_RANGE test with
s/zero/fzero/.

Please factor along the same lines as _generic_test_punch so we
don't have duplicated code in the tests.

Also, I haven't seen patches 3, 4 or 6 on the list, and they haven't
made it to the archive, either. Can you please resend them?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 1/6] generic/290: Add test for fallocate zero range
@ 2014-02-25 20:01   ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:01 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Tue, Feb 25, 2014 at 08:15:23PM +0100, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

You need a commit message that describes the test....

> ---
>  common/rc             |  14 +++++++
>  tests/generic/290     |  92 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/290.out | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/group   |   1 +
>  4 files changed, 216 insertions(+)
>  create mode 100755 tests/generic/290
>  create mode 100644 tests/generic/290.out
> 
> diff --git a/common/rc b/common/rc
> index f2c3c3a..d3ec4db 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1359,6 +1359,20 @@ _require_xfs_io_fiemap()
>  		_notrun "xfs_io fiemap command failed (no fs support?)"
>  }
>  
> +# check that xfs_io, kernel and filesystem all support fallocate with zero
> +# range
> +_require_xfs_io_falloc_zero()
> +{
> +	testfile=$TEST_DIR/$$.falloc
> +	testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> +		-c "fzero 4k 8k" $testfile 2>&1`
> +	rm -f $testfile 2>&1 > /dev/null
> +	echo $testio | grep -q "not found" && \
> +		_notrun "xfs_io fallocate zero range support is missing"
> +	echo $testio | grep -q "Operation not supported" && \
> +		_notrun "xfs_io fallocate fzero command failed (no fs support?)"
> +}
> +

Ok, we've now got 4 or 5 copies of this same set of tests for
different fallocate commands. Please factor.

>  # Check that a fs has enough free space (in 1024b blocks)
>  #
>  _require_fs_space()
> diff --git a/tests/generic/290 b/tests/generic/290
> new file mode 100755
> index 0000000..90f560a
> --- /dev/null
> +++ b/tests/generic/290
> @@ -0,0 +1,92 @@
> +#! /bin/bash
> +# FS QA Test No. 290
> +#
> +# Makes calls to fallocate zero range and checks tossed ranges
> +#
> +# Nothing should be tossed unless the range includes a page boundry
> +#
> +# Primarily tests page boundries and boundries that are
> +#  off-by-one to ensure we're only tossing what's expected
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2012 SGI.  All Rights Reserved.

Really?

Oh, it's a direct copy-n-paste of the XFS_IOC_ZERO_RANGE test with
s/zero/fzero/.

Please factor along the same lines as _generic_test_punch so we
don't have duplicated code in the tests.

Also, I haven't seen patches 3, 4 or 6 on the list, and they haven't
made it to the archive, either. Can you please resend them?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 2/6] xfstests: Add fallocate zero range operation to fsstress
  2014-02-25 19:15   ` Lukas Czerner
@ 2014-02-25 20:15     ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:15 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-ext4, xfs, linux-fsdevel

On Tue, Feb 25, 2014 at 08:15:24PM +0100, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

commit message?

> @@ -2561,6 +2567,62 @@ punch_f(int opno, long r)
>  }
>  
>  void
> +zero_f(int opno, long r)
> +{
> +#ifdef FALLOCATE
> +	int		e;
> +	pathname_t	f;
> +	int		fd;
> +	__int64_t	lr;
> +	off64_t		off;
> +	off64_t		len;
> +	struct stat64	stb;
> +	int		v;
> +	char		st[1024];

[snip]

We now have 3 copies of this fallocate code, the only difference
between the copies being the mode:

> +	int mode = FALLOC_FL_ZERO_RANGE;

passed into the fallocate function. Please factor.

> +
> +	init_pathname(&f);
> +	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
> +		if (v)
> +			printf("%d/%d: zero range - no filename\n", procid, opno);
> +		free_pathname(&f);
> +		return;
> +	}
> +	fd = open_path(&f, O_RDWR);
> +	e = fd < 0 ? errno : 0;

while you are there, get rid of the ternary operations and just use

	if (fd < 0)
		e = errno;

> +	check_cwd();

This can go after we've checked for an error on open, which avoids
needing to save the errno....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 2/6] xfstests: Add fallocate zero range operation to fsstress
@ 2014-02-25 20:15     ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:15 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Tue, Feb 25, 2014 at 08:15:24PM +0100, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

commit message?

> @@ -2561,6 +2567,62 @@ punch_f(int opno, long r)
>  }
>  
>  void
> +zero_f(int opno, long r)
> +{
> +#ifdef FALLOCATE
> +	int		e;
> +	pathname_t	f;
> +	int		fd;
> +	__int64_t	lr;
> +	off64_t		off;
> +	off64_t		len;
> +	struct stat64	stb;
> +	int		v;
> +	char		st[1024];

[snip]

We now have 3 copies of this fallocate code, the only difference
between the copies being the mode:

> +	int mode = FALLOC_FL_ZERO_RANGE;

passed into the fallocate function. Please factor.

> +
> +	init_pathname(&f);
> +	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
> +		if (v)
> +			printf("%d/%d: zero range - no filename\n", procid, opno);
> +		free_pathname(&f);
> +		return;
> +	}
> +	fd = open_path(&f, O_RDWR);
> +	e = fd < 0 ? errno : 0;

while you are there, get rid of the ternary operations and just use

	if (fd < 0)
		e = errno;

> +	check_cwd();

This can go after we've checked for an error on open, which avoids
needing to save the errno....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 3/6] xfstests: fsstress punch should always have FALLOC_FL_KEEP_SIZE set
  2014-02-25 19:15   ` Lukas Czerner
  (?)
@ 2014-02-25 20:18   ` Dave Chinner
  2014-02-26 13:04     ` Lukáš Czerner
  -1 siblings, 1 reply; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:18 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Tue, Feb 25, 2014 at 08:15:25PM +0100, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  ltp/fsstress.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index 4c3368f..42c8a5a 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
> @@ -2555,8 +2555,8 @@ punch_f(int opno, long r)
>  	off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE));
>  	off %= maxfsize;
>  	len = (off64_t)(random() % (1024 * 1024));
> -	mode |= FALLOC_FL_KEEP_SIZE & random();
>  	e = fallocate(fd, mode, (loff_t)off, (loff_t)len) < 0 ? errno : 0;
> +	mode |= FALLOC_FL_KEEP_SIZE;
>  	if (v)
>  		printf("%d/%d: punch hole(%d) %s %s %lld %lld %d\n",
>  		       procid, opno, mode,

NACK. There's nothing wrong with testing a set of parameters that
should fail in a stress test.

Regardless, the patch is wrong...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 4/6] xfstests: Add fallocate zero range operation to fsx
  2014-02-25 19:15   ` Lukas Czerner
  (?)
@ 2014-02-25 20:31   ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:31 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Tue, Feb 25, 2014 at 08:15:26PM +0100, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

commit message?

>  #define OP_CLOSEOPEN	100
> @@ -142,6 +144,7 @@ int	seed = 1;			/* -S flag */
>  int     mapped_writes = 1;              /* -W flag disables */
>  int     fallocate_calls = 1;            /* -F flag disables */
>  int     punch_hole_calls = 1;           /* -H flag disables */
> +int     zero_range_calls = 1;           /* -E flag disables */

'-E' doesn't make much sense - '-z' would be better....

>  int 	mapped_reads = 1;		/* -R flag disables it */
>  int	fsxgoodfd = 0;
>  int	o_direct;			/* -Z */
> @@ -320,6 +323,14 @@ logdump(void)
>  						     lp->args[0] + lp->args[1])
>  				prt("\t******PPPP");
>  			break;
> +		case OP_ZERO_RANGE:
> +			prt("ZERO    0x%x thru 0x%x\t(0x%x bytes)",
> +			    lp->args[0], lp->args[0] + lp->args[1] - 1,
> +			    lp->args[1]);
> +			if (badoff >= lp->args[0] && badoff <
> +						     lp->args[0] + lp->args[1])
> +				prt("\t******ZZZZ");
> +			break;
>  		case OP_SKIPPED:
>  			prt("SKIPPED (no operation)");
>  			break;
> @@ -882,6 +893,64 @@ do_punch_hole(unsigned offset, unsigned length)
>  }
>  #endif
>  
> +#ifdef FALLOC_FL_ZERO_RANGE
> +void
> +do_zero_range(unsigned offset, unsigned length)
> +{
> +	unsigned end_offset;
> +	int max_offset = 0;
> +	int max_len = 0;
> +	int mode = FALLOC_FL_ZERO_RANGE;

This is just a copy of the hole punch code with a different mode,
OP and s/punch/zero/g. Please factor.

......
> +void
> +test_zero_range()
> +{
> +#ifdef FALLOC_FL_ZERO_RANGE
> +	if (!lite && zero_range_calls) {
> +		if (fallocate(fd, FALLOC_FL_ZERO_RANGE,
> +				0, 1) && errno == EOPNOTSUPP) {
> +			if(!quiet)
> +				warn("main: filesystem does not support fallocate zero range, disabling");
> +			zero_range_calls = 0;
> +		} else
> +			ftruncate(fd, 0);
> +	}
> +#else /* ! ZERO RANGE */
> +	zero_range_calls = 0;
> +#endif
> +}

Third copy of this test code for fallocate. Please factor.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 5/6] xfstests: Define fallocate flags locally in fsx
  2014-02-25 19:15   ` Lukas Czerner
@ 2014-02-25 20:39     ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:39 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-ext4, xfs, linux-fsdevel

On Tue, Feb 25, 2014 at 08:15:27PM +0100, Lukas Czerner wrote:
> Define fallocate flags such as FALLOC_FL_PUNCH_HOLE and
> FALLOC_FL_ZERO_RANGE locally if they do not exist.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  ltp/fsx.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index b3c30db..331257e 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -35,8 +35,13 @@
>  #endif
>  #ifdef FALLOCATE
>  #include <linux/falloc.h>
> +#ifndef FALLOC_FL_PUNCH_HOLE
> +#define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
> +#endif
> +#ifndef FALLOC_FL_ZERO_RANGE
> +#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
> +#endif
>  #endif

This pattern is appearing all over the place in xfstests now.  I'd
suggest that this should really be handled by autoconf,
include/config.h and src/globals.h....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 5/6] xfstests: Define fallocate flags locally in fsx
@ 2014-02-25 20:39     ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:39 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Tue, Feb 25, 2014 at 08:15:27PM +0100, Lukas Czerner wrote:
> Define fallocate flags such as FALLOC_FL_PUNCH_HOLE and
> FALLOC_FL_ZERO_RANGE locally if they do not exist.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  ltp/fsx.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index b3c30db..331257e 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -35,8 +35,13 @@
>  #endif
>  #ifdef FALLOCATE
>  #include <linux/falloc.h>
> +#ifndef FALLOC_FL_PUNCH_HOLE
> +#define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
> +#endif
> +#ifndef FALLOC_FL_ZERO_RANGE
> +#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
> +#endif
>  #endif

This pattern is appearing all over the place in xfstests now.  I'd
suggest that this should really be handled by autoconf,
include/config.h and src/globals.h....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-25 19:15   ` Lukas Czerner
  (?)
@ 2014-02-25 20:53   ` Dave Chinner
  2014-02-25 21:01       ` Lukáš Czerner
  -1 siblings, 1 reply; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:53 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> This is copy of xfs/242. However it's better to make it file system
> specific because the range can be zeroes either directly by writing
> zeroes, or converting to unwritten extent, so the actual result might
> differ from file system to file system.

You could say the same thing about preallocation using unwritten
extents. Yet, funnily enough, we have generic tests for them because
all filesystems currently use unwritten extents for preallocation
and behave identically....

This test is no different - all filesystems currently use unwritten
extents, and so this test should be generic because all existing
filesystems *should* behave the same.

When we get a filesystem that zeros rather uses unwritten extents,
we can add a new *generic* test that tests for zeroed data extents
rather than unwritten extents. All that we will need is a method of
checking what behaviour the filesystem has and adding that to a
_requires directive to ensure the correct generic fallocate tests
are run...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 1/6] generic/290: Add test for fallocate zero range
  2014-02-25 20:01   ` Dave Chinner
  (?)
@ 2014-02-25 20:55   ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 20:55 UTC (permalink / raw)
  To: Lukas Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Wed, Feb 26, 2014 at 07:01:17AM +1100, Dave Chinner wrote:
> Also, I haven't seen patches 3, 4 or 6 on the list, and they haven't
> made it to the archive, either. Can you please resend them?

They took a couple of hours to come through, for some reason, and
were delivered by a different mailing list. So who knows what went
wrong there. No need to resend them.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-25 20:53   ` Dave Chinner
@ 2014-02-25 21:01       ` Lukáš Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-25 21:01 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-ext4, linux-fsdevel, xfs

On Wed, 26 Feb 2014, Dave Chinner wrote:

> Date: Wed, 26 Feb 2014 07:53:49 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > This is copy of xfs/242. However it's better to make it file system
> > specific because the range can be zeroes either directly by writing
> > zeroes, or converting to unwritten extent, so the actual result might
> > differ from file system to file system.
> 
> You could say the same thing about preallocation using unwritten
> extents. Yet, funnily enough, we have generic tests for them because
> all filesystems currently use unwritten extents for preallocation
> and behave identically....
> 
> This test is no different - all filesystems currently use unwritten
> extents, and so this test should be generic because all existing
> filesystems *should* behave the same.
> 
> When we get a filesystem that zeros rather uses unwritten extents,
> we can add a new *generic* test that tests for zeroed data extents
> rather than unwritten extents. All that we will need is a method of
> checking what behaviour the filesystem has and adding that to a
> _requires directive to ensure the correct generic fallocate tests
> are run...

Currently xfs/242 fails on xfs for me and it does behave differently
than ext4. Also I had to change to 242.out a bit because ext4 was
a little different. It seems to me that it was expected that when
the extent is small enough it would be overwritten by zeroes rather
than converted to unwritten, but I have not looked into
implementation.

Btw this kind of optimization is actually something I've been
thinking of as well for ext4. Rather than going though the hassle of
changing extents around it might be worth in some situation to zero
out. But that's an optimization I have not implemented yet.

-Lukas


> 
> Cheers,
> 
> Dave.
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-25 21:01       ` Lukáš Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-25 21:01 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, linux-ext4, xfs

On Wed, 26 Feb 2014, Dave Chinner wrote:

> Date: Wed, 26 Feb 2014 07:53:49 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > This is copy of xfs/242. However it's better to make it file system
> > specific because the range can be zeroes either directly by writing
> > zeroes, or converting to unwritten extent, so the actual result might
> > differ from file system to file system.
> 
> You could say the same thing about preallocation using unwritten
> extents. Yet, funnily enough, we have generic tests for them because
> all filesystems currently use unwritten extents for preallocation
> and behave identically....
> 
> This test is no different - all filesystems currently use unwritten
> extents, and so this test should be generic because all existing
> filesystems *should* behave the same.
> 
> When we get a filesystem that zeros rather uses unwritten extents,
> we can add a new *generic* test that tests for zeroed data extents
> rather than unwritten extents. All that we will need is a method of
> checking what behaviour the filesystem has and adding that to a
> _requires directive to ensure the correct generic fallocate tests
> are run...

Currently xfs/242 fails on xfs for me and it does behave differently
than ext4. Also I had to change to 242.out a bit because ext4 was
a little different. It seems to me that it was expected that when
the extent is small enough it would be overwritten by zeroes rather
than converted to unwritten, but I have not looked into
implementation.

Btw this kind of optimization is actually something I've been
thinking of as well for ext4. Rather than going though the hassle of
changing extents around it might be worth in some situation to zero
out. But that's an optimization I have not implemented yet.

-Lukas


> 
> Cheers,
> 
> Dave.
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-25 21:01       ` Lukáš Czerner
@ 2014-02-25 21:27         ` Lukáš Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-25 21:27 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-ext4, linux-fsdevel, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2837 bytes --]

On Tue, 25 Feb 2014, Lukáš Czerner wrote:

> Date: Tue, 25 Feb 2014 22:01:06 +0100 (CET)
> From: Lukáš Czerner <lczerner@redhat.com>
> To: Dave Chinner <david@fromorbit.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> 
> > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukas Czerner <lczerner@redhat.com>
> > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> >     range
> > 
> > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > This is copy of xfs/242. However it's better to make it file system
> > > specific because the range can be zeroes either directly by writing
> > > zeroes, or converting to unwritten extent, so the actual result might
> > > differ from file system to file system.
> > 
> > You could say the same thing about preallocation using unwritten
> > extents. Yet, funnily enough, we have generic tests for them because
> > all filesystems currently use unwritten extents for preallocation
> > and behave identically....
> > 
> > This test is no different - all filesystems currently use unwritten
> > extents, and so this test should be generic because all existing
> > filesystems *should* behave the same.
> > 
> > When we get a filesystem that zeros rather uses unwritten extents,
> > we can add a new *generic* test that tests for zeroed data extents
> > rather than unwritten extents. All that we will need is a method of
> > checking what behaviour the filesystem has and adding that to a
> > _requires directive to ensure the correct generic fallocate tests
> > are run...
> 
> Currently xfs/242 fails on xfs for me and it does behave differently
> than ext4. Also I had to change to 242.out a bit because ext4 was
> a little different. It seems to me that it was expected that when
> the extent is small enough it would be overwritten by zeroes rather
> than converted to unwritten, but I have not looked into
> implementation.
> 
> Btw this kind of optimization is actually something I've been
> thinking of as well for ext4. Rather than going though the hassle of
> changing extents around it might be worth in some situation to zero
> out. But that's an optimization I have not implemented yet.

Oops, I am taking it back. It's just too late and apparently I've
overlooked something.

-Lukas

> 
> -Lukas
> 
> 
> > 
> > Cheers,
> > 
> > Dave.
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-25 21:27         ` Lukáš Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-25 21:27 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, linux-ext4, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2837 bytes --]

On Tue, 25 Feb 2014, Lukáš Czerner wrote:

> Date: Tue, 25 Feb 2014 22:01:06 +0100 (CET)
> From: Lukáš Czerner <lczerner@redhat.com>
> To: Dave Chinner <david@fromorbit.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> 
> > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukas Czerner <lczerner@redhat.com>
> > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> >     range
> > 
> > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > This is copy of xfs/242. However it's better to make it file system
> > > specific because the range can be zeroes either directly by writing
> > > zeroes, or converting to unwritten extent, so the actual result might
> > > differ from file system to file system.
> > 
> > You could say the same thing about preallocation using unwritten
> > extents. Yet, funnily enough, we have generic tests for them because
> > all filesystems currently use unwritten extents for preallocation
> > and behave identically....
> > 
> > This test is no different - all filesystems currently use unwritten
> > extents, and so this test should be generic because all existing
> > filesystems *should* behave the same.
> > 
> > When we get a filesystem that zeros rather uses unwritten extents,
> > we can add a new *generic* test that tests for zeroed data extents
> > rather than unwritten extents. All that we will need is a method of
> > checking what behaviour the filesystem has and adding that to a
> > _requires directive to ensure the correct generic fallocate tests
> > are run...
> 
> Currently xfs/242 fails on xfs for me and it does behave differently
> than ext4. Also I had to change to 242.out a bit because ext4 was
> a little different. It seems to me that it was expected that when
> the extent is small enough it would be overwritten by zeroes rather
> than converted to unwritten, but I have not looked into
> implementation.
> 
> Btw this kind of optimization is actually something I've been
> thinking of as well for ext4. Rather than going though the hassle of
> changing extents around it might be worth in some situation to zero
> out. But that's an optimization I have not implemented yet.

Oops, I am taking it back. It's just too late and apparently I've
overlooked something.

-Lukas

> 
> -Lukas
> 
> 
> > 
> > Cheers,
> > 
> > Dave.
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-25 21:01       ` Lukáš Czerner
@ 2014-02-25 21:50         ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 21:50 UTC (permalink / raw)
  To: Lukáš Czerner; +Cc: linux-ext4, linux-fsdevel, xfs

On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> 
> > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukas Czerner <lczerner@redhat.com>
> > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> >     range
> > 
> > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > This is copy of xfs/242. However it's better to make it file system
> > > specific because the range can be zeroes either directly by writing
> > > zeroes, or converting to unwritten extent, so the actual result might
> > > differ from file system to file system.
> > 
> > You could say the same thing about preallocation using unwritten
> > extents. Yet, funnily enough, we have generic tests for them because
> > all filesystems currently use unwritten extents for preallocation
> > and behave identically....
> > 
> > This test is no different - all filesystems currently use unwritten
> > extents, and so this test should be generic because all existing
> > filesystems *should* behave the same.
> > 
> > When we get a filesystem that zeros rather uses unwritten extents,
> > we can add a new *generic* test that tests for zeroed data extents
> > rather than unwritten extents. All that we will need is a method of
> > checking what behaviour the filesystem has and adding that to a
> > _requires directive to ensure the correct generic fallocate tests
> > are run...
> 
> Currently xfs/242 fails on xfs for me

Really? Where's the bug report? I haven't seen a failure on xfs/242
on any of my test machines for at least a year, even on 1k block
size filesystems...

$ sudo ./check xfs/242
FSTYP         -- xfs (debug)
PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
MOUNT_OPTIONS -- /dev/vdb /mnt/scratch

xfs/242 1s ... 0s
Ran: xfs/242
Passed all 1 tests
$


> and it does behave differently than ext4.

In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
it because you wrote this test as an ext4 specific test and so
haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
path in XFS?

IOWs, how do you know that what you are seeing is not a bug in the
ext4 (or XFS for that matter) implementation?

> Also I had to change to 242.out a bit because ext4 was
> a little different. It seems to me that it was expected that when
> the extent is small enough it would be overwritten by zeroes rather
> than converted to unwritten, but I have not looked into
> implementation.

The test assumes that sub-block head and tail ranges will be zeroed,
and everything else will be converted to unwritten extents. i.e. a
single block aligned range will get converted to unwritten, but a
single unaligned block range will result in the two overlapping
blocks being zeroed (because they still contain valid data).

This is the same sub-block zeroing behaviour as is done for
hole punching - the only difference between a hole punch and a zero
range on filesystems that use unwritten extents should be that the
range being operated on has unwritten extents rather a hole.....

> Btw this kind of optimization is actually something I've been
> thinking of as well for ext4. Rather than going though the hassle of
> changing extents around it might be worth in some situation to zero
> out. But that's an optimization I have not implemented yet.

Exactly my point - until such optimisations are implemented, all the
filesystems should be behaving the same way using unwritten extents,
just like for hole punching. Hence the tests should be checking that
the behaviour is the same across filesystems, just like we do for
hole punching.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-25 21:50         ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-25 21:50 UTC (permalink / raw)
  To: Lukáš Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> 
> > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukas Czerner <lczerner@redhat.com>
> > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> >     range
> > 
> > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > This is copy of xfs/242. However it's better to make it file system
> > > specific because the range can be zeroes either directly by writing
> > > zeroes, or converting to unwritten extent, so the actual result might
> > > differ from file system to file system.
> > 
> > You could say the same thing about preallocation using unwritten
> > extents. Yet, funnily enough, we have generic tests for them because
> > all filesystems currently use unwritten extents for preallocation
> > and behave identically....
> > 
> > This test is no different - all filesystems currently use unwritten
> > extents, and so this test should be generic because all existing
> > filesystems *should* behave the same.
> > 
> > When we get a filesystem that zeros rather uses unwritten extents,
> > we can add a new *generic* test that tests for zeroed data extents
> > rather than unwritten extents. All that we will need is a method of
> > checking what behaviour the filesystem has and adding that to a
> > _requires directive to ensure the correct generic fallocate tests
> > are run...
> 
> Currently xfs/242 fails on xfs for me

Really? Where's the bug report? I haven't seen a failure on xfs/242
on any of my test machines for at least a year, even on 1k block
size filesystems...

$ sudo ./check xfs/242
FSTYP         -- xfs (debug)
PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
MOUNT_OPTIONS -- /dev/vdb /mnt/scratch

xfs/242 1s ... 0s
Ran: xfs/242
Passed all 1 tests
$


> and it does behave differently than ext4.

In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
it because you wrote this test as an ext4 specific test and so
haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
path in XFS?

IOWs, how do you know that what you are seeing is not a bug in the
ext4 (or XFS for that matter) implementation?

> Also I had to change to 242.out a bit because ext4 was
> a little different. It seems to me that it was expected that when
> the extent is small enough it would be overwritten by zeroes rather
> than converted to unwritten, but I have not looked into
> implementation.

The test assumes that sub-block head and tail ranges will be zeroed,
and everything else will be converted to unwritten extents. i.e. a
single block aligned range will get converted to unwritten, but a
single unaligned block range will result in the two overlapping
blocks being zeroed (because they still contain valid data).

This is the same sub-block zeroing behaviour as is done for
hole punching - the only difference between a hole punch and a zero
range on filesystems that use unwritten extents should be that the
range being operated on has unwritten extents rather a hole.....

> Btw this kind of optimization is actually something I've been
> thinking of as well for ext4. Rather than going though the hassle of
> changing extents around it might be worth in some situation to zero
> out. But that's an optimization I have not implemented yet.

Exactly my point - until such optimisations are implemented, all the
filesystems should be behaving the same way using unwritten extents,
just like for hole punching. Hence the tests should be checking that
the behaviour is the same across filesystems, just like we do for
hole punching.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 5/6] xfstests: Define fallocate flags locally in fsx
  2014-02-25 20:39     ` Dave Chinner
@ 2014-02-25 21:56       ` Christoph Hellwig
  -1 siblings, 0 replies; 47+ messages in thread
From: Christoph Hellwig @ 2014-02-25 21:56 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Lukas Czerner, linux-ext4, xfs, linux-fsdevel

On Wed, Feb 26, 2014 at 07:39:22AM +1100, Dave Chinner wrote:
> > index b3c30db..331257e 100644
> > --- a/ltp/fsx.c
> > +++ b/ltp/fsx.c
> > @@ -35,8 +35,13 @@
> >  #endif
> >  #ifdef FALLOCATE
> >  #include <linux/falloc.h>
> > +#ifndef FALLOC_FL_PUNCH_HOLE
> > +#define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
> > +#endif
> > +#ifndef FALLOC_FL_ZERO_RANGE
> > +#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
> > +#endif
> >  #endif
> 
> This pattern is appearing all over the place in xfstests now.  I'd
> suggest that this should really be handled by autoconf,
> include/config.h and src/globals.h....

Can we handle this nicely using autoconf, especially in the case of O_
flags that might be different for different architectures?

Either way having a single header for the various flags that might not
be present in the system headers sounds like a good plan.


^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 5/6] xfstests: Define fallocate flags locally in fsx
@ 2014-02-25 21:56       ` Christoph Hellwig
  0 siblings, 0 replies; 47+ messages in thread
From: Christoph Hellwig @ 2014-02-25 21:56 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, Lukas Czerner, linux-ext4, xfs

On Wed, Feb 26, 2014 at 07:39:22AM +1100, Dave Chinner wrote:
> > index b3c30db..331257e 100644
> > --- a/ltp/fsx.c
> > +++ b/ltp/fsx.c
> > @@ -35,8 +35,13 @@
> >  #endif
> >  #ifdef FALLOCATE
> >  #include <linux/falloc.h>
> > +#ifndef FALLOC_FL_PUNCH_HOLE
> > +#define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
> > +#endif
> > +#ifndef FALLOC_FL_ZERO_RANGE
> > +#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
> > +#endif
> >  #endif
> 
> This pattern is appearing all over the place in xfstests now.  I'd
> suggest that this should really be handled by autoconf,
> include/config.h and src/globals.h....

Can we handle this nicely using autoconf, especially in the case of O_
flags that might be different for different architectures?

Either way having a single header for the various flags that might not
be present in the system headers sounds like a good plan.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 5/6] xfstests: Define fallocate flags locally in fsx
  2014-02-25 21:56       ` Christoph Hellwig
@ 2014-02-26  6:07         ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-26  6:07 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Lukas Czerner, linux-ext4, xfs, linux-fsdevel

On Tue, Feb 25, 2014 at 01:56:16PM -0800, Christoph Hellwig wrote:
> On Wed, Feb 26, 2014 at 07:39:22AM +1100, Dave Chinner wrote:
> > > index b3c30db..331257e 100644
> > > --- a/ltp/fsx.c
> > > +++ b/ltp/fsx.c
> > > @@ -35,8 +35,13 @@
> > >  #endif
> > >  #ifdef FALLOCATE
> > >  #include <linux/falloc.h>
> > > +#ifndef FALLOC_FL_PUNCH_HOLE
> > > +#define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
> > > +#endif
> > > +#ifndef FALLOC_FL_ZERO_RANGE
> > > +#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
> > > +#endif
> > >  #endif
> > 
> > This pattern is appearing all over the place in xfstests now.  I'd
> > suggest that this should really be handled by autoconf,
> > include/config.h and src/globals.h....
> 
> Can we handle this nicely using autoconf, especially in the case of O_
> flags that might be different for different architectures?

I think so, because config.h is generated from config.h.in and
ends up looking like:

#define HAVE_SOME_FUNCTION 1

src/globals.h is controlled directly by us, and ends up looking
like:

#ifdef HAVE_SOME_FUNCTION
#include <header_for_some_function.h>
#endif

If we add all the defines that may be missing that same ifdef
section, and make sure that all code includes globals.h, then
all the code will have the "HAVE_SOME_FUNCTION" defines and the
appropriate header files and definitions included.

> Either way having a single header for the various flags that might not
> be present in the system headers sounds like a good plan.

*nod*

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 5/6] xfstests: Define fallocate flags locally in fsx
@ 2014-02-26  6:07         ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-26  6:07 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel, Lukas Czerner, linux-ext4, xfs

On Tue, Feb 25, 2014 at 01:56:16PM -0800, Christoph Hellwig wrote:
> On Wed, Feb 26, 2014 at 07:39:22AM +1100, Dave Chinner wrote:
> > > index b3c30db..331257e 100644
> > > --- a/ltp/fsx.c
> > > +++ b/ltp/fsx.c
> > > @@ -35,8 +35,13 @@
> > >  #endif
> > >  #ifdef FALLOCATE
> > >  #include <linux/falloc.h>
> > > +#ifndef FALLOC_FL_PUNCH_HOLE
> > > +#define FALLOC_FL_PUNCH_HOLE    0x02 /* de-allocates range */
> > > +#endif
> > > +#ifndef FALLOC_FL_ZERO_RANGE
> > > +#define FALLOC_FL_ZERO_RANGE    0x10 /* zeroes range */
> > > +#endif
> > >  #endif
> > 
> > This pattern is appearing all over the place in xfstests now.  I'd
> > suggest that this should really be handled by autoconf,
> > include/config.h and src/globals.h....
> 
> Can we handle this nicely using autoconf, especially in the case of O_
> flags that might be different for different architectures?

I think so, because config.h is generated from config.h.in and
ends up looking like:

#define HAVE_SOME_FUNCTION 1

src/globals.h is controlled directly by us, and ends up looking
like:

#ifdef HAVE_SOME_FUNCTION
#include <header_for_some_function.h>
#endif

If we add all the defines that may be missing that same ifdef
section, and make sure that all code includes globals.h, then
all the code will have the "HAVE_SOME_FUNCTION" defines and the
appropriate header files and definitions included.

> Either way having a single header for the various flags that might not
> be present in the system headers sounds like a good plan.

*nod*

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 3/6] xfstests: fsstress punch should always have FALLOC_FL_KEEP_SIZE set
  2014-02-25 20:18   ` Dave Chinner
@ 2014-02-26 13:04     ` Lukáš Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-26 13:04 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, linux-ext4, xfs

On Wed, 26 Feb 2014, Dave Chinner wrote:

> Date: Wed, 26 Feb 2014 07:18:54 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, xfs@oss.sgi.com, linux-fsdevel@vger.kernel.org
> Subject: Re: [PATCH 3/6] xfstests: fsstress punch should always have
>     FALLOC_FL_KEEP_SIZE set
> 
> On Tue, Feb 25, 2014 at 08:15:25PM +0100, Lukas Czerner wrote:
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> > ---
> >  ltp/fsstress.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> > index 4c3368f..42c8a5a 100644
> > --- a/ltp/fsstress.c
> > +++ b/ltp/fsstress.c
> > @@ -2555,8 +2555,8 @@ punch_f(int opno, long r)
> >  	off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE));
> >  	off %= maxfsize;
> >  	len = (off64_t)(random() % (1024 * 1024));
> > -	mode |= FALLOC_FL_KEEP_SIZE & random();
> >  	e = fallocate(fd, mode, (loff_t)off, (loff_t)len) < 0 ? errno : 0;
> > +	mode |= FALLOC_FL_KEEP_SIZE;
> >  	if (v)
> >  		printf("%d/%d: punch hole(%d) %s %s %lld %lld %d\n",
> >  		       procid, opno, mode,
> 
> NACK. There's nothing wrong with testing a set of parameters that
> should fail in a stress test.

How is this testing ? We do not actually test whether it fails when
it should and vice versa. Only thing that it does right now is
making punch hole testing less efficient.

We do not really test correctness of the arguments in fsstress.

-Lukas

> 
> Regardless, the patch is wrong...
> 
> Cheers,
> 
> Dave.
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-25 21:50         ` Dave Chinner
  (?)
@ 2014-02-26 14:24         ` Lukáš Czerner
  2014-02-26 22:01             ` Dave Chinner
  -1 siblings, 1 reply; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-26 14:24 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, linux-ext4, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5453 bytes --]

On Wed, 26 Feb 2014, Dave Chinner wrote:

> Date: Wed, 26 Feb 2014 08:50:11 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > 
> > > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > > From: Dave Chinner <david@fromorbit.com>
> > > To: Lukas Czerner <lczerner@redhat.com>
> > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > >     range
> > > 
> > > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > > This is copy of xfs/242. However it's better to make it file system
> > > > specific because the range can be zeroes either directly by writing
> > > > zeroes, or converting to unwritten extent, so the actual result might
> > > > differ from file system to file system.
> > > 
> > > You could say the same thing about preallocation using unwritten
> > > extents. Yet, funnily enough, we have generic tests for them because
> > > all filesystems currently use unwritten extents for preallocation
> > > and behave identically....
> > > 
> > > This test is no different - all filesystems currently use unwritten
> > > extents, and so this test should be generic because all existing
> > > filesystems *should* behave the same.
> > > 
> > > When we get a filesystem that zeros rather uses unwritten extents,
> > > we can add a new *generic* test that tests for zeroed data extents
> > > rather than unwritten extents. All that we will need is a method of
> > > checking what behaviour the filesystem has and adding that to a
> > > _requires directive to ensure the correct generic fallocate tests
> > > are run...
> > 
> > Currently xfs/242 fails on xfs for me
> 
> Really? Where's the bug report? I haven't seen a failure on xfs/242
> on any of my test machines for at least a year, even on 1k block
> size filesystems...
> 
> $ sudo ./check xfs/242
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> 
> xfs/242 1s ... 0s
> Ran: xfs/242
> Passed all 1 tests
> $

Here it is.

xfs/242 fails on ppc64 with latest linus tree

# uname -a
Linux ibm-p740-01-lp4.rhts.eng.bos.redhat.com 3.14.0-rc4+ #1 SMP Wed
Feb 26 08:59:48 EST 2014 ppc64 ppc64 ppc64 GNU/Linux

# ./check xfs/242
FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/ppc64 ibm-p740-01-lp4 3.14.0-rc4+
MKFS_OPTIONS  -- -f -bsize=4096 /dev/loop1
MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/loop1 /mnt/test2

xfs/242  - output mismatch (see /root/xfstests/results//xfs/242.out.bad)
    --- tests/xfs/242.out       2014-02-26 05:51:16.602579462 -0500
    +++ /root/xfstests/results//xfs/242.out.bad 2014-02-26 09:20:55.585396040 -0500
    @@ -1,76 +1,71 @@
     QA output created by 242
        1. into a hole
     0: [0..7]: hole
    -1: [8..23]: unwritten
    +1: [8..23]: data
     2: [24..39]: hole
     daa100df6e6711906b61c9ab5aa16032
    ...
    (Run 'diff -u tests/xfs/242.out /root/xfstests/results//xfs/242.out.bad'  to see the entire diff)
Ran: xfs/242
Failures: xfs/242
Failed 1 of 1 tests


Here is 242.out.bad

QA output created by 242
	1. into a hole
0: [0..7]: hole
1: [8..23]: data
2: [24..39]: hole
daa100df6e6711906b61c9ab5aa16032
	2. into allocated space
0: [0..39]: data
cc58a7417c2d7763adc45b6fcd3fa024
	3. into unwritten space
0: [0..39]: unwritten
daa100df6e6711906b61c9ab5aa16032
	4. hole -> data
0: [0..7]: hole
1: [8..31]: data
2: [32..39]: hole
cc63069677939f69a6e8f68cae6a6dac
	5. hole -> unwritten
0: [0..7]: hole
1: [8..23]: data
2: [24..31]: unwritten
3: [32..39]: hole
daa100df6e6711906b61c9ab5aa16032
	6. data -> hole
0: [0..23]: data
1: [24..39]: hole
1b3779878366498b28c702ef88c4a773
	7. data -> unwritten
0: [0..15]: data
1: [16..31]: unwritten
2: [32..39]: hole
1b3779878366498b28c702ef88c4a773
	8. unwritten -> hole
0: [0..7]: unwritten
1: [8..23]: data
2: [24..39]: hole
daa100df6e6711906b61c9ab5aa16032
	9. unwritten -> data
0: [0..15]: unwritten
1: [16..31]: data
2: [32..39]: hole
cc63069677939f69a6e8f68cae6a6dac
	10. hole -> data -> hole
0: [0..7]: hole
1: [8..31]: data
2: [32..39]: hole
daa100df6e6711906b61c9ab5aa16032
	11. data -> hole -> data
0: [0..39]: data
f6aeca13ec49e5b266cd1c913cd726e3
	12. unwritten -> data -> unwritten
0: [0..15]: unwritten
1: [16..23]: data
2: [24..39]: unwritten
daa100df6e6711906b61c9ab5aa16032
	13. data -> unwritten -> data
0: [0..7]: data
1: [8..23]: unwritten
2: [24..39]: data
f6aeca13ec49e5b266cd1c913cd726e3
	14. data -> hole @ EOF
0: [0..39]: data
e1f024eedd27ea6b1c3e9b841c850404
	15. data -> hole @ 0
0: [0..39]: data
eecb7aa303d121835de05028751d301c
	16. data -> cache cold ->hole
0: [0..39]: data
eecb7aa303d121835de05028751d301c
	17. data -> hole in single block file
0: [0..7]: data
0000000 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
*
0000200 0000 0000 0000 0000 0000 0000 0000 0000
*
0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
*

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-25 21:50         ` Dave Chinner
@ 2014-02-26 14:58           ` Lukáš Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-26 14:58 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-ext4, linux-fsdevel, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6211 bytes --]

On Wed, 26 Feb 2014, Dave Chinner wrote:

> Date: Wed, 26 Feb 2014 08:50:11 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > 
> > > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > > From: Dave Chinner <david@fromorbit.com>
> > > To: Lukas Czerner <lczerner@redhat.com>
> > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > >     range
> > > 
> > > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > > This is copy of xfs/242. However it's better to make it file system
> > > > specific because the range can be zeroes either directly by writing
> > > > zeroes, or converting to unwritten extent, so the actual result might
> > > > differ from file system to file system.
> > > 
> > > You could say the same thing about preallocation using unwritten
> > > extents. Yet, funnily enough, we have generic tests for them because
> > > all filesystems currently use unwritten extents for preallocation
> > > and behave identically....
> > > 
> > > This test is no different - all filesystems currently use unwritten
> > > extents, and so this test should be generic because all existing
> > > filesystems *should* behave the same.
> > > 
> > > When we get a filesystem that zeros rather uses unwritten extents,
> > > we can add a new *generic* test that tests for zeroed data extents
> > > rather than unwritten extents. All that we will need is a method of
> > > checking what behaviour the filesystem has and adding that to a
> > > _requires directive to ensure the correct generic fallocate tests
> > > are run...
> > 
> > Currently xfs/242 fails on xfs for me
> 
> Really? Where's the bug report? I haven't seen a failure on xfs/242
> on any of my test machines for at least a year, even on 1k block
> size filesystems...
> 
> $ sudo ./check xfs/242
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> 
> xfs/242 1s ... 0s
> Ran: xfs/242
> Passed all 1 tests
> $

Ok so once again. Yesterday was rally too late and I've
misinterpreted the diff. It's not that xfs behaves differently, but
rather ext4 behaves differently because we in fact have a code that
will zero out entire unwritten extent if it's small enough rather
than split it into unwritten and written.

What we currently do not have is the zeroout of part of initialized
extent on zero_range when the range to zero out is small enough. And
ext4 behaves the same on the edges of the range as you describe
below.

Also xfs/242 fails for me on ppc64 but I've already sent that
report.

> 
> 
> > and it does behave differently than ext4.
> 
> In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
> XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
> it because you wrote this test as an ext4 specific test and so
> haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
> path in XFS?

It does behave differently, but not because of zero_range code, but
rather when writing into uninitialized extent which is small enough.
The extent will not be split but rather converted to initialized and
respective parts will be zeroed out.

Btw that's actually the reason why we use

_filter_hole_fiemap

filtes instead of

_filter_fiemap

I've used in ext4/242.

That said I think that both tests fs specific and fs independent
have it's value so I'll create generic/242 as well by using
_filter_hole_fiemap.

And yes, I tested xfs with fzero and it does behave the same as
XFS_IOC_ZERO_RANGE.

> 
> IOWs, how do you know that what you are seeing is not a bug in the
> ext4 (or XFS for that matter) implementation?
> 
> > Also I had to change to 242.out a bit because ext4 was
> > a little different. It seems to me that it was expected that when
> > the extent is small enough it would be overwritten by zeroes rather
> > than converted to unwritten, but I have not looked into
> > implementation.
> 
> The test assumes that sub-block head and tail ranges will be zeroed,
> and everything else will be converted to unwritten extents. i.e. a
> single block aligned range will get converted to unwritten, but a
> single unaligned block range will result in the two overlapping
> blocks being zeroed (because they still contain valid data).

As I said above the difference in ext4 is that we're trying to avoid
extent split when the extent is too small. But that's no different
in punch hole testing, that's why we're using _filter_hole_fiemap
filter in generic/255.

> 
> This is the same sub-block zeroing behaviour as is done for
> hole punching - the only difference between a hole punch and a zero
> range on filesystems that use unwritten extents should be that the
> range being operated on has unwritten extents rather a hole.....
> 
> > Btw this kind of optimization is actually something I've been
> > thinking of as well for ext4. Rather than going though the hassle of
> > changing extents around it might be worth in some situation to zero
> > out. But that's an optimization I have not implemented yet.
> 
> Exactly my point - until such optimisations are implemented, all the
> filesystems should be behaving the same way using unwritten extents,
> just like for hole punching. Hence the tests should be checking that
> the behaviour is the same across filesystems, just like we do for
> hole punching.

Using _filter_hole_fiemap filter in such test we would not make a
difference between unwritten and written extent. However in the case
of zero_range this somewhat make the test much less effective so
it'll be worth having fs specific test as well as generic test I
said above.

Or we could actually directly inspect the data as we do in xfs/290, or
generic/290 respectively.

Thanks!
-Lukas

> 
> Cheers,
> 
> Dave.
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-26 14:58           ` Lukáš Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-26 14:58 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, linux-ext4, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6211 bytes --]

On Wed, 26 Feb 2014, Dave Chinner wrote:

> Date: Wed, 26 Feb 2014 08:50:11 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > 
> > > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > > From: Dave Chinner <david@fromorbit.com>
> > > To: Lukas Czerner <lczerner@redhat.com>
> > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > >     range
> > > 
> > > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > > This is copy of xfs/242. However it's better to make it file system
> > > > specific because the range can be zeroes either directly by writing
> > > > zeroes, or converting to unwritten extent, so the actual result might
> > > > differ from file system to file system.
> > > 
> > > You could say the same thing about preallocation using unwritten
> > > extents. Yet, funnily enough, we have generic tests for them because
> > > all filesystems currently use unwritten extents for preallocation
> > > and behave identically....
> > > 
> > > This test is no different - all filesystems currently use unwritten
> > > extents, and so this test should be generic because all existing
> > > filesystems *should* behave the same.
> > > 
> > > When we get a filesystem that zeros rather uses unwritten extents,
> > > we can add a new *generic* test that tests for zeroed data extents
> > > rather than unwritten extents. All that we will need is a method of
> > > checking what behaviour the filesystem has and adding that to a
> > > _requires directive to ensure the correct generic fallocate tests
> > > are run...
> > 
> > Currently xfs/242 fails on xfs for me
> 
> Really? Where's the bug report? I haven't seen a failure on xfs/242
> on any of my test machines for at least a year, even on 1k block
> size filesystems...
> 
> $ sudo ./check xfs/242
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> 
> xfs/242 1s ... 0s
> Ran: xfs/242
> Passed all 1 tests
> $

Ok so once again. Yesterday was rally too late and I've
misinterpreted the diff. It's not that xfs behaves differently, but
rather ext4 behaves differently because we in fact have a code that
will zero out entire unwritten extent if it's small enough rather
than split it into unwritten and written.

What we currently do not have is the zeroout of part of initialized
extent on zero_range when the range to zero out is small enough. And
ext4 behaves the same on the edges of the range as you describe
below.

Also xfs/242 fails for me on ppc64 but I've already sent that
report.

> 
> 
> > and it does behave differently than ext4.
> 
> In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
> XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
> it because you wrote this test as an ext4 specific test and so
> haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
> path in XFS?

It does behave differently, but not because of zero_range code, but
rather when writing into uninitialized extent which is small enough.
The extent will not be split but rather converted to initialized and
respective parts will be zeroed out.

Btw that's actually the reason why we use

_filter_hole_fiemap

filtes instead of

_filter_fiemap

I've used in ext4/242.

That said I think that both tests fs specific and fs independent
have it's value so I'll create generic/242 as well by using
_filter_hole_fiemap.

And yes, I tested xfs with fzero and it does behave the same as
XFS_IOC_ZERO_RANGE.

> 
> IOWs, how do you know that what you are seeing is not a bug in the
> ext4 (or XFS for that matter) implementation?
> 
> > Also I had to change to 242.out a bit because ext4 was
> > a little different. It seems to me that it was expected that when
> > the extent is small enough it would be overwritten by zeroes rather
> > than converted to unwritten, but I have not looked into
> > implementation.
> 
> The test assumes that sub-block head and tail ranges will be zeroed,
> and everything else will be converted to unwritten extents. i.e. a
> single block aligned range will get converted to unwritten, but a
> single unaligned block range will result in the two overlapping
> blocks being zeroed (because they still contain valid data).

As I said above the difference in ext4 is that we're trying to avoid
extent split when the extent is too small. But that's no different
in punch hole testing, that's why we're using _filter_hole_fiemap
filter in generic/255.

> 
> This is the same sub-block zeroing behaviour as is done for
> hole punching - the only difference between a hole punch and a zero
> range on filesystems that use unwritten extents should be that the
> range being operated on has unwritten extents rather a hole.....
> 
> > Btw this kind of optimization is actually something I've been
> > thinking of as well for ext4. Rather than going though the hassle of
> > changing extents around it might be worth in some situation to zero
> > out. But that's an optimization I have not implemented yet.
> 
> Exactly my point - until such optimisations are implemented, all the
> filesystems should be behaving the same way using unwritten extents,
> just like for hole punching. Hence the tests should be checking that
> the behaviour is the same across filesystems, just like we do for
> hole punching.

Using _filter_hole_fiemap filter in such test we would not make a
difference between unwritten and written extent. However in the case
of zero_range this somewhat make the test much less effective so
it'll be worth having fs specific test as well as generic test I
said above.

Or we could actually directly inspect the data as we do in xfs/290, or
generic/290 respectively.

Thanks!
-Lukas

> 
> Cheers,
> 
> Dave.
> 

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-26 14:24         ` Lukáš Czerner
@ 2014-02-26 22:01             ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-26 22:01 UTC (permalink / raw)
  To: Lukáš Czerner; +Cc: linux-ext4, linux-fsdevel, xfs

On Wed, Feb 26, 2014 at 03:24:18PM +0100, Lukáš Czerner wrote:
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> 
> > Date: Wed, 26 Feb 2014 08:50:11 +1100
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukáš Czerner <lczerner@redhat.com>
> > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> >     range
> > 
> > On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> > > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > 
> > > > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > > > From: Dave Chinner <david@fromorbit.com>
> > > > To: Lukas Czerner <lczerner@redhat.com>
> > > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > > >     range
> > > > 
> > > > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > > > This is copy of xfs/242. However it's better to make it file system
> > > > > specific because the range can be zeroes either directly by writing
> > > > > zeroes, or converting to unwritten extent, so the actual result might
> > > > > differ from file system to file system.
> > > > 
> > > > You could say the same thing about preallocation using unwritten
> > > > extents. Yet, funnily enough, we have generic tests for them because
> > > > all filesystems currently use unwritten extents for preallocation
> > > > and behave identically....
> > > > 
> > > > This test is no different - all filesystems currently use unwritten
> > > > extents, and so this test should be generic because all existing
> > > > filesystems *should* behave the same.
> > > > 
> > > > When we get a filesystem that zeros rather uses unwritten extents,
> > > > we can add a new *generic* test that tests for zeroed data extents
> > > > rather than unwritten extents. All that we will need is a method of
> > > > checking what behaviour the filesystem has and adding that to a
> > > > _requires directive to ensure the correct generic fallocate tests
> > > > are run...
> > > 
> > > Currently xfs/242 fails on xfs for me
> > 
> > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > on any of my test machines for at least a year, even on 1k block
> > size filesystems...
> > 
> > $ sudo ./check xfs/242
> > FSTYP         -- xfs (debug)
> > PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> > MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> > MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> > 
> > xfs/242 1s ... 0s
> > Ran: xfs/242
> > Passed all 1 tests
> > $
> 
> Here it is.
> 
> xfs/242 fails on ppc64 with latest linus tree

OK, that's a different kettle of fish. It's using 64k pages, right?

> # uname -a
> Linux ibm-p740-01-lp4.rhts.eng.bos.redhat.com 3.14.0-rc4+ #1 SMP Wed
> Feb 26 08:59:48 EST 2014 ppc64 ppc64 ppc64 GNU/Linux
> 
> # ./check xfs/242
> FSTYP         -- xfs (non-debug)
> PLATFORM      -- Linux/ppc64 ibm-p740-01-lp4 3.14.0-rc4+
> MKFS_OPTIONS  -- -f -bsize=4096 /dev/loop1
> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/loop1 /mnt/test2
> 
> xfs/242  - output mismatch (see /root/xfstests/results//xfs/242.out.bad)
>     --- tests/xfs/242.out       2014-02-26 05:51:16.602579462 -0500
>     +++ /root/xfstests/results//xfs/242.out.bad 2014-02-26 09:20:55.585396040 -0500
>     @@ -1,76 +1,71 @@
>      QA output created by 242
>         1. into a hole
>      0: [0..7]: hole
>     -1: [8..23]: unwritten
>     +1: [8..23]: data
>      2: [24..39]: hole
>      daa100df6e6711906b61c9ab5aa16032

So the data is correct, but the range got zeroes written to it
rather than an unwritten extent.

>     (Run 'diff -u tests/xfs/242.out /root/xfstests/results//xfs/242.out.bad'  to see the entire diff)
> Ran: xfs/242
> Failures: xfs/242
> Failed 1 of 1 tests
> 
> 
> Here is 242.out.bad

The diff would have been better.

/me goes off to diff the output

Yeah, ok, the data in all the files is correct - the md5sums all
match. What's different? Just about all unwritten extents are now
written (i.e. data) or contain some portion of written extents.

So, ZERO_RANGE has the following size threshold for converting
blocks to unwritten extents vs just zeroing them:

	granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);

So if this is a 64k page size machine, it's going to have different
extent layout compared to a 4k page size machine. The file data will
still be the same, the difference will be zeroed blocks instead of
unwritten blocks, and that's exactly what we see.

IOWs, the result in terms of data the application sees is correct,
just the extent layout representing that zeroed data is different.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-26 22:01             ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-26 22:01 UTC (permalink / raw)
  To: Lukáš Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Wed, Feb 26, 2014 at 03:24:18PM +0100, Lukáš Czerner wrote:
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> 
> > Date: Wed, 26 Feb 2014 08:50:11 +1100
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukáš Czerner <lczerner@redhat.com>
> > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> >     range
> > 
> > On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> > > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > 
> > > > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > > > From: Dave Chinner <david@fromorbit.com>
> > > > To: Lukas Czerner <lczerner@redhat.com>
> > > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > > >     range
> > > > 
> > > > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > > > This is copy of xfs/242. However it's better to make it file system
> > > > > specific because the range can be zeroes either directly by writing
> > > > > zeroes, or converting to unwritten extent, so the actual result might
> > > > > differ from file system to file system.
> > > > 
> > > > You could say the same thing about preallocation using unwritten
> > > > extents. Yet, funnily enough, we have generic tests for them because
> > > > all filesystems currently use unwritten extents for preallocation
> > > > and behave identically....
> > > > 
> > > > This test is no different - all filesystems currently use unwritten
> > > > extents, and so this test should be generic because all existing
> > > > filesystems *should* behave the same.
> > > > 
> > > > When we get a filesystem that zeros rather uses unwritten extents,
> > > > we can add a new *generic* test that tests for zeroed data extents
> > > > rather than unwritten extents. All that we will need is a method of
> > > > checking what behaviour the filesystem has and adding that to a
> > > > _requires directive to ensure the correct generic fallocate tests
> > > > are run...
> > > 
> > > Currently xfs/242 fails on xfs for me
> > 
> > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > on any of my test machines for at least a year, even on 1k block
> > size filesystems...
> > 
> > $ sudo ./check xfs/242
> > FSTYP         -- xfs (debug)
> > PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> > MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> > MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> > 
> > xfs/242 1s ... 0s
> > Ran: xfs/242
> > Passed all 1 tests
> > $
> 
> Here it is.
> 
> xfs/242 fails on ppc64 with latest linus tree

OK, that's a different kettle of fish. It's using 64k pages, right?

> # uname -a
> Linux ibm-p740-01-lp4.rhts.eng.bos.redhat.com 3.14.0-rc4+ #1 SMP Wed
> Feb 26 08:59:48 EST 2014 ppc64 ppc64 ppc64 GNU/Linux
> 
> # ./check xfs/242
> FSTYP         -- xfs (non-debug)
> PLATFORM      -- Linux/ppc64 ibm-p740-01-lp4 3.14.0-rc4+
> MKFS_OPTIONS  -- -f -bsize=4096 /dev/loop1
> MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/loop1 /mnt/test2
> 
> xfs/242  - output mismatch (see /root/xfstests/results//xfs/242.out.bad)
>     --- tests/xfs/242.out       2014-02-26 05:51:16.602579462 -0500
>     +++ /root/xfstests/results//xfs/242.out.bad 2014-02-26 09:20:55.585396040 -0500
>     @@ -1,76 +1,71 @@
>      QA output created by 242
>         1. into a hole
>      0: [0..7]: hole
>     -1: [8..23]: unwritten
>     +1: [8..23]: data
>      2: [24..39]: hole
>      daa100df6e6711906b61c9ab5aa16032

So the data is correct, but the range got zeroes written to it
rather than an unwritten extent.

>     (Run 'diff -u tests/xfs/242.out /root/xfstests/results//xfs/242.out.bad'  to see the entire diff)
> Ran: xfs/242
> Failures: xfs/242
> Failed 1 of 1 tests
> 
> 
> Here is 242.out.bad

The diff would have been better.

/me goes off to diff the output

Yeah, ok, the data in all the files is correct - the md5sums all
match. What's different? Just about all unwritten extents are now
written (i.e. data) or contain some portion of written extents.

So, ZERO_RANGE has the following size threshold for converting
blocks to unwritten extents vs just zeroing them:

	granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);

So if this is a 64k page size machine, it's going to have different
extent layout compared to a 4k page size machine. The file data will
still be the same, the difference will be zeroed blocks instead of
unwritten blocks, and that's exactly what we see.

IOWs, the result in terms of data the application sees is correct,
just the extent layout representing that zeroed data is different.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-26 14:58           ` Lukáš Czerner
@ 2014-02-26 22:17             ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-26 22:17 UTC (permalink / raw)
  To: Lukáš Czerner; +Cc: linux-ext4, linux-fsdevel, xfs

On Wed, Feb 26, 2014 at 03:58:36PM +0100, Lukáš Czerner wrote:
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > Currently xfs/242 fails on xfs for me
> > 
> > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > on any of my test machines for at least a year, even on 1k block
> > size filesystems...
> > 
> > $ sudo ./check xfs/242
> > FSTYP         -- xfs (debug)
> > PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> > MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> > MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> > 
> > xfs/242 1s ... 0s
> > Ran: xfs/242
> > Passed all 1 tests
> > $
> 
> Ok so once again. Yesterday was rally too late and I've
> misinterpreted the diff. It's not that xfs behaves differently, but
> rather ext4 behaves differently because we in fact have a code that
> will zero out entire unwritten extent if it's small enough rather
> than split it into unwritten and written.

Yes, we've come across this before, and there are several solutions.
this one is used in tests/generic/285:

# Disable extent zeroing for ext4 as that change where holes ar created
if [ "$FSTYP" = "ext4" ]; then
	DEV=`basename $TEST_DEV`
	echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
fi

> > > and it does behave differently than ext4.
> > 
> > In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
> > XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
> > it because you wrote this test as an ext4 specific test and so
> > haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
> > path in XFS?
> 
> It does behave differently, but not because of zero_range code, but
> rather when writing into uninitialized extent which is small enough.
> The extent will not be split but rather converted to initialized and
> respective parts will be zeroed out.
> 
> Btw that's actually the reason why we use
> 
> _filter_hole_fiemap
> 
> filtes instead of
> 
> _filter_fiemap
> 
> I've used in ext4/242.

Sure, but even so I think we might do better just to use the above
zeroout tune and be explicit in what we expect to happen w.r.t. data
vs holes...

> 
> That said I think that both tests fs specific and fs independent
> have it's value so I'll create generic/242 as well by using
> _filter_hole_fiemap.

Just use the first unused generic test number - trying to keep test
numbers the same across different subdirs is just going to cause
confusion....

> > hole punching - the only difference between a hole punch and a zero
> > range on filesystems that use unwritten extents should be that the
> > range being operated on has unwritten extents rather a hole.....
> > 
> > > Btw this kind of optimization is actually something I've been
> > > thinking of as well for ext4. Rather than going though the hassle of
> > > changing extents around it might be worth in some situation to zero
> > > out. But that's an optimization I have not implemented yet.
> > 
> > Exactly my point - until such optimisations are implemented, all the
> > filesystems should be behaving the same way using unwritten extents,
> > just like for hole punching. Hence the tests should be checking that
> > the behaviour is the same across filesystems, just like we do for
> > hole punching.
> 
> Using _filter_hole_fiemap filter in such test we would not make a
> difference between unwritten and written extent. However in the case
> of zero_range this somewhat make the test much less effective so
> it'll be worth having fs specific test as well as generic test I
> said above.
> 
> Or we could actually directly inspect the data as we do in xfs/290, or
> generic/290 respectively.

The md5sum does the data inspection for us. The whole point of
hole punch and zero range and so one is that they are extent
manipulation operations. If we don't check that extents have been
manipulated correctly, then we aren't testing that the key behaviour
the filesystems are supposed to display for those operations....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-26 22:17             ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-26 22:17 UTC (permalink / raw)
  To: Lukáš Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Wed, Feb 26, 2014 at 03:58:36PM +0100, Lukáš Czerner wrote:
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > Currently xfs/242 fails on xfs for me
> > 
> > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > on any of my test machines for at least a year, even on 1k block
> > size filesystems...
> > 
> > $ sudo ./check xfs/242
> > FSTYP         -- xfs (debug)
> > PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> > MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> > MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> > 
> > xfs/242 1s ... 0s
> > Ran: xfs/242
> > Passed all 1 tests
> > $
> 
> Ok so once again. Yesterday was rally too late and I've
> misinterpreted the diff. It's not that xfs behaves differently, but
> rather ext4 behaves differently because we in fact have a code that
> will zero out entire unwritten extent if it's small enough rather
> than split it into unwritten and written.

Yes, we've come across this before, and there are several solutions.
this one is used in tests/generic/285:

# Disable extent zeroing for ext4 as that change where holes ar created
if [ "$FSTYP" = "ext4" ]; then
	DEV=`basename $TEST_DEV`
	echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
fi

> > > and it does behave differently than ext4.
> > 
> > In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
> > XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
> > it because you wrote this test as an ext4 specific test and so
> > haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
> > path in XFS?
> 
> It does behave differently, but not because of zero_range code, but
> rather when writing into uninitialized extent which is small enough.
> The extent will not be split but rather converted to initialized and
> respective parts will be zeroed out.
> 
> Btw that's actually the reason why we use
> 
> _filter_hole_fiemap
> 
> filtes instead of
> 
> _filter_fiemap
> 
> I've used in ext4/242.

Sure, but even so I think we might do better just to use the above
zeroout tune and be explicit in what we expect to happen w.r.t. data
vs holes...

> 
> That said I think that both tests fs specific and fs independent
> have it's value so I'll create generic/242 as well by using
> _filter_hole_fiemap.

Just use the first unused generic test number - trying to keep test
numbers the same across different subdirs is just going to cause
confusion....

> > hole punching - the only difference between a hole punch and a zero
> > range on filesystems that use unwritten extents should be that the
> > range being operated on has unwritten extents rather a hole.....
> > 
> > > Btw this kind of optimization is actually something I've been
> > > thinking of as well for ext4. Rather than going though the hassle of
> > > changing extents around it might be worth in some situation to zero
> > > out. But that's an optimization I have not implemented yet.
> > 
> > Exactly my point - until such optimisations are implemented, all the
> > filesystems should be behaving the same way using unwritten extents,
> > just like for hole punching. Hence the tests should be checking that
> > the behaviour is the same across filesystems, just like we do for
> > hole punching.
> 
> Using _filter_hole_fiemap filter in such test we would not make a
> difference between unwritten and written extent. However in the case
> of zero_range this somewhat make the test much less effective so
> it'll be worth having fs specific test as well as generic test I
> said above.
> 
> Or we could actually directly inspect the data as we do in xfs/290, or
> generic/290 respectively.

The md5sum does the data inspection for us. The whole point of
hole punch and zero range and so one is that they are extent
manipulation operations. If we don't check that extents have been
manipulated correctly, then we aren't testing that the key behaviour
the filesystems are supposed to display for those operations....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-26 22:17             ` Dave Chinner
@ 2014-02-27 11:48               ` Lukáš Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-27 11:48 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-ext4, linux-fsdevel, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4774 bytes --]

On Thu, 27 Feb 2014, Dave Chinner wrote:

> Date: Thu, 27 Feb 2014 09:17:57 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Wed, Feb 26, 2014 at 03:58:36PM +0100, Lukáš Czerner wrote:
> > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > > Currently xfs/242 fails on xfs for me
> > > 
> > > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > > on any of my test machines for at least a year, even on 1k block
> > > size filesystems...
> > > 
> > > $ sudo ./check xfs/242
> > > FSTYP         -- xfs (debug)
> > > PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> > > MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> > > MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> > > 
> > > xfs/242 1s ... 0s
> > > Ran: xfs/242
> > > Passed all 1 tests
> > > $
> > 
> > Ok so once again. Yesterday was rally too late and I've
> > misinterpreted the diff. It's not that xfs behaves differently, but
> > rather ext4 behaves differently because we in fact have a code that
> > will zero out entire unwritten extent if it's small enough rather
> > than split it into unwritten and written.
> 
> Yes, we've come across this before, and there are several solutions.
> this one is used in tests/generic/285:
> 
> # Disable extent zeroing for ext4 as that change where holes ar created
> if [ "$FSTYP" = "ext4" ]; then
> 	DEV=`basename $TEST_DEV`
> 	echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
> fi

When testing SEEK_DATA, SEEK_HOLE this is probably just fine. But
when it comes to actually changing the extent tree and checking the
result I would rather not disable this because it is the default and
we want to test that code as well.

> 
> > > > and it does behave differently than ext4.
> > > 
> > > In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
> > > XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
> > > it because you wrote this test as an ext4 specific test and so
> > > haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
> > > path in XFS?
> > 
> > It does behave differently, but not because of zero_range code, but
> > rather when writing into uninitialized extent which is small enough.
> > The extent will not be split but rather converted to initialized and
> > respective parts will be zeroed out.
> > 
> > Btw that's actually the reason why we use
> > 
> > _filter_hole_fiemap
> > 
> > filtes instead of
> > 
> > _filter_fiemap
> > 
> > I've used in ext4/242.
> 
> Sure, but even so I think we might do better just to use the above
> zeroout tune and be explicit in what we expect to happen w.r.t. data
> vs holes...
> 
> > 
> > That said I think that both tests fs specific and fs independent
> > have it's value so I'll create generic/242 as well by using
> > _filter_hole_fiemap.
> 
> Just use the first unused generic test number - trying to keep test
> numbers the same across different subdirs is just going to cause
> confusion....

Ok.

Thanks!
-Lukas

> 
> > > hole punching - the only difference between a hole punch and a zero
> > > range on filesystems that use unwritten extents should be that the
> > > range being operated on has unwritten extents rather a hole.....
> > > 
> > > > Btw this kind of optimization is actually something I've been
> > > > thinking of as well for ext4. Rather than going though the hassle of
> > > > changing extents around it might be worth in some situation to zero
> > > > out. But that's an optimization I have not implemented yet.
> > > 
> > > Exactly my point - until such optimisations are implemented, all the
> > > filesystems should be behaving the same way using unwritten extents,
> > > just like for hole punching. Hence the tests should be checking that
> > > the behaviour is the same across filesystems, just like we do for
> > > hole punching.
> > 
> > Using _filter_hole_fiemap filter in such test we would not make a
> > difference between unwritten and written extent. However in the case
> > of zero_range this somewhat make the test much less effective so
> > it'll be worth having fs specific test as well as generic test I
> > said above.
> > 
> > Or we could actually directly inspect the data as we do in xfs/290, or
> > generic/290 respectively.
> 
> The md5sum does the data inspection for us. The whole point of
> hole punch and zero range and so one is that they are extent
> manipulation operations. If we don't check that extents have been
> manipulated correctly, then we aren't testing that the key behaviour
> the filesystems are supposed to display for those operations....
> 
> Cheers,
> 
> Dave.
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-27 11:48               ` Lukáš Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-27 11:48 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, linux-ext4, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4774 bytes --]

On Thu, 27 Feb 2014, Dave Chinner wrote:

> Date: Thu, 27 Feb 2014 09:17:57 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Wed, Feb 26, 2014 at 03:58:36PM +0100, Lukáš Czerner wrote:
> > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > > Currently xfs/242 fails on xfs for me
> > > 
> > > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > > on any of my test machines for at least a year, even on 1k block
> > > size filesystems...
> > > 
> > > $ sudo ./check xfs/242
> > > FSTYP         -- xfs (debug)
> > > PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> > > MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> > > MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> > > 
> > > xfs/242 1s ... 0s
> > > Ran: xfs/242
> > > Passed all 1 tests
> > > $
> > 
> > Ok so once again. Yesterday was rally too late and I've
> > misinterpreted the diff. It's not that xfs behaves differently, but
> > rather ext4 behaves differently because we in fact have a code that
> > will zero out entire unwritten extent if it's small enough rather
> > than split it into unwritten and written.
> 
> Yes, we've come across this before, and there are several solutions.
> this one is used in tests/generic/285:
> 
> # Disable extent zeroing for ext4 as that change where holes ar created
> if [ "$FSTYP" = "ext4" ]; then
> 	DEV=`basename $TEST_DEV`
> 	echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
> fi

When testing SEEK_DATA, SEEK_HOLE this is probably just fine. But
when it comes to actually changing the extent tree and checking the
result I would rather not disable this because it is the default and
we want to test that code as well.

> 
> > > > and it does behave differently than ext4.
> > > 
> > > In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
> > > XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
> > > it because you wrote this test as an ext4 specific test and so
> > > haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
> > > path in XFS?
> > 
> > It does behave differently, but not because of zero_range code, but
> > rather when writing into uninitialized extent which is small enough.
> > The extent will not be split but rather converted to initialized and
> > respective parts will be zeroed out.
> > 
> > Btw that's actually the reason why we use
> > 
> > _filter_hole_fiemap
> > 
> > filtes instead of
> > 
> > _filter_fiemap
> > 
> > I've used in ext4/242.
> 
> Sure, but even so I think we might do better just to use the above
> zeroout tune and be explicit in what we expect to happen w.r.t. data
> vs holes...
> 
> > 
> > That said I think that both tests fs specific and fs independent
> > have it's value so I'll create generic/242 as well by using
> > _filter_hole_fiemap.
> 
> Just use the first unused generic test number - trying to keep test
> numbers the same across different subdirs is just going to cause
> confusion....

Ok.

Thanks!
-Lukas

> 
> > > hole punching - the only difference between a hole punch and a zero
> > > range on filesystems that use unwritten extents should be that the
> > > range being operated on has unwritten extents rather a hole.....
> > > 
> > > > Btw this kind of optimization is actually something I've been
> > > > thinking of as well for ext4. Rather than going though the hassle of
> > > > changing extents around it might be worth in some situation to zero
> > > > out. But that's an optimization I have not implemented yet.
> > > 
> > > Exactly my point - until such optimisations are implemented, all the
> > > filesystems should be behaving the same way using unwritten extents,
> > > just like for hole punching. Hence the tests should be checking that
> > > the behaviour is the same across filesystems, just like we do for
> > > hole punching.
> > 
> > Using _filter_hole_fiemap filter in such test we would not make a
> > difference between unwritten and written extent. However in the case
> > of zero_range this somewhat make the test much less effective so
> > it'll be worth having fs specific test as well as generic test I
> > said above.
> > 
> > Or we could actually directly inspect the data as we do in xfs/290, or
> > generic/290 respectively.
> 
> The md5sum does the data inspection for us. The whole point of
> hole punch and zero range and so one is that they are extent
> manipulation operations. If we don't check that extents have been
> manipulated correctly, then we aren't testing that the key behaviour
> the filesystems are supposed to display for those operations....
> 
> Cheers,
> 
> Dave.
> 

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-26 22:01             ` Dave Chinner
@ 2014-02-27 12:03               ` Lukáš Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-27 12:03 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-ext4, linux-fsdevel, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5655 bytes --]

On Thu, 27 Feb 2014, Dave Chinner wrote:

> Date: Thu, 27 Feb 2014 09:01:06 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Wed, Feb 26, 2014 at 03:24:18PM +0100, Lukáš Czerner wrote:
> > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > 
> > > Date: Wed, 26 Feb 2014 08:50:11 +1100
> > > From: Dave Chinner <david@fromorbit.com>
> > > To: Lukáš Czerner <lczerner@redhat.com>
> > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > >     range
> > > 
> > > On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> > > > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > > 
> > > > > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > > > > From: Dave Chinner <david@fromorbit.com>
> > > > > To: Lukas Czerner <lczerner@redhat.com>
> > > > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > > > >     range
> > > > > 
> > > > > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > > > > This is copy of xfs/242. However it's better to make it file system
> > > > > > specific because the range can be zeroes either directly by writing
> > > > > > zeroes, or converting to unwritten extent, so the actual result might
> > > > > > differ from file system to file system.
> > > > > 
> > > > > You could say the same thing about preallocation using unwritten
> > > > > extents. Yet, funnily enough, we have generic tests for them because
> > > > > all filesystems currently use unwritten extents for preallocation
> > > > > and behave identically....
> > > > > 
> > > > > This test is no different - all filesystems currently use unwritten
> > > > > extents, and so this test should be generic because all existing
> > > > > filesystems *should* behave the same.
> > > > > 
> > > > > When we get a filesystem that zeros rather uses unwritten extents,
> > > > > we can add a new *generic* test that tests for zeroed data extents
> > > > > rather than unwritten extents. All that we will need is a method of
> > > > > checking what behaviour the filesystem has and adding that to a
> > > > > _requires directive to ensure the correct generic fallocate tests
> > > > > are run...
> > > > 
> > > > Currently xfs/242 fails on xfs for me
> > > 
> > > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > > on any of my test machines for at least a year, even on 1k block
> > > size filesystems...
> > > 
> > > $ sudo ./check xfs/242
> > > FSTYP         -- xfs (debug)
> > > PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> > > MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> > > MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> > > 
> > > xfs/242 1s ... 0s
> > > Ran: xfs/242
> > > Passed all 1 tests
> > > $
> > 
> > Here it is.
> > 
> > xfs/242 fails on ppc64 with latest linus tree
> 
> OK, that's a different kettle of fish. It's using 64k pages, right?

64k pages, yes.

> 
> > # uname -a
> > Linux ibm-p740-01-lp4.rhts.eng.bos.redhat.com 3.14.0-rc4+ #1 SMP Wed
> > Feb 26 08:59:48 EST 2014 ppc64 ppc64 ppc64 GNU/Linux
> > 
> > # ./check xfs/242
> > FSTYP         -- xfs (non-debug)
> > PLATFORM      -- Linux/ppc64 ibm-p740-01-lp4 3.14.0-rc4+
> > MKFS_OPTIONS  -- -f -bsize=4096 /dev/loop1
> > MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/loop1 /mnt/test2
> > 
> > xfs/242  - output mismatch (see /root/xfstests/results//xfs/242.out.bad)
> >     --- tests/xfs/242.out       2014-02-26 05:51:16.602579462 -0500
> >     +++ /root/xfstests/results//xfs/242.out.bad 2014-02-26 09:20:55.585396040 -0500
> >     @@ -1,76 +1,71 @@
> >      QA output created by 242
> >         1. into a hole
> >      0: [0..7]: hole
> >     -1: [8..23]: unwritten
> >     +1: [8..23]: data
> >      2: [24..39]: hole
> >      daa100df6e6711906b61c9ab5aa16032
> 
> So the data is correct, but the range got zeroes written to it
> rather than an unwritten extent.
> 
> >     (Run 'diff -u tests/xfs/242.out /root/xfstests/results//xfs/242.out.bad'  to see the entire diff)
> > Ran: xfs/242
> > Failures: xfs/242
> > Failed 1 of 1 tests
> > 
> > 
> > Here is 242.out.bad
> 
> The diff would have been better.
> 
> /me goes off to diff the output
> 
> Yeah, ok, the data in all the files is correct - the md5sums all
> match. What's different? Just about all unwritten extents are now
> written (i.e. data) or contain some portion of written extents.
> 
> So, ZERO_RANGE has the following size threshold for converting
> blocks to unwritten extents vs just zeroing them:
> 
> 	granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
> 
> So if this is a 64k page size machine, it's going to have different
> extent layout compared to a 4k page size machine. The file data will
> still be the same, the difference will be zeroed blocks instead of
> unwritten blocks, and that's exactly what we see.
> 
> IOWs, the result in terms of data the application sees is correct,
> just the extent layout representing that zeroed data is different.

Ok, so that's yet another difference between xfs and ext4 code which
makes having generic test even more complicated. So as I said before
I'll make the generic test (using _filter_hole_fiemap) and then ext4
specific test as well to really make sure that the extent
manipulation is right.

Thanks!
-Lukas

> 
> Cheers,
> 
> Dave.
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-27 12:03               ` Lukáš Czerner
  0 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-27 12:03 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, linux-ext4, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5655 bytes --]

On Thu, 27 Feb 2014, Dave Chinner wrote:

> Date: Thu, 27 Feb 2014 09:01:06 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Wed, Feb 26, 2014 at 03:24:18PM +0100, Lukáš Czerner wrote:
> > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > 
> > > Date: Wed, 26 Feb 2014 08:50:11 +1100
> > > From: Dave Chinner <david@fromorbit.com>
> > > To: Lukáš Czerner <lczerner@redhat.com>
> > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > >     range
> > > 
> > > On Tue, Feb 25, 2014 at 10:01:06PM +0100, Lukáš Czerner wrote:
> > > > On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > > 
> > > > > Date: Wed, 26 Feb 2014 07:53:49 +1100
> > > > > From: Dave Chinner <david@fromorbit.com>
> > > > > To: Lukas Czerner <lczerner@redhat.com>
> > > > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> > > > > Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
> > > > >     range
> > > > > 
> > > > > On Tue, Feb 25, 2014 at 08:15:28PM +0100, Lukas Czerner wrote:
> > > > > > This is copy of xfs/242. However it's better to make it file system
> > > > > > specific because the range can be zeroes either directly by writing
> > > > > > zeroes, or converting to unwritten extent, so the actual result might
> > > > > > differ from file system to file system.
> > > > > 
> > > > > You could say the same thing about preallocation using unwritten
> > > > > extents. Yet, funnily enough, we have generic tests for them because
> > > > > all filesystems currently use unwritten extents for preallocation
> > > > > and behave identically....
> > > > > 
> > > > > This test is no different - all filesystems currently use unwritten
> > > > > extents, and so this test should be generic because all existing
> > > > > filesystems *should* behave the same.
> > > > > 
> > > > > When we get a filesystem that zeros rather uses unwritten extents,
> > > > > we can add a new *generic* test that tests for zeroed data extents
> > > > > rather than unwritten extents. All that we will need is a method of
> > > > > checking what behaviour the filesystem has and adding that to a
> > > > > _requires directive to ensure the correct generic fallocate tests
> > > > > are run...
> > > > 
> > > > Currently xfs/242 fails on xfs for me
> > > 
> > > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > > on any of my test machines for at least a year, even on 1k block
> > > size filesystems...
> > > 
> > > $ sudo ./check xfs/242
> > > FSTYP         -- xfs (debug)
> > > PLATFORM      -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> > > MKFS_OPTIONS  -- -f -bsize=4096 /dev/vdb
> > > MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
> > > 
> > > xfs/242 1s ... 0s
> > > Ran: xfs/242
> > > Passed all 1 tests
> > > $
> > 
> > Here it is.
> > 
> > xfs/242 fails on ppc64 with latest linus tree
> 
> OK, that's a different kettle of fish. It's using 64k pages, right?

64k pages, yes.

> 
> > # uname -a
> > Linux ibm-p740-01-lp4.rhts.eng.bos.redhat.com 3.14.0-rc4+ #1 SMP Wed
> > Feb 26 08:59:48 EST 2014 ppc64 ppc64 ppc64 GNU/Linux
> > 
> > # ./check xfs/242
> > FSTYP         -- xfs (non-debug)
> > PLATFORM      -- Linux/ppc64 ibm-p740-01-lp4 3.14.0-rc4+
> > MKFS_OPTIONS  -- -f -bsize=4096 /dev/loop1
> > MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/loop1 /mnt/test2
> > 
> > xfs/242  - output mismatch (see /root/xfstests/results//xfs/242.out.bad)
> >     --- tests/xfs/242.out       2014-02-26 05:51:16.602579462 -0500
> >     +++ /root/xfstests/results//xfs/242.out.bad 2014-02-26 09:20:55.585396040 -0500
> >     @@ -1,76 +1,71 @@
> >      QA output created by 242
> >         1. into a hole
> >      0: [0..7]: hole
> >     -1: [8..23]: unwritten
> >     +1: [8..23]: data
> >      2: [24..39]: hole
> >      daa100df6e6711906b61c9ab5aa16032
> 
> So the data is correct, but the range got zeroes written to it
> rather than an unwritten extent.
> 
> >     (Run 'diff -u tests/xfs/242.out /root/xfstests/results//xfs/242.out.bad'  to see the entire diff)
> > Ran: xfs/242
> > Failures: xfs/242
> > Failed 1 of 1 tests
> > 
> > 
> > Here is 242.out.bad
> 
> The diff would have been better.
> 
> /me goes off to diff the output
> 
> Yeah, ok, the data in all the files is correct - the md5sums all
> match. What's different? Just about all unwritten extents are now
> written (i.e. data) or contain some portion of written extents.
> 
> So, ZERO_RANGE has the following size threshold for converting
> blocks to unwritten extents vs just zeroing them:
> 
> 	granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
> 
> So if this is a 64k page size machine, it's going to have different
> extent layout compared to a 4k page size machine. The file data will
> still be the same, the difference will be zeroed blocks instead of
> unwritten blocks, and that's exactly what we see.
> 
> IOWs, the result in terms of data the application sees is correct,
> just the extent layout representing that zeroed data is different.

Ok, so that's yet another difference between xfs and ext4 code which
makes having generic test even more complicated. So as I said before
I'll make the generic test (using _filter_hole_fiemap) and then ext4
specific test as well to really make sure that the extent
manipulation is right.

Thanks!
-Lukas

> 
> Cheers,
> 
> Dave.
> 

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-27 12:03               ` Lukáš Czerner
@ 2014-02-27 19:35                 ` Dave Chinner
  -1 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-27 19:35 UTC (permalink / raw)
  To: Lukáš Czerner; +Cc: linux-ext4, linux-fsdevel, xfs

On Thu, Feb 27, 2014 at 01:03:09PM +0100, Lukáš Czerner wrote:
> On Thu, 27 Feb 2014, Dave Chinner wrote:
> > > xfs/242 fails on ppc64 with latest linus tree
> > 
> > OK, that's a different kettle of fish. It's using 64k pages, right?
> 
> 64k pages, yes.
....
> > Yeah, ok, the data in all the files is correct - the md5sums all
> > match. What's different? Just about all unwritten extents are now
> > written (i.e. data) or contain some portion of written extents.
> > 
> > So, ZERO_RANGE has the following size threshold for converting
> > blocks to unwritten extents vs just zeroing them:
> > 
> > 	granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
> > 
> > So if this is a 64k page size machine, it's going to have different
> > extent layout compared to a 4k page size machine. The file data will
> > still be the same, the difference will be zeroed blocks instead of
> > unwritten blocks, and that's exactly what we see.
> > 
> > IOWs, the result in terms of data the application sees is correct,
> > just the extent layout representing that zeroed data is different.
> 
> Ok, so that's yet another difference between xfs and ext4 code which
> makes having generic test even more complicated.

It actually makes the need for a generic test more important. A
generic test should handle the differences between block/page size
behaviours - the issue is that xfs/242 was written and tested on 4k
page machines, not 64k page machines.

We've already got the "multiple" code in _generic_test_punch to
increase the size of the regions being worked on, so the simple fix
here is to increase the sizes so that 64k page and 4k page behaviour
is the same and results in the same extent layout. This is the
normal way tests evolve as people run them on different hardware and
configurations....

> So as I said before
> I'll make the generic test (using _filter_hole_fiemap) and then ext4
> specific test as well to really make sure that the extent
> manipulation is right.

Which ignores the fact that if you turn off zeroout on ext4 then a
generic test can also determine that the extent manipulation is
correct. I really don't see the need for an ext4 specific test
here...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
@ 2014-02-27 19:35                 ` Dave Chinner
  0 siblings, 0 replies; 47+ messages in thread
From: Dave Chinner @ 2014-02-27 19:35 UTC (permalink / raw)
  To: Lukáš Czerner; +Cc: linux-fsdevel, linux-ext4, xfs

On Thu, Feb 27, 2014 at 01:03:09PM +0100, Lukáš Czerner wrote:
> On Thu, 27 Feb 2014, Dave Chinner wrote:
> > > xfs/242 fails on ppc64 with latest linus tree
> > 
> > OK, that's a different kettle of fish. It's using 64k pages, right?
> 
> 64k pages, yes.
....
> > Yeah, ok, the data in all the files is correct - the md5sums all
> > match. What's different? Just about all unwritten extents are now
> > written (i.e. data) or contain some portion of written extents.
> > 
> > So, ZERO_RANGE has the following size threshold for converting
> > blocks to unwritten extents vs just zeroing them:
> > 
> > 	granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
> > 
> > So if this is a 64k page size machine, it's going to have different
> > extent layout compared to a 4k page size machine. The file data will
> > still be the same, the difference will be zeroed blocks instead of
> > unwritten blocks, and that's exactly what we see.
> > 
> > IOWs, the result in terms of data the application sees is correct,
> > just the extent layout representing that zeroed data is different.
> 
> Ok, so that's yet another difference between xfs and ext4 code which
> makes having generic test even more complicated.

It actually makes the need for a generic test more important. A
generic test should handle the differences between block/page size
behaviours - the issue is that xfs/242 was written and tested on 4k
page machines, not 64k page machines.

We've already got the "multiple" code in _generic_test_punch to
increase the size of the regions being worked on, so the simple fix
here is to increase the sizes so that 64k page and 4k page behaviour
is the same and results in the same extent layout. This is the
normal way tests evolve as people run them on different hardware and
configurations....

> So as I said before
> I'll make the generic test (using _filter_hole_fiemap) and then ext4
> specific test as well to really make sure that the extent
> manipulation is right.

Which ignores the fact that if you turn off zeroout on ext4 then a
generic test can also determine that the extent manipulation is
correct. I really don't see the need for an ext4 specific test
here...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
  2014-02-27 19:35                 ` Dave Chinner
  (?)
@ 2014-02-28 12:38                 ` Lukáš Czerner
  -1 siblings, 0 replies; 47+ messages in thread
From: Lukáš Czerner @ 2014-02-28 12:38 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-fsdevel, linux-ext4, xfs

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3381 bytes --]

On Fri, 28 Feb 2014, Dave Chinner wrote:

> Date: Fri, 28 Feb 2014 06:35:33 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero
>     range
> 
> On Thu, Feb 27, 2014 at 01:03:09PM +0100, Lukáš Czerner wrote:
> > On Thu, 27 Feb 2014, Dave Chinner wrote:
> > > > xfs/242 fails on ppc64 with latest linus tree
> > > 
> > > OK, that's a different kettle of fish. It's using 64k pages, right?
> > 
> > 64k pages, yes.
> ....
> > > Yeah, ok, the data in all the files is correct - the md5sums all
> > > match. What's different? Just about all unwritten extents are now
> > > written (i.e. data) or contain some portion of written extents.
> > > 
> > > So, ZERO_RANGE has the following size threshold for converting
> > > blocks to unwritten extents vs just zeroing them:
> > > 
> > > 	granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
> > > 
> > > So if this is a 64k page size machine, it's going to have different
> > > extent layout compared to a 4k page size machine. The file data will
> > > still be the same, the difference will be zeroed blocks instead of
> > > unwritten blocks, and that's exactly what we see.
> > > 
> > > IOWs, the result in terms of data the application sees is correct,
> > > just the extent layout representing that zeroed data is different.
> > 
> > Ok, so that's yet another difference between xfs and ext4 code which
> > makes having generic test even more complicated.
> 
> It actually makes the need for a generic test more important. A
> generic test should handle the differences between block/page size
> behaviours - the issue is that xfs/242 was written and tested on 4k
> page machines, not 64k page machines.
> 
> We've already got the "multiple" code in _generic_test_punch to
> increase the size of the regions being worked on, so the simple fix
> here is to increase the sizes so that 64k page and 4k page behaviour
> is the same and results in the same extent layout. This is the
> normal way tests evolve as people run them on different hardware and
> configurations....
> 
> > So as I said before
> > I'll make the generic test (using _filter_hole_fiemap) and then ext4
> > specific test as well to really make sure that the extent
> > manipulation is right.
> 
> Which ignores the fact that if you turn off zeroout on ext4 then a
> generic test can also determine that the extent manipulation is
> correct. I really don't see the need for an ext4 specific test
> here...

I disagree, you assume that there is not a problem with the zeroout
code. And if there is, then the test would not be able to catch
that. And yes, bug in the zeroout code might affect zero range as
well even though we do not normally do zeroout on zero range except
block unaligned edges of the range.

Also ext4 does not need different testing for different page sizes.
That said I usually do ppc64 testing to test the operations with
granularity smaller than page size (sub page size zero out etc..).
With your proposal this goes away because we would actually test the
operation on larger extents - that's not helpful at all.

-Lukas

> 
> Cheers,
> 
> Dave.
> 

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 47+ messages in thread

end of thread, other threads:[~2014-02-28 12:38 UTC | newest]

Thread overview: 47+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-25 19:15 [PATCH 1/6] generic/290: Add test for fallocate zero range Lukas Czerner
2014-02-25 19:15 ` Lukas Czerner
2014-02-25 19:15 ` [PATCH 2/6] xfstests: Add fallocate zero range operation to fsstress Lukas Czerner
2014-02-25 19:15   ` Lukas Czerner
2014-02-25 20:15   ` Dave Chinner
2014-02-25 20:15     ` Dave Chinner
2014-02-25 19:15 ` [PATCH 3/6] xfstests: fsstress punch should always have FALLOC_FL_KEEP_SIZE set Lukas Czerner
2014-02-25 19:15   ` Lukas Czerner
2014-02-25 20:18   ` Dave Chinner
2014-02-26 13:04     ` Lukáš Czerner
2014-02-25 19:15 ` [PATCH 4/6] xfstests: Add fallocate zero range operation to fsx Lukas Czerner
2014-02-25 19:15   ` Lukas Czerner
2014-02-25 20:31   ` Dave Chinner
2014-02-25 19:15 ` [PATCH 5/6] xfstests: Define fallocate flags locally in fsx Lukas Czerner
2014-02-25 19:15   ` Lukas Czerner
2014-02-25 20:39   ` Dave Chinner
2014-02-25 20:39     ` Dave Chinner
2014-02-25 21:56     ` Christoph Hellwig
2014-02-25 21:56       ` Christoph Hellwig
2014-02-26  6:07       ` Dave Chinner
2014-02-26  6:07         ` Dave Chinner
2014-02-25 19:15 ` [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range Lukas Czerner
2014-02-25 19:15   ` Lukas Czerner
2014-02-25 20:53   ` Dave Chinner
2014-02-25 21:01     ` Lukáš Czerner
2014-02-25 21:01       ` Lukáš Czerner
2014-02-25 21:27       ` Lukáš Czerner
2014-02-25 21:27         ` Lukáš Czerner
2014-02-25 21:50       ` Dave Chinner
2014-02-25 21:50         ` Dave Chinner
2014-02-26 14:24         ` Lukáš Czerner
2014-02-26 22:01           ` Dave Chinner
2014-02-26 22:01             ` Dave Chinner
2014-02-27 12:03             ` Lukáš Czerner
2014-02-27 12:03               ` Lukáš Czerner
2014-02-27 19:35               ` Dave Chinner
2014-02-27 19:35                 ` Dave Chinner
2014-02-28 12:38                 ` Lukáš Czerner
2014-02-26 14:58         ` Lukáš Czerner
2014-02-26 14:58           ` Lukáš Czerner
2014-02-26 22:17           ` Dave Chinner
2014-02-26 22:17             ` Dave Chinner
2014-02-27 11:48             ` Lukáš Czerner
2014-02-27 11:48               ` Lukáš Czerner
2014-02-25 20:01 ` [PATCH 1/6] generic/290: Add " Dave Chinner
2014-02-25 20:01   ` Dave Chinner
2014-02-25 20:55   ` Dave Chinner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.