Skip to contentSkip to conversation

generic: test for seeing unseen fsync errors on newly open files

PATCH Superseded Jeff Layton 4 participants 10 messages Apr 27, 2018 – May 8, 2018

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Jeff Layton

author
#0 lore raw

PATCH 1/1 generic: test for seeing unseen fsync errors on newly open files

From: Jeff Layton <[email protected]>

This adds a regression test for the following kernel patch:

errseq: Always report a writeback error once

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

This used to work with older kernels that reported the error to only one fd, but with the errseq_t changes we lost the ability to see errors that occurred before the open. The above patch restores that behavior.

Signed-off-by Jeff Layton <[email protected]>

---

This patch currently fails on mainline kernels, but I'll be sending a pull request to Linus in the near future for the above patch.

 src/Makefile               |   2 +-
 src/fsync-open-after-err.c | 167 +++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999          |  95 ++++++++++++++++++++++++++
 tests/generic/999.out      |   3 +
 tests/generic/group        |   1 +
 5 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 src/fsync-open-after-err.c
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out
src/Makefile +1 -1
index 0d3feae1eeb2..3dc9b0da9c3a 100644
@@ -15,7 +15,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
 	holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd \
 	t_mmap_cow_race t_mmap_fallocate fsync-err t_mmap_write_ro \
 	t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
-	t_ofd_locks
+	t_ofd_locks fsync-open-after-err
 
 LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
src/fsync-open-after-err.c new file +167 -0

167 lines not on this page · raw message

tests/generic/999 new file +95 -0

95 lines not on this page · raw message

tests/generic/999.out new file +3 -0
new file mode 100644
index 000000000000..2e48492ff6d1
@@ -0,0 +1,3 @@
+QA output created by 999
+Format and mount
+Test passed!
tests/generic/group +1 -0
index ea8e51b35e79..48f491a5c32b 100644
@@ -486,3 +486,4 @@
 481 auto quick log metadata
 482 auto metadata replay
 483 auto quick log metadata
+999 auto quick
Signature · 1 line
2.14.3
#1 lore raw

Hi,

Quoted 4 lines · Jeff Layton

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

;)

src/fsync-open-after-err.c line 131

Quoted 8 lines
src/fsync-open-after-err.c new file +8 -0
+	ret = pwrite(fd1, buf, bufsize, 0);
+	if (ret < 0) {
+		printf("Second write on fd1 failed: %m\n");
+		return 1;
+	}
+
+	/* Ensure writeback occurs, but don't scrape the error */
+	sync();

It might be a good idea to also add a second version of this that additionally evicts inodes after the sync? I think that should be simulatable with "echo 2 > /proc/sys/vm/drop_caches" or such? That'd obviously fail for now...

Thanks for the test,

Andres Freund

author
#2 lore raw
Quoted 11 lines · Andres Freund

Hi,

Quoted 4 lines · Jeff Layton

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

;)

Quoted 14 lines

src/fsync-open-after-err.c line 131

Quoted 8 lines
src/fsync-open-after-err.c new file +8 -0
+	ret = pwrite(fd1, buf, bufsize, 0);
+	if (ret < 0) {
+		printf("Second write on fd1 failed: %m\n");
+		return 1;
+	}
+
+	/* Ensure writeback occurs, but don't scrape the error */
+	sync();

It might be a good idea to also add a second version of this that additionally evicts inodes after the sync? I think that should be simulatable with "echo 2 > /proc/sys/vm/drop_caches" or such? That'd obviously fail for now...

We could. In this test, I'm explicitly holding the fd1 open it while we open fd2 after the sync to ensure that it sticks around in the cache.

TBH: I'm a little leery of keeping inodes with errors on them in the cache as it seems like an open-ended commitment. We may not be able to keep them forever and may eventually have to eliminate some in order to make forward progress.

If we do go that route, we may very well need drop_caches to purge those inodes as well, to give admins a way to clear such inodes globally without having to explicitly open+fsync them.

In any case, I'd prefer to wait until we at least have a proposed patch for that piece before we go adding tests for it.

Quoted 7 lines

