[1/9,v2,BZ,16892] Add linux-generic lowlevellock.h

Thanks both for the review. I've made those changes, updated patch follows.

To my surprise the code was not quite identical so I re-ran the nptl tests on arm. Still no regressions - I see nptl/tst-cond21 failing intermittently but it looks like it's timing sensitive and I don't see how this change could cause a problem here.

OK?


2014-06-09  Bernard Ogden  <bernie.ogden@linaro.org>

	[BZ #16892]
	* sysdeps/unix/sysv/linux/lowlevellock.h: New file.





On 11 Jun 2014, at 19:47, Richard Henderson <rth@twiddle.net> wrote:

> Many instances of:
> 
>> +    if(INTERNAL_SYSCALL_ERROR_P (__ret, __err))				      \
> 
> Missed space after IF.
> 
>> +      __ret = -(INTERNAL_SYSCALL_ERRNO (__ret, __err));			      \
> 
> Silly extra parenthesis around INTERNAL_SYSCALL_ERRNO.
> 
> 
> r~

Looks OK to me.

On 6/12/2014 5:41 PM, Bernard Ogden wrote:
> Thanks both for the review. I've made those changes, updated patch follows.
>
> To my surprise the code was not quite identical so I re-ran the nptl tests on arm. Still no regressions - I see nptl/tst-cond21 failing intermittently but it looks like it's timing sensitive and I don't see how this change could cause a problem here.
>
> OK?
>
>
> 2014-06-09  Bernard Ogden  <bernie.ogden@linaro.org>
>
> 	[BZ #16892]
> 	* sysdeps/unix/sysv/linux/lowlevellock.h: New file.
>
>
> diff --git a/sysdeps/unix/sysv/linux/lowlevellock.h b/sysdeps/unix/sysv/linux/lowlevellock.h
> new file mode 100644
> index 0000000..efba7a7
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/lowlevellock.h
> @@ -0,0 +1,313 @@
> +/* Copyright (C) 2011-2014 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _LOWLEVELLOCK_H
> +#define _LOWLEVELLOCK_H	1
> +
> +#include <time.h>
> +#include <sys/param.h>
> +#include <bits/pthreadtypes.h>
> +#include <atomic.h>
> +#include <sysdep.h>
> +#include <kernel-features.h>
> +
> +
> +#define FUTEX_WAIT		0
> +#define FUTEX_WAKE		1
> +#define FUTEX_REQUEUE		3
> +#define FUTEX_CMP_REQUEUE	4
> +#define FUTEX_WAKE_OP		5
> +#define FUTEX_OP_CLEAR_WAKE_IF_GT_ONE	((4 << 24) | 1)
> +#define FUTEX_LOCK_PI		6
> +#define FUTEX_UNLOCK_PI		7
> +#define FUTEX_TRYLOCK_PI	8
> +#define FUTEX_WAIT_BITSET	9
> +#define FUTEX_WAKE_BITSET	10
> +#define FUTEX_WAIT_REQUEUE_PI	11
> +#define FUTEX_CMP_REQUEUE_PI	12
> +#define FUTEX_PRIVATE_FLAG	128
> +#define FUTEX_CLOCK_REALTIME	256
> +
> +#define FUTEX_BITSET_MATCH_ANY	0xffffffff
> +
> +/* Values for 'private' parameter of locking macros.  Yes, the
> +   definition seems to be backwards.  But it is not.  The bit will be
> +   reversed before passing to the system call.  */
> +#define LLL_PRIVATE	0
> +#define LLL_SHARED	FUTEX_PRIVATE_FLAG
> +
> +
> +#if !defined NOT_IN_libc || defined IS_IN_rtld
> +/* In libc.so or ld.so all futexes are private.  */
> +# ifdef __ASSUME_PRIVATE_FUTEX
> +#  define __lll_private_flag(fl, private) \
> +  ((fl) | FUTEX_PRIVATE_FLAG)
> +# else
> +#  define __lll_private_flag(fl, private) \
> +  ((fl) | THREAD_GETMEM (THREAD_SELF, header.private_futex))
> +# endif
> +#else
> +# ifdef __ASSUME_PRIVATE_FUTEX
> +#  define __lll_private_flag(fl, private) \
> +  (((fl) | FUTEX_PRIVATE_FLAG) ^ (private))
> +# else
> +#  define __lll_private_flag(fl, private) \
> +  (__builtin_constant_p (private)					      \
> +   ? ((private) == 0							      \
> +      ? ((fl) | THREAD_GETMEM (THREAD_SELF, header.private_futex))	      \
> +      : (fl))								      \
> +   : ((fl) | (((private) ^ FUTEX_PRIVATE_FLAG)				      \
> +	      & THREAD_GETMEM (THREAD_SELF, header.private_futex))))
> +# endif
> +#endif
> +
> +#define INLINE_FUTEX_SYSCALL(nr, args...) \
> +  ({									      \
> +    INTERNAL_SYSCALL_DECL (__err);					      \
> +    long int __ret = INTERNAL_SYSCALL (futex, __err, nr, args);		      \
> +    if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (__ret, __err)))	      \
> +        __ret = -INTERNAL_SYSCALL_ERRNO (__ret, __err);			      \
> +    __ret;								      \
> +  })
> +
> +#define lll_futex_wait(futexp, val, private) \
> +  lll_futex_timed_wait (futexp, val, NULL, private)
> +
> +#define lll_futex_timed_wait(futexp, val, timespec, private) \
> +  ({									      \
> +    INLINE_FUTEX_SYSCALL (4, (futexp),					      \
> +                          __lll_private_flag (FUTEX_WAIT, private),	      \
> +                          (val), (timespec));				      \
> +  })
> +
> +#define lll_futex_timed_wait_bitset(futexp, val, timespec, clockbit, private) \
> +  ({									      \
> +    int __op = FUTEX_WAIT_BITSET | clockbit;				      \
> +    INLINE_FUTEX_SYSCALL (6, (futexp),					      \
> +			  __lll_private_flag (__op, private),		      \
> +			  (val), (timespec), NULL /* Unused.  */, 	      \
> +			  FUTEX_BITSET_MATCH_ANY);			      \
> +  })
> +
> +#define lll_futex_wake(futexp, nr, private) \
> +  ({									      \
> +    INLINE_FUTEX_SYSCALL (4, (futexp),					      \
> +			  __lll_private_flag (FUTEX_WAKE, private),	      \
> +			  (nr), 0);					      \
> +  })
> +
> +#define lll_robust_dead(futexv, private) \
> +  do									      \
> +    {									      \
> +      int *__futexp = &(futexv);					      \
> +      atomic_or (__futexp, FUTEX_OWNER_DIED);				      \
> +      lll_futex_wake (__futexp, 1, private);				      \
> +    }									      \
> +  while (0)
> +
> +/* Returns non-zero if error happened, zero if success.  */
> +#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
> +  ({									      \
> +    INLINE_FUTEX_SYSCALL (6, (futexp),					      \
> +			  __lll_private_flag (FUTEX_CMP_REQUEUE, private),    \
> +			  (nr_wake), (nr_move), (mutex), (val));	      \
> +  })
> +
> +/* Returns non-zero if error happened, zero if success.  */
> +#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
> +  ({									      \
> +    INLINE_FUTEX_SYSCALL (6, (futexp),					      \
> +			  __lll_private_flag (FUTEX_WAKE_OP, private),        \
> +			  (nr_wake), (nr_wake2), (futexp2),		      \
> +			  FUTEX_OP_CLEAR_WAKE_IF_GT_ONE);		      \
> +  })
> +
> +/* Priority Inheritance support.  */
> +#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
> +  lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
> +
> +#define lll_futex_timed_wait_requeue_pi(futexp, val, timespec, clockbit,      \
> +					mutex, private)			      \
> +  ({									      \
> +    int __op = FUTEX_WAIT_REQUEUE_PI | clockbit;			      \
> +    INLINE_FUTEX_SYSCALL (5, (futexp),					      \
> +			  __lll_private_flag (__op, private),		      \
> +			  (val), (timespec), mutex);	 		      \
> +  })
> +
> +#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, val, priv)  \
> +  ({									      \
> +    INLINE_FUTEX_SYSCALL (6, (futexp),					      \
> +			  __lll_private_flag (FUTEX_CMP_REQUEUE_PI, priv),    \
> +			  (nr_wake), (nr_move), (mutex), (val));	      \
> +  })
> +
> +
> +static inline int __attribute__ ((always_inline))
> +__lll_trylock (int *futex)
> +{
> +  return atomic_compare_and_exchange_bool_acq (futex, 1, 0);
> +}
> +#define lll_trylock(lock)	__lll_trylock (&(lock))
> +
> +
> +static inline int __attribute__ ((always_inline))
> +__lll_cond_trylock (int *futex)
> +{
> +  return atomic_compare_and_exchange_bool_acq (futex, 2, 0);
> +}
> +#define lll_cond_trylock(lock)	__lll_cond_trylock (&(lock))
> +
> +
> +static inline int __attribute__ ((always_inline))
> +__lll_robust_trylock (int *futex, int id)
> +{
> +  return atomic_compare_and_exchange_bool_acq (futex, id, 0);
> +}
> +#define lll_robust_trylock(lock, id) \
> +  __lll_robust_trylock (&(lock), id)
> +
> +extern void __lll_lock_wait_private (int *futex) attribute_hidden;
> +extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
> +extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
> +
> +static inline void __attribute__ ((always_inline))
> +__lll_lock (int *futex, int private)
> +{
> +  if (atomic_compare_and_exchange_bool_acq (futex, 1, 0) != 0)
> +    {
> +      if (__builtin_constant_p (private) && private == LLL_PRIVATE)
> +	__lll_lock_wait_private (futex);
> +      else
> +	__lll_lock_wait (futex, private);
> +    }
> +}
> +#define lll_lock(futex, private) __lll_lock (&(futex), private)
> +
> +
> +static inline int __attribute__ ((always_inline))
> +__lll_robust_lock (int *futex, int id, int private)
> +{
> +  int result = 0;
> +  if (atomic_compare_and_exchange_bool_acq (futex, id, 0) != 0)
> +    result = __lll_robust_lock_wait (futex, private);
> +  return result;
> +}
> +#define lll_robust_lock(futex, id, private) \
> +  __lll_robust_lock (&(futex), id, private)
> +
> +
> +static inline void __attribute__ ((always_inline))
> +__lll_cond_lock (int *futex, int private)
> +{
> +  if (atomic_compare_and_exchange_bool_acq (futex, 2, 0) != 0)
> +    __lll_lock_wait (futex, private);
> +}
> +#define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
> +
> +
> +#define lll_robust_cond_lock(futex, id, private) \
> +  __lll_robust_lock (&(futex), (id) | FUTEX_WAITERS, private)
> +
> +
> +extern int __lll_timedlock_wait (int *futex, const struct timespec *,
> +				 int private) attribute_hidden;
> +extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
> +					int private) attribute_hidden;
> +
> +static inline int __attribute__ ((always_inline))
> +__lll_timedlock (int *futex, const struct timespec *abstime, int private)
> +{
> +  int result = 0;
> +  if (atomic_compare_and_exchange_bool_acq (futex, 1, 0) != 0)
> +    result = __lll_timedlock_wait (futex, abstime, private);
> +  return result;
> +}
> +#define lll_timedlock(futex, abstime, private) \
> +  __lll_timedlock (&(futex), abstime, private)
> +
> +
> +static inline int __attribute__ ((always_inline))
> +__lll_robust_timedlock (int *futex, const struct timespec *abstime,
> +			int id, int private)
> +{
> +  int result = 0;
> +  if (atomic_compare_and_exchange_bool_acq (futex, id, 0) != 0)
> +    result = __lll_robust_timedlock_wait (futex, abstime, private);
> +  return result;
> +}
> +#define lll_robust_timedlock(futex, abstime, id, private) \
> +  __lll_robust_timedlock (&(futex), abstime, id, private)
> +
> +
> +#define __lll_unlock(futex, private) \
> +  (void)							\
> +    ({ int *__futex = (futex);					\
> +       int __oldval = atomic_exchange_rel (__futex, 0);		\
> +       if (__glibc_unlikely (__oldval > 1))			\
> +	 lll_futex_wake (__futex, 1, private);			\
> +    })
> +#define lll_unlock(futex, private) __lll_unlock(&(futex), private)
> +
> +
> +#define __lll_robust_unlock(futex, private) \
> +  (void)							\
> +    ({ int *__futex = (futex);					\
> +       int __oldval = atomic_exchange_rel (__futex, 0);		\
> +       if (__glibc_unlikely (__oldval & FUTEX_WAITERS))		\
> +	 lll_futex_wake (__futex, 1, private);			\
> +    })
> +#define lll_robust_unlock(futex, private) \
> +  __lll_robust_unlock(&(futex), private)
> +
> +
> +#define lll_islocked(futex) \
> +  (futex != 0)
> +
> +/* Initializers for lock.  */
> +#define LLL_LOCK_INITIALIZER		(0)
> +#define LLL_LOCK_INITIALIZER_LOCKED	(1)
> +
> +/* The states of a lock are:
> +    0  -  untaken
> +    1  -  taken by one user
> +   >1  -  taken by more users */
> +
> +/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
> +   wakeup when the clone terminates.  The memory location contains the
> +   thread ID while the clone is running and is reset to zero
> +   afterwards.	*/
> +#define lll_wait_tid(tid) \
> +  do {							\
> +    __typeof (tid) __tid;				\
> +    while ((__tid = (tid)) != 0)			\
> +      lll_futex_wait (&(tid), __tid, LLL_SHARED);	\
> +  } while (0)
> +
> +extern int __lll_timedwait_tid (int *, const struct timespec *)
> +  attribute_hidden;
> +
> +#define lll_timedwait_tid(tid, abstime) \
> +  ({							\
> +    int __res = 0;					\
> +    if ((tid) != 0)					\
> +      __res = __lll_timedwait_tid (&(tid), (abstime));	\
> +    __res;						\
> +  })
> +
> +#endif /* lowlevellock.h */
>
>
>
> On 11 Jun 2014, at 19:47, Richard Henderson <rth@twiddle.net> wrote:
>
>> Many instances of:
>>
>>> +    if(INTERNAL_SYSCALL_ERROR_P (__ret, __err))				      \
>> Missed space after IF.
>>
>>> +      __ret = -(INTERNAL_SYSCALL_ERRNO (__ret, __err));			      \
>> Silly extra parenthesis around INTERNAL_SYSCALL_ERRNO.
>>
>>
>> r~

