All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] mmap16: extend checkpoint wait time during fs fill
@ 2017-02-10 13:38 Jan Stancek
  2017-02-13 10:02 ` Cyril Hrubis
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Stancek @ 2017-02-10 13:38 UTC (permalink / raw)
  To: ltp

I'm seeing this test rarely failing on slower systems:
mmap16      0  TINFO  :  Using test device LTP_DEV='/dev/loop0'
mmap16      0  TINFO  :  Formatting /dev/loop0 with ext4 opts='-b 1024' extra opts='10240'
mmap16      1  TBROK  :  tst_checkpoint.c:136: mmap16.c:223: tst_checkpoint_wait(0, 10000): errno=ETIMEDOUT(110): Connection timed out
mmap16      2  TBROK  :  tst_checkpoint.c:136: Remaining cases broken
mmap16      1  TBROK  :  tst_checkpoint.c:149: mmap16.c:125: tst_checkpoint_wake(0, 1, 10000): errno=ETIMEDOUT(110): Connection timed out
mmap16      2  TBROK  :  tst_checkpoint.c:149: Remaining cases broken

duration=52 termination_type=exited termination_id=2 corefile=no
cutime=0 cstime=132

What appears to be happening is that filling entire fs with 1024 chunks
takes longer than 10 seconds, so child hits TBROK while waiting for
parent. Parent eventually finishes, but then is unable to wake
child, which already finished.

Based on output above it looks like it took ~30 seconds to fill the fs
and then we hit 2 timeouts, making total of ~50 seconds. This patch
extends this particular checkpoint wait to 60 seconds.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/mmap/mmap16.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/mmap/mmap16.c b/testcases/kernel/syscalls/mmap/mmap16.c
index 3f2676bdc2d2..b3f51fd18c92 100644
--- a/testcases/kernel/syscalls/mmap/mmap16.c
+++ b/testcases/kernel/syscalls/mmap/mmap16.c
@@ -220,7 +220,11 @@ static void do_child(void)
 	 * kernel writepage is called, that means data corruption.
 	 */
 	TST_SAFE_CHECKPOINT_WAKE(NULL, 0);
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+
+	/* Give parent enough time to consume FS free blocks */
+	if (tst_checkpoint_wait(0, 60*1000))
+		tst_brkm(TBROK | TERRNO, cleanup,
+			"wait for parent to fill entire space");
 
 	for (offset = FS_BLOCKSIZE; offset < page_size; offset += FS_BLOCKSIZE)
 		addr[offset] = 'a';
-- 
1.8.3.1


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

* [LTP] [PATCH] mmap16: extend checkpoint wait time during fs fill
  2017-02-10 13:38 [LTP] [PATCH] mmap16: extend checkpoint wait time during fs fill Jan Stancek
@ 2017-02-13 10:02 ` Cyril Hrubis
  2017-02-13 13:33   ` Jan Stancek
  0 siblings, 1 reply; 3+ messages in thread
From: Cyril Hrubis @ 2017-02-13 10:02 UTC (permalink / raw)
  To: ltp

Hi!
> +
> +	/* Give parent enough time to consume FS free blocks */
> +	if (tst_checkpoint_wait(0, 60*1000))
> +		tst_brkm(TBROK | TERRNO, cleanup,
> +			"wait for parent to fill entire space");

Hrm, I wonder why I hardcoded the timeout into the library. It would be
much better if we could have used the safe variant here.

What about we add TST_SAFE_CHECKPOINT_WAIT2() that would take a timeout
parameter. Pretty much the same as we do for WAKE2 that takes a number
of processes to wake as a paramter.

Something like this:

diff --git a/include/old/old_checkpoint.h b/include/old/old_checkpoint.h
index 37bdf92..c8ffc92 100644
--- a/include/old/old_checkpoint.h
+++ b/include/old/old_checkpoint.h
@@ -41,7 +41,10 @@
 	tst_checkpoint_init(__FILE__, __LINE__, cleanup_fn)
 
 #define TST_SAFE_CHECKPOINT_WAIT(cleanup_fn, id) \
-        tst_safe_checkpoint_wait(__FILE__, __LINE__, cleanup_fn, id);
+        tst_safe_checkpoint_wait(__FILE__, __LINE__, cleanup_fn, id, 0);
+
+#define TST_SAFE_CHECKPOINT_WAIT2(cleanup_fn, id, msec_timeout) \
+        tst_safe_checkpoint_wait(__FILE__, __LINE__, cleanup_fn, id, msec_timeout);
 
 #define TST_SAFE_CHECKPOINT_WAKE(cleanup_fn, id) \
         tst_safe_checkpoint_wake(__FILE__, __LINE__, cleanup_fn, id, 1);
@@ -51,6 +54,6 @@
 
 #define TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(cleanup_fn, id) \
         tst_safe_checkpoint_wake(__FILE__, __LINE__, cleanup_fn, id, 1); \
-        tst_safe_checkpoint_wait(__FILE__, __LINE__, cleanup_fn, id);
+        tst_safe_checkpoint_wait(__FILE__, __LINE__, cleanup_fn, id, 0);
 
 #endif /* OLD_CHECKPOINT__ */
diff --git a/include/tst_checkpoint.h b/include/tst_checkpoint.h
index ae5a68b..e0dc1a5 100644
--- a/include/tst_checkpoint.h
+++ b/include/tst_checkpoint.h
@@ -21,7 +21,10 @@
 #include "tst_checkpoint_fn.h"
 
 #define TST_CHECKPOINT_WAIT(id) \
-        tst_safe_checkpoint_wait(__FILE__, __LINE__, NULL, id);
+        tst_safe_checkpoint_wait(__FILE__, __LINE__, NULL, id, 0);
+
+#define TST_CHECKPOINT_WAIT2(id, msec_timeout) \
+        tst_safe_checkpoint_wait(__FILE__, __LINE__, NULL, id, msec_timeout);
 
 #define TST_CHECKPOINT_WAKE(id) \
         tst_safe_checkpoint_wake(__FILE__, __LINE__, NULL, id, 1);
diff --git a/include/tst_checkpoint_fn.h b/include/tst_checkpoint_fn.h
index ebfd22b..0730fb0 100644
--- a/include/tst_checkpoint_fn.h
+++ b/include/tst_checkpoint_fn.h
@@ -45,7 +45,8 @@ int tst_checkpoint_wake(unsigned int id, unsigned int nr_wake,
                         unsigned int msec_timeout);
 
 void tst_safe_checkpoint_wait(const char *file, const int lineno,
-                              void (*cleanup_fn)(void), unsigned int id);
+                              void (*cleanup_fn)(void), unsigned int id,
+			      unsigned int msec_timeout);
 
 void tst_safe_checkpoint_wake(const char *file, const int lineno,
                               void (*cleanup_fn)(void), unsigned int id,
diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 5d12ac9..a23bd02 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -126,14 +126,20 @@ int tst_checkpoint_wake(unsigned int id, unsigned int nr_wake,
 }
 
 void tst_safe_checkpoint_wait(const char *file, const int lineno,
-                              void (*cleanup_fn)(void), unsigned int id)
+                              void (*cleanup_fn)(void), unsigned int id,
+			      unsigned int msec_timeout)
 {
-	int ret = tst_checkpoint_wait(id, DEFAULT_MSEC_TIMEOUT);
+	int ret;
+
+	if (!msec_timeout)
+		msec_timeout = DEFAULT_MSEC_TIMEOUT;
+
+	ret = tst_checkpoint_wait(id, msec_timeout);
 
 	if (ret) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 		         "%s:%d: tst_checkpoint_wait(%u, %i)",
-		         file, lineno, id, DEFAULT_MSEC_TIMEOUT);
+		         file, lineno, id, msec_timeout);
 	}
 }
 
-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH] mmap16: extend checkpoint wait time during fs fill
  2017-02-13 10:02 ` Cyril Hrubis
