[3/5] nptl: Replace non cancellable pause/nanosleep with futex
Commit Message
To help y2038 work avoid duplicate all the logic of nanosleep on
non cancellable version, the patch replaces it with a new futex
operation, lll_timedwait. The changes are:
- Add a expected value for __lll_clocklock_wait, so it can be used
to wait for generic values.
- Remove its internal atomic operation and move the logic to
__lll_clocklock. It makes __lll_clocklock_wait even more generic
and __lll_clocklock slight faster on fast-path (since it won't
require a function call anymore).
- Add lll_timedwait, which uses __lll_clocklock_wait, to replace both
__pause_nocancel and __nanosleep_nocancel.
It also allows remove the sparc32 __lll_clocklock_wait implementation
(since it is similar to the generic one).
Checked on x86_64-linux-gnu, sparcv9-linux-gnu, and i686-linux-gnu.
---
nptl/lll_timedlock_wait.c | 35 ++++++++--------
nptl/pthread_mutex_lock.c | 3 +-
nptl/pthread_mutex_timedlock.c | 20 ++--------
sysdeps/nptl/lowlevellock.h | 15 ++++++-
sysdeps/sparc/sparc32/lll_timedlock_wait.c | 1 -
sysdeps/sparc/sparc32/lowlevellock.c | 42 --------------------
sysdeps/unix/sysv/linux/sparc/lowlevellock.h | 22 +++++++++-
7 files changed, 56 insertions(+), 82 deletions(-)
delete mode 100644 sysdeps/sparc/sparc32/lll_timedlock_wait.c
Comments
On 10/30/19 4:00 PM, Adhemerval Zanella wrote:
> To help y2038 work avoid duplicate all the logic of nanosleep on
> non cancellable version, the patch replaces it with a new futex
> operation, lll_timedwait. The changes are:
OK for master.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> - Add a expected value for __lll_clocklock_wait, so it can be used
> to wait for generic values.
OK.
> - Remove its internal atomic operation and move the logic to
> __lll_clocklock. It makes __lll_clocklock_wait even more generic
> and __lll_clocklock slight faster on fast-path (since it won't
> require a function call anymore).
OK.
> - Add lll_timedwait, which uses __lll_clocklock_wait, to replace both
> __pause_nocancel and __nanosleep_nocancel.
OK.
> It also allows remove the sparc32 __lll_clocklock_wait implementation
> (since it is similar to the generic one).
>
> Checked on x86_64-linux-gnu, sparcv9-linux-gnu, and i686-linux-gnu.
> ---
> nptl/lll_timedlock_wait.c | 35 ++++++++--------
> nptl/pthread_mutex_lock.c | 3 +-
> nptl/pthread_mutex_timedlock.c | 20 ++--------
> sysdeps/nptl/lowlevellock.h | 15 ++++++-
> sysdeps/sparc/sparc32/lll_timedlock_wait.c | 1 -
> sysdeps/sparc/sparc32/lowlevellock.c | 42 --------------------
> sysdeps/unix/sysv/linux/sparc/lowlevellock.h | 22 +++++++++-
> 7 files changed, 56 insertions(+), 82 deletions(-)
> delete mode 100644 sysdeps/sparc/sparc32/lll_timedlock_wait.c
>
> diff --git a/nptl/lll_timedlock_wait.c b/nptl/lll_timedlock_wait.c
> index cd3cc3d371..952b042555 100644
> --- a/nptl/lll_timedlock_wait.c
> +++ b/nptl/lll_timedlock_wait.c
> @@ -25,39 +25,38 @@
>
>
> int
> -__lll_clocklock_wait (int *futex, clockid_t clockid,
> +__lll_clocklock_wait (int *futex, int val, clockid_t clockid,
> const struct timespec *abstime, int private)
> {
> - /* Reject invalid timeouts. */
> - if (! valid_nanoseconds (abstime->tv_nsec))
> - return EINVAL;
> + struct timespec ts, *tsp = NULL;
>
> - /* Try locking. */
> - while (atomic_exchange_acq (futex, 2) != 0)
> + if (abstime != NULL)
> {
> - struct timespec ts;
> + /* Reject invalid timeouts. */
> + if (! valid_nanoseconds (abstime->tv_nsec))
> + return EINVAL;
>
> - /* Get the current time. This can only fail if clockid is not
> - valid. */
> + /* Get the current time. This can only fail if clockid is not valid. */
> if (__glibc_unlikely (__clock_gettime (clockid, &ts) != 0))
> return EINVAL;
>
> /* Compute relative timeout. */
> - struct timespec rt;
> - rt.tv_sec = abstime->tv_sec - ts.tv_sec;
> - rt.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
> - if (rt.tv_nsec < 0)
> + ts.tv_sec = abstime->tv_sec - ts.tv_sec;
> + ts.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
> + if (ts.tv_nsec < 0)
> {
> - rt.tv_nsec += 1000000000;
> - --rt.tv_sec;
> + ts.tv_nsec += 1000000000;
> + --ts.tv_sec;
> }
>
> - if (rt.tv_sec < 0)
> + if (ts.tv_sec < 0)
> return ETIMEDOUT;
>
> - /* If *futex == 2, wait until woken or timeout. */
> - lll_futex_timed_wait (futex, 2, &rt, private);
> + tsp = &ts;
> }
>
> + /* If *futex == val, wait until woken or timeout. */
> + lll_futex_timed_wait (futex, val, tsp, private);
> +
> return 0;
> }
OK.
> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
> index 0ab890d815..ace436d5a6 100644
> --- a/nptl/pthread_mutex_lock.c
> +++ b/nptl/pthread_mutex_lock.c
> @@ -434,7 +434,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
>
> /* Delay the thread indefinitely. */
> while (1)
> - __pause_nocancel ();
> + lll_timedwait (&(int){0}, 0, 0 /* ignored */, NULL,
> + private);
OK.
> }
>
> oldval = mutex->__data.__lock;
> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> index c9bb3b9176..490064d8cf 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -401,22 +401,10 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>
> /* Delay the thread until the timeout is reached.
> Then return ETIMEDOUT. */
> - struct timespec reltime;
> - struct timespec now;
> -
> - INTERNAL_SYSCALL (clock_gettime, __err, 2, clockid,
> - &now);
> - reltime.tv_sec = abstime->tv_sec - now.tv_sec;
> - reltime.tv_nsec = abstime->tv_nsec - now.tv_nsec;
> - if (reltime.tv_nsec < 0)
> - {
> - reltime.tv_nsec += 1000000000;
> - --reltime.tv_sec;
> - }
> - if (reltime.tv_sec >= 0)
> - while (__nanosleep_nocancel (&reltime, &reltime) != 0)
> - continue;
> -
> + do
> + e = lll_timedwait (&(int){0}, 0, clockid, abstime,
> + private);
> + while (e != ETIMEDOUT);
OK.
> return ETIMEDOUT;
> }
>
> diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
> index e3f006537a..92298f3b7e 100644
> --- a/sysdeps/nptl/lowlevellock.h
> +++ b/sysdeps/nptl/lowlevellock.h
> @@ -21,6 +21,7 @@
>
> #include <atomic.h>
> #include <lowlevellock-futex.h>
> +#include <time.h>
>
> /* Low-level locks use a combination of atomic operations (to acquire and
> release lock ownership) and futex operations (to block until the state
> @@ -121,10 +122,12 @@ extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
> #define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
>
>
> -extern int __lll_clocklock_wait (int *futex, clockid_t,
> +extern int __lll_clocklock_wait (int *futex, int val, clockid_t,
> const struct timespec *,
> int private) attribute_hidden;
>
> +#define lll_timedwait(futex, val, clockid, abstime, private) \
> + __lll_clocklock_wait (futex, val, clockid, abstime, private)
OK.
>
> /* As __lll_lock, but with an absolute timeout measured against the clock
> specified in CLOCKID. If the timeout occurs then return ETIMEDOUT. If
> @@ -136,7 +139,15 @@ extern int __lll_clocklock_wait (int *futex, clockid_t,
> \
> if (__glibc_unlikely \
> (atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \
> - __val = __lll_clocklock_wait (__futex, clockid, abstime, private); \
> + { \
> + while (atomic_exchange_acq (futex, 2) != 0) \
> + { \
> + __val = __lll_clocklock_wait (__futex, 2, clockid, \
> + abstime, private); \
OK.
> + if (__val == EINVAL || __val == ETIMEDOUT) \
> + break; \
> + } \
> + } \
> __val; \
> })
> #define lll_clocklock(futex, clockid, abstime, private) \
> diff --git a/sysdeps/sparc/sparc32/lll_timedlock_wait.c b/sysdeps/sparc/sparc32/lll_timedlock_wait.c
> deleted file mode 100644
> index bd639a7091..0000000000
> --- a/sysdeps/sparc/sparc32/lll_timedlock_wait.c
> +++ /dev/null
> @@ -1 +0,0 @@
> -/* __lll_clocklock_wait is in lowlevellock.c. */
> diff --git a/sysdeps/sparc/sparc32/lowlevellock.c b/sysdeps/sparc/sparc32/lowlevellock.c
> index 074ecf0636..1a0ab4d9f2 100644
> --- a/sysdeps/sparc/sparc32/lowlevellock.c
> +++ b/sysdeps/sparc/sparc32/lowlevellock.c
> @@ -50,46 +50,4 @@ __lll_lock_wait (int *futex, int private)
> }
> while (atomic_compare_and_exchange_val_24_acq (futex, 2, 0) != 0);
> }
> -
> -
> -int
> -__lll_clocklock_wait (int *futex, clockid_t clockid,
> - const struct timespec *abstime, int private)
> -{
> - /* Reject invalid timeouts. */
> - if (! valid_nanoseconds (abstime->tv_nsec))
> - return EINVAL;
> -
> - do
> - {
> - struct timespec ts;
> - struct timespec rt;
> -
> - /* Get the current time. This can only fail if clockid is not
> - valid. */
> - if (__glibc_unlikely (__clock_gettime (clockid, &ts) != 0))
> - return EINVAL;
> -
> - /* Compute relative timeout. */
> - rt.tv_sec = abstime->tv_sec - ts.tv_sec;
> - rt.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
> - if (rt.tv_nsec < 0)
> - {
> - rt.tv_nsec += 1000000000;
> - --rt.tv_sec;
> - }
> -
> - /* Already timed out? */
> - if (rt.tv_sec < 0)
> - return ETIMEDOUT;
> -
> - /* Wait. */
> - int oldval = atomic_compare_and_exchange_val_24_acq (futex, 2, 1);
> - if (oldval != 0)
> - lll_futex_timed_wait (futex, 2, &rt, private);
> - }
> - while (atomic_compare_and_exchange_val_24_acq (futex, 2, 0) != 0);
> -
> - return 0;
> -}
OK. Nice to see this code go.
> #endif
> diff --git a/sysdeps/unix/sysv/linux/sparc/lowlevellock.h b/sysdeps/unix/sysv/linux/sparc/lowlevellock.h
> index 34441f8ff5..6c005e8ffa 100644
> --- a/sysdeps/unix/sysv/linux/sparc/lowlevellock.h
> +++ b/sysdeps/unix/sysv/linux/sparc/lowlevellock.h
> @@ -24,6 +24,7 @@
> #include <bits/pthreadtypes.h>
> #include <atomic.h>
> #include <kernel-features.h>
> +#include <errno.h>
>
> #include <lowlevellock-futex.h>
>
> @@ -75,9 +76,13 @@ __lll_cond_lock (int *futex, int private)
> #define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
>
>
> -extern int __lll_clocklock_wait (int *futex, clockid_t, const struct timespec *,
> +extern int __lll_clocklock_wait (int *futex, clockid_t, int val,
> + const struct timespec *,
> int private) attribute_hidden;
>
> +#define lll_timedwait(futex, val, clockid, abstime, private) \
> + __lll_clocklock_wait (futex, val, clockid, abstime, private)
> +
> static inline int
> __attribute__ ((always_inline))
> __lll_clocklock (int *futex, clockid_t clockid,
> @@ -87,7 +92,20 @@ __lll_clocklock (int *futex, clockid_t clockid,
> int result = 0;
>
> if (__glibc_unlikely (val != 0))
> - result = __lll_clocklock_wait (futex, clockid, abstime, private);
> + {
> + do
> + {
> + int oldval = atomic_compare_and_exchange_val_24_acq (futex, val, 1);
> + if (oldval != 0)
> + {
> + result = __lll_clocklock_wait (futex, 2, clockid, abstime,
> + private);
> + if (result == EINVAL || result == ETIMEDOUT)
> + break;
> + }
> + }
> + while (atomic_compare_and_exchange_val_24_acq (futex, val, 0) != 0);
> + }
> return result;
> }
> #define lll_clocklock(futex, clockid, abstime, private) \
>
OK.
@@ -25,39 +25,38 @@
int
-__lll_clocklock_wait (int *futex, clockid_t clockid,
+__lll_clocklock_wait (int *futex, int val, clockid_t clockid,
const struct timespec *abstime, int private)
{
- /* Reject invalid timeouts. */
- if (! valid_nanoseconds (abstime->tv_nsec))
- return EINVAL;
+ struct timespec ts, *tsp = NULL;
- /* Try locking. */
- while (atomic_exchange_acq (futex, 2) != 0)
+ if (abstime != NULL)
{
- struct timespec ts;
+ /* Reject invalid timeouts. */
+ if (! valid_nanoseconds (abstime->tv_nsec))
+ return EINVAL;
- /* Get the current time. This can only fail if clockid is not
- valid. */
+ /* Get the current time. This can only fail if clockid is not valid. */
if (__glibc_unlikely (__clock_gettime (clockid, &ts) != 0))
return EINVAL;
/* Compute relative timeout. */
- struct timespec rt;
- rt.tv_sec = abstime->tv_sec - ts.tv_sec;
- rt.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
- if (rt.tv_nsec < 0)
+ ts.tv_sec = abstime->tv_sec - ts.tv_sec;
+ ts.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
+ if (ts.tv_nsec < 0)
{
- rt.tv_nsec += 1000000000;
- --rt.tv_sec;
+ ts.tv_nsec += 1000000000;
+ --ts.tv_sec;
}
- if (rt.tv_sec < 0)
+ if (ts.tv_sec < 0)
return ETIMEDOUT;
- /* If *futex == 2, wait until woken or timeout. */
- lll_futex_timed_wait (futex, 2, &rt, private);
+ tsp = &ts;
}
+ /* If *futex == val, wait until woken or timeout. */
+ lll_futex_timed_wait (futex, val, tsp, private);
+
return 0;
}
@@ -434,7 +434,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
/* Delay the thread indefinitely. */
while (1)
- __pause_nocancel ();
+ lll_timedwait (&(int){0}, 0, 0 /* ignored */, NULL,
+ private);
}
oldval = mutex->__data.__lock;
@@ -401,22 +401,10 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
/* Delay the thread until the timeout is reached.
Then return ETIMEDOUT. */
- struct timespec reltime;
- struct timespec now;
-
- INTERNAL_SYSCALL (clock_gettime, __err, 2, clockid,
- &now);
- reltime.tv_sec = abstime->tv_sec - now.tv_sec;
- reltime.tv_nsec = abstime->tv_nsec - now.tv_nsec;
- if (reltime.tv_nsec < 0)
- {
- reltime.tv_nsec += 1000000000;
- --reltime.tv_sec;
- }
- if (reltime.tv_sec >= 0)
- while (__nanosleep_nocancel (&reltime, &reltime) != 0)
- continue;
-
+ do
+ e = lll_timedwait (&(int){0}, 0, clockid, abstime,
+ private);
+ while (e != ETIMEDOUT);
return ETIMEDOUT;
}
@@ -21,6 +21,7 @@
#include <atomic.h>
#include <lowlevellock-futex.h>
+#include <time.h>
/* Low-level locks use a combination of atomic operations (to acquire and
release lock ownership) and futex operations (to block until the state
@@ -121,10 +122,12 @@ extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
#define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
-extern int __lll_clocklock_wait (int *futex, clockid_t,
+extern int __lll_clocklock_wait (int *futex, int val, clockid_t,
const struct timespec *,
int private) attribute_hidden;
+#define lll_timedwait(futex, val, clockid, abstime, private) \
+ __lll_clocklock_wait (futex, val, clockid, abstime, private)
/* As __lll_lock, but with an absolute timeout measured against the clock
specified in CLOCKID. If the timeout occurs then return ETIMEDOUT. If
@@ -136,7 +139,15 @@ extern int __lll_clocklock_wait (int *futex, clockid_t,
\
if (__glibc_unlikely \
(atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \
- __val = __lll_clocklock_wait (__futex, clockid, abstime, private); \
+ { \
+ while (atomic_exchange_acq (futex, 2) != 0) \
+ { \
+ __val = __lll_clocklock_wait (__futex, 2, clockid, \
+ abstime, private); \
+ if (__val == EINVAL || __val == ETIMEDOUT) \
+ break; \
+ } \
+ } \
__val; \
})
#define lll_clocklock(futex, clockid, abstime, private) \
deleted file mode 100644
@@ -1 +0,0 @@
-/* __lll_clocklock_wait is in lowlevellock.c. */
@@ -50,46 +50,4 @@ __lll_lock_wait (int *futex, int private)
}
while (atomic_compare_and_exchange_val_24_acq (futex, 2, 0) != 0);
}
-
-
-int
-__lll_clocklock_wait (int *futex, clockid_t clockid,
- const struct timespec *abstime, int private)
-{
- /* Reject invalid timeouts. */
- if (! valid_nanoseconds (abstime->tv_nsec))
- return EINVAL;
-
- do
- {
- struct timespec ts;
- struct timespec rt;
-
- /* Get the current time. This can only fail if clockid is not
- valid. */
- if (__glibc_unlikely (__clock_gettime (clockid, &ts) != 0))
- return EINVAL;
-
- /* Compute relative timeout. */
- rt.tv_sec = abstime->tv_sec - ts.tv_sec;
- rt.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
- if (rt.tv_nsec < 0)
- {
- rt.tv_nsec += 1000000000;
- --rt.tv_sec;
- }
-
- /* Already timed out? */
- if (rt.tv_sec < 0)
- return ETIMEDOUT;
-
- /* Wait. */
- int oldval = atomic_compare_and_exchange_val_24_acq (futex, 2, 1);
- if (oldval != 0)
- lll_futex_timed_wait (futex, 2, &rt, private);
- }
- while (atomic_compare_and_exchange_val_24_acq (futex, 2, 0) != 0);
-
- return 0;
-}
#endif
@@ -24,6 +24,7 @@
#include <bits/pthreadtypes.h>
#include <atomic.h>
#include <kernel-features.h>
+#include <errno.h>
#include <lowlevellock-futex.h>
@@ -75,9 +76,13 @@ __lll_cond_lock (int *futex, int private)
#define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
-extern int __lll_clocklock_wait (int *futex, clockid_t, const struct timespec *,
+extern int __lll_clocklock_wait (int *futex, clockid_t, int val,
+ const struct timespec *,
int private) attribute_hidden;
+#define lll_timedwait(futex, val, clockid, abstime, private) \
+ __lll_clocklock_wait (futex, val, clockid, abstime, private)
+
static inline int
__attribute__ ((always_inline))
__lll_clocklock (int *futex, clockid_t clockid,
@@ -87,7 +92,20 @@ __lll_clocklock (int *futex, clockid_t clockid,
int result = 0;
if (__glibc_unlikely (val != 0))
- result = __lll_clocklock_wait (futex, clockid, abstime, private);
+ {
+ do
+ {
+ int oldval = atomic_compare_and_exchange_val_24_acq (futex, val, 1);
+ if (oldval != 0)
+ {
+ result = __lll_clocklock_wait (futex, 2, clockid, abstime,
+ private);
+ if (result == EINVAL || result == ETIMEDOUT)
+ break;
+ }
+ }
+ while (atomic_compare_and_exchange_val_24_acq (futex, val, 0) != 0);
+ }
return result;
}
#define lll_clocklock(futex, clockid, abstime, private) \