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

[wangyf <wangyf-fnst@cn.fujitsu.com>]

在 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.