| Message ID | 20191030200052.497-3-adhemerval.zanella@linaro.org |
|---|---|
| State | Accepted |
| Commit | 215078017fd25fd64074e25ccd3dde0f6f19d4fe |
| Headers | show |
| Series | [1/5] Consolidate futex-internal.h | expand |
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. -- Cheers, Carlos.
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; } 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); } 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); 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) /* 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) \ 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; -} #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) \
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 -- 2.17.1