From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by sourceware.org (Postfix) with ESMTPS id 80A163858D38 for ; Wed, 12 Aug 2020 22:04:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 80A163858D38 Received: by mail-il1-x144.google.com with SMTP id 77so3401868ilc.5 for ; Wed, 12 Aug 2020 15:04:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NyN/8Kgvs1S8dNzQyefWFB2F7EI7GWnPb+Co2dxIc/Y=; b=BJitpnm0pLxnbFF06GwLMXlim7bsULtnCccgwrP9oSvlngxj900FjdkkPngfBV/k+0 ye4d2ERZF3VFPgnWidZxKLUv1OvQ7cWsgvYy2a2EVclNunE/7qhCps+O5KlhBA7IBiTU wuhjbcvUo6GzQ6cKk2WMWszOqHRu03ZP6SU5WK0fjnlDE0sEFDj44fpVNRlazE2PPHHJ 5NoxWyP16MyvjT24npcoJVLmvZhiJn5ftfeX+w63wnYeuP/exl5bCrgNjh0F7iP7p9mr kHU3lFguDblKBGvxTCb26x1zZFpKfWOQRDBgYApVVFe2gNybIkfwdthrxaP168XW27VR vZRg== X-Gm-Message-State: AOAM532C6wOqyBdiaPgDJIT4FR1d4Tsc72yaXZj8CvXcuC5y9N0ct73f zcC83Z4zDNDQPIHVPQsUT3RXwVL5jKSyIdHIpOQ= X-Google-Smtp-Source: ABdhPJwucXYO7b9eqt5b7z+MFVkwmtYLWVaYinLJpYYntCNOC0jbiqMzV4obh0OGndSRRnwY3B8oQntkVBdq/UGG4uU= X-Received: by 2002:a92:4989:: with SMTP id k9mr1797900ilg.177.1597269868884; Wed, 12 Aug 2020 15:04:28 -0700 (PDT) MIME-Version: 1.0 References: <20200812102205.14816-1-lukma@denx.de> In-Reply-To: <20200812102205.14816-1-lukma@denx.de> From: Alistair Francis Date: Wed, 12 Aug 2020 14:54:02 -0700 Message-ID: Subject: Re: [PATCH v5] y2038: nptl: Convert pthread_{clock|timed}join_np to support 64 bit time To: Lukasz Majewski Cc: Joseph Myers , Paul Eggert , Adhemerval Zanella , Arnd Bergmann , Alistair Francis , GNU C Library , Florian Weimer , "Carlos O'Donell" , Stepan Golosunov , Andreas Schwab , Zack Weinberg Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Aug 2020 22:04:31 -0000 On Wed, Aug 12, 2020 at 3:22 AM Lukasz Majewski wrote: > > The pthread_clockjoin_np and pthread_timedjoin_np have been converted to > support 64 bit time. > > This change introduces new futex_timed_wait_cancel64 function in > ./sysdeps/nptl/futex-internal.h, which uses futex_time64 where possible > and tries to replace low-level preprocessor macros from > lowlevellock-futex.h > The pthread_{timed|clock}join_np only accept absolute time. Moreover, > there is no need to check for NULL passed as *abstime pointer as > clockwait_tid() always passes struct __timespec64. > > For systems with __TIMESIZE != 64 && __WORDSIZE == 32: > - Conversions between 64 bit time to 32 bit are necessary > - Redirection to __pthread_{clock|timed}join_np64 will provide support > for 64 bit time > > 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_{timed|clock}join_np64 and > __pthread_{timed|clock}join_np. > > --- > Changes for v2: > - Update the commit message > > Changes for v3: > - Use const qualifier for futex_timed_wait_cancel64() timeout parameter > - Use INTERNAL_SYSCALL_CALL macro instead of INTERNAL_SYSCALL > - Use LIBC_CANCEL_{RESET|ASYNC} macro instead of CANCEL_{RESET|ASYNC} > - Add switch clause with listing possible error values returned by > futex{_time64} syscall > > Changes for v4: > > - Update comment when -EOVERFLOW is returned from futex syscall > - Use INTERNAL_SYSCALL_CANCELL instead of INTERNAL_SYSCALL_CALL > (with or without LIBC_CANCEL_ASYNC/LIBC_CANCEL_RESET) to make > the code smaller as it hides cancellation points handling. > > Changes for v5: > > - Refactor the code to avoid using goto Looks good. Reviewed-by: Alistair Francis Alistair > --- > nptl/pthreadP.h | 16 ++++++++++- > nptl/pthread_clockjoin.c | 19 +++++++++++-- > nptl/pthread_join_common.c | 19 +++++++------ > nptl/pthread_timedjoin.c | 18 ++++++++++-- > sysdeps/nptl/futex-internal.h | 53 +++++++++++++++++++++++++++++++++++ > 5 files changed, 111 insertions(+), 14 deletions(-) > > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h > index 6f94d6be31..99713c8447 100644 > --- a/nptl/pthreadP.h > +++ b/nptl/pthreadP.h > @@ -458,6 +458,20 @@ extern int __pthread_cond_init (pthread_cond_t *cond, > libc_hidden_proto (__pthread_cond_init) > extern int __pthread_cond_signal (pthread_cond_t *cond); > 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 > +#else > +extern int __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return, > + clockid_t clockid, > + const struct __timespec64 *abstime); > +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) > +#endif > + > extern int __pthread_cond_timedwait (pthread_cond_t *cond, > pthread_mutex_t *mutex, > const struct timespec *abstime); > @@ -488,7 +502,7 @@ extern int __pthread_enable_asynccancel (void) attribute_hidden; > extern void __pthread_disable_asynccancel (int oldtype) attribute_hidden; > extern void __pthread_testcancel (void); > extern int __pthread_clockjoin_ex (pthread_t, void **, clockid_t, > - const struct timespec *, bool) > + const struct __timespec64 *, bool) > attribute_hidden; > extern int __pthread_sigmask (int, const sigset_t *, sigset_t *); > libc_hidden_proto (__pthread_sigmask); > diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c > index a3e7f37e3b..3cd780f688 100644 > --- a/nptl/pthread_clockjoin.c > +++ b/nptl/pthread_clockjoin.c > @@ -16,14 +16,27 @@ > License along with the GNU C Library; if not, see > . */ > > +#include > #include "pthreadP.h" > > int > -__pthread_clockjoin_np (pthread_t threadid, void **thread_return, > - clockid_t clockid, > - const struct timespec *abstime) > +__pthread_clockjoin_np64 (pthread_t threadid, void **thread_return, > + clockid_t clockid, const struct __timespec64 *abstime) > { > return __pthread_clockjoin_ex (threadid, thread_return, > clockid, abstime, true); > } > + > +#if __TIMESIZE != 64 > +libc_hidden_def (__pthread_clockjoin_np64) > + > +int > +__pthread_clockjoin_np (pthread_t threadid, void **thread_return, > + clockid_t clockid, const struct timespec *abstime) > +{ > + struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime); > + > + return __pthread_clockjoin_np64 (threadid, thread_return, clockid, &ts64); > +} > +#endif > weak_alias (__pthread_clockjoin_np, pthread_clockjoin_np) > diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c > index a96ceafde4..67d8e2b780 100644 > --- a/nptl/pthread_join_common.c > +++ b/nptl/pthread_join_common.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > > static void > cleanup (void *arg) > @@ -37,9 +38,11 @@ cleanup (void *arg) > afterwards. The kernel up to version 3.16.3 does not use the private futex > operations for futex wake-up when the clone terminates. */ > static int > -clockwait_tid (pid_t *tidp, clockid_t clockid, const struct timespec *abstime) > +clockwait_tid (pid_t *tidp, clockid_t clockid, > + const struct __timespec64 *abstime) > { > pid_t tid; > + int ret; > > if (! valid_nanoseconds (abstime->tv_nsec)) > return EINVAL; > @@ -47,11 +50,11 @@ clockwait_tid (pid_t *tidp, clockid_t clockid, const struct timespec *abstime) > /* Repeat until thread terminated. */ > while ((tid = *tidp) != 0) > { > - struct timespec rt; > + struct __timespec64 rt; > > /* Get the current time. This can only fail if clockid is > invalid. */ > - if (__glibc_unlikely (__clock_gettime (clockid, &rt))) > + if (__glibc_unlikely (__clock_gettime64 (clockid, &rt))) > return EINVAL; > > /* Compute relative timeout. */ > @@ -70,9 +73,9 @@ clockwait_tid (pid_t *tidp, clockid_t clockid, const struct timespec *abstime) > /* If *tidp == tid, wait until thread terminates or the wait times out. > The kernel up to version 3.16.3 does not use the private futex > operations for futex wake-up when the clone terminates. */ > - if (lll_futex_timed_wait_cancel (tidp, tid, &rt, LLL_SHARED) > - == -ETIMEDOUT) > - return ETIMEDOUT; > + ret = futex_timed_wait_cancel64 (tidp, tid, &rt, LLL_SHARED); > + if (ret == -ETIMEDOUT || ret == -EOVERFLOW) > + return -ret; > } > > return 0; > @@ -80,8 +83,8 @@ clockwait_tid (pid_t *tidp, clockid_t clockid, const struct timespec *abstime) > > int > __pthread_clockjoin_ex (pthread_t threadid, void **thread_return, > - clockid_t clockid, > - const struct timespec *abstime, bool block) > + clockid_t clockid, > + const struct __timespec64 *abstime, bool block) > { > struct pthread *pd = (struct pthread *) threadid; > > diff --git a/nptl/pthread_timedjoin.c b/nptl/pthread_timedjoin.c > index dd7038dcf7..6164ae7060 100644 > --- a/nptl/pthread_timedjoin.c > +++ b/nptl/pthread_timedjoin.c > @@ -16,13 +16,27 @@ > License along with the GNU C Library; if not, see > . */ > > +#include > #include "pthreadP.h" > > int > -__pthread_timedjoin_np (pthread_t threadid, void **thread_return, > - const struct timespec *abstime) > +__pthread_timedjoin_np64 (pthread_t threadid, void **thread_return, > + const struct __timespec64 *abstime) > { > return __pthread_clockjoin_ex (threadid, thread_return, > CLOCK_REALTIME, abstime, true); > } > + > +#if __TIMESIZE != 64 > +libc_hidden_def (__pthread_timedjoin_np64) > + > +int > +__pthread_timedjoin_np (pthread_t threadid, void **thread_return, > + const struct timespec *abstime) > +{ > + struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime); > + > + return __pthread_timedjoin_np64 (threadid, thread_return, &ts64); > +} > +#endif > weak_alias (__pthread_timedjoin_np, pthread_timedjoin_np) > diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h > index d622122ddc..1d038e4949 100644 > --- a/sysdeps/nptl/futex-internal.h > +++ b/sysdeps/nptl/futex-internal.h > @@ -467,4 +467,57 @@ futex_unlock_pi (unsigned int *futex_word, int private) > } > } > > +#ifndef __NR_futex_time64 > +# define __NR_futex_time64 __NR_futex > +#endif > + > +static __always_inline int > +futex_timed_wait_cancel64 (pid_t *tidp, pid_t tid, > + const struct __timespec64 *timeout, int private) > +{ > + int err = INTERNAL_SYSCALL_CANCEL (futex_time64, tidp, > + __lll_private_flag > + (FUTEX_WAIT, private), tid, timeout); > +#ifndef __ASSUME_TIME64_SYSCALLS > + if (err == -ENOSYS) > + { > + struct timespec ts32; > + if (in_time_t_range (timeout->tv_sec)) > + { > + ts32 = valid_timespec64_to_timespec (*timeout); > + > + err = INTERNAL_SYSCALL_CANCEL (futex, tidp, > + __lll_private_flag (FUTEX_WAIT, > + private), > + tid, &ts32); > + } > + else > + err = -EOVERFLOW; > + } > +#endif > + switch (err) > + { > + case 0: > + case -EAGAIN: > + case -EINTR: > + case -ETIMEDOUT: > + case -EDEADLK: > + case -ENOSYS: > + case -EOVERFLOW: /* Passed absolute timeout uses 64 bit time_t type, but > + underlying kernel does not support 64 bit time_t futex > + syscalls. */ > + case -EPERM: /* The caller is not allowed to attach itself to the futex. > + Used to check if PI futexes are supported by the > + kernel. */ > + return -err; > + > + 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 -EFAULT: /* Must have been caused by a glibc or application bug. */ > + /* No other errors are documented at this time. */ > + default: > + futex_fatal_error (); > + } > +} > #endif /* futex-internal.h */ > -- > 2.20.1 >