Bernard Ogden <bernie.ogden@linaro.org> writes:

> +static inline int __attribute__ ((always_inline))
> +__lll_trylock (int *futex)
> +{
> +  return atomic_compare_and_exchange_bool_acq (futex, 1, 0);
> +}

You cannot use inlines here, it breaks coldfire:

In file included from ../sysdeps/unix/sysv/linux/m68k/coldfire/sysdep.h:21:0,
                 from x.c:1:
../sysdeps/unix/sysv/linux/lowlevellock.h: In function ‘__lll_trylock’:
../sysdeps/unix/sysdep.h:34:31: error: ‘SYS_atomic_cmpxchg_32’ undeclared (first use in this function)
 #define SYS_ify(syscall_name) SYS_##syscall_name
                               ^
../sysdeps/unix/sysv/linux/m68k/coldfire/nptl/bits/atomic.h:74:29: note: in expansion of macro ‘SYS_ify’
       = (__typeof (oldval)) SYS_ify (atomic_cmpxchg_32);  \
                             ^
../include/atomic.h:138:6: note: in expansion of macro ‘atomic_compare_and_exchange_val_acq’
      atomic_compare_and_exchange_val_acq (mem, newval, __atg3_old)       \
      ^
../sysdeps/unix/sysv/linux/lowlevellock.h:186:10: note: in expansion of macro ‘atomic_compare_and_exchange_bool_acq’
   return atomic_compare_and_exchange_bool_acq (futex, 1, 0);
          ^
../sysdeps/unix/sysdep.h:34:31: note: each undeclared identifier is reported only once for each function it appears in
 #define SYS_ify(syscall_name) SYS_##syscall_name
                               ^
../sysdeps/unix/sysv/linux/m68k/coldfire/nptl/bits/atomic.h:74:29: note: in expansion of macro ‘SYS_ify’
       = (__typeof (oldval)) SYS_ify (atomic_cmpxchg_32);  \
                             ^
../include/atomic.h:138:6: note: in expansion of macro ‘atomic_compare_and_exchange_val_acq’
      atomic_compare_and_exchange_val_acq (mem, newval, __atg3_old)       \
      ^
../sysdeps/unix/sysv/linux/lowlevellock.h:186:10: note: in expansion of macro ‘atomic_compare_and_exchange_bool_acq’
   return atomic_compare_and_exchange_bool_acq (futex, 1, 0);
          ^

Andreas.