Thanks for the test,

Andres Freund
--
To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
In reply to Jeff Layton

src/fsync-open-after-err.c

Quoted 50 lines · Jeff Layton

From: Jeff Layton <[email protected]>

This adds a regression test for the following kernel patch:

errseq: Always report a writeback error once

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

This used to work with older kernels that reported the error to only one fd, but with the errseq_t changes we lost the ability to see errors that occurred before the open. The above patch restores that behavior.

Signed-off-by Jeff Layton <[email protected]>

---

This patch currently fails on mainline kernels, but I'll be sending a pull request to Linus in the near future for the above patch.

 src/Makefile               |   2 +-
 src/fsync-open-after-err.c | 167 +++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999          |  95 ++++++++++++++++++++++++++
 tests/generic/999.out      |   3 +
 tests/generic/group        |   1 +
 5 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 src/fsync-open-after-err.c
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out
src/Makefile +1 -1
index 0d3feae1eeb2..3dc9b0da9c3a 100644
@@ -15,7 +15,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
        holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd \
        t_mmap_cow_race t_mmap_fallocate fsync-err t_mmap_write_ro \
        t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
-       t_ofd_locks
+       t_ofd_locks fsync-open-after-err
 
 LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
        preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
src/fsync-open-after-err.c new file +0 -0
new file mode 100644
index 000000000000..3dcf936eb94a

Jeff,

It is an anti pattern for xfstests to add a single purpose C program for things that could be implemented otherwise.

AFAICT, This program doesn't do anything that you cannot do with existing bash helpers and existing programs.

So either add a flag to fsync-err to enable the new test or use xfs_io fsync (make sure it really returns the error) and keep file open with bash tricks.

Thanks,
Amir.

author
#4 lore raw
Quoted 60 lines · Amir Goldstein

src/fsync-open-after-err.c

Quoted 50 lines · Jeff Layton

From: Jeff Layton <[email protected]>

This adds a regression test for the following kernel patch:

errseq: Always report a writeback error once

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

This used to work with older kernels that reported the error to only one fd, but with the errseq_t changes we lost the ability to see errors that occurred before the open. The above patch restores that behavior.

Signed-off-by Jeff Layton <[email protected]>

---

This patch currently fails on mainline kernels, but I'll be sending a pull request to Linus in the near future for the above patch.

 src/Makefile               |   2 +-
 src/fsync-open-after-err.c | 167 +++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999          |  95 ++++++++++++++++++++++++++
 tests/generic/999.out      |   3 +
 tests/generic/group        |   1 +
 5 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 src/fsync-open-after-err.c
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out
src/Makefile +1 -1
index 0d3feae1eeb2..3dc9b0da9c3a 100644
@@ -15,7 +15,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
        holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd \
        t_mmap_cow_race t_mmap_fallocate fsync-err t_mmap_write_ro \
        t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
-       t_ofd_locks
+       t_ofd_locks fsync-open-after-err
 
 LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
        preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
src/fsync-open-after-err.c new file +0 -0
new file mode 100644
index 000000000000..3dcf936eb94a

Jeff,

It is an anti pattern for xfstests to add a single purpose C program for things that could be implemented otherwise.

AFAICT, This program doesn't do anything that you cannot do with existing bash helpers and existing programs.

Quoted 6 lines

So either add a flag to fsync-err to enable the new test or use xfs_io fsync (make sure it really returns the error) and keep file open with bash tricks.

Thanks,
Amir.

Ok. Let's drop this patch for now and I'll see if I can code it up with scripts somehow.

Thanks,

Signature · 1 line
Jeff Layton <[email protected]>
author
#5 lore raw

PATCH v2 1/1 generic: test for seeing unseen fsync errors on newly open files

In reply to Jeff Layton

From: Jeff Layton <[email protected]>

This adds a regression test for the following kernel patch:

errseq: Always report a writeback error once

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

This used to work with older kernels that reported the error to only one fd, but with the errseq_t changes we lost the ability to see errors that occurred before the open. The above patch restores that behavior.