@ 2017-02-13 13:33   ` Jan Stancek
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Stancek @ 2017-02-13 13:33 UTC (permalink / raw)
  To: ltp


----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Monday, 13 February, 2017 11:02:34 AM
> Subject: Re: [LTP] [PATCH] mmap16: extend checkpoint wait time during fs fill
> 
> Hi!
> > +
> > +	/* Give parent enough time to consume FS free blocks */
> > +	if (tst_checkpoint_wait(0, 60*1000))
> > +		tst_brkm(TBROK | TERRNO, cleanup,
> > +			"wait for parent to fill entire space");
> 
> Hrm, I wonder why I hardcoded the timeout into the library. It would be
> much better if we could have used the safe variant here.
> 
> What about we add TST_SAFE_CHECKPOINT_WAIT2() that would take a timeout
> parameter. Pretty much the same as we do for WAKE2 that takes a number
> of processes to wake as a paramter.

Fine by me. Can you push it?
I can rebase & push patch using new TST_SAFE_CHECKPOINT_WAIT2().

Regards,
Jan

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

end of thread, other threads:[~2017-02-13 13:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-10 13:38 [LTP] [PATCH] mmap16: extend checkpoint wait time during fs fill Jan Stancek
2017-02-13 10:02 ` Cyril Hrubis
2017-02-13 13:33   ` Jan Stancek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.