[PATCH] generic: busy loop of dd and rm test

[PATCH] generic: busy loop of dd and rm test

[Wang Yanfeng]

generic/326 is a case about testing whether writing failed on
NO_SPACE in a busy loop of write and delete when disk almost full.
It is a long-term problem since very beginning in btrfs, and has been
fixed by patchset titled "btrfs: Fix no_space on dd and rm loop" from
zhaolei@cn.fujitsu.com.

Signed-off-by: Wang Yanfeng <wangyf-fnst@cn.fujitsu.com>
---
 tests/generic/326     | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/326.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 76 insertions(+)
 create mode 100755 tests/generic/326
 create mode 100644 tests/generic/326.out

diff --git a/tests/generic/326 b/tests/generic/326
new file mode 100755
index 0000000..1ab1ac3
--- /dev/null
+++ b/tests/generic/326
@@ -0,0 +1,73 @@
+#! /bin/bash
+# FS QA Test No. 326
+#
+# TEST busy loop of write and delete in a filesystem.
+# Sometimes writes will failed on NO_SPACE when disk almost full
+# in btrfs. It is long-term problem since very beginning for btrfs
+#
+# This issue was fixed by the following linux kernel patch
+#
+# btrfs: Fix no_space on dd and rm loop < from zhaolei@cn.fujitsu.com >
+#	btrfs: Fix NO_SPACE bug caused by delayed-iput
+#	btrfs: Support busy loop of write and delete
+#	btrfs: add WARN_ON() to check is space_info op current
+#	btrfs: Set relative data on clear btrfs_block_group_cache->pinned
+#	btrfs: Adjust commit-transaction condition to avoid NO_SPACE more
+#	btrfs: Fix tail space processing in find_free_dev_extent()
+#	btrfs: fix condition of commit transaction
+#	btrfs: wait for delayed iputs on no space
+#-----------------------------------------------------------------------
+# Copyright (c) 2015 Fujitsu.  All Rights Reserved.
+# Author: Wang Yanfeng <wangyf-fnst@cn.fujitsu.com>
+#
+# 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"
+
+status=1
+trap "exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_need_to_be_root
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+rm -f $seqres.full
+
+dev_size=$((512 * 1024 * 1024))
+_scratch_mkfs_sized $dev_size >>$seqres.full 2>&1
+_scratch_mount
+file_size_m=$(($dev_size * 75 / 100 / 1024 / 1024))
+
+for ((i = 0; i < 10; i++)); do
+	echo "loop $i" >>$seqres.full
+
+	dd if=/dev/zero of="$SCRATCH_MNT"/file0 bs=1M count="$file_size_m" \
+	>>$seqres.full 2>&1 || _fail "dd failed"
+
+	rm -f "$SCRATCH_MNT"/file0 || _fail "rm failed"
+done
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/326.out b/tests/generic/326.out
new file mode 100644
index 0000000..4ac0db5
--- /dev/null
+++ b/tests/generic/326.out
@@ -0,0 +1,2 @@
+QA output created by 326
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 0c8964c..393a2cb 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -194,3 +194,4 @@
 323 auto aio stress
 324 auto fsr quick
 325 auto quick data log
+326 auto
-- 
1.9.1

Re: [PATCH] generic: busy loop of dd and rm test

[Dave Chinner]

On Thu, Jun 11, 2015 at 08:52:35AM +0800, Wang Yanfeng wrote:
> generic/326 is a case about testing whether writing failed on
> NO_SPACE in a busy loop of write and delete when disk almost full.
> It is a long-term problem since very beginning in btrfs, and has been
> fixed by patchset titled "btrfs: Fix no_space on dd and rm loop" from
> zhaolei@cn.fujitsu.com.
> 
> Signed-off-by: Wang Yanfeng <wangyf-fnst@cn.fujitsu.com>
....
> @@ -0,0 +1,73 @@
> +#! /bin/bash
> +# FS QA Test No. 326
> +#
> +# TEST busy loop of write and delete in a filesystem.
> +# Sometimes writes will failed on NO_SPACE when disk almost full
> +# in btrfs. It is long-term problem since very beginning for btrfs
> +#
> +# This issue was fixed by the following linux kernel patch
> +#
> +# btrfs: Fix no_space on dd and rm loop < from zhaolei@cn.fujitsu.com >
> +#	btrfs: Fix NO_SPACE bug caused by delayed-iput
> +#	btrfs: Support busy loop of write and delete
> +#	btrfs: add WARN_ON() to check is space_info op current
> +#	btrfs: Set relative data on clear btrfs_block_group_cache->pinned
> +#	btrfs: Adjust commit-transaction condition to avoid NO_SPACE more
> +#	btrfs: Fix tail space processing in find_free_dev_extent()
> +#	btrfs: fix condition of commit transaction
> +#	btrfs: wait for delayed iputs on no space

This list of kernel commits does not belong in the test. If you
really need to put this anywhere, it belongs in the commit
message.

> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +status=1
> +trap "exit \$status" 0 1 2 3 15

Why did you remove the "tmp=/tmp/$$" line from the new test
template?

> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_need_to_be_root
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +
> +rm -f $seqres.full
> +
> +dev_size=$((512 * 1024 * 1024))

