| Message ID | 20161117192051.GE74211@jaegeuk (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, Nov 18, 2016 at 05:57:45PM -0800, Jaegeuk Kim wrote: > This patch adds tests/generic/391 to test fsync and fdatasync with power-cuts. > > The rule to check is: > 1) fsync should guarantee all the inode metadata after power-cut, > 2) fdatasync should guarantee i_size and i_blocks at least after power-cut. > > Note that, in XFS, it allocates more blocks when doing writes, so it should > close the file before fsync in order to get actual i_blocks after power-cut. > > Suggested-by: Chao Yu <yuchao0@huawei.com> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > --- > tests/generic/391 | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/391.out | 11 ++++ > tests/generic/group | 1 + > 3 files changed, 161 insertions(+) > create mode 100755 tests/generic/391 > create mode 100644 tests/generic/391.out > > diff --git a/tests/generic/391 b/tests/generic/391 > new file mode 100755 > index 0000000..121e01d > --- /dev/null > +++ b/tests/generic/391 > @@ -0,0 +1,149 @@ > +#! /bin/bash > +# FS QA Test 391 > +# > +# Test inode's metadata after fsync or fdatasync calls. > +# In the case of fsync, filesystem should recover all the inode metadata, while > +# recovering i_blocks and i_size at least for fdatasync. > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2016 Jaegeuk Kim. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +#----------------------------------------------------------------------- > +# > + > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > +. ./common/punch > + > +# real QA test starts here > +_supported_fs generic > +_supported_os Linux > + > +rm -f $seqres.full > +_require_scratch > +_require_scratch_shutdown > +_require_xfs_io_command "fpunch" > + > +_scratch_mkfs >/dev/null 2>&1 > +_scratch_mount > + > +testfile=$SCRATCH_MNT/testfile > + > +f_stat() > +{ > + $XFS_IO_PROG -r -c "stat -v" $testfile >$tmp.$1 > +} > + > +f_sync() > +{ > + $XFS_IO_PROG -c "$1" $testfile >/dev/null > +} This is factoring overkill and... > + > +# check inode metadata after shutdown > +check_inode_metadata() > +{ > + # prepare syncing metadata > + f_sync $1 > + > + # get inode metadata #1 > + f_stat before > + > + # shutdown > + src/godown $SCRATCH_MNT >> $seqres.full > + _scratch_cycle_mount > + > + # get inode metadata #2 > + f_stat after > + > + # compare #1 and #2 > + diff $tmp.before $tmp.after >$tmp.diff > + > + # fdatasync only cares about i_size and i_blocks > + if [ "$1" = "fdatasync" ]; then > + cat $tmp.diff | grep stat.size > + cat $tmp.diff | grep stat.blocks > + else > + cat $tmp.diff > + fi .... forces you to do things like this. We have the stat binary for getting specific fields from files. We use it all over the place for exactly this sort of thing. See, for example, _defrag(), which also does this "stat before/stat after/compare results" for specific metadata. i.e.: $ stat -c "blocks: %b size: %s" Makefile blocks: 8 size: 3069 $ check_inode_metadata() { sync_mode=$1 stat_opts="" if [ $sync_mode = "fdatasync" ]; then stat_opts='-c "blocks: %b size: %s"' fi $XFS_IO_PROG -c "$sync_mode" $testfile | _filter_xfs_io before=`stat $stat_opts $testfile` ... after=`stat $stat_opts $testfile` if [ $before != $after ]; then echo "Before: $before" echo "After: $after" status=1; # this is a failure! fi > + cat $tmp.before >> $seqres.full > + cat $tmp.after >> $seqres.full > + > + rm $testfile > +} > + > +# append XX KB with f{data}sync, followed by power-cut > +test_i_size() > +{ > + $XFS_IO_PROG -f -c "pwrite 0 4M" \ > + -c "fsync" \ > + -c "pwrite 4M $2" \ > + $testfile >/dev/null Make these all truncate to the final size first like this: $XFS_IO_PROG -f -c "truncate 4M" \ -c "pwrite 0 4M" \ -c "fsync" \ -c "pwrite 4M $2" \ $testfile | _filter_xfs_io check_inode_metadata $1 And that will make all the problems with XFS preallocation go away, because the initial truncate to set the file size will turn off the preallocation. > + $XFS_IO_PROG -f -c "pwrite 0 4M" \ > + -c "fsync" \ > + $testfile >/dev/null > + sleep 1 > + touch $testfile > + check_inode_metadata $1 > +} > + > +# punch XX KB with f{data}sync, followed by power-cut > +test_punch() > +{ > + echo "==== fpunch $2 test with $1 ====" | tee -a $seqres.full > + $XFS_IO_PROG -f -c "pwrite 0 4202496" \ > + -c "fsync" \ > + -c "fpunch 4194304 $2"\ > + $testfile >/dev/null > + check_inode_metadata $1 > +} > + > +for i in fsync fdatasync > +do > + test_i_size $i 1024 > + test_i_size $i 4096 > + test_i_time $i > + test_punch $i 1024 > + test_punch $i 4096 > +done > + > +rm -f $tmp.* No need for this here. > + > +# success, all done > +status=0 Set that at the start before we run the test so that if we detect a failure we can set status=1 for exit to handle appropriately. Cheers, Dave.
On Mon, Nov 21, 2016 at 08:19:58AM +1100, Dave Chinner wrote: > On Fri, Nov 18, 2016 at 05:57:45PM -0800, Jaegeuk Kim wrote: > > This patch adds tests/generic/391 to test fsync and fdatasync with power-cuts. > > > > The rule to check is: > > 1) fsync should guarantee all the inode metadata after power-cut, > > 2) fdatasync should guarantee i_size and i_blocks at least after power-cut. > > > > Note that, in XFS, it allocates more blocks when doing writes, so it should > > close the file before fsync in order to get actual i_blocks after power-cut. > > > > Suggested-by: Chao Yu <yuchao0@huawei.com> > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > --- > > tests/generic/391 | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/391.out | 11 ++++ > > tests/generic/group | 1 + > > 3 files changed, 161 insertions(+) > > create mode 100755 tests/generic/391 > > create mode 100644 tests/generic/391.out > > > > diff --git a/tests/generic/391 b/tests/generic/391 > > new file mode 100755 > > index 0000000..121e01d > > --- /dev/null > > +++ b/tests/generic/391 > > @@ -0,0 +1,149 @@ > > +#! /bin/bash > > +# FS QA Test 391 > > +# > > +# Test inode's metadata after fsync or fdatasync calls. > > +# In the case of fsync, filesystem should recover all the inode metadata, while > > +# recovering i_blocks and i_size at least for fdatasync. > > +# > > +#----------------------------------------------------------------------- > > +# Copyright (c) 2016 Jaegeuk Kim. All Rights Reserved. > > +# > > +# This program is free software; you can redistribute it and/or > > +# modify it under the terms of the GNU General Public License as > > +# published by the Free Software Foundation. > > +# > > +# This program is distributed in the hope that it would be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program; if not, write the Free Software Foundation, > > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > > +#----------------------------------------------------------------------- > > +# > > + > > +seq=`basename $0` > > +seqres=$RESULT_DIR/$seq > > +echo "QA output created by $seq" > > + > > +here=`pwd` > > +tmp=/tmp/$$ > > +status=1 # failure is the default! > > +trap "_cleanup; exit \$status" 0 1 2 3 15 > > + > > +_cleanup() > > +{ > > + cd / > > + rm -f $tmp.* > > +} > > + > > +# get standard environment, filters and checks > > +. ./common/rc > > +. ./common/filter > > +. ./common/punch > > + > > +# real QA test starts here > > +_supported_fs generic > > +_supported_os Linux > > + > > +rm -f $seqres.full > > +_require_scratch > > +_require_scratch_shutdown > > +_require_xfs_io_command "fpunch" > > + > > +_scratch_mkfs >/dev/null 2>&1 > > +_scratch_mount > > + > > +testfile=$SCRATCH_MNT/testfile > > + > > +f_stat() > > +{ > > + $XFS_IO_PROG -r -c "stat -v" $testfile >$tmp.$1 > > +} > > + > > +f_sync() > > +{ > > + $XFS_IO_PROG -c "$1" $testfile >/dev/null > > +} > > This is factoring overkill and... > > > + > > +# check inode metadata after shutdown > > +check_inode_metadata() > > +{ > > + # prepare syncing metadata > > + f_sync $1 > > + > > + # get inode metadata #1 > > + f_stat before > > + > > + # shutdown > > + src/godown $SCRATCH_MNT >> $seqres.full > > + _scratch_cycle_mount > > + > > + # get inode metadata #2 > > + f_stat after > > + > > + # compare #1 and #2 > > + diff $tmp.before $tmp.after >$tmp.diff > > + > > + # fdatasync only cares about i_size and i_blocks > > + if [ "$1" = "fdatasync" ]; then > > + cat $tmp.diff | grep stat.size > > + cat $tmp.diff | grep stat.blocks > > + else > > + cat $tmp.diff > > + fi > > .... forces you to do things like this. > > We have the stat binary for getting specific fields from files. We > use it all over the place for exactly this sort of thing. See, for > example, _defrag(), which also does this "stat before/stat > after/compare results" for specific metadata. > > i.e.: > > $ stat -c "blocks: %b size: %s" Makefile > blocks: 8 size: 3069 > $ > > check_inode_metadata() > { > sync_mode=$1 > > stat_opts="" > if [ $sync_mode = "fdatasync" ]; then > stat_opts='-c "blocks: %b size: %s"' > fi > > $XFS_IO_PROG -c "$sync_mode" $testfile | _filter_xfs_io > before=`stat $stat_opts $testfile` > ... > after=`stat $stat_opts $testfile` > > if [ $before != $after ]; then > echo "Before: $before" > echo "After: $after" > status=1; # this is a failure! > fi > > > + cat $tmp.before >> $seqres.full > > + cat $tmp.after >> $seqres.full > > + > > + rm $testfile > > +} > > + > > +# append XX KB with f{data}sync, followed by power-cut > > +test_i_size() > > +{ > > + $XFS_IO_PROG -f -c "pwrite 0 4M" \ > > + -c "fsync" \ > > + -c "pwrite 4M $2" \ > > + $testfile >/dev/null > > Make these all truncate to the final size first like this: > > $XFS_IO_PROG -f -c "truncate 4M" \ > -c "pwrite 0 4M" \ > -c "fsync" \ > -c "pwrite 4M $2" \ > $testfile | _filter_xfs_io > > check_inode_metadata $1 > > And that will make all the problems with XFS preallocation go away, > because the initial truncate to set the file size will turn off the > preallocation. > > > + $XFS_IO_PROG -f -c "pwrite 0 4M" \ > > + -c "fsync" \ > > + $testfile >/dev/null > > + sleep 1 > > + touch $testfile > > + check_inode_metadata $1 > > +} > > + > > +# punch XX KB with f{data}sync, followed by power-cut > > +test_punch() > > +{ > > + echo "==== fpunch $2 test with $1 ====" | tee -a $seqres.full > > + $XFS_IO_PROG -f -c "pwrite 0 4202496" \ > > + -c "fsync" \ > > + -c "fpunch 4194304 $2"\ > > + $testfile >/dev/null > > + check_inode_metadata $1 > > +} > > + > > +for i in fsync fdatasync > > +do > > + test_i_size $i 1024 > > + test_i_size $i 4096 > > + test_i_time $i > > + test_punch $i 1024 > > + test_punch $i 4096 > > +done > > + > > +rm -f $tmp.* > > No need for this here. > > > + > > +# success, all done > > +status=0 > > Set that at the start before we run the test so that if we detect a > failure we can set status=1 for exit to handle appropriately. Thank you so much for the review. Agreed to all, and let me send v5. Thanks, > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/generic/391 b/tests/generic/391 new file mode 100644 index 0000000..2b95151 --- /dev/null +++ b/tests/generic/391 @@ -0,0 +1,136 @@ +#! /bin/bash +# FS QA Test 391 +# +# Test inode's metadata after fsync or fdatasync calls. +# In the case of fsync, filesystem should recover all the inode metadata, while +# recovering i_blocks and i_size at least for fdatasync. +# +#----------------------------------------------------------------------- +# Copyright (c) 2016 Jaegeuk Kim. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter +. ./common/punch + +# real QA test starts here +_supported_fs generic +_supported_os Linux + +rm -f $seqres.full +_require_scratch +_require_scratch_shutdown +_require_xfs_io_command "fpunch" + +_scratch_mkfs >/dev/null 2>&1 +_scratch_mount + +testfile=$SCRATCH_MNT/testfile + +# check inode metadata after shutdown +check_inode_metadata() +{ + $XFS_IO_PROG -r -c "stat -v" $1 >$tmp.before + src/godown $SCRATCH_MNT >> $seqres.full + _scratch_cycle_mount + $XFS_IO_PROG -r -c "stat -v" $1 >$tmp.after + + if [ $FSTYP = xfs ]; then + sed -i '/stat.blocks/d' $tmp.before + sed -i '/stat.blocks/d' $tmp.after + fi + diff $tmp.before $tmp.after >$tmp.diff + + if [ "$2" = "fdatasync" ]; then + cat $tmp.diff | grep stat.size + cat $tmp.diff | grep stat.blocks + else + cat $tmp.diff $dont_care + fi + cat $tmp.before >> $seqres.full + cat $tmp.after >> $seqres.full +} + +# append XX KB with f{data}sync, followed by power-cut +test_i_size() +{ + echo "==== i_size $2 test with $1 ====" | tee -a $seqres.full + $XFS_IO_PROG -f -c "pwrite 0 4M" \ + -c "fsync" \ + -c "pwrite 4M $2" \ + -c "$1" \ + $testfile >/dev/null + check_inode_metadata $testfile $1 +} + +# update times with f{data}sync, followed by power-cut +test_i_time() +{ + echo "==== i_time test with $1 ====" | tee -a $seqres.full + $XFS_IO_PROG -f -c "pwrite 0 4M" \ + -c "fsync" \ + $testfile >/dev/null + sleep 1 + touch $testfile + $XFS_IO_PROG -c "$1" \ + $testfile >/dev/null + check_inode_metadata $testfile $1 +} + +# punch XX KB with f{data}sync, followed by power-cut +test_punch() +{ + echo "==== fpunch $2 test with $1 ====" | tee -a $seqres.full + $XFS_IO_PROG -f -c "pwrite 0 4202496" \ + -c "fsync" \ + -c "fpunch 4194304 $2"\ + -c "$1" \ + $testfile >/dev/null + + check_inode_metadata $testfile $1 +} + +for i in fsync fdatasync +do + test_i_size $i 1024 + test_i_size $i 4096 + test_i_time $i + test_punch $i 1024 + test_punch $i 4096 +done + +rm -f $tmp.* + +# success, all done +status=0 +exit diff --git a/tests/generic/391.out b/tests/generic/391.out new file mode 100644 index 0000000..7c66776 --- /dev/null +++ b/tests/generic/391.out @@ -0,0 +1,11 @@ +QA output created by 391 +==== i_size 1024 test with fsync ==== +==== i_size 4096 test with fsync ==== +==== i_time test with fsync ==== +==== fpunch 1024 test with fsync ==== +==== fpunch 4096 test with fsync ==== +==== i_size 1024 test with fdatasync ==== +==== i_size 4096 test with fdatasync ==== +==== i_time test with fdatasync ==== +==== fpunch 1024 test with fdatasync ==== +==== fpunch 4096 test with fdatasync ==== diff --git a/tests/generic/group b/tests/generic/group index 08007d7..9de3415 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -392,3 +392,4 @@ 387 auto clone 388 auto log metadata 389 auto quick acl +391 auto quick metadata
This patch adds tests/generic/391 to test fsync and fdatasync with power-cuts. The rule to check is: 1) fsync should guarantee all the inode metadata after power-cut, 2) fdatasync should guarantee i_size and i_blocks at least after power-cut. Suggested-by: Chao Yu <yuchao0@huawei.com> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- tests/generic/391 | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/391.out | 11 ++++ tests/generic/group | 1 + 3 files changed, 148 insertions(+) create mode 100644 tests/generic/391 create mode 100644 tests/generic/391.out