[PATCH 0/3] Some fsx improvements

[PATCH 0/3] Some fsx improvements

[Josef Bacik]

I was fixing a i_size problem in btrfs and I used fsx to reproduce the issue,
but I needed to make a few changes to actually reproduce the problem.  The
individual explanations are in the patch changelogs themselves.  One thing that
may trip people up is that I've done system("echo 3 > /proc/sys/vm/drop_caches")
for dropping caches, but this is consistent with several other tools we have in
fstests.  These are all small and straightforward fixes, and don't really affect
any of the other tests.  The only exception is generic/521 which is a soak test,
I figure we want that thing to cover as much ground as possible, so enabling the
close+open operation is desired there.  Thanks,

Josef

[PATCH 1/3] ltp/fsx: do size check after closeopen operation

[Josef Bacik]

I was running down a i_size problem and was missing the failure until
the next iteration of fsx operations because we do the file size check
_after_ the closeopen operation.  Move it after the closeopen operation
so we can catch problems where the file gets messed up on disk.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 ltp/fsx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 00001117..c74b13c2 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -2211,10 +2211,10 @@ have_op:
 		check_contents();
 
 out:
-	if (sizechecks && testcalls > simulatedopcount)
-		check_size();
 	if (closeopen)
 		docloseopen();
+	if (sizechecks && testcalls > simulatedopcount)
+		check_size();
 	return 1;
 }
 
-- 
2.23.0

[PATCH 2/3] ltp/fsx: drop caches if we're doing closeopen

[Josef Bacik]

fsx has a closeopen option where it will close and then re-open the
file.  This is handy, but what is really more useful is to drop the file
from cache completely, so add a drop_caches into this operation so that
the file is read back completely from disk to be really evil.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 ltp/fsx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index c74b13c2..e519367b 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1778,6 +1778,10 @@ docloseopen(void)
 		prterr("docloseopen: close");
 		report_failure(180);
 	}
+	if (system("echo 3 > /proc/sys/vm/drop_caches")) {
+		prterr("docloseopen: drop_caches");
+		report_failure(213);
+	}
 	fd = open(fname, O_RDWR|o_direct, 0);
 	if (fd < 0) {
 		prterr("docloseopen: open");
-- 
2.23.0

[PATCH 3/3] generic/521: add close+open operations to the fsx run

[Josef Bacik]

I was fixing a issue with i_size setting in btrfs and generic/521 was
what I used to reproduce the problem.  However I needed the close+open
operation to trigger the issue.  This is a soak test, so add this
option to increase the coverage of this test.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/generic/521 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/521 b/tests/generic/521
index e8bc36e4..f0fc575e 100755
--- a/tests/generic/521
+++ b/tests/generic/521
@@ -52,6 +52,7 @@ fsx_args+=(-r $min_dio_sz)
 fsx_args+=(-t $min_dio_sz)
 fsx_args+=(-w $min_dio_sz)
 fsx_args+=(-Z)
+fsx_args+=(-c 10)
 
 run_fsx "${fsx_args[@]}" | sed -e '/^fsx.*/d'
 
-- 
2.23.0

Re: [PATCH 1/3] ltp/fsx: do size check after closeopen operation

[Eryu Guan]

On Tue, Jan 07, 2020 at 11:55:40AM -0500, Josef Bacik wrote:
> I was running down a i_size problem and was missing the failure until
> the next iteration of fsx operations because we do the file size check
> _after_ the closeopen operation.  Move it after the closeopen operation

I think you mean "_before_" here? And that's why we move it _after_
closeopen?

Thanks,
Eryu

> so we can catch problems where the file gets messed up on disk.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  ltp/fsx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 00001117..c74b13c2 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -2211,10 +2211,10 @@ have_op:
>  		check_contents();
>  
>  out:
> -	if (sizechecks && testcalls > simulatedopcount)
> -		check_size();
>  	if (closeopen)
>  		docloseopen();
> +	if (sizechecks && testcalls > simulatedopcount)
> +		check_size();
>  	return 1;
>  }
>  
> -- 
> 2.23.0
> 

Re: [PATCH 3/3] generic/521: add close+open operations to the fsx run

[Eryu Guan]

On Tue, Jan 07, 2020 at 11:55:42AM -0500, Josef Bacik wrote:
> I was fixing a issue with i_size setting in btrfs and generic/521 was
> what I used to reproduce the problem.  However I needed the close+open
> operation to trigger the issue.  This is a soak test, so add this
> option to increase the coverage of this test.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  tests/generic/521 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/521 b/tests/generic/521
> index e8bc36e4..f0fc575e 100755
> --- a/tests/generic/521
> +++ b/tests/generic/521
> @@ -52,6 +52,7 @@ fsx_args+=(-r $min_dio_sz)
>  fsx_args+=(-t $min_dio_sz)
>  fsx_args+=(-w $min_dio_sz)
>  fsx_args+=(-Z)
> +fsx_args+=(-c 10)

This looks fine to me, but my only concern is that this floods dmesg
because every drop cache records a dmesg info, and the useful part of
dmesg may be lost. How about "-c 10000"? As the default op number is 1
million, with "-c 10000" we only have 100 dmesg entries. But I'm not
sure if that's enough for you to reproduce the bug.

Thanks,
Eryu

>  
>  run_fsx "${fsx_args[@]}" | sed -e '/^fsx.*/d'
>  
> -- 
> 2.23.0
>