Signed-off-by Jeff Layton <[email protected]>
---
 tests/generic/999     | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  5 +++
 tests/generic/group   |  1 +
 3 files changed, 105 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out
tests/generic/999 new file +99 -0
new file mode 100755
index 000000000000..081409f16100
@@ -0,0 +1,99 @@
+#! /bin/bash
+# FS QA Test No. XXX
+#
+# Open a file several times, write to it, fsync on all fds and make sure that
+# they all return 0. Change the device to start throwing errors. Write again
+# on all fds and fsync on all fds. Ensure that we get errors on all of them.
+# Then fsync on all one last time and verify that all return 0.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018, Jeff Layton <[email protected]>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.* $testdir
+	_dmerror_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmerror
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
+
+_require_dm_target error
+_require_test_program fsync-open-after-err
+_require_test_program dmerror
+
+rm -f $seqres.full
+
+echo "Format and mount"
+_scratch_mkfs > $seqres.full 2>&1
+_dmerror_init
+_dmerror_mount
+
+datalen=65536
+_require_fs_space $SCRATCH_MNT $datalen
+
+# use fd 5 to hold file open
+testfile=$SCRATCH_MNT/fsync-open-after-err
+exec 5>$testfile
+
+# write some data to file and fsync it out
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c fsync $testfile
+
+# flip device to non-working mode
+_dmerror_load_error_table
+
+# rewrite the data, call sync to ensure it's written back w/o scraping error
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c sync $testfile
+
+# heal the device error
+_dmerror_load_working_table
+
+# open again and call fsync
+echo "The following fsync should fail with EIO:"
+$XFS_IO_PROG -c fsync $testfile
+echo "done"
+
+# close file
+exec 5>&-
+
+# success, all done
+_dmerror_unmount
+_dmerror_cleanup
+
+# fs may be corrupt after this -- attempt to repair it
+_repair_scratch_fs >> $seqres.full
+status=0
+exit
tests/generic/999.out new file +5 -0
new file mode 100644
index 000000000000..38d2d7f6495f
@@ -0,0 +1,5 @@
+QA output created by 999
+Format and mount
+The following fsync should fail with EIO:
+fsync: Input/output error
+done
tests/generic/group +1 -0
index ea8e51b35e79..48f491a5c32b 100644
@@ -486,3 +486,4 @@
 481 auto quick log metadata
 482 auto metadata replay
 483 auto quick log metadata
+999 auto quick
Signature · 1 line
2.14.3
Reviewed-by on v2 1/1
#6 lore raw
Quoted 11 lines · Jeff Layton

From: Jeff Layton <[email protected]>

This adds a regression test for the following kernel patch:

errseq: Always report a writeback error once

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

Quoted 6 lines

This used to work with older kernels that reported the error to only one fd, but with the errseq_t changes we lost the ability to see errors that occurred before the open. The above patch restores that behavior.

Signed-off-by Jeff Layton <[email protected]>

Looks good. Minus a few nits, you can add:

Reviewed-by Amir Goldstein <[email protected]>
Quoted 53 lines
---
 tests/generic/999     | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  5 +++
 tests/generic/group   |  1 +
 3 files changed, 105 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out
tests/generic/999 new file +39 -0
new file mode 100755
index 000000000000..081409f16100
@@ -0,0 +1,99 @@
+#! /bin/bash
+# FS QA Test No. XXX
+#
+# Open a file several times, write to it, fsync on all fds and make sure that
+# they all return 0. Change the device to start throwing errors. Write again
+# on all fds and fsync on all fds. Ensure that we get errors on all of them.
+# Then fsync on all one last time and verify that all return 0.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018, Jeff Layton <[email protected]>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+       cd /
+       rm -rf $tmp.* $testdir
+       _dmerror_cleanup

tests/generic/999 line 49

Quoted 10 lines
tests/generic/999 new file +10 -0
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmerror
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch

Better _require_scratch_nocheck if you expect fs errors.

tests/generic/999 line 56

Quoted 7 lines
tests/generic/999 new file +7 -0
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
+
+_require_dm_target error
+_require_test_program fsync-open-after-err
+_require_test_program dmerror

