[17/18] linux: Only use 64-bit syscall if required for internal futex
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
For !__ASSUME_TIME64_SYSCALLS there is no need to issue a 64-bit syscall
if the provided timeout fits in a 32-bit one. The 64-bit usage should
be rare since the timeout is a relative one.
Checked on i686-linux-gnu on a 4.15 kernel and on a 5.11 kernel
(with and without --enable-kernel=5.1) and on x86_64-linux-gnu.
---
nptl/futex-internal.c | 52 +++++++++++++++++++++++------------
sysdeps/nptl/futex-internal.h | 24 ++++++++++------
2 files changed, 50 insertions(+), 26 deletions(-)
Comments
On Thu, 17 Jun 2021 08:51:03 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> For !__ASSUME_TIME64_SYSCALLS there is no need to issue a 64-bit
> syscall if the provided timeout fits in a 32-bit one. The 64-bit
> usage should be rare since the timeout is a relative one.
>
> Checked on i686-linux-gnu on a 4.15 kernel and on a 5.11 kernel
> (with and without --enable-kernel=5.1) and on x86_64-linux-gnu.
> ---
> nptl/futex-internal.c | 52
> +++++++++++++++++++++++------------ sysdeps/nptl/futex-internal.h |
> 24 ++++++++++------ 2 files changed, 50 insertions(+), 26 deletions(-)
>
> diff --git a/nptl/futex-internal.c b/nptl/futex-internal.c
> index 850bf4fd83..277e60986e 100644
> --- a/nptl/futex-internal.c
> +++ b/nptl/futex-internal.c
> @@ -32,9 +32,6 @@ __futex_abstimed_wait_common32 (unsigned int*
> futex_word, struct timespec ts32, *pts32 = NULL;
> if (abstime != NULL)
> {
> - if (! in_time_t_range (abstime->tv_sec))
> - return -EOVERFLOW;
> -
> ts32 = valid_timespec64_to_timespec (*abstime);
> pts32 = &ts32;
> }
> @@ -52,12 +49,28 @@ __futex_abstimed_wait_common32 (unsigned int*
> futex_word,
> static int
> __futex_abstimed_wait_common64 (unsigned int* futex_word,
> - unsigned int expected, clockid_t
> clockid,
> + unsigned int expected, int op,
> const struct __timespec64* abstime,
> int private, bool cancel)
> {
> - unsigned int clockbit;
> + if (cancel)
> + return INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op,
> expected,
> + abstime, NULL /* Unused. */,
> + FUTEX_BITSET_MATCH_ANY);
> + else
> + return INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> expected,
> + abstime, NULL /* Ununsed. */,
> + FUTEX_BITSET_MATCH_ANY);
> +}
> +
> +static int
> +__futex_abstimed_wait_common (unsigned int* futex_word,
> + unsigned int expected, clockid_t
> clockid,
> + const struct __timespec64* abstime,
> + int private, bool cancel)
> +{
> int err;
> + unsigned int clockbit;
>
> /* Work around the fact that the kernel rejects negative timeout
> values despite them being valid. */
> @@ -70,16 +83,19 @@ __futex_abstimed_wait_common64 (unsigned int*
> futex_word, clockbit = (clockid == CLOCK_REALTIME) ?
> FUTEX_CLOCK_REALTIME : 0; int op = __lll_private_flag
> (FUTEX_WAIT_BITSET | clockbit, private);
> - if (cancel)
> - err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op,
> expected,
> - abstime, NULL /* Unused. */,
> - FUTEX_BITSET_MATCH_ANY);
> +#ifdef __ASSUME_TIME64_SYSCALLS
> + err = __futex_abstimed_wait_common64 (futex_word, expected, op,
> abstime,
> + private, cancel);
> +#else
> + bool is32bit = abstime != NULL ? in_time_t_range (abstime->tv_sec)
> : true;
> + if (!is32bit)
> + {
> + err = __futex_abstimed_wait_common64 (futex_word, expected,
> op, abstime,
> + private, cancel);
> + if (err == -ENOSYS)
> + err = -EOVERFLOW;
> + }
> else
> - err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> expected,
> - abstime, NULL /* Ununsed. */,
> - FUTEX_BITSET_MATCH_ANY);
> -#ifndef __ASSUME_TIME64_SYSCALLS
> - if (err == -ENOSYS)
> err = __futex_abstimed_wait_common32 (futex_word, expected, op,
> abstime, private, cancel);
> #endif
> @@ -109,8 +125,8 @@ __futex_abstimed_wait64 (unsigned int*
> futex_word, unsigned int expected, clockid_t clockid,
> const struct __timespec64* abstime, int
> private) {
> - return __futex_abstimed_wait_common64 (futex_word, expected,
> clockid,
> - abstime, private, false);
> + return __futex_abstimed_wait_common (futex_word, expected, clockid,
> + abstime, private, false);
> }
> libc_hidden_def (__futex_abstimed_wait64)
>
> @@ -120,7 +136,7 @@ __futex_abstimed_wait_cancelable64 (unsigned int*
> futex_word, const struct __timespec64* abstime,
> int private)
> {
> - return __futex_abstimed_wait_common64 (futex_word, expected,
> clockid,
> - abstime, private, true);
> + return __futex_abstimed_wait_common (futex_word, expected, clockid,
> + abstime, private, true);
> }
> libc_hidden_def (__futex_abstimed_wait_cancelable64)
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index 969ab2bf4b..b54fdd44c1 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -254,15 +254,23 @@ static __always_inline int
> futex_lock_pi64 (int *futex_word, const struct __timespec64 *abstime,
> int private)
> {
> - int err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word,
> - __lll_private_flag
> - (FUTEX_LOCK_PI, private), 0,
> abstime); -#ifndef __ASSUME_TIME64_SYSCALLS
> - if (err == -ENOSYS)
> + int err;
> +#ifdef __ASSUME_TIME64_SYSCALLS
> + err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word,
> + __lll_private_flag (FUTEX_LOCK_PI,
> private), 0,
> + abstime);
> +#else
> + bool is32bit = abstime != NULL ? in_time_t_range (abstime->tv_sec)
> : true;
> + if (!is32bit)
> + {
> + err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word,
> + __lll_private_flag
> (FUTEX_LOCK_PI, private),
> + 0, abstime);
> + if (err == -ENOSYS)
> + err = -EOVERFLOW;
> + }
> + else
> {
> - if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> - return EOVERFLOW;
> -
> struct timespec ts32;
> if (abstime != NULL)
> ts32 = valid_timespec64_to_timespec (*abstime);
Reviewed-by: Lukasz Majewski <lukma@denx.de>
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
@@ -32,9 +32,6 @@ __futex_abstimed_wait_common32 (unsigned int* futex_word,
struct timespec ts32, *pts32 = NULL;
if (abstime != NULL)
{
- if (! in_time_t_range (abstime->tv_sec))
- return -EOVERFLOW;
-
ts32 = valid_timespec64_to_timespec (*abstime);
pts32 = &ts32;
}
@@ -52,12 +49,28 @@ __futex_abstimed_wait_common32 (unsigned int* futex_word,
static int
__futex_abstimed_wait_common64 (unsigned int* futex_word,
- unsigned int expected, clockid_t clockid,
+ unsigned int expected, int op,
const struct __timespec64* abstime,
int private, bool cancel)
{
- unsigned int clockbit;
+ if (cancel)
+ return INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected,
+ abstime, NULL /* Unused. */,
+ FUTEX_BITSET_MATCH_ANY);
+ else
+ return INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, expected,
+ abstime, NULL /* Ununsed. */,
+ FUTEX_BITSET_MATCH_ANY);
+}
+
+static int
+__futex_abstimed_wait_common (unsigned int* futex_word,
+ unsigned int expected, clockid_t clockid,
+ const struct __timespec64* abstime,
+ int private, bool cancel)
+{
int err;
+ unsigned int clockbit;
/* Work around the fact that the kernel rejects negative timeout values
despite them being valid. */
@@ -70,16 +83,19 @@ __futex_abstimed_wait_common64 (unsigned int* futex_word,
clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
- if (cancel)
- err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected,
- abstime, NULL /* Unused. */,
- FUTEX_BITSET_MATCH_ANY);
+#ifdef __ASSUME_TIME64_SYSCALLS
+ err = __futex_abstimed_wait_common64 (futex_word, expected, op, abstime,
+ private, cancel);
+#else
+ bool is32bit = abstime != NULL ? in_time_t_range (abstime->tv_sec) : true;
+ if (!is32bit)
+ {
+ err = __futex_abstimed_wait_common64 (futex_word, expected, op, abstime,
+ private, cancel);
+ if (err == -ENOSYS)
+ err = -EOVERFLOW;
+ }
else
- err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, expected,
- abstime, NULL /* Ununsed. */,
- FUTEX_BITSET_MATCH_ANY);
-#ifndef __ASSUME_TIME64_SYSCALLS
- if (err == -ENOSYS)
err = __futex_abstimed_wait_common32 (futex_word, expected, op, abstime,
private, cancel);
#endif
@@ -109,8 +125,8 @@ __futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
clockid_t clockid,
const struct __timespec64* abstime, int private)
{
- return __futex_abstimed_wait_common64 (futex_word, expected, clockid,
- abstime, private, false);
+ return __futex_abstimed_wait_common (futex_word, expected, clockid,
+ abstime, private, false);
}
libc_hidden_def (__futex_abstimed_wait64)
@@ -120,7 +136,7 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
const struct __timespec64* abstime,
int private)
{
- return __futex_abstimed_wait_common64 (futex_word, expected, clockid,
- abstime, private, true);
+ return __futex_abstimed_wait_common (futex_word, expected, clockid,
+ abstime, private, true);
}
libc_hidden_def (__futex_abstimed_wait_cancelable64)
@@ -254,15 +254,23 @@ static __always_inline int
futex_lock_pi64 (int *futex_word, const struct __timespec64 *abstime,
int private)
{
- int err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word,
- __lll_private_flag
- (FUTEX_LOCK_PI, private), 0, abstime);
-#ifndef __ASSUME_TIME64_SYSCALLS
- if (err == -ENOSYS)
+ int err;
+#ifdef __ASSUME_TIME64_SYSCALLS
+ err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word,
+ __lll_private_flag (FUTEX_LOCK_PI, private), 0,
+ abstime);
+#else
+ bool is32bit = abstime != NULL ? in_time_t_range (abstime->tv_sec) : true;
+ if (!is32bit)
+ {
+ err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word,
+ __lll_private_flag (FUTEX_LOCK_PI, private),
+ 0, abstime);
+ if (err == -ENOSYS)
+ err = -EOVERFLOW;
+ }
+ else
{
- if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
- return EOVERFLOW;
-
struct timespec ts32;
if (abstime != NULL)
ts32 = valid_timespec64_to_timespec (*abstime);