[patch] testsuite: pthread_cond_wait.exp: Avoid a race

[patch] testsuite: pthread_cond_wait.exp: Avoid a race

[Jan Kratochvil]

Hi,

-PASS: gdb.threads/pthread_cond_wait.exp: backtrace in blocked thread
+FAIL: gdb.threads/pthread_cond_wait.exp: backtrace in blocked thread

-Thread 2 (Thread 0xf7dd6b40 (LWP 4893)):^M
+Thread 2 (Thread 0xf7dd6b40 (LWP 4754)):^M
 #0  0xf7fdc430 in __kernel_vsyscall ()^M
-#1  0xf7fc012c in pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S:172^M
-#2  0x08048683 in cond_wait (cond=0xf7dd6308, mut=0xf7dd6338) at gdb/testsuite/gdb.threads/pthread_cond_wait.c:28^M
-#3  0x080486ce in noreturn () at gdb/testsuite/gdb.threads/pthread_cond_wait.c:43^M
-#4  0x080486db in forever_pthread (unused=0x0) at gdb/testsuite/gdb.threads/pthread_cond_wait.c:49^M
-#5  0xf7fbcadf in start_thread (arg=0xf7dd6b40) at pthread_create.c:309^M
-#6  0xf7ecd42e in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:133^M
+#1  0xf7fc2dd2 in __lll_lock_wait_private () at ../nptl/sysdeps/unix/sysv/linux/i386/i486/lowlevellock.S:98^M
+#2  0xf7fbd7b2 in _L_lock_3175 () from /lib/libpthread.so.0^M
+#3  0xf7fbccc9 in start_thread (arg=0xf7dd6b40) at pthread_create.c:298^M
+#4  0xf7ecd42e in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:133^M
 ^M
-Thread 1 (Thread 0xf7dd76c0 (LWP 4745)):^M
+Thread 1 (Thread 0xf7dd76c0 (LWP 4628)):^M
 #0  break_me () at gdb/testsuite/gdb.threads/pthread_cond_wait.c:56^M
 #1  0x08048738 in main () at gdb/testsuite/gdb.threads/pthread_cond_wait.c:68^M
-(gdb) PASS: gdb.threads/pthread_cond_wait.exp: backtrace in blocked thread
+(gdb) FAIL: gdb.threads/pthread_cond_wait.exp: backtrace in blocked thread


It is just too racy.  I will check in the easy workaround below, it is sure not
a real fix.


Thanks,
Jan


gdb/testsuite/
2013-02-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.threads/pthread_cond_wait.c (main): Remove variable ts.  Replace
	nanosleep by sleep.