Does it really require those programs?
I think you managed without them..

Quoted 31 lines
tests/generic/999 new file +31 -0
+
+rm -f $seqres.full
+
+echo "Format and mount"
+_scratch_mkfs > $seqres.full 2>&1
+_dmerror_init
+_dmerror_mount
+
+datalen=65536
+_require_fs_space $SCRATCH_MNT $datalen
+
+# use fd 5 to hold file open
+testfile=$SCRATCH_MNT/fsync-open-after-err
+exec 5>$testfile
+
+# write some data to file and fsync it out
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c fsync $testfile
+
+# flip device to non-working mode
+_dmerror_load_error_table
+
+# rewrite the data, call sync to ensure it's written back w/o scraping error
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c sync $testfile
+
+# heal the device error
+_dmerror_load_working_table
+
+# open again and call fsync
+echo "The following fsync should fail with EIO:"
+$XFS_IO_PROG -c fsync $testfile
+echo "done"

tests/generic/999 line 97

Quoted 10 lines
tests/generic/999 new file +10 -0
+
+# close file
+exec 5>&-
+
+# success, all done
+_dmerror_unmount
+_dmerror_cleanup
+
+# fs may be corrupt after this -- attempt to repair it
+_repair_scratch_fs >> $seqres.full

Do we need that? Doesn't seem relevant to the test??

Thanks,
Amir.

author
#7 lore raw

PATCH v3 1/1 generic: test for seeing unseen fsync errors on newly open files

In reply to Jeff Layton

From: Jeff Layton <[email protected]>

This adds a regression test for the following kernel patch:

errseq: Always report a writeback error once

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

This used to work with older kernels that reported the error to only one fd, but with the errseq_t changes we lost the ability to see errors that occurred before the open. The above patch restores that behavior.

Signed-off-by Jeff Layton <[email protected]>
Reviewed-by Amir Goldstein <[email protected]>
---
 tests/generic/999     | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  5 +++
 tests/generic/group   |  1 +
 3 files changed, 101 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out
tests/generic/999 new file +95 -0
new file mode 100755
index 000000000000..b816fd3bb423
@@ -0,0 +1,95 @@
+#! /bin/bash
+# FS QA Test No. XXX
+#
+# Open a file several times, write to it, fsync on all fds and make sure that
+# they all return 0. Change the device to start throwing errors. Write again
+# on all fds and fsync on all fds. Ensure that we get errors on all of them.
+# Then fsync on all one last time and verify that all return 0.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018, Jeff Layton <[email protected]>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.* $testdir
+	_dmerror_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmerror
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch_nocheck
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
+
+_require_dm_target error
+
+rm -f $seqres.full
+
+echo "Format and mount"
+_scratch_mkfs > $seqres.full 2>&1
+_dmerror_init
+_dmerror_mount
+
+datalen=65536
+_require_fs_space $SCRATCH_MNT $datalen
+
+# use fd 5 to hold file open
+testfile=$SCRATCH_MNT/fsync-open-after-err
+exec 5>$testfile
+
+# write some data to file and fsync it out
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c fsync $testfile
+
+# flip device to non-working mode
+_dmerror_load_error_table
+
+# rewrite the data, call sync to ensure it's written back w/o scraping error
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c sync $testfile
+
+# heal the device error
+_dmerror_load_working_table
+
+# open again and call fsync
+echo "The following fsync should fail with EIO:"
+$XFS_IO_PROG -c fsync $testfile
+echo "done"
+
+# close file
+exec 5>&-
+
+# success, all done
+_dmerror_unmount
+_dmerror_cleanup
+
+status=0
+exit
tests/generic/999.out new file +5 -0
new file mode 100644
index 000000000000..38d2d7f6495f
@@ -0,0 +1,5 @@
+QA output created by 999
+Format and mount
+The following fsync should fail with EIO:
+fsync: Input/output error
+done
tests/generic/group +1 -0
index ea8e51b35e79..48f491a5c32b 100644
@@ -486,3 +486,4 @@
 481 auto quick log metadata
 482 auto metadata replay
 483 auto quick log metadata
