[PATCH v3] generic: make 17[1-4] work well when btrfs compression is enabled

[PATCH v3] generic: make 17[1-4] work well when btrfs compression is enabled

[Wang Xiaoguang]

When enabling btrfs compression, original codes can not fill fs
correctly, here we introduce _fill_fs() in common/rc, which'll keep
creating and writing files until enospc error occurs. Note _fill_fs
is copied from tests/generic/256, but with some minor modifications.

Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
---
V2: In common/, I did't find an existing function suitable for
    these 4 test cases to fill fs, so I still use _pwrite_byte() with
    a big enough file length fo fill fs. Note, for btrfs, metadata space
    still is not full, only data space is full, but it's OK for these
    4 test cases.

    All these 4 cases pass in xfs and btrfs(without compression), if
    btrfs has compression enabled, these 4 cases will fail for false
    enospc error, I have sent kernel patches to fix this bug.

V3: Introduce  _fill_fs in common/rc to fill fs.
---
 common/rc         | 50 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/171 |  4 +---
 tests/generic/172 |  4 ++--
 tests/generic/173 |  4 +---
 tests/generic/174 |  4 +---
 tests/generic/256 | 65 +++++--------------------------------------------------
 6 files changed, 60 insertions(+), 71 deletions(-)

diff --git a/common/rc b/common/rc
index 7a9fc90..0e1ac5d 100644
--- a/common/rc
+++ b/common/rc
@@ -4003,6 +4003,56 @@ _require_xfs_mkfs_without_validation()
 	fi
 }
 
