Re: [PATCH siddhesh/wait_bitset] Use FUTEX_WAIT_BITSET forpthread_cond_timedwait for non-x86: ppc, s390

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

Hi,

Sorry, there was a bug in my patch in the clockbit condition that I had
fixed during testing but forgot to bring back into the patch during
submission.  Here's an updated patch in which everything else is the
same except for the flipped condition for clockbit.  Verified once
again on powerpc and s390.

Also coming up shortly is another patch with a similar change for
pthread_rwlock_timed*lock and __lll_robust_timedlock_wait.

Regards,
Siddhesh

nptl/ChangeLog:

	* pthread_cond_timedwait.c (__pthread_cond_timedwait): Time out
	if absolute timeout is negative.
	[__ASSUME_FUTEX_CLOCK_REALTIME &&
	lll_futex_timed_wait_bitset]: Use lll_futex_timed_wait_bitset.
	* sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
	(lll_futex_timed_wait_bitset): New macro.
	* sysdeps/unix/sysv/linux/s390/lowlevellock.h
	(lll_futex_timed_wait_bitset): Likewise.

diff --git a/nptl/pthread_cond_timedwait.c b/nptl/pthread_cond_timedwait.c
index 51a34ba..2fcbc57 100644
--- a/nptl/pthread_cond_timedwait.c
+++ b/nptl/pthread_cond_timedwait.c
@@ -80,6 +80,11 @@ __pthread_cond_timedwait (cond, mutex, abstime)
   ++cond->__data.__futex;
   cond->__data.__nwaiters += 1 << COND_NWAITERS_SHIFT;
 
+  /* Work around the fact that the kernel rejects negative timeout values
+     despite them being valid.  */
+  if (__builtin_expect (abstime->tv_sec < 0, 0))
+    goto timeout;
+
   /* Remember the mutex we are using here.  If there is already a
      different address store this is a bad user bug.  Do not store
      anything for pshared condvars.  */
@@ -104,9 +109,11 @@ __pthread_cond_timedwait (cond, mutex, abstime)
 
   while (1)
     {
+#if (!defined __ASSUME_FUTEX_CLOCK_REALTIME \
+     || !defined lll_futex_timed_wait_bitset)
       struct timespec rt;
       {
-#ifdef __NR_clock_gettime
+# ifdef __NR_clock_gettime
 	INTERNAL_SYSCALL_DECL (err);
 	int ret;
 	ret = INTERNAL_VSYSCALL (clock_gettime, err, 2,
@@ -116,7 +123,7 @@ __pthread_cond_timedwait (cond, mutex, abstime)
 	/* Convert the absolute timeout value to a relative timeout.  */
 	rt.tv_sec = abstime->tv_sec - rt.tv_sec;
 	rt.tv_nsec = abstime->tv_nsec - rt.tv_nsec;
-#else
+# else
 	/* Get the current time.  So far we support only one clock.  */
 	struct timeval tv;
 	(void) gettimeofday (&tv, NULL);
@@ -124,7 +131,7 @@ __pthread_cond_timedwait (cond, mutex, abstime)
 	/* Convert the absolute timeout value to a relative timeout.  */
 	rt.tv_sec = abstime->tv_sec - tv.tv_sec;
 	rt.tv_nsec = abstime->tv_nsec - tv.tv_usec * 1000;
-#endif
+# endif
       }
       if (rt.tv_nsec < 0)
 	{
@@ -139,6 +146,7 @@ __pthread_cond_timedwait (cond, mutex, abstime)
 
 	  goto timeout;
 	}
+#endif
 
       unsigned int futex_val = cond->__data.__futex;
 
@@ -148,9 +156,17 @@ __pthread_cond_timedwait (cond, mutex, abstime)
       /* Enable asynchronous cancellation.  Required by the standard.  */
       cbuffer.oldtype = __pthread_enable_asynccancel ();
 
+#if (!defined __ASSUME_FUTEX_CLOCK_REALTIME \
+     || !defined lll_futex_timed_wait_bitset)
       /* Wait until woken by signal or broadcast.  */
       err = lll_futex_timed_wait (&cond->__data.__futex,
 				  futex_val, &rt, pshared);
+#else
+      unsigned int clockbit = (cond->__data.__nwaiters & 1
+			       ? 0 : FUTEX_CLOCK_REALTIME);
+      err = lll_futex_timed_wait_bitset (&cond->__data.__futex, futex_val,
+					 abstime, clockbit, pshared);
+#endif
 
       /* Disable asynchronous cancellation.  */
       __pthread_disable_asynccancel (cbuffer.oldtype);
diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
index 406c290..17e63c6 100644
--- a/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
@@ -88,6 +88,19 @@
     INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
   })
 
+#define lll_futex_timed_wait_bitset(futexp, val, timespec, clockbit, private) \
+  ({									      \
+    INTERNAL_SYSCALL_DECL (__err);					      \
+    long int __ret;							      \
+    int __op = FUTEX_WAIT_BITSET | clockbit;				      \
+									      \
+    __ret = INTERNAL_SYSCALL (futex, __err, 6, (futexp),		      \
+			      __lll_private_flag (__op, private),	      \
+			      (val), (timespec), NULL /* Unused.  */, 	      \
+			      FUTEX_BITSET_MATCH_ANY);			      \
+    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
+  })
+
 #define lll_futex_wake(futexp, nr, private) \
   ({									      \
     INTERNAL_SYSCALL_DECL (__err);					      \
diff --git a/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h
index 9709282..0b7110f 100644
--- a/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h
@@ -93,6 +93,26 @@
     __result;								      \
   })
 
+#define lll_futex_timed_wait_bitset(futexp, val, timespec, clockbit, private) \
+  ({									      \
+    register unsigned long int __r2 asm ("2") = (unsigned long int) (futexp); \
+    register unsigned long int __r3 asm ("3")				      \
+      = __lll_private_flag ((FUTEX_WAIT_BITSET | clockbit), private);	      \
+    register unsigned long int __r4 asm ("4") = (long int) (val);	      \
+    register unsigned long int __r5 asm ("5") = (long int) (timespec);	      \
+    register unsigned long int __r6 asm ("6") = (unsigned long int) (NULL);   \
+    register unsigned long int __r7 asm ("7")				      \
+      = (unsigned int) (FUTEX_BITSET_MATCH_ANY);			      \
+    register unsigned long __result asm ("2");				      \
+									      \
+    __asm __volatile ("svc %b1"						      \
+		      : "=d" (__result)					      \
+		      : "i" (SYS_futex), "0" (__r2), "d" (__r3),	      \
+			"d" (__r4), "d" (__r5), "d" (__r6), "d" (__r7)	      \
+		      : "cc", "memory" );				      \
+    __result;								      \
+  })
+
 
 #define lll_futex_wake(futex, nr, private) \
   ({									      \