+999 auto quick
Signature · 1 line
2.14.3
#8 lore raw
Quoted 32 lines · Jeff Layton

From: Jeff Layton <[email protected]>

This adds a regression test for the following kernel patch:

errseq: Always report a writeback error once

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

This used to work with older kernels that reported the error to only one fd, but with the errseq_t changes we lost the ability to see errors that occurred before the open. The above patch restores that behavior.

Signed-off-by Jeff Layton <[email protected]>
Reviewed-by Amir Goldstein <[email protected]>
---
 tests/generic/999     | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  5 +++
 tests/generic/group   |  1 +
 3 files changed, 101 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out
tests/generic/999 new file +0 -0
new file mode 100755
index 000000000000..b816fd3bb423

tests/generic/999 line 2

Quoted 2 lines
tests/generic/999 new file +2 -0
@@ -0,0 +1,95 @@
+#! /bin/bash
+# FS QA Test No. XXX

^^^ Need a test number here :)

Quoted 73 lines
tests/generic/999 new file +73 -0
+#
+# Open a file several times, write to it, fsync on all fds and make sure that
+# they all return 0. Change the device to start throwing errors. Write again
+# on all fds and fsync on all fds. Ensure that we get errors on all of them.
+# Then fsync on all one last time and verify that all return 0.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018, Jeff Layton <[email protected]>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.* $testdir
+	_dmerror_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmerror
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch_nocheck
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
+
+_require_dm_target error
+
+rm -f $seqres.full
+
+echo "Format and mount"
+_scratch_mkfs > $seqres.full 2>&1
+_dmerror_init
+_dmerror_mount
+
+datalen=65536
+_require_fs_space $SCRATCH_MNT $datalen
+
+# use fd 5 to hold file open
+testfile=$SCRATCH_MNT/fsync-open-after-err
+exec 5>$testfile
+
+# write some data to file and fsync it out
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c fsync $testfile
+
+# flip device to non-working mode
+_dmerror_load_error_table
+

tests/generic/999 line 85

Quoted 10 lines
tests/generic/999 new file +10 -0
+# rewrite the data, call sync to ensure it's written back w/o scraping error
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c sync $testfile
+
+# heal the device error
+_dmerror_load_working_table
+
+# open again and call fsync
+echo "The following fsync should fail with EIO:"
+$XFS_IO_PROG -c fsync $testfile
+echo "done"

I built latest Linus tree, which should contain the mentioned fix, and I saw different results on xfs, ext4 and btrfs.

XFS fails the test as:
-fsync: Input/output error +/mnt/scratch/fsync-open-after-err: Input/output error

While btrfs fails as:
-fsync: Input/output error +/mnt/scratch/fsync-open-after-err: Read-only file system

And both XFS and btrfs behave in the same way no matter I have the fix applied or not (tested on v4.16 kernel and v4.17-rc3+ kernel).

Only ext4 passes the test with v4.17-rc3+ kernel, and fails on v4.16 kernel as expected:
-fsync: Input/output error

Could you please take a look?

Thanks,
Eryu

Quoted 37 lines
tests/generic/999 new file +10 -0
+
+# close file
+exec 5>&-
+
+# success, all done
+_dmerror_unmount
+_dmerror_cleanup
+
+status=0
+exit
tests/generic/999.out new file +5 -0
new file mode 100644
index 000000000000..38d2d7f6495f
@@ -0,0 +1,5 @@
+QA output created by 999
+Format and mount
+The following fsync should fail with EIO:
+fsync: Input/output error
+done
tests/generic/group +1 -0
index ea8e51b35e79..48f491a5c32b 100644
@@ -486,3 +486,4 @@
 481 auto quick log metadata
 482 auto metadata replay
 483 auto quick log metadata
+999 auto quick
Signature · 6 lines
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#9 lore raw
Quoted 125 lines · Eryu Guan
Quoted 32 lines · Jeff Layton

From: Jeff Layton <[email protected]>

This adds a regression test for the following kernel patch:

errseq: Always report a writeback error once

