diff mbox series

xfs/014: try a few times to create speculative preallocations

Message ID 20220104020417.GB31566@magnolia (mailing list archive)
State New, archived
Headers show
Series xfs/014: try a few times to create speculative preallocations | expand

Commit Message

Darrick J. Wong Jan. 4, 2022, 2:04 a.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

This test checks that speculative file preallocations are transferred to
threads writing other files when space is low.  Since we have background
threads to clear those preallocations, it's possible that the test
program might not get a speculative preallocation on the first try.

This problem has become more pronounced since the introduction of
background inode inactivation since userspace no longer has direct
control over the timing of file blocks being released from unlinked
files.  As a result, the author has seen an increase in sporadic
warnings from this test about speculative preallocations not appearing.

Therefore, modify the function to try up to five times to create the
speculative preallocation before emitting warnings that then cause
golden output failures.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/014 |   41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

Comments

Zorro Lang Jan. 5, 2022, 4:19 p.m. UTC | #1 
On Mon, Jan 03, 2022 at 06:04:17PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This test checks that speculative file preallocations are transferred to
> threads writing other files when space is low.  Since we have background
> threads to clear those preallocations, it's possible that the test
> program might not get a speculative preallocation on the first try.
> 
> This problem has become more pronounced since the introduction of
> background inode inactivation since userspace no longer has direct
> control over the timing of file blocks being released from unlinked
> files.  As a result, the author has seen an increase in sporadic
> warnings from this test about speculative preallocations not appearing.
> 
> Therefore, modify the function to try up to five times to create the
> speculative preallocation before emitting warnings that then cause
> golden output failures.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/014 |   41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/xfs/014 b/tests/xfs/014
> index a605b359..1f0ebac3 100755
> --- a/tests/xfs/014
> +++ b/tests/xfs/014
> @@ -33,27 +33,36 @@ _cleanup()
>  # failure.
>  _spec_prealloc_file()
>  {
> -	file=$1
> +	local file=$1
> +	local prealloc_size=0
> +	local i=0
>  
> -	rm -f $file
> +	# Now that we have background garbage collection processes that can be
> +	# triggered by low space/quota conditions, it's possible that we won't
> +	# succeed in creating a speculative preallocation on the first try.
> +	for ((tries = 0; tries < 5 && prealloc_size == 0; tries++)); do
> +		rm -f $file
>  
> -	# a few file extending open-write-close cycles should be enough to
> -	# trigger the fs to retain preallocation. write 256k in 32k intervals to
> -	# be sure
> -	for i in $(seq 0 32768 262144); do
> -		$XFS_IO_PROG -f -c "pwrite $i 32k" $file >> $seqres.full
> +		# a few file extending open-write-close cycles should be enough
> +		# to trigger the fs to retain preallocation. write 256k in 32k
> +		# intervals to be sure
> +		for i in $(seq 0 32768 262144); do
> +			$XFS_IO_PROG -f -c "pwrite $i 32k" $file >> $seqres.full
> +		done
> +
> +		# write a 4k aligned amount of data to keep the calculations
> +		# simple
> +		$XFS_IO_PROG -c "pwrite 0 128m" $file >> $seqres.full
> +
> +		size=`_get_filesize $file`
> +		blocks=`stat -c "%b" $file`
> +		blocksize=`stat -c "%B" $file`
> +
> +		prealloc_size=$((blocks * blocksize - size))

So we only try same pwrite operations 5 times, and only check the prealloc_size after 5
times done? Should we break from this loop once prealloc_size > 0?

Thanks,
Zorro

>  	done
>  
> -	# write a 4k aligned amount of data to keep the calculations simple
> -	$XFS_IO_PROG -c "pwrite 0 128m" $file >> $seqres.full
> -
> -	size=`_get_filesize $file`
> -	blocks=`stat -c "%b" $file`
> -	blocksize=`stat -c "%B" $file`
> -
> -	prealloc_size=$((blocks * blocksize - size))
>  	if [ $prealloc_size -eq 0 ]; then
> -		echo "Warning: No speculative preallocation for $file." \
> +		echo "Warning: No speculative preallocation for $file after $tries iterations." \
>  			"Check use of the allocsize= mount option."
>  	fi
>  
>
Darrick J. Wong Jan. 5, 2022, 7:09 p.m. UTC | #2 
On Thu, Jan 06, 2022 at 12:19:05AM +0800, Zorro Lang wrote:
> On Mon, Jan 03, 2022 at 06:04:17PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > This test checks that speculative file preallocations are transferred to
> > threads writing other files when space is low.  Since we have background
> > threads to clear those preallocations, it's possible that the test
> > program might not get a speculative preallocation on the first try.
> > 
> > This problem has become more pronounced since the introduction of
> > background inode inactivation since userspace no longer has direct
> > control over the timing of file blocks being released from unlinked
> > files.  As a result, the author has seen an increase in sporadic
> > warnings from this test about speculative preallocations not appearing.
> > 
> > Therefore, modify the function to try up to five times to create the
> > speculative preallocation before emitting warnings that then cause
> > golden output failures.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/014 |   41 +++++++++++++++++++++++++----------------
> >  1 file changed, 25 insertions(+), 16 deletions(-)
> > 
> > diff --git a/tests/xfs/014 b/tests/xfs/014
> > index a605b359..1f0ebac3 100755
> > --- a/tests/xfs/014
> > +++ b/tests/xfs/014
> > @@ -33,27 +33,36 @@ _cleanup()
> >  # failure.
> >  _spec_prealloc_file()
> >  {
> > -	file=$1
> > +	local file=$1
> > +	local prealloc_size=0
> > +	local i=0
> >  
> > -	rm -f $file
> > +	# Now that we have background garbage collection processes that can be
> > +	# triggered by low space/quota conditions, it's possible that we won't
> > +	# succeed in creating a speculative preallocation on the first try.
> > +	for ((tries = 0; tries < 5 && prealloc_size == 0; tries++)); do
> > +		rm -f $file
> >  
> > -	# a few file extending open-write-close cycles should be enough to
> > -	# trigger the fs to retain preallocation. write 256k in 32k intervals to
> > -	# be sure
> > -	for i in $(seq 0 32768 262144); do
> > -		$XFS_IO_PROG -f -c "pwrite $i 32k" $file >> $seqres.full
> > +		# a few file extending open-write-close cycles should be enough
> > +		# to trigger the fs to retain preallocation. write 256k in 32k
> > +		# intervals to be sure
> > +		for i in $(seq 0 32768 262144); do
> > +			$XFS_IO_PROG -f -c "pwrite $i 32k" $file >> $seqres.full
> > +		done
> > +
> > +		# write a 4k aligned amount of data to keep the calculations
> > +		# simple
> > +		$XFS_IO_PROG -c "pwrite 0 128m" $file >> $seqres.full
> > +
> > +		size=`_get_filesize $file`
> > +		blocks=`stat -c "%b" $file`
> > +		blocksize=`stat -c "%B" $file`
> > +
> > +		prealloc_size=$((blocks * blocksize - size))
> 
> So we only try same pwrite operations 5 times, and only check the prealloc_size after 5
> times done? Should we break from this loop once prealloc_size > 0?

The second clause of the for loop tests for that, does it not?

--D

> 
> Thanks,
> Zorro
> 
> >  	done
> >  
> > -	# write a 4k aligned amount of data to keep the calculations simple
> > -	$XFS_IO_PROG -c "pwrite 0 128m" $file >> $seqres.full
> > -
> > -	size=`_get_filesize $file`
> > -	blocks=`stat -c "%b" $file`
> > -	blocksize=`stat -c "%B" $file`
> > -
> > -	prealloc_size=$((blocks * blocksize - size))
> >  	if [ $prealloc_size -eq 0 ]; then
> > -		echo "Warning: No speculative preallocation for $file." \
> > +		echo "Warning: No speculative preallocation for $file after $tries iterations." \
> >  			"Check use of the allocsize= mount option."
> >  	fi
> >  
> > 
>
Zorro Lang Jan. 6, 2022, 2:14 a.m. UTC | #3 
On Wed, Jan 05, 2022 at 11:09:57AM -0800, Darrick J. Wong wrote:
> On Thu, Jan 06, 2022 at 12:19:05AM +0800, Zorro Lang wrote:
> > On Mon, Jan 03, 2022 at 06:04:17PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > This test checks that speculative file preallocations are transferred to
> > > threads writing other files when space is low.  Since we have background
> > > threads to clear those preallocations, it's possible that the test
> > > program might not get a speculative preallocation on the first try.
> > > 
> > > This problem has become more pronounced since the introduction of
> > > background inode inactivation since userspace no longer has direct
> > > control over the timing of file blocks being released from unlinked
> > > files.  As a result, the author has seen an increase in sporadic
> > > warnings from this test about speculative preallocations not appearing.
> > > 
> > > Therefore, modify the function to try up to five times to create the
> > > speculative preallocation before emitting warnings that then cause
> > > golden output failures.
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > >  tests/xfs/014 |   41 +++++++++++++++++++++++++----------------
> > >  1 file changed, 25 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/tests/xfs/014 b/tests/xfs/014
> > > index a605b359..1f0ebac3 100755
> > > --- a/tests/xfs/014
> > > +++ b/tests/xfs/014
> > > @@ -33,27 +33,36 @@ _cleanup()
> > >  # failure.
> > >  _spec_prealloc_file()
> > >  {
> > > -	file=$1
> > > +	local file=$1
> > > +	local prealloc_size=0
> > > +	local i=0
> > >  
> > > -	rm -f $file
> > > +	# Now that we have background garbage collection processes that can be
> > > +	# triggered by low space/quota conditions, it's possible that we won't
> > > +	# succeed in creating a speculative preallocation on the first try.
> > > +	for ((tries = 0; tries < 5 && prealloc_size == 0; tries++)); do
> > > +		rm -f $file
> > >  
> > > -	# a few file extending open-write-close cycles should be enough to
> > > -	# trigger the fs to retain preallocation. write 256k in 32k intervals to
> > > -	# be sure
> > > -	for i in $(seq 0 32768 262144); do
> > > -		$XFS_IO_PROG -f -c "pwrite $i 32k" $file >> $seqres.full
> > > +		# a few file extending open-write-close cycles should be enough
> > > +		# to trigger the fs to retain preallocation. write 256k in 32k
> > > +		# intervals to be sure
> > > +		for i in $(seq 0 32768 262144); do
> > > +			$XFS_IO_PROG -f -c "pwrite $i 32k" $file >> $seqres.full
> > > +		done
> > > +
> > > +		# write a 4k aligned amount of data to keep the calculations
> > > +		# simple
> > > +		$XFS_IO_PROG -c "pwrite 0 128m" $file >> $seqres.full
> > > +
> > > +		size=`_get_filesize $file`
> > > +		blocks=`stat -c "%b" $file`
> > > +		blocksize=`stat -c "%B" $file`
> > > +
> > > +		prealloc_size=$((blocks * blocksize - size))
> > 
> > So we only try same pwrite operations 5 times, and only check the prealloc_size after 5
> > times done? Should we break from this loop once prealloc_size > 0?
> 
> The second clause of the for loop tests for that, does it not?

Oh, yes, sorry I missed the "&& prealloc_size == 0", I thought you just gave it 5 tries :)
So this patch is good to me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

> 
> --D
> 
> > 
> > Thanks,
> > Zorro
> > 
> > >  	done
> > >  
> > > -	# write a 4k aligned amount of data to keep the calculations simple
> > > -	$XFS_IO_PROG -c "pwrite 0 128m" $file >> $seqres.full
> > > -
> > > -	size=`_get_filesize $file`
> > > -	blocks=`stat -c "%b" $file`
> > > -	blocksize=`stat -c "%B" $file`
> > > -
> > > -	prealloc_size=$((blocks * blocksize - size))
> > >  	if [ $prealloc_size -eq 0 ]; then
> > > -		echo "Warning: No speculative preallocation for $file." \
> > > +		echo "Warning: No speculative preallocation for $file after $tries iterations." \
> > >  			"Check use of the allocsize= mount option."
> > >  	fi
> > >  
> > > 
> > 
>
diff mbox series

Patch

diff --git a/tests/xfs/014 b/tests/xfs/014
index a605b359..1f0ebac3 100755
--- a/tests/xfs/014
+++ b/tests/xfs/014
@@ -33,27 +33,36 @@  _cleanup()
 # failure.
 _spec_prealloc_file()
 {
-	file=$1
+	local file=$1
+	local prealloc_size=0
+	local i=0
 
-	rm -f $file
+	# Now that we have background garbage collection processes that can be
+	# triggered by low space/quota conditions, it's possible that we won't
+	# succeed in creating a speculative preallocation on the first try.
+	for ((tries = 0; tries < 5 && prealloc_size == 0; tries++)); do
+		rm -f $file
 
-	# a few file extending open-write-close cycles should be enough to
-	# trigger the fs to retain preallocation. write 256k in 32k intervals to
-	# be sure
-	for i in $(seq 0 32768 262144); do
-		$XFS_IO_PROG -f -c "pwrite $i 32k" $file >> $seqres.full
+		# a few file extending open-write-close cycles should be enough
+		# to trigger the fs to retain preallocation. write 256k in 32k
+		# intervals to be sure
+		for i in $(seq 0 32768 262144); do
+			$XFS_IO_PROG -f -c "pwrite $i 32k" $file >> $seqres.full
+		done
+
+		# write a 4k aligned amount of data to keep the calculations
+		# simple
+		$XFS_IO_PROG -c "pwrite 0 128m" $file >> $seqres.full
+
+		size=`_get_filesize $file`
+		blocks=`stat -c "%b" $file`
+		blocksize=`stat -c "%B" $file`
+
+		prealloc_size=$((blocks * blocksize - size))
 	done
 
-	# write a 4k aligned amount of data to keep the calculations simple
-	$XFS_IO_PROG -c "pwrite 0 128m" $file >> $seqres.full
-
-	size=`_get_filesize $file`
-	blocks=`stat -c "%b" $file`
-	blocksize=`stat -c "%B" $file`
-
-	prealloc_size=$((blocks * blocksize - size))
 	if [ $prealloc_size -eq 0 ]; then
-		echo "Warning: No speculative preallocation for $file." \
+		echo "Warning: No speculative preallocation for $file after $tries iterations." \
 			"Check use of the allocsize= mount option."
 	fi