[PATCH] xfstests 251: fix fitrim support test

[PATCH] xfstests 251: fix fitrim support test

[Eric Sandeen]

On my ext4 filesystem, the simple "did fstrim work" test passes,
because it asks to free all blocks in the first 10m of the fs,
and those 10m are full of filesystem metadata.  Because no blocks
are free, no blocks are trimmed, and we get success returned.

But then when the test runs I'm flooded with error messages, because
it's a hard drive not an ssd...

So we need to step through the fs until we either free a block,
or encounter an error.

I think this is ugly bash, if anyone has a better plan I'm all ears.

(also change FSTRIM to FITRIM in the failure message, it seems
to be intended to print the ioctl name ...)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/251 b/251
index fa3d74a..5ab0a87 100755
--- a/251
+++ b/251
@@ -73,7 +73,19 @@ _fail()
 
 _check_fstrim_support()
 {
-	$here/src/fstrim -l 10M $SCRATCH_MNT &> /dev/null
+	# Go until error or until something gets trimmed
+	step=1048576
+	start=0
+	retval=0
+	nonetrimmed=1
+
+	while [ $retval -eq 0 ] && [ $nonetrimmed -ne 0 ]; do
+		result=`$here/src/fstrim -v -s $start -l $step $SCRATCH_MNT 2>&1`
+		retval=$?
+		[ "${result:0:1}" -eq "0" ] && nonetrimmed=1
+		start=$(( $start + $step ))
+	done
+	return $retval
 }
 
 ##
diff --git a/src/fstrim.c b/src/fstrim.c
index f1f37ec..ad7fd6a 100644
--- a/src/fstrim.c
+++ b/src/fstrim.c
@@ -236,7 +236,7 @@ int main(int argc, char **argv)
 	}
 
 	if (ioctl(fd, FITRIM, opts->range)) {
-		fprintf(stderr, "%s: FSTRIM: %s\n", program_name,
+		fprintf(stderr, "%s: FITRIM %s\n", program_name,
 			strerror(errno));
 		goto free_opts;
 	}

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests 251: fix fitrim support test

[Lukas Czerner]

On Fri, 13 May 2011, Eric Sandeen wrote:

> On my ext4 filesystem, the simple "did fstrim work" test passes,
> because it asks to free all blocks in the first 10m of the fs,
> and those 10m are full of filesystem metadata.  Because no blocks
> are free, no blocks are trimmed, and we get success returned.
> 
> But then when the test runs I'm flooded with error messages, because
> it's a hard drive not an ssd...
> 
> So we need to step through the fs until we either free a block,
> or encounter an error.
> 
> I think this is ugly bash, if anyone has a better plan I'm all ears.
> 
> (also change FSTRIM to FITRIM in the failure message, it seems
> to be intended to print the ioctl name ...)

Hi Eric,

this is actually a filesystem bug found unintentionally by this test :)
and it is already fixed upstream 4143179218960a70d821a425e3c23ce44aa93dee
for ext4.. So I think we better leave it as it is, since this is
unwanted behaviour and should be detected.

What we should fix however, is when fstrim fails after successful fist
test, so the test exits and report failure, rather than printing tons of
error messages.

Thanks!
-Lukas

> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/251 b/251
> index fa3d74a..5ab0a87 100755
> --- a/251
> +++ b/251
> @@ -73,7 +73,19 @@ _fail()
>  
>  _check_fstrim_support()
>  {
> -	$here/src/fstrim -l 10M $SCRATCH_MNT &> /dev/null
> +	# Go until error or until something gets trimmed
> +	step=1048576
> +	start=0
> +	retval=0
> +	nonetrimmed=1
> +
> +	while [ $retval -eq 0 ] && [ $nonetrimmed -ne 0 ]; do
> +		result=`$here/src/fstrim -v -s $start -l $step $SCRATCH_MNT 2>&1`
> +		retval=$?
> +		[ "${result:0:1}" -eq "0" ] && nonetrimmed=1
> +		start=$(( $start + $step ))
> +	done
> +	return $retval
>  }
>  
>  ##
> diff --git a/src/fstrim.c b/src/fstrim.c
> index f1f37ec..ad7fd6a 100644
> --- a/src/fstrim.c
> +++ b/src/fstrim.c
> @@ -236,7 +236,7 @@ int main(int argc, char **argv)
>  	}
>  
>  	if (ioctl(fd, FITRIM, opts->range)) {
> -		fprintf(stderr, "%s: FSTRIM: %s\n", program_name,
> +		fprintf(stderr, "%s: FITRIM %s\n", program_name,
>  			strerror(errno));
>  		goto free_opts;
>  	}
> 

-- 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests 251: fix fitrim support test

[Eric Sandeen]

On 5/16/11 4:19 AM, Lukas Czerner wrote:
> On Fri, 13 May 2011, Eric Sandeen wrote:
> 
>> On my ext4 filesystem, the simple "did fstrim work" test passes,
>> because it asks to free all blocks in the first 10m of the fs,
>> and those 10m are full of filesystem metadata.  Because no blocks
>> are free, no blocks are trimmed, and we get success returned.
>>
>> But then when the test runs I'm flooded with error messages, because
>> it's a hard drive not an ssd...
>>
>> So we need to step through the fs until we either free a block,
>> or encounter an error.
>>
>> I think this is ugly bash, if anyone has a better plan I'm all ears.
>>
>> (also change FSTRIM to FITRIM in the failure message, it seems
>> to be intended to print the ioctl name ...)
> 
> Hi Eric,
> 
> this is actually a filesystem bug found unintentionally by this test :)
> and it is already fixed upstream 4143179218960a70d821a425e3c23ce44aa93dee
> for ext4.. So I think we better leave it as it is, since this is
> unwanted behaviour and should be detected.

Oh, of course.  Makes much more sense, thanks!



> What we should fix however, is when fstrim fails after successful fist
> test, so the test exits and report failure, rather than printing tons of
> error messages.

That'd be easy enough ...

-Eric
 
> Thanks!
> -Lukas
> 
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/251 b/251
>> index fa3d74a..5ab0a87 100755
>> --- a/251
>> +++ b/251
>> @@ -73,7 +73,19 @@ _fail()
>>  
>>  _check_fstrim_support()
>>  {
>> -	$here/src/fstrim -l 10M $SCRATCH_MNT &> /dev/null
>> +	# Go until error or until something gets trimmed
>> +	step=1048576
>> +	start=0
>> +	retval=0
>> +	nonetrimmed=1
>> +
>> +	while [ $retval -eq 0 ] && [ $nonetrimmed -ne 0 ]; do
>> +		result=`$here/src/fstrim -v -s $start -l $step $SCRATCH_MNT 2>&1`
>> +		retval=$?
>> +		[ "${result:0:1}" -eq "0" ] && nonetrimmed=1
>> +		start=$(( $start + $step ))
>> +	done
>> +	return $retval
>>  }
>>  
>>  ##
>> diff --git a/src/fstrim.c b/src/fstrim.c
>> index f1f37ec..ad7fd6a 100644
>> --- a/src/fstrim.c
>> +++ b/src/fstrim.c
>> @@ -236,7 +236,7 @@ int main(int argc, char **argv)
>>  	}
>>  
>>  	if (ioctl(fd, FITRIM, opts->range)) {
>> -		fprintf(stderr, "%s: FSTRIM: %s\n", program_name,
>> +		fprintf(stderr, "%s: FITRIM %s\n", program_name,
>>  			strerror(errno));
>>  		goto free_opts;
>>  	}
>>
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs