[PATCH] generic/70: Use scratch device instead of test.

[PATCH] generic/70: Use scratch device instead of test.

[Nikolay Borisov]

At the end of the test the fsstress work dir is deleted, this suggests
the test really requires a scratch device and not a test device. Change
it accordingly.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 tests/generic/070 | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/generic/070 b/tests/generic/070
index 678344fa011d..c98f445ae4b8 100755
--- a/tests/generic/070
+++ b/tests/generic/070
@@ -13,7 +13,6 @@ _begin_fstest attr udf auto quick stress
 _cleanup()
 {
     cd /
-    rm -rf $TEST_DIR/fsstress
     rm -f $tmp.*
 }
 
@@ -24,11 +23,14 @@ _cleanup()
 # real QA test starts here
 _supported_fs generic
 
-_require_test
+_require_scratch
 _require_attrs
 
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount
+
 FSSTRESS_ARGS=`_scale_fsstress_args \
-	-d $TEST_DIR/fsstress \
+	-d $SCRATCH_MNT \
 	-f allocsp=0 \
 	-f freesp=0 \
 	-f bulkstat=0 \
@@ -40,5 +42,7 @@ FSSTRESS_ARGS=`_scale_fsstress_args \
         -p 1 -n 10000 -S c`
 $FSSTRESS_PROG $FSSTRESS_ARGS >$seqres.full 2>&1
 
+_scratch_unmount
+
 status=$?
 exit
-- 
2.17.1

Re: [PATCH] generic/70: Use scratch device instead of test.

[Zorro Lang]

On Thu, Aug 19, 2021 at 04:13:55PM +0300, Nikolay Borisov wrote:
> At the end of the test the fsstress work dir is deleted, this suggests
> the test really requires a scratch device and not a test device. Change
> it accordingly.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---

Hmm... is there any more specific reasons to explain why have to change this
TEST_DIR to SCRATCH_MNT? For example, this test hit ENOSPC in TEST_DIR, or
it takes too much time to remove $TEST_DIR/fsstress in _cleanup, or use
SCRATCH_DEV can cover more things, and so on. I think more specific reason
can help to push this patch be merged. After all there's not a rule says
if a case don't need to keep its work dir, then must use SCRATCH_DEV. Except
the maintainer is more inclined to accept that :)

Thanks,
Zorro

>  tests/generic/070 | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/generic/070 b/tests/generic/070
> index 678344fa011d..c98f445ae4b8 100755
> --- a/tests/generic/070
> +++ b/tests/generic/070
> @@ -13,7 +13,6 @@ _begin_fstest attr udf auto quick stress
>  _cleanup()
>  {
>      cd /
> -    rm -rf $TEST_DIR/fsstress
>      rm -f $tmp.*
>  }
>  
> @@ -24,11 +23,14 @@ _cleanup()
>  # real QA test starts here
>  _supported_fs generic
>  
> -_require_test
> +_require_scratch
>  _require_attrs
>  
> +_scratch_mkfs >/dev/null 2>&1
> +_scratch_mount
> +
>  FSSTRESS_ARGS=`_scale_fsstress_args \
> -	-d $TEST_DIR/fsstress \
> +	-d $SCRATCH_MNT \
>  	-f allocsp=0 \
>  	-f freesp=0 \
>  	-f bulkstat=0 \
> @@ -40,5 +42,7 @@ FSSTRESS_ARGS=`_scale_fsstress_args \
>          -p 1 -n 10000 -S c`
>  $FSSTRESS_PROG $FSSTRESS_ARGS >$seqres.full 2>&1
>  
> +_scratch_unmount
> +
>  status=$?
>  exit
> -- 
> 2.17.1
> 

Re: [PATCH] generic/70: Use scratch device instead of test.

[Nikolay Borisov]

On 19.08.21 г. 18:00, Zorro Lang wrote:
> On Thu, Aug 19, 2021 at 04:13:55PM +0300, Nikolay Borisov wrote:
>> At the end of the test the fsstress work dir is deleted, this suggests
>> the test really requires a scratch device and not a test device. Change
>> it accordingly.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
> 
> Hmm... is there any more specific reasons to explain why have to change this
> TEST_DIR to SCRATCH_MNT? For example, this test hit ENOSPC in TEST_DIR, or
> it takes too much time to remove $TEST_DIR/fsstress in _cleanup, or use
> SCRATCH_DEV can cover more things, and so on. I think more specific reason
> can help to push this patch be merged. After all there's not a rule says
> if a case don't need to keep its work dir, then must use SCRATCH_DEV. Except
> the maintainer is more inclined to accept that :)

None of the reasons above, I just thought that stuff in $TEST_DEV lives
there so that it can simulate an 'aging' system. In this case this
doesn't seem to be the reason for it as we delete everything.