diff --git a/gdb/testsuite/gdb.threads/pthread_cond_wait.c b/gdb/testsuite/gdb.threads/pthread_cond_wait.c
index a639e41..3c28e13 100644
--- a/gdb/testsuite/gdb.threads/pthread_cond_wait.c
+++ b/gdb/testsuite/gdb.threads/pthread_cond_wait.c
@@ -59,12 +59,11 @@ int
 main (void)
 {
   pthread_t forever;
-  const struct timespec ts = { 0, 10000000 }; /* 0.01 sec */
 
   pthread_create (&forever, NULL, forever_pthread, NULL);
   for (;;)
     {
-      nanosleep (&ts, NULL);
+      sleep (2);
       break_me();
     }
 

Re: [patch] testsuite: pthread_cond_wait.exp: Avoid a race

[Joel Brobecker]

> It is just too racy.  I will check in the easy workaround below, it is
> sure not a real fix.

2 seconds feels much much much too long, however. I try to avoid
these types of delays, because collectively they make a significant
dent on the overall amount of time it takes to run the testsuite.

I am trying to think of a way to rewrite the program to avoid
the delay entirely, but I don't think it's possible, because
we need to the thread to run until it blocks in pthread_cond_wait.

Can we try a smaller value? For instance, if we went from 0.01
second to 0.1 seconds, we'd increase the time by a factor of 10.
In today's CPU times, 0.1 second enough to do a huge amount of
processing, no?

-- 
Joel

Re: [patch] testsuite: pthread_cond_wait.exp: Avoid a race

[Jan Kratochvil]

On Tue, 12 Feb 2013 18:23:01 +0100, Joel Brobecker wrote:
> > It is just too racy.  I will check in the easy workaround below, it is
> > sure not a real fix.
> 
> 2 seconds feels much much much too long, however.

2 seconds is a general delay standard in the testsuite so I do not find it
would hurt so much:
	$ grep 'sleep 2' */*.exp|wc -l
	20


> I try to avoid
> these types of delays, because collectively they make a significant
> dent on the overall amount of time it takes to run the testsuite.

The testsuite is already directory-parallelized and soon Tom should introduce
even a file-parallelization, so sleep should not hurt much.


> I am trying to think of a way to rewrite the program to avoid
> the delay entirely,

Yes, I understand the sleep is terribly but as it is so widespread in the
testsuite I just found it a "good enough" fix.  Sure I can keep it only among
my local non-upstreamed testsuite races fixes so that I can continue running
the daily regression testing:
	gdbserverasyncnonstop.patch
	watchpointfork2.patch
	gccpr4.patch
	valgrind-kill.patch


> but I don't think it's possible,

IMO there should be retrying.  If it does not find the symbol then resume it
for another 0.1 sec or even 1 sec as such case does not commonly happen.


> Can we try a smaller value?

No.  IMO even those 2 seconds is too long.  The machine has load about 20-30
when running the parallelized testsuites (I always run 3 at once) and I would
like to parallelize it even more if the testsuite already was not so fragily
gainst even higher loads.


Thanks,
Jan

Re: [patch] testsuite: pthread_cond_wait.exp: Avoid a race

[Joel Brobecker]

> 2 seconds is a general delay standard in the testsuite so I do not find it
> would hurt so much:
> 	$ grep 'sleep 2' */*.exp|wc -l
> 	20

OK, it will be fine, I suppose, especially if we can file-parallelize
the testsuite.

-- 
Joel

Re: [patch] testsuite: pthread_cond_wait.exp: Avoid a race

[Tom Tromey]

Joel> OK, it will be fine, I suppose, especially if we can file-parallelize
Joel> the testsuite.

I have it working -- not in my totally ideal form, but I'm reaching
diminishing returns and also boredom.  I need to look into some
remote-testing things, and also write up an RFC about changes to the
tests.  I keep postponing this, mostly due to unrelated things, but I'll
get to it sooner rather than later.

Anyway, as part of this, I looked at test suite timings.  I ran each
.exp file separately and looked at the elapsed time it took.

Some interesting bits:

octave:3> mean(Tdata)
ans =  1.1680
octave:4> min(Tdata)
ans =  0.070000
octave:5> max(Tdata)
ans =  80.780
octave:6> std(Tdata)
ans =  4.1273

So, most tests are quite fast, but we have some huge outliers.  The 80
second one is break-interp.exp, but there are others, see appended.

Crunching the numbers, something I'm not totally confident I did
correctly, says that we can scale the test suite up to about -j12.
After that break-interp will start dominating the time.

Tom

All times further than 2 standard deviations from the mean:

octave> sort(Tdata(find(Tdata > mean(Tdata) + 2*std(Tdata))), 'descend')
ans =

   80.7800
   60.4900
   40.2700
   34.3500
   22.0500
   20.0000
   19.8900
   19.6500
   19.3000
   15.4500
   14.7300
   14.5400
   13.5700
   10.2300
    9.9100
    9.7000

[commit] [patch] testsuite: pthread_cond_wait.exp: Avoid a race

[Jan Kratochvil]

On Tue, 12 Feb 2013 18:38:55 +0100, Joel Brobecker wrote:
> > 2 seconds is a general delay standard in the testsuite so I do not find it
> > would hurt so much:
> > 	$ grep 'sleep 2' */*.exp|wc -l
> > 	20
> 
> OK, it will be fine, I suppose, especially if we can file-parallelize
> the testsuite.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-02/msg00102.html


Thanks,
Jan

Re: [patch] testsuite: pthread_cond_wait.exp: Avoid a race

[Jan Kratochvil]

On Tue, 12 Feb 2013 18:29:50 +0100, Jan Kratochvil wrote:
> > Can we try a smaller value?
> 
> No.  IMO even those 2 seconds is too long.  The machine has load about 20-30
                                   too short
> when running the parallelized testsuites (I always run 3 at once) and I would
> like to parallelize it even more if the testsuite already was not so fragily
> gainst even higher loads.