Message ID | 20201019145739.32361-1-lukma@denx.de |
---|---|
State | Committed |
Delegated to: | Lukasz Majewski |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8966E388A43C; Mon, 19 Oct 2020 14:59:02 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-out.m-online.net (mail-out.m-online.net [IPv6:2001:a60:0:28:0:1:25:1]) by sourceware.org (Postfix) with ESMTPS id 3A0C63857C4F for <libc-alpha@sourceware.org>; Mon, 19 Oct 2020 14:58:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3A0C63857C4F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=lukma@denx.de Received: from frontend01.mail.m-online.net (unknown [192.168.8.182]) by mail-out.m-online.net (Postfix) with ESMTP id 4CFKdr2nCxz1rwvQ; Mon, 19 Oct 2020 16:58:56 +0200 (CEST) Received: from localhost (dynscan1.mnet-online.de [192.168.6.70]) by mail.m-online.net (Postfix) with ESMTP id 4CFKdr1BJ8z1r0m0; Mon, 19 Oct 2020 16:58:56 +0200 (CEST) X-Virus-Scanned: amavisd-new at mnet-online.de Received: from mail.mnet-online.de ([192.168.8.182]) by localhost (dynscan1.mail.m-online.net [192.168.6.70]) (amavisd-new, port 10024) with ESMTP id g9bV3hIceIMG; Mon, 19 Oct 2020 16:58:54 +0200 (CEST) X-Auth-Info: //CU9WbW3VP/CWIuYQvgcGaanNBm+ntWW9/6g2WqQEo= Received: from localhost.localdomain (85-222-111-42.dynamic.chello.pl [85.222.111.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.mnet-online.de (Postfix) with ESMTPSA; Mon, 19 Oct 2020 16:58:54 +0200 (CEST) From: Lukasz Majewski <lukma@denx.de> To: Joseph Myers <joseph@codesourcery.com>, Paul Eggert <eggert@cs.ucla.edu>, Adhemerval Zanella <adhemerval.zanella@linaro.org> Subject: [PATCH] y2038: nptl: Provide __futex_clock_wait_bitset64 to support 64 bit bitset Date: Mon, 19 Oct 2020 16:57:39 +0200 Message-Id: <20201019145739.32361-1-lukma@denx.de> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-16.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, 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 <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> Cc: Florian Weimer <fweimer@redhat.com>, Stefan Liebler <stli@linux.ibm.com>, GNU C Library <libc-alpha@sourceware.org>, Andreas Schwab <schwab@suse.de>, Stepan Golosunov <stepan@golosunov.pp.ru>, Alistair Francis <alistair.francis@wdc.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
y2038: nptl: Provide __futex_clock_wait_bitset64 to support 64 bit bitset
|
|
Commit Message
Lukasz Majewski
Oct. 19, 2020, 2:57 p.m. UTC
The commit: "y2038: nptl: Convert pthread_mutex_{clock|timed}lock to support 64 bit" SHA1: 29e9874a048f47e2d46c40253036c8d2de921548 introduced support for 64 bit timeouts. Unfortunately, it was missing the code for bitset - i.e. lll_futex_clock_wait_bitset C preprocessor macro was used. As a result the 64 bit struct __timespec64 was coerced to 32 bit struct timespec and regression visible as timeout was observed (nptl/tst-robust10 on s390). Reported-by: Stefan Liebler <stli@linux.ibm.com> --- nptl/pthread_mutex_timedlock.c | 2 +- sysdeps/nptl/futex-internal.c | 48 +++++++++++++++++++++++++++++++++- sysdeps/nptl/futex-internal.h | 5 ++++ 3 files changed, 53 insertions(+), 2 deletions(-)
Comments
On 10/19/20 4:57 PM, Lukasz Majewski wrote: > The commit: > "y2038: nptl: Convert pthread_mutex_{clock|timed}lock to support 64 bit" > SHA1: 29e9874a048f47e2d46c40253036c8d2de921548 > > introduced support for 64 bit timeouts. Unfortunately, it was missing the > code for bitset - i.e. lll_futex_clock_wait_bitset C preprocessor macro > was used. As a result the 64 bit struct __timespec64 was coerced to 32 > bit struct timespec and regression visible as timeout was observed > (nptl/tst-robust10 on s390). > > Reported-by: Stefan Liebler <stli@linux.ibm.com> I've successfully run some tests on s390 (31bit) / x86 (32bit) with this patch. - strace output of nptl/tst-robust10 on s390 (31bit): 10:41:37.553933 futex_time64(0x406144, FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2150823950, {tv_sec=1603183298, tv_nsec=553918404}, FUTEX_BITSET_MATCH_ANY) = -1 ETIMEDOUT (Connection timed out) <1.000090> => Now there is only this single FUTEX_WAIT_BITSET call which times out after a second. - strace output of nptl/tst-robust10 on x86 (32bit): 10:45:50.985125 futex_time64(0x804f0f4, FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2147912575, {tv_sec=1603183551, tv_nsec=985109825}, FUTEX_BITSET_MATCH_ANY) = -1 ETIMEDOUT (Connection timed out) <1.000439> => Now tv_nsec is not zero anymore and it times out after a second. I have one question - see below. Thanks, Stefan > --- > nptl/pthread_mutex_timedlock.c | 2 +- > sysdeps/nptl/futex-internal.c | 48 +++++++++++++++++++++++++++++++++- > sysdeps/nptl/futex-internal.h | 5 ++++ > 3 files changed, 53 insertions(+), 2 deletions(-) > > diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c > index dc40399f02..fe9e651f6c 100644 > --- a/nptl/pthread_mutex_timedlock.c > +++ b/nptl/pthread_mutex_timedlock.c > @@ -268,7 +268,7 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex, > assume_other_futex_waiters |= FUTEX_WAITERS; > > /* Block using the futex. */ > - int err = lll_futex_clock_wait_bitset (&mutex->__data.__lock, > + int err = __futex_clock_wait_bitset64 (&mutex->__data.__lock, > oldval, clockid, abstime, > PTHREAD_ROBUST_MUTEX_PSHARED (mutex)); > /* The futex call timed out. */ > diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c > index 2e1919f056..a978ad0ad2 100644 > --- a/sysdeps/nptl/futex-internal.c > +++ b/sysdeps/nptl/futex-internal.c > @@ -70,7 +70,28 @@ __futex_abstimed_wait32 (unsigned int* futex_word, > abstime != NULL ? &ts32 : NULL, > NULL /* Unused. */, FUTEX_BITSET_MATCH_ANY); > } > -#endif > + > +static int > +__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid, > + const struct __timespec64 *abstime, int private) > +{ > + struct timespec ts32; > + > + if (abstime != NULL && ! in_time_t_range (abstime->tv_sec)) > + return -EOVERFLOW; > + > + const unsigned int clockbit = > + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; > + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); > + > + if (abstime != NULL) > + ts32 = valid_timespec64_to_timespec (*abstime); > + > + return INTERNAL_SYSCALL_CALL (futex, futexp, op, val, > + abstime != NULL ? &ts32 : NULL, > + NULL /* Unused. */, FUTEX_BITSET_MATCH_ANY); > +} > +#endif /* ! __ASSUME_TIME64_SYSCALLS */ > > int > __futex_abstimed_wait_cancelable64 (unsigned int* futex_word, > @@ -222,3 +243,28 @@ __futex_clocklock_wait64 (int *futex, int val, clockid_t clockid, > > return -err; > } > + > +int > +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid, > + const struct __timespec64 *abstime, > + int private) > +{ > + long int ret; Is there a reason why long int is used instead of int (which is also returned by this function)? > + if (! lll_futex_supported_clockid (clockid)) > + { > + return -EINVAL; > + } > + > + const unsigned int clockbit = > + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; > + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); > + > + ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val, > + abstime, NULL /* Unused. */, > + FUTEX_BITSET_MATCH_ANY); > +#ifndef __ASSUME_TIME64_SYSCALLS > + if (ret == -ENOSYS) > + ret = __futex_clock_wait_bitset32 (futexp, val, clockid, abstime, private); > +#endif > + return ret; > +} > diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h > index 8a5f62768f..cd356e4fa8 100644 > --- a/sysdeps/nptl/futex-internal.h > +++ b/sysdeps/nptl/futex-internal.h > @@ -603,4 +603,9 @@ __futex_clocklock64 (int *futex, clockid_t clockid, > return err; > } > > +int > +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid, > + const struct __timespec64 *abstime, > + int private) attribute_hidden; > + > #endif /* futex-internal.h */ >
Hi Stefan, Thank you for testing. > On 10/19/20 4:57 PM, Lukasz Majewski wrote: > > The commit: > > "y2038: nptl: Convert pthread_mutex_{clock|timed}lock to support 64 > > bit" SHA1: 29e9874a048f47e2d46c40253036c8d2de921548 > > > > introduced support for 64 bit timeouts. Unfortunately, it was > > missing the code for bitset - i.e. lll_futex_clock_wait_bitset C > > preprocessor macro was used. As a result the 64 bit struct > > __timespec64 was coerced to 32 bit struct timespec and regression > > visible as timeout was observed (nptl/tst-robust10 on s390). > > > > Reported-by: Stefan Liebler <stli@linux.ibm.com> > > I've successfully run some tests on s390 (31bit) / x86 (32bit) with > this patch. > - strace output of nptl/tst-robust10 on s390 (31bit): > 10:41:37.553933 futex_time64(0x406144, > FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2150823950, > {tv_sec=1603183298, tv_nsec=553918404}, FUTEX_BITSET_MATCH_ANY) = -1 > ETIMEDOUT (Connection timed out) <1.000090> > => Now there is only this single FUTEX_WAIT_BITSET call which times > out after a second. The tv_sec=1603183298 now seems to be correct. > > - strace output of nptl/tst-robust10 on x86 (32bit): > 10:45:50.985125 futex_time64(0x804f0f4, > FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2147912575, > {tv_sec=1603183551, tv_nsec=985109825}, FUTEX_BITSET_MATCH_ANY) = -1 > ETIMEDOUT (Connection timed out) <1.000439> > => Now tv_nsec is not zero anymore and it times out after a second. > > I have one question - see below. > > Thanks, > Stefan > > > --- > > nptl/pthread_mutex_timedlock.c | 2 +- > > sysdeps/nptl/futex-internal.c | 48 > > +++++++++++++++++++++++++++++++++- sysdeps/nptl/futex-internal.h | > > 5 ++++ 3 files changed, 53 insertions(+), 2 deletions(-) > > > > diff --git a/nptl/pthread_mutex_timedlock.c > > b/nptl/pthread_mutex_timedlock.c index dc40399f02..fe9e651f6c 100644 > > --- a/nptl/pthread_mutex_timedlock.c > > +++ b/nptl/pthread_mutex_timedlock.c > > @@ -268,7 +268,7 @@ __pthread_mutex_clocklock_common > > (pthread_mutex_t *mutex, assume_other_futex_waiters |= > > FUTEX_WAITERS; > > /* Block using the futex. */ > > - int err = lll_futex_clock_wait_bitset > > (&mutex->__data.__lock, > > + int err = __futex_clock_wait_bitset64 > > (&mutex->__data.__lock, oldval, clockid, abstime, > > PTHREAD_ROBUST_MUTEX_PSHARED (mutex)); > > /* The futex call timed out. */ > > diff --git a/sysdeps/nptl/futex-internal.c > > b/sysdeps/nptl/futex-internal.c index 2e1919f056..a978ad0ad2 100644 > > --- a/sysdeps/nptl/futex-internal.c > > +++ b/sysdeps/nptl/futex-internal.c > > @@ -70,7 +70,28 @@ __futex_abstimed_wait32 (unsigned int* > > futex_word, abstime != NULL ? &ts32 : NULL, > > NULL /* Unused. */, > > FUTEX_BITSET_MATCH_ANY); } > > -#endif > > + > > +static int > > +__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t > > clockid, > > + const struct __timespec64 *abstime, > > int private) +{ > > + struct timespec ts32; > > + > > + if (abstime != NULL && ! in_time_t_range (abstime->tv_sec)) > > + return -EOVERFLOW; > > + > > + const unsigned int clockbit = > > + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; > > + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, > > private); + > > + if (abstime != NULL) > > + ts32 = valid_timespec64_to_timespec (*abstime); > > + > > + return INTERNAL_SYSCALL_CALL (futex, futexp, op, val, > > + abstime != NULL ? &ts32 : NULL, > > + NULL /* Unused. */, > > FUTEX_BITSET_MATCH_ANY); +} > > +#endif /* ! __ASSUME_TIME64_SYSCALLS */ > > > > int > > __futex_abstimed_wait_cancelable64 (unsigned int* futex_word, > > @@ -222,3 +243,28 @@ __futex_clocklock_wait64 (int *futex, int val, > > clockid_t clockid, > > return -err; > > } > > + > > +int > > +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t > > clockid, > > + const struct __timespec64 *abstime, > > + int private) > > +{ > > + long int relowlevellock-futex.ht; > Is there a reason why long int is used instead of int (which is also > returned by this function)? The "long int" type for ret was in the original lll_futex_clock_wait_bitset() macro in: ./sysdeps/nptl/lowlevellock-futex.h (line 102). It is also returned when this macro is pasted. The reason may be that lll_syscall_futex() macro also uses long int to get return value from INTERNAL_SYSCALL() (Line 70 in the above file). Then depending on architecture either long or int are used to get the status of the syscall. However, in the end the value returned by lll_futex_clock_wait_bitset() is casted to int anyway: git grep -n "= lll_futex_clock_wait_bitset" nptl/pthread_mutex_timedlock.c:271: int err = lll_futex_clock_wait_bitset (&mutex->__data.__lock, sysdeps/nptl/futex-internal.h:288: int err = lll_futex_clock_wait_bitset (futex_word, expected, sysdeps/nptl/futex-internal.h:324: int err = lll_futex_clock_wait_bitset (futex_word, expected, Hence, IMHO it would be correct to change 'long int' to 'int'. Or am I wrong? > > + if (! lll_futex_supported_clockid (clockid)) > > + { > > + return -EINVAL; > > + } > > + > > + const unsigned int clockbit = > > + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; > > + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, > > private); + > > + ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val, > > + abstime, NULL /* Unused. */, > > + FUTEX_BITSET_MATCH_ANY); > > +#ifndef __ASSUME_TIME64_SYSCALLS > > + if (ret == -ENOSYS) > > + ret = __futex_clock_wait_bitset32 (futexp, val, clockid, > > abstime, private); +#endif > > + return ret; > > +} > > diff --git a/sysdeps/nptl/futex-internal.h > > b/sysdeps/nptl/futex-internal.h index 8a5f62768f..cd356e4fa8 100644 > > --- a/sysdeps/nptl/futex-internal.h > > +++ b/sysdeps/nptl/futex-internal.h > > @@ -603,4 +603,9 @@ __futex_clocklock64 (int *futex, clockid_t > > clockid, return err; > > } > > > > +int > > +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t > > clockid, > > + const struct __timespec64 *abstime, > > + int private) attribute_hidden; > > + > > #endif /* futex-internal.h */ > > > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On 10/20/20 3:31 PM, Lukasz Majewski wrote: > Hi Stefan, > > Thank you for testing. > >> On 10/19/20 4:57 PM, Lukasz Majewski wrote: >>> The commit: >>> "y2038: nptl: Convert pthread_mutex_{clock|timed}lock to support 64 >>> bit" SHA1: 29e9874a048f47e2d46c40253036c8d2de921548 >>> >>> introduced support for 64 bit timeouts. Unfortunately, it was >>> missing the code for bitset - i.e. lll_futex_clock_wait_bitset C >>> preprocessor macro was used. As a result the 64 bit struct >>> __timespec64 was coerced to 32 bit struct timespec and regression >>> visible as timeout was observed (nptl/tst-robust10 on s390). >>> >>> Reported-by: Stefan Liebler <stli@linux.ibm.com> >> >> I've successfully run some tests on s390 (31bit) / x86 (32bit) with >> this patch. >> - strace output of nptl/tst-robust10 on s390 (31bit): >> 10:41:37.553933 futex_time64(0x406144, >> FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2150823950, >> {tv_sec=1603183298, tv_nsec=553918404}, FUTEX_BITSET_MATCH_ANY) = -1 >> ETIMEDOUT (Connection timed out) <1.000090> >> => Now there is only this single FUTEX_WAIT_BITSET call which times >> out after a second. > > The tv_sec=1603183298 now seems to be correct. > >> >> - strace output of nptl/tst-robust10 on x86 (32bit): >> 10:45:50.985125 futex_time64(0x804f0f4, >> FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2147912575, >> {tv_sec=1603183551, tv_nsec=985109825}, FUTEX_BITSET_MATCH_ANY) = -1 >> ETIMEDOUT (Connection timed out) <1.000439> >> => Now tv_nsec is not zero anymore and it times out after a second. >> >> I have one question - see below. >> >> Thanks, >> Stefan >> >>> --- >>> nptl/pthread_mutex_timedlock.c | 2 +- >>> sysdeps/nptl/futex-internal.c | 48 >>> +++++++++++++++++++++++++++++++++- sysdeps/nptl/futex-internal.h | >>> 5 ++++ 3 files changed, 53 insertions(+), 2 deletions(-) >>> >>> diff --git a/nptl/pthread_mutex_timedlock.c >>> b/nptl/pthread_mutex_timedlock.c index dc40399f02..fe9e651f6c 100644 >>> --- a/nptl/pthread_mutex_timedlock.c >>> +++ b/nptl/pthread_mutex_timedlock.c >>> @@ -268,7 +268,7 @@ __pthread_mutex_clocklock_common >>> (pthread_mutex_t *mutex, assume_other_futex_waiters |= >>> FUTEX_WAITERS; >>> /* Block using the futex. */ >>> - int err = lll_futex_clock_wait_bitset >>> (&mutex->__data.__lock, >>> + int err = __futex_clock_wait_bitset64 >>> (&mutex->__data.__lock, oldval, clockid, abstime, >>> PTHREAD_ROBUST_MUTEX_PSHARED (mutex)); >>> /* The futex call timed out. */ >>> diff --git a/sysdeps/nptl/futex-internal.c >>> b/sysdeps/nptl/futex-internal.c index 2e1919f056..a978ad0ad2 100644 >>> --- a/sysdeps/nptl/futex-internal.c >>> +++ b/sysdeps/nptl/futex-internal.c >>> @@ -70,7 +70,28 @@ __futex_abstimed_wait32 (unsigned int* >>> futex_word, abstime != NULL ? &ts32 : NULL, >>> NULL /* Unused. */, >>> FUTEX_BITSET_MATCH_ANY); } >>> -#endif >>> + >>> +static int >>> +__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t >>> clockid, >>> + const struct __timespec64 *abstime, >>> int private) +{ >>> + struct timespec ts32; >>> + >>> + if (abstime != NULL && ! in_time_t_range (abstime->tv_sec)) >>> + return -EOVERFLOW; >>> + >>> + const unsigned int clockbit = >>> + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; >>> + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, >>> private); + >>> + if (abstime != NULL) >>> + ts32 = valid_timespec64_to_timespec (*abstime); >>> + >>> + return INTERNAL_SYSCALL_CALL (futex, futexp, op, val, >>> + abstime != NULL ? &ts32 : NULL, >>> + NULL /* Unused. */, >>> FUTEX_BITSET_MATCH_ANY); +} >>> +#endif /* ! __ASSUME_TIME64_SYSCALLS */ >>> >>> int >>> __futex_abstimed_wait_cancelable64 (unsigned int* futex_word, >>> @@ -222,3 +243,28 @@ __futex_clocklock_wait64 (int *futex, int val, >>> clockid_t clockid, >>> return -err; >>> } >>> + >>> +int >>> +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t >>> clockid, >>> + const struct __timespec64 *abstime, >>> + int private) >>> +{ >>> + long int relowlevellock-futex.ht; >> Is there a reason why long int is used instead of int (which is also >> returned by this function)? > > The "long int" type for ret was in the original > lll_futex_clock_wait_bitset() macro in: > ./sysdeps/nptl/lowlevellock-futex.h (line 102). > > It is also returned when this macro is pasted. > > The reason may be that lll_syscall_futex() macro also uses long int to > get return value from INTERNAL_SYSCALL() (Line 70 in the above file). > > Then depending on architecture either long or int are used to get the > status of the syscall. > > > However, in the end the value returned by lll_futex_clock_wait_bitset() > is casted to int anyway: > > git grep -n "= lll_futex_clock_wait_bitset" > nptl/pthread_mutex_timedlock.c:271: > int err = lll_futex_clock_wait_bitset (&mutex->__data.__lock, > > sysdeps/nptl/futex-internal.h:288: > int err = lll_futex_clock_wait_bitset (futex_word, expected, > > sysdeps/nptl/futex-internal.h:324: > int err = lll_futex_clock_wait_bitset (futex_word, expected, > > Hence, IMHO it would be correct to change 'long int' to 'int'. Or am I > wrong? Yes, I think this is okay. > >>> + if (! lll_futex_supported_clockid (clockid)) >>> + { >>> + return -EINVAL; >>> + } >>> + >>> + const unsigned int clockbit = >>> + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; >>> + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, >>> private); + >>> + ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val, >>> + abstime, NULL /* Unused. */, >>> + FUTEX_BITSET_MATCH_ANY); >>> +#ifndef __ASSUME_TIME64_SYSCALLS >>> + if (ret == -ENOSYS) >>> + ret = __futex_clock_wait_bitset32 (futexp, val, clockid, >>> abstime, private); +#endif >>> + return ret; >>> +} >>> diff --git a/sysdeps/nptl/futex-internal.h >>> b/sysdeps/nptl/futex-internal.h index 8a5f62768f..cd356e4fa8 100644 >>> --- a/sysdeps/nptl/futex-internal.h >>> +++ b/sysdeps/nptl/futex-internal.h >>> @@ -603,4 +603,9 @@ __futex_clocklock64 (int *futex, clockid_t >>> clockid, return err; >>> } >>> >>> +int >>> +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t >>> clockid, >>> + const struct __timespec64 *abstime, >>> + int private) attribute_hidden; >>> + >>> #endif /* futex-internal.h */ >>> >> > > > Best regards, > > Lukasz Majewski > > -- > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de >
diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c index dc40399f02..fe9e651f6c 100644 --- a/nptl/pthread_mutex_timedlock.c +++ b/nptl/pthread_mutex_timedlock.c @@ -268,7 +268,7 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex, assume_other_futex_waiters |= FUTEX_WAITERS; /* Block using the futex. */ - int err = lll_futex_clock_wait_bitset (&mutex->__data.__lock, + int err = __futex_clock_wait_bitset64 (&mutex->__data.__lock, oldval, clockid, abstime, PTHREAD_ROBUST_MUTEX_PSHARED (mutex)); /* The futex call timed out. */ diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c index 2e1919f056..a978ad0ad2 100644 --- a/sysdeps/nptl/futex-internal.c +++ b/sysdeps/nptl/futex-internal.c @@ -70,7 +70,28 @@ __futex_abstimed_wait32 (unsigned int* futex_word, abstime != NULL ? &ts32 : NULL, NULL /* Unused. */, FUTEX_BITSET_MATCH_ANY); } -#endif + +static int +__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid, + const struct __timespec64 *abstime, int private) +{ + struct timespec ts32; + + if (abstime != NULL && ! in_time_t_range (abstime->tv_sec)) + return -EOVERFLOW; + + const unsigned int clockbit = + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); + + if (abstime != NULL) + ts32 = valid_timespec64_to_timespec (*abstime); + + return INTERNAL_SYSCALL_CALL (futex, futexp, op, val, + abstime != NULL ? &ts32 : NULL, + NULL /* Unused. */, FUTEX_BITSET_MATCH_ANY); +} +#endif /* ! __ASSUME_TIME64_SYSCALLS */ int __futex_abstimed_wait_cancelable64 (unsigned int* futex_word, @@ -222,3 +243,28 @@ __futex_clocklock_wait64 (int *futex, int val, clockid_t clockid, return -err; } + +int +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid, + const struct __timespec64 *abstime, + int private) +{ + long int ret; + if (! lll_futex_supported_clockid (clockid)) + { + return -EINVAL; + } + + const unsigned int clockbit = + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0; + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); + + ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val, + abstime, NULL /* Unused. */, + FUTEX_BITSET_MATCH_ANY); +#ifndef __ASSUME_TIME64_SYSCALLS + if (ret == -ENOSYS) + ret = __futex_clock_wait_bitset32 (futexp, val, clockid, abstime, private); +#endif + return ret; +} diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h index 8a5f62768f..cd356e4fa8 100644 --- a/sysdeps/nptl/futex-internal.h +++ b/sysdeps/nptl/futex-internal.h @@ -603,4 +603,9 @@ __futex_clocklock64 (int *futex, clockid_t clockid, return err; } +int +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid, + const struct __timespec64 *abstime, + int private) attribute_hidden; + #endif /* futex-internal.h */