FSTests Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] Some fsx improvements
@ 2020-01-07 16:55 Josef Bacik
  2020-01-07 16:55 ` [PATCH 1/3] ltp/fsx: do size check after closeopen operation Josef Bacik
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Josef Bacik @ 2020-01-07 16:55 UTC (permalink / raw)
  To: fstests, linux-btrfs

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] ltp/fsx: do size check after closeopen operation
  2020-01-07 16:55 [PATCH 0/3] Some fsx improvements Josef Bacik
@ 2020-01-07 16:55 ` Josef Bacik
  2020-02-01  7:42   ` Eryu Guan
  2020-01-07 16:55 ` [PATCH 2/3] ltp/fsx: drop caches if we're doing closeopen Josef Bacik
  2020-01-07 16:55 ` [PATCH 3/3] generic/521: add close+open operations to the fsx run Josef Bacik
  2 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-01-07 16:55 UTC (permalink / raw)
  To: fstests, linux-btrfs

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] ltp/fsx: drop caches if we're doing closeopen
  2020-01-07 16:55 [PATCH 0/3] Some fsx improvements Josef Bacik
  2020-01-07 16:55 ` [PATCH 1/3] ltp/fsx: do size check after closeopen operation Josef Bacik
@ 2020-01-07 16:55 ` Josef Bacik
  2020-01-07 16:55 ` [PATCH 3/3] generic/521: add close+open operations to the fsx run Josef Bacik
  2 siblings, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2020-01-07 16:55 UTC (permalink / raw)
  To: fstests, linux-btrfs

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] generic/521: add close+open operations to the fsx run
  2020-01-07 16:55 [PATCH 0/3] Some fsx improvements Josef Bacik
  2020-01-07 16:55 ` [PATCH 1/3] ltp/fsx: do size check after closeopen operation Josef Bacik
  2020-01-07 16:55 ` [PATCH 2/3] ltp/fsx: drop caches if we're doing closeopen Josef Bacik
@ 2020-01-07 16:55 ` Josef Bacik
  2020-02-01  8:06   ` Eryu Guan
  2 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-01-07 16:55 UTC (permalink / raw)
  To: fstests, linux-btrfs

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] ltp/fsx: do size check after closeopen operation
  2020-01-07 16:55 ` [PATCH 1/3] ltp/fsx: do size check after closeopen operation Josef Bacik
@ 2020-02-01  7:42   ` Eryu Guan
  0 siblings, 0 replies; 6+ messages in thread
From: Eryu Guan @ 2020-02-01  7:42 UTC (permalink / raw)
  To: Josef Bacik; +Cc: fstests, linux-btrfs

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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] generic/521: add close+open operations to the fsx run
  2020-01-07 16:55 ` [PATCH 3/3] generic/521: add close+open operations to the fsx run Josef Bacik
@ 2020-02-01  8:06   ` Eryu Guan
  0 siblings, 0 replies; 6+ messages in thread
From: Eryu Guan @ 2020-02-01  8:06 UTC (permalink / raw)
  To: Josef Bacik; +Cc: fstests, linux-btrfs

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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07 16:55 [PATCH 0/3] Some fsx improvements Josef Bacik
2020-01-07 16:55 ` [PATCH 1/3] ltp/fsx: do size check after closeopen operation Josef Bacik
2020-02-01  7:42   ` Eryu Guan
2020-01-07 16:55 ` [PATCH 2/3] ltp/fsx: drop caches if we're doing closeopen Josef Bacik
2020-01-07 16:55 ` [PATCH 3/3] generic/521: add close+open operations to the fsx run Josef Bacik
2020-02-01  8:06   ` Eryu Guan

FSTests Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/fstests/0 fstests/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 fstests fstests/ https://lore.kernel.org/fstests \
		fstests@vger.kernel.org
	public-inbox-index fstests

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.fstests


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git