|
From: Florian K. <br...@ac...> - 2011-07-27 20:30:01
|
Hi Bart,
there is currently a compile failure on older system z machines
because FUTEX_PRIVATE_FLAG is not defined anywhere.
The patch below fixes it.
What do you think?
Florian
Index: drd/drd_pthread_intercepts.c
===================================================================
--- drd/drd_pthread_intercepts.c (revision 11924)
+++ drd/drd_pthread_intercepts.c (working copy)
@@ -188,7 +188,7 @@
static void DRD_(sema_down)(DrdSema* sema)
{
while (sema->counter == 0) {
-#ifdef __linux__
+#if defined(__linux__) && defined(FUTEX_PRIVATE_FLAG)
syscall(__NR_futex, (UWord)&sema->counter,
FUTEX_WAIT | FUTEX_PRIVATE_FLAG, 0);
#else
@@ -201,7 +201,7 @@
static void DRD_(sema_up)(DrdSema* sema)
{
sema->counter++;
-#ifdef __linux__
+#if defined(__linux__) && defined(FUTEX_PRIVATE_FLAG)
syscall(__NR_futex, (UWord)&sema->counter,
FUTEX_WAKE | FUTEX_PRIVATE_FLAG, 1);
#endif
|
|
From: Bart V. A. <bva...@ac...> - 2011-07-28 09:23:45
|
On Wed, Jul 27, 2011 at 10:29 PM, Florian Krohm <br...@ac...> wrote:
> there is currently a compile failure on older system z machines
> because FUTEX_PRIVATE_FLAG is not defined anywhere.
> The patch below fixes it.
> What do you think?
>
> Florian
>
>
> Index: drd/drd_pthread_intercepts.c
> ===================================================================
> --- drd/drd_pthread_intercepts.c (revision 11924)
> +++ drd/drd_pthread_intercepts.c (working copy)
> @@ -188,7 +188,7 @@
> static void DRD_(sema_down)(DrdSema* sema)
> {
> while (sema->counter == 0) {
> -#ifdef __linux__
> +#if defined(__linux__) && defined(FUTEX_PRIVATE_FLAG)
> syscall(__NR_futex, (UWord)&sema->counter,
> FUTEX_WAIT | FUTEX_PRIVATE_FLAG, 0);
> #else
> @@ -201,7 +201,7 @@
> static void DRD_(sema_up)(DrdSema* sema)
> {
> sema->counter++;
> -#ifdef __linux__
> +#if defined(__linux__) && defined(FUTEX_PRIVATE_FLAG)
> syscall(__NR_futex, (UWord)&sema->counter,
> FUTEX_WAKE | FUTEX_PRIVATE_FLAG, 1);
> #endif
I'd rather prefer a patch like the (untested) patch below:
Index: drd/drd_pthread_intercepts.c
===================================================================
--- drd/drd_pthread_intercepts.c (revision 11922)
+++ drd/drd_pthread_intercepts.c (working copy)
@@ -49,6 +49,7 @@
#endif
#include <assert.h> /* assert() */
+#include <errno.h>
#include <pthread.h> /* pthread_mutex_t */
#include <semaphore.h> /* sem_t */
#include <stdint.h> /* uintptr_t */
@@ -58,7 +59,10 @@
#ifdef __linux__
#include <asm/unistd.h> /* __NR_futex */
#include <linux/futex.h> /* FUTEX_WAIT */
+#ifndef FUTEX_PRIVATE_FLAG
+#define FUTEX_PRIVATE_FLAG 0
#endif
+#endif
#include "config.h" /* HAVE_PTHREAD_MUTEX_ADAPTIVE_NP etc. */
#include "drd_basics.h" /* DRD_() */
#include "drd_clientreq.h"
@@ -187,13 +191,18 @@
static void DRD_(sema_down)(DrdSema* sema)
{
+ int res = ENOSYS;
+
while (sema->counter == 0) {
-#ifdef __linux__
- syscall(__NR_futex, (UWord)&sema->counter,
- FUTEX_WAIT | FUTEX_PRIVATE_FLAG, 0);
-#else
- sched_yield();
+#if defined(__linux__) && defined(__NR_futex)
+ if (syscall(__NR_futex, (UWord)&sema->counter,
+ FUTEX_WAIT | FUTEX_PRIVATE_FLAG, 0) == 0)
+ res = 0;
+ else
+ res = errno;
#endif
+ if (res != 0 && res != EWOULDBLOCK)
+ sched_yield();
}
sema->counter--;
}
@@ -201,7 +210,7 @@
static void DRD_(sema_up)(DrdSema* sema)
{
sema->counter++;
-#ifdef __linux__
+#if defined(__linux__) && defined(__NR_futex)
syscall(__NR_futex, (UWord)&sema->counter,
FUTEX_WAKE | FUTEX_PRIVATE_FLAG, 1);
#endif
|
|
From: Florian K. <br...@ac...> - 2011-07-28 13:50:17
|
On 07/28/2011 05:23 AM, Bart Van Assche wrote:
>
> I'd rather prefer a patch like the (untested) patch below:
>
> Index: drd/drd_pthread_intercepts.c
> ===================================================================
> --- drd/drd_pthread_intercepts.c (revision 11922)
> +++ drd/drd_pthread_intercepts.c (working copy)
> @@ -49,6 +49,7 @@
> #endif
>
> #include <assert.h> /* assert() */
> +#include <errno.h>
> #include <pthread.h> /* pthread_mutex_t */
> #include <semaphore.h> /* sem_t */
> #include <stdint.h> /* uintptr_t */
> @@ -58,7 +59,10 @@
> #ifdef __linux__
> #include <asm/unistd.h> /* __NR_futex */
> #include <linux/futex.h> /* FUTEX_WAIT */
> +#ifndef FUTEX_PRIVATE_FLAG
> +#define FUTEX_PRIVATE_FLAG 0
> #endif
> +#endif
> #include "config.h" /* HAVE_PTHREAD_MUTEX_ADAPTIVE_NP etc. */
> #include "drd_basics.h" /* DRD_() */
> #include "drd_clientreq.h"
> @@ -187,13 +191,18 @@
>
> static void DRD_(sema_down)(DrdSema* sema)
> {
> + int res = ENOSYS;
> +
> while (sema->counter == 0) {
> -#ifdef __linux__
> - syscall(__NR_futex, (UWord)&sema->counter,
> - FUTEX_WAIT | FUTEX_PRIVATE_FLAG, 0);
> -#else
> - sched_yield();
> +#if defined(__linux__) && defined(__NR_futex)
> + if (syscall(__NR_futex, (UWord)&sema->counter,
> + FUTEX_WAIT | FUTEX_PRIVATE_FLAG, 0) == 0)
> + res = 0;
> + else
> + res = errno;
> #endif
> + if (res != 0 && res != EWOULDBLOCK)
> + sched_yield();
> }
> sema->counter--;
> }
> @@ -201,7 +210,7 @@
> static void DRD_(sema_up)(DrdSema* sema)
> {
> sema->counter++;
> -#ifdef __linux__
> +#if defined(__linux__) && defined(__NR_futex)
> syscall(__NR_futex, (UWord)&sema->counter,
> FUTEX_WAKE | FUTEX_PRIVATE_FLAG, 1);
> #endif
>
With that patch it compiles and links fine.
I don't run DRD tests on that old machine so I can't say whether
this is going to be OK. Some of them make the machine hang...
I'd say go ahead and apply it. The next nightly build will tell
whether there are any regressions.
Florian
|
|
From: Bart V. A. <bva...@ac...> - 2011-07-28 15:12:17
|
On Thu, Jul 28, 2011 at 3:50 PM, Florian Krohm <br...@ac...> wrote: > With that patch it compiles and links fine. > I don't run DRD tests on that old machine so I can't say whether > this is going to be OK. Some of them make the machine hang... > I'd say go ahead and apply it. The next nightly build will tell > whether there are any regressions. Strange. I've never noticed something similar on any of the systems I have access to (Linux/i386, Linux/x86_64, Linux/ppc and Darwin). On all these systems DRD runs fine - not only the regression tests but also real-world multithreaded applications. Bart. |
|
From: Florian K. <br...@ac...> - 2011-07-28 16:07:19
|
On 07/28/2011 11:11 AM, Bart Van Assche wrote: > > Strange. I've never noticed something similar on any of the systems I > have access to (Linux/i386, Linux/x86_64, Linux/ppc and Darwin). On > all these systems DRD runs fine - not only the regression tests but > also real-world multithreaded applications. > I don't doubt it. The nightly system z build which happens on a z196 (latest model) shows only 3 testcase failures in DRD: drd/tests/tc04_free_lock (stderr) drd/tests/tc09_bad_unlock (stderr) drd/tests/tc23_bogus_condwait (stderr) Not sure what the nature of the failure is. Christian has done more testing with DRD on newer system z models and feels that it runs well on those. This is what we state in README.s390 So the problem I'm seeing is more likely elsewhere. Florian |
|
From: Bart V. A. <bva...@ac...> - 2011-07-28 16:22:14
|
On Thu, Jul 28, 2011 at 6:07 PM, Florian Krohm <br...@ac...> wrote: > I don't doubt it. The nightly system z build which happens on a z196 > (latest model) shows only 3 testcase failures in DRD: > drd/tests/tc04_free_lock (stderr) > drd/tests/tc09_bad_unlock (stderr) > drd/tests/tc23_bogus_condwait (stderr) > > Not sure what the nature of the failure is. > Christian has done more testing with DRD on newer system z models and > feels that it runs well on those. This is what we state in README.s390 > So the problem I'm seeing is more likely elsewhere. I've noticed that the "diff" attachment is missing from the nightly build output e-mailed from the systems administered by Christian ? Having that output sent together with the nightly build output itself would make it possible to analyze regression test failures without having to e-mail the person who is managing these asking for more information. Bart. |
|
From: Florian K. <br...@ac...> - 2011-07-28 16:41:40
|
On 07/28/2011 12:21 PM, Bart Van Assche wrote: > > I've noticed that the "diff" attachment is missing from the nightly > build output e-mailed from the systems administered by Christian ? yeah, I noticed this myself. And the svn revision number is missing, too. Ah well, I'm sure Christian will fix it when he's back from his leave in 3 weeks. Florian |
|
From: Christian B. <bor...@de...> - 2011-07-28 18:49:20
|
On 28/07/11 18:41, Florian Krohm wrote: > On 07/28/2011 12:21 PM, Bart Van Assche wrote: >> >> I've noticed that the "diff" attachment is missing from the nightly >> build output e-mailed from the systems administered by Christian ? > > yeah, I noticed this myself. And the svn revision number is missing, > too. Ah well, I'm sure Christian will fix it when he's back from his > leave in 3 weeks. Just checked my mail. I tried a short hack on the sles system. Lets see, if that worked out. I have to look at tc04 and tc09, but tc23 is broken even without valgrind: [...] r= pthread_cond_wait(&cv, (pthread_mutex_t*)(1 + (char*)&mx[0]) ); [...] this will cause a compare and swap instruction on a misaligned address, which causes a sigill. |