Message ID | 20200830224445.31879-1-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | [v3] y2038: nptl: Convert pthread_cond_{clock|timed}wait to support 64 bit time | expand |
On 30/08/2020 19:44, Lukasz Majewski wrote: > The pthread_cond_clockwait and pthread_cond_timedwait have been converted > to support 64 bit time. > > This change introduces new futex_abstimed_wait_cancelable64 function in > ./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where possible > and tries to replace low-level preprocessor macros from > lowlevellock-futex.h > The pthread_cond_{clock|timed}wait only accepts absolute time. Moreover, > there is no need to check for NULL passed as *abstime pointer as > __pthread_cond_wait_common() always passes non-NULL struct __timespec64 > pointer to futex_abstimed_wait_cancellable64(). > > For systems with __TIMESIZE != 64 && __WORDSIZE == 32: > - Conversions between 64 bit time to 32 bit are necessary > - Redirection to __pthread_cond_{clock|timed}wait64 will provide support > for 64 bit time > > The futex_abstimed_wait_cancelable64 function has been put into a separate > file on the purpose - to avoid issues apparent on the m68k architecture > related to small number of available registers (there is not enough > registers to put all necessary arguments in them if the above function > would be added to futex-internal.h with __always_inline attribute). > > In fact - new function - namely __futex_abstimed_wait_cancellable32 is > used to reduce number of needed registers (as some in-register values are > stored on the stack when function call is made). > > Build tests: > ./src/scripts/build-many-glibcs.py glibcs > > Run-time tests: > - Run specific tests on ARM/x86 32bit systems (qemu): > https://github.com/lmajewski/meta-y2038 and run tests: > https://github.com/lmajewski/y2038-tests/commits/master > > Above tests were performed with Y2038 redirection applied as well as without > to test the proper usage of both __pthread_cond_{clock|timed}wait64 and > __pthread_cond_{clock|timed}wait. LGTM with attribute_hidden fixes below. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > --- > Changes for v3: > > - Change the file name from futex-helpers.c to futex-internal.c > - Use INTERNAL_SYSCALL_CANCEL() instead of INTERNAL_SYSCALL_CALL and remove not > necessary __pthread_{disable|enable}_asynccancel () > - Remove not needed blank lines > - Do not change code formatting not related to this patch > - Add attribute_hidden instead of libpthread_hidden_def > - Rewrite the __futex_abstimed_wait_cancelable64 to have a separate function > call for 32 bit time syscall fallback > - Remove sysdeps/unix/sysv/linux/m68k/futex-helpers.c as it is not needed > anymore > --- > nptl/pthreadP.h | 11 ++++ > nptl/pthread_cond_wait.c | 43 +++++++++++++--- > sysdeps/nptl/Makefile | 2 +- > sysdeps/nptl/futex-internal.c | 96 +++++++++++++++++++++++++++++++++++ > sysdeps/nptl/futex-internal.h | 11 ++++ > 5 files changed, 154 insertions(+), 9 deletions(-) > create mode 100644 sysdeps/nptl/futex-internal.c > > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h > index 99713c8447..9bb44c8535 100644 > --- a/nptl/pthreadP.h > +++ b/nptl/pthreadP.h > @@ -462,6 +462,8 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex); > #if __TIMESIZE == 64 > # define __pthread_clockjoin_np64 __pthread_clockjoin_np > # define __pthread_timedjoin_np64 __pthread_timedjoin_np > +# define __pthread_cond_timedwait64 __pthread_cond_timedwait > +# define __pthread_cond_clockwait64 __pthread_cond_clockwait > #else > extern int __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return, > clockid_t clockid, > @@ -470,6 +472,15 @@ libc_hidden_proto (__pthread_clockjoin_np64) > extern int __pthread_timedjoin_np64 (pthread_t threadid, void **thread_return, > const struct __timespec64 *abstime); > libc_hidden_proto (__pthread_timedjoin_np64) > +extern int __pthread_cond_timedwait64 (pthread_cond_t *cond, > + pthread_mutex_t *mutex, > + const struct __timespec64 *abstime); > +libpthread_hidden_proto (__pthread_cond_timedwait64) > +extern int __pthread_cond_clockwait64 (pthread_cond_t *cond, > + pthread_mutex_t *mutex, > + clockid_t clockid, > + const struct __timespec64 *abstime); > +libpthread_hidden_proto (__pthread_cond_clockwait64) > #endif > > extern int __pthread_cond_timedwait (pthread_cond_t *cond, Ok. > diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c > index 85ddbc1011..7d158d553f 100644 > --- a/nptl/pthread_cond_wait.c > +++ b/nptl/pthread_cond_wait.c > @@ -378,8 +378,7 @@ __condvar_cleanup_waiting (void *arg) > */ > static __always_inline int > __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex, > - clockid_t clockid, > - const struct timespec *abstime) > + clockid_t clockid, const struct __timespec64 *abstime) > { > const int maxspin = 0; > int err; > @@ -517,7 +516,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex, > err = ETIMEDOUT; > else > { > - err = futex_abstimed_wait_cancelable > + err = __futex_abstimed_wait_cancelable64 > (cond->__data.__g_signals + g, 0, clockid, abstime, > private); > } Ok. > @@ -640,8 +639,8 @@ __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex) > > /* See __pthread_cond_wait_common. */ > int > -__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex, > - const struct timespec *abstime) > +__pthread_cond_timedwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex, > + const struct __timespec64 *abstime) > { > /* Check parameter validity. This should also tell the compiler that > it can assume that abstime is not NULL. */ Ok. > @@ -655,6 +654,20 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex, > ? CLOCK_MONOTONIC : CLOCK_REALTIME; > return __pthread_cond_wait_common (cond, mutex, clockid, abstime); > } > + > +#if __TIMESIZE != 64 > +libpthread_hidden_def (__pthread_cond_timedwait64) > + > +int > +__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex, > + const struct timespec *abstime) > +{ > + struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime); > + > + return __pthread_cond_timedwait64 (cond, mutex, &ts64); > +} > +#endif > + > versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait, > GLIBC_2_3_2); > versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait, Ok. > @@ -662,9 +675,9 @@ versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait, > > /* See __pthread_cond_wait_common. */ > int > -__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex, > - clockid_t clockid, > - const struct timespec *abstime) > +__pthread_cond_clockwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex, > + clockid_t clockid, > + const struct __timespec64 *abstime) > { > /* Check parameter validity. This should also tell the compiler that > it can assume that abstime is not NULL. */ Ok. > @@ -676,4 +689,18 @@ __pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex, > > return __pthread_cond_wait_common (cond, mutex, clockid, abstime); > } > + > +#if __TIMESIZE != 64 > +libpthread_hidden_def (__pthread_cond_clockwait64) > + > +int > +__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex, > + clockid_t clockid, > + const struct timespec *abstime) > +{ > + struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime); > + > + return __pthread_cond_clockwait64 (cond, mutex, clockid, &ts64); > +} > +#endif Ok. > weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait); > diff --git a/sysdeps/nptl/Makefile b/sysdeps/nptl/Makefile > index 0631a870c8..a65be3b7ea 100644 > --- a/sysdeps/nptl/Makefile > +++ b/sysdeps/nptl/Makefile > @@ -17,7 +17,7 @@ > # <https://www.gnu.org/licenses/>. > > ifeq ($(subdir),nptl) > -libpthread-sysdep_routines += errno-loc > +libpthread-sysdep_routines += errno-loc futex-internal > endif > > ifeq ($(subdir),rt) Ok. > diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c > new file mode 100644 > index 0000000000..692820b0b0 > --- /dev/null > +++ b/sysdeps/nptl/futex-internal.c > @@ -0,0 +1,96 @@ > +/* futex helper functions for glibc-internal use. > + Copyright (C) 2020 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + 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 > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <sysdep.h> > +#include <time.h> > +#include <futex-internal.h> > +#include <kernel-features.h> > + > +#ifndef __ASSUME_TIME64_SYSCALLS > +static int > +__futex_abstimed_wait_cancellable32 (unsigned int* futex_word, > + unsigned int expected, clockid_t clockid, > + const struct __timespec64* abstime, > + int private) > +{ > + if (! in_time_t_range (abstime->tv_sec)) > + return -EOVERFLOW; > + > + unsigned int clockbit = (clockid == CLOCK_REALTIME) > + ? FUTEX_CLOCK_REALTIME : 0; > + int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); > + > + struct timespec ts32 = valid_timespec64_to_timespec (*abstime); > + return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected, > + &ts32, NULL /* Unused. */, > + FUTEX_BITSET_MATCH_ANY); > +} > +#endif > + > +int > +attribute_hidden There is no need to add this, the function prototype should handle it. > +__futex_abstimed_wait_cancelable64 (unsigned int* futex_word, > + unsigned int expected, clockid_t clockid, > + const struct __timespec64* abstime, > + int private) > +{ > + unsigned int clockbit; > + int err; > + > + /* Work around the fact that the kernel rejects negative timeout values > + despite them being valid. */ > + if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec < 0))) > + return ETIMEDOUT; > + > + if (! lll_futex_supported_clockid (clockid)) > + return EINVAL; > + > + clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; > + int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); > + > + err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected, > + abstime, NULL /* Unused. */, > + FUTEX_BITSET_MATCH_ANY); > +#ifndef __ASSUME_TIME64_SYSCALLS > +if (err == -ENOSYS) > + err = __futex_abstimed_wait_cancellable32 (futex_word, expected, > + clockid, abstime, private); > +#endif > + > + switch (err) > + { > + case 0: > + case -EAGAIN: > + case -EINTR: > + case -ETIMEDOUT: > + case -EOVERFLOW: /* Passed absolute timeout uses 64 bit time_t type, but > + underlying kernel does not support 64 bit time_t futex > + syscalls. */ > + return -err; > + > + case -EFAULT: /* Must have been caused by a glibc or application bug. */ > + case -EINVAL: /* Either due to wrong alignment or due to the timeout not > + being normalized. Must have been caused by a glibc or > + application bug. */ > + case -ENOSYS: /* Must have been caused by a glibc bug. */ > + /* No other errors are documented at this time. */ > + default: > + futex_fatal_error (); > + } > +} Ok. > diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h > index 159aae82dc..3c4a3edb55 100644 > --- a/sysdeps/nptl/futex-internal.h > +++ b/sysdeps/nptl/futex-internal.h > @@ -519,4 +519,15 @@ futex_timed_wait_cancel64 (pid_t *tidp, pid_t tid, > futex_fatal_error (); > } > } > + > +/* The futex_abstimed_wait_cancelable64 has been moved to a separate file > + to avoid problems with exhausting available registers on some architectures > + - e.g. on m68k architecture. */ > +int > +attribute_hidden Move the attribute_hidden to the end of the function prototype (commit 30e5069c7d4 fix a issue where the __MALLOC_DEPRECATED is empty on some configurations, it should not happen here since attribute_hidden will be always defined with supported configurations though). > +__futex_abstimed_wait_cancelable64 (unsigned int* futex_word, > + unsigned int expected, clockid_t clockid, > + const struct __timespec64* abstime, > + int private); > + > #endif /* futex-internal.h */ >
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 99713c8447..9bb44c8535 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -462,6 +462,8 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex); #if __TIMESIZE == 64 # define __pthread_clockjoin_np64 __pthread_clockjoin_np # define __pthread_timedjoin_np64 __pthread_timedjoin_np +# define __pthread_cond_timedwait64 __pthread_cond_timedwait +# define __pthread_cond_clockwait64 __pthread_cond_clockwait #else extern int __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return, clockid_t clockid, @@ -470,6 +472,15 @@ libc_hidden_proto (__pthread_clockjoin_np64) extern int __pthread_timedjoin_np64 (pthread_t threadid, void **thread_return, const struct __timespec64 *abstime); libc_hidden_proto (__pthread_timedjoin_np64) +extern int __pthread_cond_timedwait64 (pthread_cond_t *cond, + pthread_mutex_t *mutex, + const struct __timespec64 *abstime); +libpthread_hidden_proto (__pthread_cond_timedwait64) +extern int __pthread_cond_clockwait64 (pthread_cond_t *cond, + pthread_mutex_t *mutex, + clockid_t clockid, + const struct __timespec64 *abstime); +libpthread_hidden_proto (__pthread_cond_clockwait64) #endif extern int __pthread_cond_timedwait (pthread_cond_t *cond, diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c index 85ddbc1011..7d158d553f 100644 --- a/nptl/pthread_cond_wait.c +++ b/nptl/pthread_cond_wait.c @@ -378,8 +378,7 @@ __condvar_cleanup_waiting (void *arg) */ static __always_inline int __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex, - clockid_t clockid, - const struct timespec *abstime) + clockid_t clockid, const struct __timespec64 *abstime) { const int maxspin = 0; int err; @@ -517,7 +516,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex, err = ETIMEDOUT; else { - err = futex_abstimed_wait_cancelable + err = __futex_abstimed_wait_cancelable64 (cond->__data.__g_signals + g, 0, clockid, abstime, private); } @@ -640,8 +639,8 @@ __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex) /* See __pthread_cond_wait_common. */ int -__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex, - const struct timespec *abstime) +__pthread_cond_timedwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex, + const struct __timespec64 *abstime) { /* Check parameter validity. This should also tell the compiler that it can assume that abstime is not NULL. */ @@ -655,6 +654,20 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex, ? CLOCK_MONOTONIC : CLOCK_REALTIME; return __pthread_cond_wait_common (cond, mutex, clockid, abstime); } + +#if __TIMESIZE != 64 +libpthread_hidden_def (__pthread_cond_timedwait64) + +int +__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex, + const struct timespec *abstime) +{ + struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime); + + return __pthread_cond_timedwait64 (cond, mutex, &ts64); +} +#endif + versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait, GLIBC_2_3_2); versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait, @@ -662,9 +675,9 @@ versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait, /* See __pthread_cond_wait_common. */ int -__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex, - clockid_t clockid, - const struct timespec *abstime) +__pthread_cond_clockwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex, + clockid_t clockid, + const struct __timespec64 *abstime) { /* Check parameter validity. This should also tell the compiler that it can assume that abstime is not NULL. */ @@ -676,4 +689,18 @@ __pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex, return __pthread_cond_wait_common (cond, mutex, clockid, abstime); } + +#if __TIMESIZE != 64 +libpthread_hidden_def (__pthread_cond_clockwait64) + +int +__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex, + clockid_t clockid, + const struct timespec *abstime) +{ + struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime); + + return __pthread_cond_clockwait64 (cond, mutex, clockid, &ts64); +} +#endif weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait); diff --git a/sysdeps/nptl/Makefile b/sysdeps/nptl/Makefile index 0631a870c8..a65be3b7ea 100644 --- a/sysdeps/nptl/Makefile +++ b/sysdeps/nptl/Makefile @@ -17,7 +17,7 @@ # <https://www.gnu.org/licenses/>. ifeq ($(subdir),nptl) -libpthread-sysdep_routines += errno-loc +libpthread-sysdep_routines += errno-loc futex-internal endif ifeq ($(subdir),rt) diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c new file mode 100644 index 0000000000..692820b0b0 --- /dev/null +++ b/sysdeps/nptl/futex-internal.c @@ -0,0 +1,96 @@ +/* futex helper functions for glibc-internal use. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + 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 + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <sysdep.h> +#include <time.h> +#include <futex-internal.h> +#include <kernel-features.h> + +#ifndef __ASSUME_TIME64_SYSCALLS +static int +__futex_abstimed_wait_cancellable32 (unsigned int* futex_word, + unsigned int expected, clockid_t clockid, + const struct __timespec64* abstime, + int private) +{ + if (! in_time_t_range (abstime->tv_sec)) + return -EOVERFLOW; + + unsigned int clockbit = (clockid == CLOCK_REALTIME) + ? FUTEX_CLOCK_REALTIME : 0; + int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); + + struct timespec ts32 = valid_timespec64_to_timespec (*abstime); + return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected, + &ts32, NULL /* Unused. */, + FUTEX_BITSET_MATCH_ANY); +} +#endif + +int +attribute_hidden +__futex_abstimed_wait_cancelable64 (unsigned int* futex_word, + unsigned int expected, clockid_t clockid, + const struct __timespec64* abstime, + int private) +{ + unsigned int clockbit; + int err; + + /* Work around the fact that the kernel rejects negative timeout values + despite them being valid. */ + if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec < 0))) + return ETIMEDOUT; + + if (! lll_futex_supported_clockid (clockid)) + return EINVAL; + + clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; + int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); + + err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected, + abstime, NULL /* Unused. */, + FUTEX_BITSET_MATCH_ANY); +#ifndef __ASSUME_TIME64_SYSCALLS +if (err == -ENOSYS) + err = __futex_abstimed_wait_cancellable32 (futex_word, expected, + clockid, abstime, private); +#endif + + switch (err) + { + case 0: + case -EAGAIN: + case -EINTR: + case -ETIMEDOUT: + case -EOVERFLOW: /* Passed absolute timeout uses 64 bit time_t type, but + underlying kernel does not support 64 bit time_t futex + syscalls. */ + return -err; + + case -EFAULT: /* Must have been caused by a glibc or application bug. */ + case -EINVAL: /* Either due to wrong alignment or due to the timeout not + being normalized. Must have been caused by a glibc or + application bug. */ + case -ENOSYS: /* Must have been caused by a glibc bug. */ + /* No other errors are documented at this time. */ + default: + futex_fatal_error (); + } +} diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h index 159aae82dc..3c4a3edb55 100644 --- a/sysdeps/nptl/futex-internal.h +++ b/sysdeps/nptl/futex-internal.h @@ -519,4 +519,15 @@ futex_timed_wait_cancel64 (pid_t *tidp, pid_t tid, futex_fatal_error (); } } + +/* The futex_abstimed_wait_cancelable64 has been moved to a separate file + to avoid problems with exhausting available registers on some architectures + - e.g. on m68k architecture. */ +int +attribute_hidden +__futex_abstimed_wait_cancelable64 (unsigned int* futex_word, + unsigned int expected, clockid_t clockid, + const struct __timespec64* abstime, + int private); + #endif /* futex-internal.h */