This patch might very well be moot.
> 
> Thanks,
> Zorro
> 
>>  tests/generic/070 | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/generic/070 b/tests/generic/070
>> index 678344fa011d..c98f445ae4b8 100755
>> --- a/tests/generic/070
>> +++ b/tests/generic/070
>> @@ -13,7 +13,6 @@ _begin_fstest attr udf auto quick stress
>>  _cleanup()
>>  {
>>      cd /
>> -    rm -rf $TEST_DIR/fsstress
>>      rm -f $tmp.*
>>  }
>>  
>> @@ -24,11 +23,14 @@ _cleanup()
>>  # real QA test starts here
>>  _supported_fs generic
>>  
>> -_require_test
>> +_require_scratch
>>  _require_attrs
>>  
>> +_scratch_mkfs >/dev/null 2>&1
>> +_scratch_mount
>> +
>>  FSSTRESS_ARGS=`_scale_fsstress_args \
>> -	-d $TEST_DIR/fsstress \
>> +	-d $SCRATCH_MNT \
>>  	-f allocsp=0 \
>>  	-f freesp=0 \
>>  	-f bulkstat=0 \
>> @@ -40,5 +42,7 @@ FSSTRESS_ARGS=`_scale_fsstress_args \
>>          -p 1 -n 10000 -S c`
>>  $FSSTRESS_PROG $FSSTRESS_ARGS >$seqres.full 2>&1
>>  
>> +_scratch_unmount
>> +
>>  status=$?
>>  exit
>> -- 
>> 2.17.1
>>
> 

Re: [PATCH] generic/70: Use scratch device instead of test.

[Theodore Ts'o]

On Thu, Aug 19, 2021 at 05:56:31PM +0300, Nikolay Borisov wrote:
> On 19.08.21 г. 18:00, Zorro Lang wrote:
> > On Thu, Aug 19, 2021 at 04:13:55PM +0300, Nikolay Borisov wrote:
> >> At the end of the test the fsstress work dir is deleted, this suggests
> >> the test really requires a scratch device and not a test device. Change
> >> it accordingly.
> >>
> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> >> ---
> > 
> > Hmm... is there any more specific reasons to explain why have to change this
> > TEST_DIR to SCRATCH_MNT? For example, this test hit ENOSPC in TEST_DIR, or
> > it takes too much time to remove $TEST_DIR/fsstress in _cleanup, or use
> > SCRATCH_DEV can cover more things, and so on. I think more specific reason
> > can help to push this patch be merged. After all there's not a rule says
> > if a case don't need to keep its work dir, then must use SCRATCH_DEV. Except
> > the maintainer is more inclined to accept that :)
> 
> None of the reasons above, I just thought that stuff in $TEST_DEV lives
> there so that it can simulate an 'aging' system. In this case this
> doesn't seem to be the reason for it as we delete everything.

Using SCRATCH_DEV requires that the file system be reformatted, since
it doesn't necessarily exist.  So that means extra flash writes, and
extra time to actually do the file system format.  In addition,
sometimes SCRATCH_DEV might not be present for some test environments.

Hence, unless you need to use SCRATCH_DEV (because the file system
needs to be formatted with specific mkfs options, etc.), I think the
preference should be to use TEST_DIR.  It doesn't *hurt* that we
create the work directory, in TEST_DIR and then delete it afterwawrds
--- where as using SCRATCH_DEV dosn't actually buy you anything, and
might actually slow the test down a tiny bit.

Cheers,

					- Ted

Re: [PATCH] generic/70: Use scratch device instead of test.

[Dave Chinner]

On Thu, Aug 19, 2021 at 05:56:31PM +0300, Nikolay Borisov wrote:
> 
> 
> On 19.08.21 г. 18:00, Zorro Lang wrote:
> > On Thu, Aug 19, 2021 at 04:13:55PM +0300, Nikolay Borisov wrote:
> >> At the end of the test the fsstress work dir is deleted, this suggests
> >> the test really requires a scratch device and not a test device. Change
> >> it accordingly.
> >>
> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> >> ---
> > 
> > Hmm... is there any more specific reasons to explain why have to change this
> > TEST_DIR to SCRATCH_MNT? For example, this test hit ENOSPC in TEST_DIR, or
> > it takes too much time to remove $TEST_DIR/fsstress in _cleanup, or use
> > SCRATCH_DEV can cover more things, and so on. I think more specific reason
> > can help to push this patch be merged. After all there's not a rule says
> > if a case don't need to keep its work dir, then must use SCRATCH_DEV. Except
> > the maintainer is more inclined to accept that :)
> 
> None of the reasons above, I just thought that stuff in $TEST_DEV lives
> there so that it can simulate an 'aging' system. In this case this
> doesn't seem to be the reason for it as we delete everything.

That's most certainly aging the filesystem. Just because the test
deletes everything it wrote doesn't mean that the filesystem is
unchanged after the test runs. e.g. log structured or COW filesytems
can have very different structures at the end of the test, even
though the current view of the filesystem appears unchanged...

IOWs, tests that don't require a specific filesystem configuration
should be run on the test device, not the scratch device...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com