So a 512MB filesystem...

> +_scratch_mkfs_sized $dev_size >>$seqres.full 2>&1
> +_scratch_mount
> +file_size_m=$(($dev_size * 75 / 100 / 1024 / 1024))

and a ~400MB file, right? So you don't actually need this
magic calculation, because:

> +for ((i = 0; i < 10; i++)); do
> +	echo "loop $i" >>$seqres.full
> +
> +	dd if=/dev/zero of="$SCRATCH_MNT"/file0 bs=1M count="$file_size_m" \
> +	>>$seqres.full 2>&1 || _fail "dd failed"

You shouldn't be using dd for this, nor should you be using _fail:

	$XFS_IO_PROG -f -c "pwrite -b 1m 0 400m" $SCRATCH_MNT"/file0 | \
		_filter_xfs_io | _filter_scratch

If the write fails, then xfs_io will dump an unexpected error
message to the output, and the golden output mismatch will fail the
test.


> +	rm -f "$SCRATCH_MNT"/file0 || _fail "rm failed"

Same here - rm will output an error that will be caughti if it
fails...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] generic: busy loop of dd and rm test

[wangyf]

在 2015年06月18日 05:44, Dave Chinner 写道:
> On Thu, Jun 11, 2015 at 08:52:35AM +0800, Wang Yanfeng wrote:
>> generic/326 is a case about testing whether writing failed on
>> NO_SPACE in a busy loop of write and delete when disk almost full.
>> It is a long-term problem since very beginning in btrfs, and has been
>> fixed by patchset titled "btrfs: Fix no_space on dd and rm loop" from
>> zhaolei@cn.fujitsu.com.
>>
>> Signed-off-by: Wang Yanfeng <wangyf-fnst@cn.fujitsu.com>
> ....
>> @@ -0,0 +1,73 @@
>> +#! /bin/bash
>> +# FS QA Test No. 326
>> +#
>> +# TEST busy loop of write and delete in a filesystem.
>> +# Sometimes writes will failed on NO_SPACE when disk almost full
>> +# in btrfs. It is long-term problem since very beginning for btrfs
>> +#
>> +# This issue was fixed by the following linux kernel patch
>> +#
>> +# btrfs: Fix no_space on dd and rm loop < from zhaolei@cn.fujitsu.com >
>> +#	btrfs: Fix NO_SPACE bug caused by delayed-iput
>> +#	btrfs: Support busy loop of write and delete
>> +#	btrfs: add WARN_ON() to check is space_info op current
>> +#	btrfs: Set relative data on clear btrfs_block_group_cache->pinned
>> +#	btrfs: Adjust commit-transaction condition to avoid NO_SPACE more
>> +#	btrfs: Fix tail space processing in find_free_dev_extent()
>> +#	btrfs: fix condition of commit transaction
>> +#	btrfs: wait for delayed iputs on no space
> This list of kernel commits does not belong in the test. If you
> really need to put this anywhere, it belongs in the commit
> message.
Maybe we can use the sentence " The issue was fixed by the patchset named
'btrfs: Fix no_space on dd and rm loop'< from zhaolei@cn.fujitsu.com> "  
to replace it
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +status=1
>> +trap "exit \$status" 0 1 2 3 15
> Why did you remove the "tmp=/tmp/$$" line from the new test
> template?
o,,,, at first I think I didn't use the tmp dir. I didn't notice it in 
the new template
I will add it.
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +_need_to_be_root
>> +_supported_fs generic
>> +_supported_os Linux
>> +_require_scratch
>> +
>> +rm -f $seqres.full
>> +
>> +dev_size=$((512 * 1024 * 1024))
> So a 512MB filesystem...
If we choose a small device, the Bug will be easy to trigger.
In btrfs (other fs I don't know), the required smallest disk space is 256MB.
But in this situation,  we can only use about 100MB~ space, and it 
appears too small,
so double it, I choose 512M as the size of the file system.
>> +_scratch_mkfs_sized $dev_size >>$seqres.full 2>&1
>> +_scratch_mount
>> +file_size_m=$(($dev_size * 75 / 100 / 1024 / 1024))
> and a ~400MB file, right? So you don't actually need this
> magic calculation, because:
>
>> +for ((i = 0; i < 10; i++)); do
>> +	echo "loop $i" >>$seqres.full
>> +
>> +	dd if=/dev/zero of="$SCRATCH_MNT"/file0 bs=1M count="$file_size_m" \
>> +	>>$seqres.full 2>&1 || _fail "dd failed"
> You shouldn't be using dd for this, nor should you be using _fail:
>
> 	$XFS_IO_PROG -f -c "pwrite -b 1m 0 400m" $SCRATCH_MNT"/file0 | \
> 		_filter_xfs_io | _filter_scratch
>
> If the write fails, then xfs_io will dump an unexpected error
> message to the output, and the golden output mismatch will fail the
> test.
>
>
>> +	rm -f "$SCRATCH_MNT"/file0 || _fail "rm failed"
> Same here - rm will output an error that will be caughti if it
> fails...
Agree, I will make a new patch under your advice.
>
> Cheers,
>
> Dave.