This is motivated by some rather odd behavior done by the PostgreSQL project. The main database writers will offload the fsync calls to a separate process, which can open files after a writeback error has already occurred.

This used to work with older kernels that reported the error to only one fd, but with the errseq_t changes we lost the ability to see errors that occurred before the open. The above patch restores that behavior.

Signed-off-by Jeff Layton <[email protected]>
Reviewed-by Amir Goldstein <[email protected]>
---
 tests/generic/999     | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  5 +++
 tests/generic/group   |  1 +
 3 files changed, 101 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out
tests/generic/999 new file +0 -0
new file mode 100755
index 000000000000..b816fd3bb423

tests/generic/999 line 2

Quoted 2 lines
tests/generic/999 new file +2 -0
@@ -0,0 +1,95 @@
+#! /bin/bash
+# FS QA Test No. XXX

^^^ Need a test number here :)

Quoted 73 lines
tests/generic/999 new file +73 -0
+#
+# Open a file several times, write to it, fsync on all fds and make sure that
+# they all return 0. Change the device to start throwing errors. Write again
+# on all fds and fsync on all fds. Ensure that we get errors on all of them.
+# Then fsync on all one last time and verify that all return 0.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018, Jeff Layton <[email protected]>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.* $testdir
+	_dmerror_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmerror
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch_nocheck
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
+
+_require_dm_target error
+
+rm -f $seqres.full
+
+echo "Format and mount"
+_scratch_mkfs > $seqres.full 2>&1
+_dmerror_init
+_dmerror_mount
+
+datalen=65536
+_require_fs_space $SCRATCH_MNT $datalen
+
+# use fd 5 to hold file open
+testfile=$SCRATCH_MNT/fsync-open-after-err
+exec 5>$testfile
+
+# write some data to file and fsync it out
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c fsync $testfile
+
+# flip device to non-working mode
+_dmerror_load_error_table
+

tests/generic/999 line 85

Quoted 10 lines
tests/generic/999 new file +10 -0
+# rewrite the data, call sync to ensure it's written back w/o scraping error
+$XFS_IO_PROG -c "pwrite -q 0 $datalen" -c sync $testfile
+
+# heal the device error
+_dmerror_load_working_table
+
+# open again and call fsync
+echo "The following fsync should fail with EIO:"
+$XFS_IO_PROG -c fsync $testfile
+echo "done"

I built latest Linus tree, which should contain the mentioned fix, and I saw different results on xfs, ext4 and btrfs.

Quoted 3 lines

XFS fails the test as:
-fsync: Input/output error +/mnt/scratch/fsync-open-after-err: Input/output error

I think I see the problem. I was testing this with a logdev. I think we'll need a _require_logdev directive in the test. I'll send a v4 in a bit with that and some other corrections.

Quoted 6 lines

While btrfs fails as:
-fsync: Input/output error +/mnt/scratch/fsync-open-after-err: Read-only file system

I think btrfs will require a specialized testcase (like we did for some of the earlier tests in this area). I'll send a separate patch for that one.

Quoted 51 lines

And both XFS and btrfs behave in the same way no matter I have the fix applied or not (tested on v4.16 kernel and v4.17-rc3+ kernel).

Only ext4 passes the test with v4.17-rc3+ kernel, and fails on v4.16 kernel as expected:
-fsync: Input/output error

Could you please take a look?

Thanks,
Eryu

Quoted 37 lines
tests/generic/999 new file +10 -0
+
+# close file
+exec 5>&-
+
+# success, all done
+_dmerror_unmount
+_dmerror_cleanup
+
+status=0
+exit
tests/generic/999.out new file +5 -0
new file mode 100644
index 000000000000..38d2d7f6495f
@@ -0,0 +1,5 @@
+QA output created by 999
+Format and mount
+The following fsync should fail with EIO:
+fsync: Input/output error
+done
tests/generic/group +1 -0
index ea8e51b35e79..48f491a5c32b 100644
@@ -486,3 +486,4 @@
 481 auto quick log metadata
 482 auto metadata replay
 483 auto quick log metadata
+999 auto quick
Signature · 6 lines
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 1 line
Jeff Layton <[email protected]>