+# Fill a file system by repeatedly creating files in the given folder
+# starting with the given file size.  Files are reduced in size when
+# they can no longer fit until no more files can be created.
+_fill_fs()
+{
+	local file_size=$1
+	local dir=$2
+	local block_size=$3
+	local switch_user=$4
+	local file_count=1
+	local bytes_written=0
+
+	if [ $# -ne 4 ]; then
+		echo "Usage: _fill_fs filesize dir blocksize"
+		exit 1
+	fi
+
+	if [ $switch_user -eq 0 ]; then
+		mkdir -p $dir
+	else
+		_user_do "mkdir -p $dir"
+	fi
+
+	if [ $file_size -lt $block_size ]; then
+		$file_size = $block_size
+	fi
+
+	while [ $file_size -ge $block_size ]; do
+		bytes_written=0
+		if [ $switch_user -eq 0 ]; then
+			$XFS_IO_PROG -f -c "pwrite 0 $file_size" \
+				$dir/$file_count > /dev/null
+		else
+			_user_do "$XFS_IO_PROG -f -c \"pwrite 0 $file_size\" \
+				$dir/$file_count > /dev/null"
+		fi
+
+		if [ -f $dir/$file_count ]; then
+			bytes_written=$(stat -c '%s' $dir/$file_count)
+		fi
+
+		# If there was no room to make the file, then divide it in
+		# half, and keep going
+		if [ $bytes_written -lt $file_size ]; then
+			file_size=$((file_size / 2))
+		fi
+		file_count=$((file_count + 1))
+	done
+}
+
 init_rc
 
 ################################################################################
diff --git a/tests/generic/171 b/tests/generic/171
index a69f798..fde8ef2 100755
--- a/tests/generic/171
+++ b/tests/generic/171
@@ -75,9 +75,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
 sync
 
 echo "Allocate the rest of the space"
-nr_free=$(stat -f -c '%f' $testdir)
-touch $testdir/file0 $testdir/file1
-_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
+_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
 sync
 
 echo "CoW the big file"
diff --git a/tests/generic/172 b/tests/generic/172
index 8192290..3625546 100755
--- a/tests/generic/172
+++ b/tests/generic/172
@@ -57,6 +57,7 @@ testdir=$SCRATCH_MNT/test-$seq
 mkdir $testdir
 
 echo "Reformat with appropriate size"
+blksz="$(get_block_size $testdir)"
 umount $SCRATCH_MNT
 
 file_size=$((168 * 1024 * 1024))
@@ -72,8 +73,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
 sync
 
 echo "Allocate the rest of the space"
-touch $testdir/file0 $testdir/file1
-_pwrite_byte 0x61 0 $fs_size $testdir/eat_my_space >> $seqres.full 2>&1
+_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
 sync
 
 echo "CoW the big file"
diff --git a/tests/generic/173 b/tests/generic/173
index e35597f..3d583b1 100755
--- a/tests/generic/173
+++ b/tests/generic/173
@@ -75,9 +75,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
 sync
 
 echo "Allocate the rest of the space"
-nr_free=$(stat -f -c '%f' $testdir)
-touch $testdir/file0 $testdir/file1
-_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
+_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
 sync
 
 echo "mmap CoW the big file"
diff --git a/tests/generic/174 b/tests/generic/174
index e58d64b..bd7fc11 100755
--- a/tests/generic/174
+++ b/tests/generic/174
@@ -76,9 +76,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
 sync
 
 echo "Allocate the rest of the space"
-nr_free=$(stat -f -c '%f' $testdir)
-touch $testdir/file0 $testdir/file1
-_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
+_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
 sync
 
 echo "CoW the big file"
diff --git a/tests/generic/256 b/tests/generic/256
index cfbf790..9488648 100755
--- a/tests/generic/256
+++ b/tests/generic/256
@@ -53,64 +53,6 @@ _require_test
 
 testfile=$TEST_DIR/256.$$
 
-# _fill_fs()
-#
-# Fills a file system by repeatedly creating files in the given folder
-# starting with the given file size.  Files are reduced in size when
-# they can no longer fit untill no more files can be created.
-#
-# This routine is used by _test_full_fs_punch to test that a hole may
-# still be punched when the disk is full by borrowing reserved blocks.
-# All files are created as a non root user to prevent reserved blocks
-# from being consumed.
-#
-_fill_fs() {
-	local file_size=$1
-	local dir=$2
-	local block_size=$3
-	local file_count=1
-	local bytes_written=0
-
-	if [ $# -ne 3 ]
-	then
-		echo "USAGE: _fill_fs filesize dir block size"
-		exit 1
-	fi
-
-	# Creation of files or folders
-	# must not be done as root or
-	# reserved blocks will be consumed
-	_user_do "mkdir -p $dir &> /dev/null"
-	if [ $? -ne 0 ] ; then
-		return 0
-	fi
-
-	if [ $file_size -lt $block_size ]
-	then
-		$file_size = $block_size
-	fi
-
-	while [ $file_size -ge $block_size ]
-	do
-		bytes_written=0
-		_user_do "$XFS_IO_PROG -f -c \"pwrite 0 $file_size\" $dir/$file_count.bin &> /dev/null"
-
-		if [ -f $dir/$file_count.bin ]
-		then
-			bytes_written=`$XFS_IO_PROG -c "stat"  $dir/$file_count.bin | grep stat.size | cut -d ' ' -f3`
-		fi
-
-		# If there was no room to make the file,
-		# then divide it in half, and keep going
-		if [ $bytes_written -lt $file_size ]
-		then
-			file_size=$(( $file_size / 2 ))
-		fi
-		file_count=$(( $file_count + 1 ))
-
-	done
-}
-
 # _test_full_fs_punch()
 #
 # This function will test that a hole may be punched
@@ -144,7 +86,10 @@ _test_full_fs_punch()
 		-c "fsync" $file_name &> /dev/null
 	chmod 666 $file_name
 
-	_fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size
+	# All files are created as a non root user to prevent reserved blocks
+	# from being consumed.
+	_fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size 1 \
+		> /dev/null 2>&1
 
 	for (( i=0; i<$iterations; i++ ))
 	do
@@ -159,7 +104,7 @@ _test_full_fs_punch()
 
 		hole_offset=$(( $hole_offset + $hole_len + $hole_interval ))
 
-		_fill_fs $hole_len $path/fill.$i $block_size
+		_fill_fs $hole_len $path/fill.$i $block_size 1 > /dev/null 2>&1
 
 	done
 }
-- 
2.9.0

Re: [PATCH v3] generic: make 17[1-4] work well when btrfs compression is enabled

[Eryu Guan]

On Wed, Oct 26, 2016 at 05:52:11PM +0800, Wang Xiaoguang wrote:
> When enabling btrfs compression, original codes can not fill fs
> correctly, here we introduce _fill_fs() in common/rc, which'll keep
> creating and writing files until enospc error occurs. Note _fill_fs
> is copied from tests/generic/256, but with some minor modifications.
> 
> Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>

Looks fine to me overall, generic/17[1-4] and generic/256 passed on xfs,
btrfs and btrfs with compress. But I'd like Darrick to review it as well :)

> ---
> V2: In common/, I did't find an existing function suitable for
>     these 4 test cases to fill fs, so I still use _pwrite_byte() with
>     a big enough file length fo fill fs. Note, for btrfs, metadata space
>     still is not full, only data space is full, but it's OK for these
>     4 test cases.
> 
>     All these 4 cases pass in xfs and btrfs(without compression), if
>     btrfs has compression enabled, these 4 cases will fail for false
>     enospc error, I have sent kernel patches to fix this bug.
> 
> V3: Introduce  _fill_fs in common/rc to fill fs.
> ---
>  common/rc         | 50 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/171 |  4 +---
>  tests/generic/172 |  4 ++--
>  tests/generic/173 |  4 +---
>  tests/generic/174 |  4 +---
>  tests/generic/256 | 65 +++++--------------------------------------------------
>  6 files changed, 60 insertions(+), 71 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 7a9fc90..0e1ac5d 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4003,6 +4003,56 @@ _require_xfs_mkfs_without_validation()
>  	fi
>  }
>  
> +# Fill a file system by repeatedly creating files in the given folder
> +# starting with the given file size.  Files are reduced in size when
> +# they can no longer fit until no more files can be created.
> +_fill_fs()
> +{
> +	local file_size=$1
> +	local dir=$2
> +	local block_size=$3
> +	local switch_user=$4
> +	local file_count=1
> +	local bytes_written=0
> +
> +	if [ $# -ne 4 ]; then
> +		echo "Usage: _fill_fs filesize dir blocksize"

The usage info here is wrong, missing the "switch user" argument.

Thanks,
Eryu

Re: [PATCH v3] generic: make 17[1-4] work well when btrfs compression is enabled

[Darrick J. Wong]

On Wed, Oct 26, 2016 at 05:52:11PM +0800, Wang Xiaoguang wrote:
> When enabling btrfs compression, original codes can not fill fs
> correctly, here we introduce _fill_fs() in common/rc, which'll keep
> creating and writing files until enospc error occurs. Note _fill_fs
> is copied from tests/generic/256, but with some minor modifications.
> 
> Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
> ---
> V2: In common/, I did't find an existing function suitable for
>     these 4 test cases to fill fs, so I still use _pwrite_byte() with
>     a big enough file length fo fill fs. Note, for btrfs, metadata space
>     still is not full, only data space is full, but it's OK for these
>     4 test cases.
> 
>     All these 4 cases pass in xfs and btrfs(without compression), if
>     btrfs has compression enabled, these 4 cases will fail for false
>     enospc error, I have sent kernel patches to fix this bug.
> 
> V3: Introduce  _fill_fs in common/rc to fill fs.
> ---
>  common/rc         | 50 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/171 |  4 +---
>  tests/generic/172 |  4 ++--
>  tests/generic/173 |  4 +---
>  tests/generic/174 |  4 +---
>  tests/generic/256 | 65 +++++--------------------------------------------------
>  6 files changed, 60 insertions(+), 71 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 7a9fc90..0e1ac5d 100644
> --- a/common/rc
> +++ b/common/rc

Would you mind moving this to common/populate, where I've been (slowly)
collecting all the "create stuff inside the filesystem" routines?

(It's part of a slow-moving effort to declutter common/rc.)

> @@ -4003,6 +4003,56 @@ _require_xfs_mkfs_without_validation()
>  	fi
>  }
>  
> +# Fill a file system by repeatedly creating files in the given folder
> +# starting with the given file size.  Files are reduced in size when
> +# they can no longer fit until no more files can be created.
> +_fill_fs()
> +{
> +	local file_size=$1
> +	local dir=$2
> +	local block_size=$3
> +	local switch_user=$4
> +	local file_count=1
> +	local bytes_written=0
> +
> +	if [ $# -ne 4 ]; then
> +		echo "Usage: _fill_fs filesize dir blocksize"
> +		exit 1
> +	fi
> +
> +	if [ $switch_user -eq 0 ]; then
> +		mkdir -p $dir
> +	else
> +		_user_do "mkdir -p $dir"
> +	fi
> +
> +	if [ $file_size -lt $block_size ]; then
> +		$file_size = $block_size
> +	fi
> +
> +	while [ $file_size -ge $block_size ]; do
> +		bytes_written=0
> +		if [ $switch_user -eq 0 ]; then
> +			$XFS_IO_PROG -f -c "pwrite 0 $file_size" \
> +				$dir/$file_count > /dev/null

If you want to speed this up some more you could pass "-b 8388608" in
the pwrite string so that xfs_io will allocate an 8MB memory buffer
internally when writing out data.

$XFS_IO_PROG -f -c "pwrite -b 8388608 0 $file_size" $dir/$file_count

> +		else
> +			_user_do "$XFS_IO_PROG -f -c \"pwrite 0 $file_size\" \
> +				$dir/$file_count > /dev/null"

Also, why redirect to /dev/null?  I think helper functions generally let
the individual test decide where the stdout & stderr output ultimately
end up.  Most tests seem to dump to $seqres.full or /dev/null, but we
should let the test decide where potentially useful diagnostic
information ends up.

/me also wonders if falloc would be a faster way to consume disk blocks
than pwrite, but ... <shrug>

> +		fi
> +
> +		if [ -f $dir/$file_count ]; then
> +			bytes_written=$(stat -c '%s' $dir/$file_count)
> +		fi
> +
> +		# If there was no room to make the file, then divide it in
> +		# half, and keep going
> +		if [ $bytes_written -lt $file_size ]; then
> +			file_size=$((file_size / 2))
> +		fi
> +		file_count=$((file_count + 1))
> +	done
> +}
> +
>  init_rc
>  
>  ################################################################################
> diff --git a/tests/generic/171 b/tests/generic/171
> index a69f798..fde8ef2 100755
> --- a/tests/generic/171
> +++ b/tests/generic/171
> @@ -75,9 +75,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
>  sync
>  
>  echo "Allocate the rest of the space"
> -nr_free=$(stat -f -c '%f' $testdir)
> -touch $testdir/file0 $testdir/file1
> -_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
> +_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1

Given that _fill_fs has exponential backoff to try to really fill the fs,
why not call it with $((blksz * nr_free)) instead of a fixed 32MB?

--D

>  sync
>  
>  echo "CoW the big file"
> diff --git a/tests/generic/172 b/tests/generic/172
> index 8192290..3625546 100755
> --- a/tests/generic/172
> +++ b/tests/generic/172
> @@ -57,6 +57,7 @@ testdir=$SCRATCH_MNT/test-$seq
>  mkdir $testdir
>  
>  echo "Reformat with appropriate size"
> +blksz="$(get_block_size $testdir)"
>  umount $SCRATCH_MNT
>  
>  file_size=$((168 * 1024 * 1024))
> @@ -72,8 +73,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
>  sync
>  
>  echo "Allocate the rest of the space"
> -touch $testdir/file0 $testdir/file1
> -_pwrite_byte 0x61 0 $fs_size $testdir/eat_my_space >> $seqres.full 2>&1
> +_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
>  sync
>  
>  echo "CoW the big file"
> diff --git a/tests/generic/173 b/tests/generic/173
> index e35597f..3d583b1 100755
> --- a/tests/generic/173
> +++ b/tests/generic/173
> @@ -75,9 +75,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
>  sync
>  
>  echo "Allocate the rest of the space"
> -nr_free=$(stat -f -c '%f' $testdir)
> -touch $testdir/file0 $testdir/file1
> -_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
> +_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
>  sync
>  
>  echo "mmap CoW the big file"
> diff --git a/tests/generic/174 b/tests/generic/174
> index e58d64b..bd7fc11 100755
> --- a/tests/generic/174
> +++ b/tests/generic/174
> @@ -76,9 +76,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
>  sync
>  
>  echo "Allocate the rest of the space"
> -nr_free=$(stat -f -c '%f' $testdir)
> -touch $testdir/file0 $testdir/file1
> -_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
> +_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
>  sync
>  
>  echo "CoW the big file"
> diff --git a/tests/generic/256 b/tests/generic/256
> index cfbf790..9488648 100755
> --- a/tests/generic/256
> +++ b/tests/generic/256
> @@ -53,64 +53,6 @@ _require_test
>  
>  testfile=$TEST_DIR/256.$$
>  
> -# _fill_fs()
> -#
> -# Fills a file system by repeatedly creating files in the given folder
> -# starting with the given file size.  Files are reduced in size when
> -# they can no longer fit untill no more files can be created.
> -#
> -# This routine is used by _test_full_fs_punch to test that a hole may
> -# still be punched when the disk is full by borrowing reserved blocks.
> -# All files are created as a non root user to prevent reserved blocks
> -# from being consumed.
> -#
> -_fill_fs() {
> -	local file_size=$1
> -	local dir=$2
> -	local block_size=$3
> -	local file_count=1
> -	local bytes_written=0
> -
> -	if [ $# -ne 3 ]
> -	then
> -		echo "USAGE: _fill_fs filesize dir block size"
> -		exit 1
> -	fi
> -
> -	# Creation of files or folders
> -	# must not be done as root or
> -	# reserved blocks will be consumed
> -	_user_do "mkdir -p $dir &> /dev/null"
> -	if [ $? -ne 0 ] ; then
> -		return 0
> -	fi
> -
> -	if [ $file_size -lt $block_size ]
> -	then
> -		$file_size = $block_size
> -	fi
> -
> -	while [ $file_size -ge $block_size ]
> -	do
> -		bytes_written=0
> -		_user_do "$XFS_IO_PROG -f -c \"pwrite 0 $file_size\" $dir/$file_count.bin &> /dev/null"
> -
> -		if [ -f $dir/$file_count.bin ]
> -		then
> -			bytes_written=`$XFS_IO_PROG -c "stat"  $dir/$file_count.bin | grep stat.size | cut -d ' ' -f3`
> -		fi
> -
> -		# If there was no room to make the file,
> -		# then divide it in half, and keep going
> -		if [ $bytes_written -lt $file_size ]
> -		then
> -			file_size=$(( $file_size / 2 ))
> -		fi
> -		file_count=$(( $file_count + 1 ))
> -
> -	done
> -}
> -
>  # _test_full_fs_punch()
>  #
>  # This function will test that a hole may be punched
> @@ -144,7 +86,10 @@ _test_full_fs_punch()
>  		-c "fsync" $file_name &> /dev/null
>  	chmod 666 $file_name
>  
> -	_fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size
> +	# All files are created as a non root user to prevent reserved blocks
> +	# from being consumed.
> +	_fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size 1 \
> +		> /dev/null 2>&1
>  
>  	for (( i=0; i<$iterations; i++ ))
>  	do
> @@ -159,7 +104,7 @@ _test_full_fs_punch()
>  
>  		hole_offset=$(( $hole_offset + $hole_len + $hole_interval ))
>  
> -		_fill_fs $hole_len $path/fill.$i $block_size
> +		_fill_fs $hole_len $path/fill.$i $block_size 1 > /dev/null 2>&1
>  
>  	done
>  }
> -- 
> 2.9.0
> 
> 
> 
> --
> 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

Re: [PATCH v3] generic: make 17[1-4] work well when btrfs compression is enabled

[Wang Xiaoguang]

hi,

On 10/28/2016 01:13 AM, Darrick J. Wong wrote:
> On Wed, Oct 26, 2016 at 05:52:11PM +0800, Wang Xiaoguang wrote:
>> When enabling btrfs compression, original codes can not fill fs
>> correctly, here we introduce _fill_fs() in common/rc, which'll keep
>> creating and writing files until enospc error occurs. Note _fill_fs
>> is copied from tests/generic/256, but with some minor modifications.
>>
>> Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
>> ---
>> V2: In common/, I did't find an existing function suitable for
>>      these 4 test cases to fill fs, so I still use _pwrite_byte() with
>>      a big enough file length fo fill fs. Note, for btrfs, metadata space
>>      still is not full, only data space is full, but it's OK for these
>>      4 test cases.
>>
>>      All these 4 cases pass in xfs and btrfs(without compression), if
>>      btrfs has compression enabled, these 4 cases will fail for false
>>      enospc error, I have sent kernel patches to fix this bug.
>>
>> V3: Introduce  _fill_fs in common/rc to fill fs.
>> ---
>>   common/rc         | 50 ++++++++++++++++++++++++++++++++++++++++++
>>   tests/generic/171 |  4 +---
>>   tests/generic/172 |  4 ++--
>>   tests/generic/173 |  4 +---
>>   tests/generic/174 |  4 +---
>>   tests/generic/256 | 65 +++++--------------------------------------------------
>>   6 files changed, 60 insertions(+), 71 deletions(-)
>>
>> diff --git a/common/rc b/common/rc
>> index 7a9fc90..0e1ac5d 100644
>> --- a/common/rc
>> +++ b/common/rc
> Would you mind moving this to common/populate, where I've been (slowly)
> collecting all the "create stuff inside the filesystem" routines?
OK, sure.
>
> (It's part of a slow-moving effort to declutter common/rc.)
>
>> @@ -4003,6 +4003,56 @@ _require_xfs_mkfs_without_validation()
>>   	fi
>>   }
>>   
>> +# Fill a file system by repeatedly creating files in the given folder
>> +# starting with the given file size.  Files are reduced in size when
>> +# they can no longer fit until no more files can be created.
>> +_fill_fs()
>> +{
>> +	local file_size=$1
>> +	local dir=$2
>> +	local block_size=$3
>> +	local switch_user=$4
>> +	local file_count=1
>> +	local bytes_written=0
>> +
>> +	if [ $# -ne 4 ]; then
>> +		echo "Usage: _fill_fs filesize dir blocksize"
>> +		exit 1
>> +	fi
>> +
>> +	if [ $switch_user -eq 0 ]; then
>> +		mkdir -p $dir
>> +	else
>> +		_user_do "mkdir -p $dir"
>> +	fi
>> +
>> +	if [ $file_size -lt $block_size ]; then
>> +		$file_size = $block_size
>> +	fi
>> +
>> +	while [ $file_size -ge $block_size ]; do
>> +		bytes_written=0
>> +		if [ $switch_user -eq 0 ]; then
>> +			$XFS_IO_PROG -f -c "pwrite 0 $file_size" \
>> +				$dir/$file_count > /dev/null
> If you want to speed this up some more you could pass "-b 8388608" in
> the pwrite string so that xfs_io will allocate an 8MB memory buffer
> internally when writing out data.
>
> $XFS_IO_PROG -f -c "pwrite -b 8388608 0 $file_size" $dir/$file_count
Thanks for this info, I'll use it.
>
>> +		else
>> +			_user_do "$XFS_IO_PROG -f -c \"pwrite 0 $file_size\" \
>> +				$dir/$file_count > /dev/null"
> Also, why redirect to /dev/null?  I think helper functions generally let
> the individual test decide where the stdout & stderr output ultimately
> end up.  Most tests seem to dump to $seqres.full or /dev/null, but we
> should let the test decide where potentially useful diagnostic
> information ends up.
Agree :)

> /me also wonders if falloc would be a faster way to consume disk blocks
> than pwrite, but ... <shrug>
Oh, it is. Falloc is not affected by compression.

>> +		fi
>> +
>> +		if [ -f $dir/$file_count ]; then
>> +			bytes_written=$(stat -c '%s' $dir/$file_count)
>> +		fi
>> +
>> +		# If there was no room to make the file, then divide it in
>> +		# half, and keep going
>> +		if [ $bytes_written -lt $file_size ]; then
>> +			file_size=$((file_size / 2))
>> +		fi
>> +		file_count=$((file_count + 1))
>> +	done
>> +}
>> +
>>   init_rc
>>   
>>   ################################################################################
>> diff --git a/tests/generic/171 b/tests/generic/171
>> index a69f798..fde8ef2 100755
>> --- a/tests/generic/171
>> +++ b/tests/generic/171
>> @@ -75,9 +75,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
>>   sync
>>   
>>   echo "Allocate the rest of the space"
>> -nr_free=$(stat -f -c '%f' $testdir)
>> -touch $testdir/file0 $testdir/file1
>> -_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
>> +_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
> Given that _fill_fs has exponential backoff to try to really fill the fs,
> why not call it with $((blksz * nr_free)) instead of a fixed 32MB?
OK, I'll send v4 soon.
Thanks for all your comments.

Regards,
Xiaoguang Wang
>
> --D
>
>>   sync
>>   
>>   echo "CoW the big file"
>> diff --git a/tests/generic/172 b/tests/generic/172
>> index 8192290..3625546 100755
>> --- a/tests/generic/172
>> +++ b/tests/generic/172
>> @@ -57,6 +57,7 @@ testdir=$SCRATCH_MNT/test-$seq
>>   mkdir $testdir
>>   
>>   echo "Reformat with appropriate size"
>> +blksz="$(get_block_size $testdir)"
>>   umount $SCRATCH_MNT
>>   
>>   file_size=$((168 * 1024 * 1024))
>> @@ -72,8 +73,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
>>   sync
>>   
>>   echo "Allocate the rest of the space"
>> -touch $testdir/file0 $testdir/file1
>> -_pwrite_byte 0x61 0 $fs_size $testdir/eat_my_space >> $seqres.full 2>&1
>> +_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
>>   sync
>>   
>>   echo "CoW the big file"
>> diff --git a/tests/generic/173 b/tests/generic/173
>> index e35597f..3d583b1 100755
>> --- a/tests/generic/173
>> +++ b/tests/generic/173
>> @@ -75,9 +75,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
>>   sync
>>   
>>   echo "Allocate the rest of the space"
>> -nr_free=$(stat -f -c '%f' $testdir)
>> -touch $testdir/file0 $testdir/file1
>> -_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
>> +_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
>>   sync
>>   
>>   echo "mmap CoW the big file"
>> diff --git a/tests/generic/174 b/tests/generic/174
>> index e58d64b..bd7fc11 100755
>> --- a/tests/generic/174
>> +++ b/tests/generic/174
>> @@ -76,9 +76,7 @@ _cp_reflink $testdir/bigfile $testdir/clonefile
>>   sync
>>   
>>   echo "Allocate the rest of the space"
>> -nr_free=$(stat -f -c '%f' $testdir)
>> -touch $testdir/file0 $testdir/file1
>> -_pwrite_byte 0x61 0 $((blksz * nr_free)) $testdir/eat_my_space >> $seqres.full 2>&1
>> +_fill_fs $((32 * 1024 * 1024)) $testdir/space $blksz 0 >> $seqres.full 2>&1
>>   sync
>>   
>>   echo "CoW the big file"
>> diff --git a/tests/generic/256 b/tests/generic/256
>> index cfbf790..9488648 100755
>> --- a/tests/generic/256
>> +++ b/tests/generic/256
>> @@ -53,64 +53,6 @@ _require_test
>>   
>>   testfile=$TEST_DIR/256.$$
>>   
>> -# _fill_fs()
>> -#
>> -# Fills a file system by repeatedly creating files in the given folder
>> -# starting with the given file size.  Files are reduced in size when
>> -# they can no longer fit untill no more files can be created.
>> -#
>> -# This routine is used by _test_full_fs_punch to test that a hole may
>> -# still be punched when the disk is full by borrowing reserved blocks.
>> -# All files are created as a non root user to prevent reserved blocks
>> -# from being consumed.
>> -#
>> -_fill_fs() {
>> -	local file_size=$1
>> -	local dir=$2
>> -	local block_size=$3
>> -	local file_count=1
>> -	local bytes_written=0
>> -
>> -	if [ $# -ne 3 ]
>> -	then
>> -		echo "USAGE: _fill_fs filesize dir block size"
>> -		exit 1
>> -	fi
>> -
>> -	# Creation of files or folders
>> -	# must not be done as root or
>> -	# reserved blocks will be consumed
>> -	_user_do "mkdir -p $dir &> /dev/null"
>> -	if [ $? -ne 0 ] ; then
>> -		return 0
>> -	fi
>> -
>> -	if [ $file_size -lt $block_size ]
>> -	then
>> -		$file_size = $block_size
>> -	fi
>> -
>> -	while [ $file_size -ge $block_size ]
>> -	do
>> -		bytes_written=0
>> -		_user_do "$XFS_IO_PROG -f -c \"pwrite 0 $file_size\" $dir/$file_count.bin &> /dev/null"
>> -
>> -		if [ -f $dir/$file_count.bin ]
>> -		then
>> -			bytes_written=`$XFS_IO_PROG -c "stat"  $dir/$file_count.bin | grep stat.size | cut -d ' ' -f3`
>> -		fi
>> -
>> -		# If there was no room to make the file,
>> -		# then divide it in half, and keep going
>> -		if [ $bytes_written -lt $file_size ]
>> -		then
>> -			file_size=$(( $file_size / 2 ))
>> -		fi
>> -		file_count=$(( $file_count + 1 ))
>> -
>> -	done
>> -}
>> -
>>   # _test_full_fs_punch()
>>   #
>>   # This function will test that a hole may be punched
>> @@ -144,7 +86,10 @@ _test_full_fs_punch()
>>   		-c "fsync" $file_name &> /dev/null
>>   	chmod 666 $file_name
>>   
>> -	_fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size
>> +	# All files are created as a non root user to prevent reserved blocks
>> +	# from being consumed.
>> +	_fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size 1 \
>> +		> /dev/null 2>&1
>>   
>>   	for (( i=0; i<$iterations; i++ ))
>>   	do
>> @@ -159,7 +104,7 @@ _test_full_fs_punch()
>>   
>>   		hole_offset=$(( $hole_offset + $hole_len + $hole_interval ))
>>   
>> -		_fill_fs $hole_len $path/fill.$i $block_size
>> +		_fill_fs $hole_len $path/fill.$i $block_size 1 > /dev/null 2>&1
>>   
>>   	done
>>   }
>> -- 
>> 2.9.0
>>
>>
>>
>> --
>> 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
>

Re: [PATCH v3] generic: make 17[1-4] work well when btrfs compression is enabled

[Wang Xiaoguang]

hi,

On 10/27/2016 07:25 PM, Eryu Guan wrote:
> On Wed, Oct 26, 2016 at 05:52:11PM +0800, Wang Xiaoguang wrote:
>> When enabling btrfs compression, original codes can not fill fs
>> correctly, here we introduce _fill_fs() in common/rc, which'll keep
>> creating and writing files until enospc error occurs. Note _fill_fs
>> is copied from tests/generic/256, but with some minor modifications.
>>
>> Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
> Looks fine to me overall, generic/17[1-4] and generic/256 passed on xfs,
> btrfs and btrfs with compress. But I'd like Darrick to review it as well :)
Could you please give me your btrfs's kernel version?
When enabling btrfs compression run these 4 test cases, I often got
enospc error, it seems that you didn't run into enospc errors.

>
>> ---
>> V2: In common/, I did't find an existing function suitable for
>>      these 4 test cases to fill fs, so I still use _pwrite_byte() with
>>      a big enough file length fo fill fs. Note, for btrfs, metadata space
>>      still is not full, only data space is full, but it's OK for these
>>      4 test cases.
>>
>>      All these 4 cases pass in xfs and btrfs(without compression), if
>>      btrfs has compression enabled, these 4 cases will fail for false
>>      enospc error, I have sent kernel patches to fix this bug.
>>
>> V3: Introduce  _fill_fs in common/rc to fill fs.
>> ---
>>   common/rc         | 50 ++++++++++++++++++++++++++++++++++++++++++
>>   tests/generic/171 |  4 +---
>>   tests/generic/172 |  4 ++--
>>   tests/generic/173 |  4 +---
>>   tests/generic/174 |  4 +---
>>   tests/generic/256 | 65 +++++--------------------------------------------------
>>   6 files changed, 60 insertions(+), 71 deletions(-)
>>
>> diff --git a/common/rc b/common/rc
>> index 7a9fc90..0e1ac5d 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -4003,6 +4003,56 @@ _require_xfs_mkfs_without_validation()
>>   	fi
>>   }
>>   
>> +# Fill a file system by repeatedly creating files in the given folder
>> +# starting with the given file size.  Files are reduced in size when
>> +# they can no longer fit until no more files can be created.
>> +_fill_fs()
>> +{
>> +	local file_size=$1
>> +	local dir=$2
>> +	local block_size=$3
>> +	local switch_user=$4
>> +	local file_count=1
>> +	local bytes_written=0
>> +
>> +	if [ $# -ne 4 ]; then
>> +		echo "Usage: _fill_fs filesize dir blocksize"
> The usage info here is wrong, missing the "switch user" argument.
Thanks for review, I'll fix it in v4.

Regards,
Xiaoguang Wang
>
> Thanks,
> Eryu
>
>

Re: [PATCH v3] generic: make 17[1-4] work well when btrfs compression is enabled

[Eryu Guan]

On Fri, Oct 28, 2016 at 03:00:29PM +0800, Wang Xiaoguang wrote:
> hi,
> 
> On 10/28/2016 01:13 AM, Darrick J. Wong wrote:
> > On Wed, Oct 26, 2016 at 05:52:11PM +0800, Wang Xiaoguang wrote:
> > > When enabling btrfs compression, original codes can not fill fs
> > > correctly, here we introduce _fill_fs() in common/rc, which'll keep
> > > creating and writing files until enospc error occurs. Note _fill_fs
> > > is copied from tests/generic/256, but with some minor modifications.
> > > 
> > > Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
> > > ---
> > > V2: In common/, I did't find an existing function suitable for
> > >      these 4 test cases to fill fs, so I still use _pwrite_byte() with
> > >      a big enough file length fo fill fs. Note, for btrfs, metadata space
> > >      still is not full, only data space is full, but it's OK for these
> > >      4 test cases.
> > > 
> > >      All these 4 cases pass in xfs and btrfs(without compression), if
> > >      btrfs has compression enabled, these 4 cases will fail for false
> > >      enospc error, I have sent kernel patches to fix this bug.
> > > 
> > > V3: Introduce  _fill_fs in common/rc to fill fs.
> > > ---
> > >   common/rc         | 50 ++++++++++++++++++++++++++++++++++++++++++
> > >   tests/generic/171 |  4 +---
> > >   tests/generic/172 |  4 ++--
> > >   tests/generic/173 |  4 +---
> > >   tests/generic/174 |  4 +---
> > >   tests/generic/256 | 65 +++++--------------------------------------------------
> > >   6 files changed, 60 insertions(+), 71 deletions(-)
> > > 
> > > diff --git a/common/rc b/common/rc
> > > index 7a9fc90..0e1ac5d 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > Would you mind moving this to common/populate, where I've been (slowly)
> > collecting all the "create stuff inside the filesystem" routines?
> OK, sure.
> > 
> > (It's part of a slow-moving effort to declutter common/rc.)
> > 
> > > @@ -4003,6 +4003,56 @@ _require_xfs_mkfs_without_validation()
> > >   	fi
> > >   }
> > > +# Fill a file system by repeatedly creating files in the given folder
> > > +# starting with the given file size.  Files are reduced in size when
> > > +# they can no longer fit until no more files can be created.
> > > +_fill_fs()
> > > +{
> > > +	local file_size=$1
> > > +	local dir=$2
> > > +	local block_size=$3
> > > +	local switch_user=$4
> > > +	local file_count=1
> > > +	local bytes_written=0
> > > +
> > > +	if [ $# -ne 4 ]; then
> > > +		echo "Usage: _fill_fs filesize dir blocksize"
> > > +		exit 1
> > > +	fi
> > > +
> > > +	if [ $switch_user -eq 0 ]; then
> > > +		mkdir -p $dir
> > > +	else
> > > +		_user_do "mkdir -p $dir"
> > > +	fi
> > > +
> > > +	if [ $file_size -lt $block_size ]; then
> > > +		$file_size = $block_size
> > > +	fi
> > > +
> > > +	while [ $file_size -ge $block_size ]; do
> > > +		bytes_written=0
> > > +		if [ $switch_user -eq 0 ]; then
> > > +			$XFS_IO_PROG -f -c "pwrite 0 $file_size" \
> > > +				$dir/$file_count > /dev/null
> > If you want to speed this up some more you could pass "-b 8388608" in
> > the pwrite string so that xfs_io will allocate an 8MB memory buffer
> > internally when writing out data.
> > 
> > $XFS_IO_PROG -f -c "pwrite -b 8388608 0 $file_size" $dir/$file_count
> Thanks for this info, I'll use it.
> > 
> > > +		else
> > > +			_user_do "$XFS_IO_PROG -f -c \"pwrite 0 $file_size\" \
> > > +				$dir/$file_count > /dev/null"
> > Also, why redirect to /dev/null?  I think helper functions generally let
> > the individual test decide where the stdout & stderr output ultimately
> > end up.  Most tests seem to dump to $seqres.full or /dev/null, but we
> > should let the test decide where potentially useful diagnostic
> > information ends up.
> Agree :)
> 
> > /me also wonders if falloc would be a faster way to consume disk blocks
> > than pwrite, but ... <shrug>
> Oh, it is. Falloc is not affected by compression.

One problem with falloc is that not all filesystems support it, e.g.
ext3 and ext2. So you have to fall back to pwrite if falloc is not
supported.

Thanks,
Eryu

Re: [PATCH v3] generic: make 17[1-4] work well when btrfs compression is enabled

[Eryu Guan]

On Fri, Oct 28, 2016 at 03:05:55PM +0800, Wang Xiaoguang wrote:
> hi,
> 
> On 10/27/2016 07:25 PM, Eryu Guan wrote:
> > On Wed, Oct 26, 2016 at 05:52:11PM +0800, Wang Xiaoguang wrote:
> > > When enabling btrfs compression, original codes can not fill fs
> > > correctly, here we introduce _fill_fs() in common/rc, which'll keep
> > > creating and writing files until enospc error occurs. Note _fill_fs
> > > is copied from tests/generic/256, but with some minor modifications.
> > > 
> > > Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
> > Looks fine to me overall, generic/17[1-4] and generic/256 passed on xfs,
> > btrfs and btrfs with compress. But I'd like Darrick to review it as well :)
> Could you please give me your btrfs's kernel version?
> When enabling btrfs compression run these 4 test cases, I often got
> enospc error, it seems that you didn't run into enospc errors.

I'm using 4.9-rc1 kernel, with both test_dev and scratch_dev 15G in
size, btrfs-progs v4.6

Thanks,
Eryu