* [RFC 01/10] doc: Fix wording and formatting in ./support/README
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-07 15:11 ` Florian Weimer
2020-07-07 15:08 ` [RFC 02/10] y2038: Convert timespec_* from posix-timer.h to be Y2038 safe Lukasz Majewski
` (10 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
---
support/README | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/support/README b/support/README
index ae2c41caa8..bbd3d588f2 100644
--- a/support/README
+++ b/support/README
@@ -4,8 +4,8 @@ not) and tests.
# Error-checking wrappers
-These wrappers test for error return codes an terminate the process on
-error. They are declared in these header files:
+These wrappers test for error return codes and terminate the process on
+error. They are declared in these header files:
* support.h
* xsignal.h
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 01/10] doc: Fix wording and formatting in ./support/README
2020-07-07 15:08 ` [RFC 01/10] doc: Fix wording and formatting in ./support/README Lukasz Majewski
@ 2020-07-07 15:11 ` Florian Weimer
0 siblings, 0 replies; 30+ messages in thread
From: Florian Weimer @ 2020-07-07 15:11 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Alistair Francis,
Arnd Bergmann, Alistair Francis, GNU C Library,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
* Lukasz Majewski:
> ---
> support/README | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/support/README b/support/README
> index ae2c41caa8..bbd3d588f2 100644
> --- a/support/README
> +++ b/support/README
> @@ -4,8 +4,8 @@ not) and tests.
>
> # Error-checking wrappers
>
> -These wrappers test for error return codes an terminate the process on
> -error. They are declared in these header files:
> +These wrappers test for error return codes and terminate the process on
> +error. They are declared in these header files:
We generally use two spaces after a sentence-ending period in glibc.
Thanks,
Florian
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 02/10] y2038: Convert timespec_* from posix-timer.h to be Y2038 safe
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
2020-07-07 15:08 ` [RFC 01/10] doc: Fix wording and formatting in ./support/README Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-07 15:08 ` [RFC 03/10] y2038: Convert __lll_futex* functions to use futex_time64 when available Lukasz Majewski
` (9 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
Static inline functions - timespec_compare, timespec_sum and
timespec_diff - from ./sysdeps/pthread/posix-timer.h have been
converted to support 64 bit time on ports with __WORDSIZE == 32
&& __TIMESIZE != 64.
The change was focused on using struct __timespec64.
Tested with glibc/glibc-many-build --keep failed glibcs
---
sysdeps/pthread/posix-timer.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/sysdeps/pthread/posix-timer.h b/sysdeps/pthread/posix-timer.h
index 81fc8a2394..86bb123b7a 100644
--- a/sysdeps/pthread/posix-timer.h
+++ b/sysdeps/pthread/posix-timer.h
@@ -115,7 +115,8 @@ timer_delref (struct timer_node *timer)
/* Timespec helper routines. */
static inline int
__attribute ((always_inline))
-timespec_compare (const struct timespec *left, const struct timespec *right)
+timespec_compare (const struct __timespec64 *left,
+ const struct __timespec64 *right)
{
if (left->tv_sec < right->tv_sec)
return -1;
@@ -131,8 +132,8 @@ timespec_compare (const struct timespec *left, const struct timespec *right)
}
static inline void
-timespec_add (struct timespec *sum, const struct timespec *left,
- const struct timespec *right)
+timespec_add (struct __timespec64 *sum, const struct __timespec64 *left,
+ const struct __timespec64 *right)
{
sum->tv_sec = left->tv_sec + right->tv_sec;
sum->tv_nsec = left->tv_nsec + right->tv_nsec;
@@ -145,8 +146,8 @@ timespec_add (struct timespec *sum, const struct timespec *left,
}
static inline void
-timespec_sub (struct timespec *diff, const struct timespec *left,
- const struct timespec *right)
+timespec_sub (struct __timespec64 *diff, const struct __timespec64 *left,
+ const struct __timespec64 *right)
{
diff->tv_sec = left->tv_sec - right->tv_sec;
diff->tv_nsec = left->tv_nsec - right->tv_nsec;
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [RFC 03/10] y2038: Convert __lll_futex* functions to use futex_time64 when available
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
2020-07-07 15:08 ` [RFC 01/10] doc: Fix wording and formatting in ./support/README Lukasz Majewski
2020-07-07 15:08 ` [RFC 02/10] y2038: Convert timespec_* from posix-timer.h to be Y2038 safe Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-12 13:43 ` Lukasz Majewski
2020-07-07 15:08 ` [RFC 04/10] y2038: Replace struct timespec with __timespec64 in futex-internal.h Lukasz Majewski
` (8 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
As the 'futex' syscall is wrapped with a C preprocessor macros, it was
necessary to provide new set of those to replace call it with Y2038
safe 'futex_time64' syscall.
The current code expand wrapper's arguments as __VA_ARGS__ when more than
three arguments are passed. For the conversion it was necessary to also
have 'timeout' explicitly passed, and hence the introduction of
lll_futex_syscall_time64_4 and lll_futex_syscall_time64 functions.
The former expects exactly 4 arguments - timeout is the last one.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
sysdeps/nptl/lowlevellock-futex.h | 80 ++++++++++++++++++++++++++++---
sysdeps/nptl/lowlevellock.h | 2 +-
2 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/sysdeps/nptl/lowlevellock-futex.h b/sysdeps/nptl/lowlevellock-futex.h
index 2209ca76a1..a686773db4 100644
--- a/sysdeps/nptl/lowlevellock-futex.h
+++ b/sysdeps/nptl/lowlevellock-futex.h
@@ -65,14 +65,82 @@
(((fl) | FUTEX_PRIVATE_FLAG) ^ (private))
# endif
-# define lll_futex_syscall(nargs, futexp, op, ...) \
+# define __lll_futex_syscall(nargs, futexp, op, ...) \
({ \
- long int __ret = INTERNAL_SYSCALL (futex, nargs, futexp, op, \
+ long int __ret = INTERNAL_SYSCALL (futex, nargs, futexp, op, \
__VA_ARGS__); \
(__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (__ret)) \
? -INTERNAL_SYSCALL_ERRNO (__ret) : 0); \
})
+# define __lll_futex_syscall64(nargs, futexp, op, ...) \
+ ({ \
+ long int __ret = INTERNAL_SYSCALL (futex_time64, nargs, futexp, op, \
+ __VA_ARGS__); \
+ (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (__ret)) \
+ ? -INTERNAL_SYSCALL_ERRNO (__ret) : 0); \
+ })
+
+# ifdef __ASSUME_TIME64_SYSCALLS
+# ifndef __NR_futex_time64
+# define __NR_futex_time64 __NR_futex
+# endif
+# define lll_futex_syscall(nargs, futexp, op, ...) \
+ __lll_futex_syscall64(nargs, futexp, op, __VA_ARGS__)
+# define lll_futex_syscall_time64_4(nargs, futexp, op, val, timeout)\
+ __lll_futex_syscall64(nargs, futexp, op, val, timeout)
+# define lll_futex_syscall_time64(nargs, futexp, op, val, timeout, ...)\
+ __lll_futex_syscall64(nargs, futexp, op, val, timeout, __VA_ARGS__)
+# else
+inline static long int
+__lll_futex_syscall_time_check (long int __ret, const struct __timespec64 *t)
+{
+ if (! (__ret == 0 || errno != ENOSYS))
+ if (t != NULL && ! in_time_t_range (t->tv_sec))
+ __ret = ({ errno = (EOVERFLOW); -1l; });
+ return __ret;
+}
+# define lll_futex_syscall(nargs, futexp, op, ...) \
+ ({ \
+ long int __ret = \
+ __lll_futex_syscall64(nargs, futexp, op, __VA_ARGS__); \
+ if (! (__ret == 0 || errno != ENOSYS)) \
+ __ret = \
+ __lll_futex_syscall(nargs, futexp, op, __VA_ARGS__); \
+ __ret; \
+ })
+
+# define lll_futex_syscall_time64(nargs, futexp, op, val, timeout, ...)\
+ ({ \
+ struct timespec __ts32; \
+ if (timeout != NULL) \
+ __ts32 = valid_timespec64_to_timespec (*((struct __timespec64*) timeout)); \
+ long int __ret = \
+ __lll_futex_syscall64(nargs, futexp, op, val, timeout,\
+ __VA_ARGS__); \
+ __ret = __lll_futex_syscall_time_check (__ret, timeout); \
+ if (__ret != 0) \
+ __ret = __lll_futex_syscall(nargs, futexp, op, val, \
+ timeout != NULL ? &__ts32 : NULL, \
+ __VA_ARGS__); \
+ __ret; \
+ })
+
+# define lll_futex_syscall_time64_4(nargs, futexp, op, val, timeout)\
+ ({ \
+ struct timespec __ts32; \
+ if (timeout != NULL) \
+ __ts32 = valid_timespec64_to_timespec (*((struct __timespec64*) timeout)); \
+ long int __ret = \
+ __lll_futex_syscall64(nargs, futexp, op, val, timeout); \
+ __ret = __lll_futex_syscall_time_check (__ret, timeout); \
+ if (__ret != 0) \
+ __ret = __lll_futex_syscall(nargs, futexp, op, val, \
+ timeout != NULL ? &__ts32 : NULL); \
+ __ret; \
+ })
+
+# endif
/* For most of these macros, the return value is never really used.
Nevertheless, the protocol is that each one returns a negated errno
code for failure or zero for success. (Note that the corresponding
@@ -85,7 +153,7 @@
lll_futex_timed_wait (futexp, val, NULL, private)
# define lll_futex_timed_wait(futexp, val, timeout, private) \
- lll_futex_syscall (4, futexp, \
+ lll_futex_syscall_time64_4 (4, futexp, \
__lll_private_flag (FUTEX_WAIT, private), \
val, timeout)
@@ -107,7 +175,7 @@
const int op = \
__lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); \
\
- __ret = lll_futex_syscall (6, futexp, op, val, \
+ __ret = lll_futex_syscall_time64 (6, futexp, op, val, \
timeout, NULL /* Unused. */, \
FUTEX_BITSET_MATCH_ANY); \
} \
@@ -118,7 +186,7 @@
/* Wake up up to NR waiters on FUTEXP. */
# define lll_futex_wake(futexp, nr, private) \
- lll_futex_syscall (4, futexp, \
+ lll_futex_syscall_time64_4 (4, futexp, \
__lll_private_flag (FUTEX_WAKE, private), nr, 0)
/* Wake up up to NR_WAKE waiters on FUTEXP. Move up to NR_MOVE of the
@@ -159,7 +227,7 @@
/* Like lll_futex_wait_requeue_pi, but with a timeout. */
# define lll_futex_timed_wait_requeue_pi(futexp, val, timeout, clockbit, \
mutex, private) \
- lll_futex_syscall (5, futexp, \
+ lll_futex_syscall_time64 (5, futexp, \
__lll_private_flag (FUTEX_WAIT_REQUEUE_PI \
| (clockbit), private), \
val, timeout, mutex)
diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
index 68b3be8819..864152e609 100644
--- a/sysdeps/nptl/lowlevellock.h
+++ b/sysdeps/nptl/lowlevellock.h
@@ -123,7 +123,7 @@ extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
extern int __lll_clocklock_wait (int *futex, int val, clockid_t,
- const struct timespec *,
+ const struct __timespec64 *,
int private) attribute_hidden;
#define lll_timedwait(futex, val, clockid, abstime, private) \
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 03/10] y2038: Convert __lll_futex* functions to use futex_time64 when available
2020-07-07 15:08 ` [RFC 03/10] y2038: Convert __lll_futex* functions to use futex_time64 when available Lukasz Majewski
@ 2020-07-12 13:43 ` Lukasz Majewski
2020-07-13 21:08 ` Joseph Myers
0 siblings, 1 reply; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-12 13:43 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg
[-- Attachment #1: Type: text/plain, Size: 7073 bytes --]
Dear Community,
> As the 'futex' syscall is wrapped with a C preprocessor macros, it was
> necessary to provide new set of those to replace call it with Y2038
> safe 'futex_time64' syscall.
>
> The current code expand wrapper's arguments as __VA_ARGS__ when more
> than three arguments are passed. For the conversion it was necessary
> to also have 'timeout' explicitly passed, and hence the introduction
> of lll_futex_syscall_time64_4 and lll_futex_syscall_time64 functions.
> The former expects exactly 4 arguments - timeout is the last one.
>
Comments are more than welcome.
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
> sysdeps/nptl/lowlevellock-futex.h | 80
> ++++++++++++++++++++++++++++--- sysdeps/nptl/lowlevellock.h |
> 2 +- 2 files changed, 75 insertions(+), 7 deletions(-)
>
> diff --git a/sysdeps/nptl/lowlevellock-futex.h
> b/sysdeps/nptl/lowlevellock-futex.h index 2209ca76a1..a686773db4
> 100644 --- a/sysdeps/nptl/lowlevellock-futex.h
> +++ b/sysdeps/nptl/lowlevellock-futex.h
> @@ -65,14 +65,82 @@
> (((fl) | FUTEX_PRIVATE_FLAG) ^ (private))
> # endif
>
> -# define lll_futex_syscall(nargs, futexp, op, ...)
> \ +# define __lll_futex_syscall(nargs, futexp, op, ...)
> \ ({
> \
> - long int __ret = INTERNAL_SYSCALL (futex, nargs, futexp, op,
> \
> + long int __ret = INTERNAL_SYSCALL (futex, nargs, futexp, op,
> \ __VA_ARGS__); \
> (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (__ret))
> \ ? -INTERNAL_SYSCALL_ERRNO (__ret) : 0);
> \ })
>
> +# define __lll_futex_syscall64(nargs, futexp, op, ...) \
> + ({
> \
> + long int __ret = INTERNAL_SYSCALL (futex_time64, nargs, futexp,
> op, \
> + __VA_ARGS__);
> \
> + (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (__ret))
> \
> + ? -INTERNAL_SYSCALL_ERRNO (__ret) : 0);
> \
> + })
> +
> +# ifdef __ASSUME_TIME64_SYSCALLS
> +# ifndef __NR_futex_time64
> +# define __NR_futex_time64 __NR_futex
> +# endif
> +# define lll_futex_syscall(nargs, futexp, op, ...)
> \
> + __lll_futex_syscall64(nargs, futexp, op, __VA_ARGS__)
> +# define lll_futex_syscall_time64_4(nargs, futexp, op, val,
> timeout)\
> + __lll_futex_syscall64(nargs, futexp, op, val, timeout)
> +# define lll_futex_syscall_time64(nargs, futexp, op, val, timeout,
> ...)\
> + __lll_futex_syscall64(nargs, futexp, op, val, timeout, __VA_ARGS__)
> +# else
> +inline static long int
> +__lll_futex_syscall_time_check (long int __ret, const struct
> __timespec64 *t) +{
> + if (! (__ret == 0 || errno != ENOSYS))
> + if (t != NULL && ! in_time_t_range (t->tv_sec))
> + __ret = ({ errno = (EOVERFLOW); -1l; });
> + return __ret;
> +}
> +# define lll_futex_syscall(nargs, futexp, op, ...)
> \
> + ({
> \
> + long int __ret =
> \
> + __lll_futex_syscall64(nargs, futexp, op, __VA_ARGS__); \
> + if (! (__ret == 0 || errno != ENOSYS))
> \
> + __ret =
> \
> + __lll_futex_syscall(nargs, futexp, op, __VA_ARGS__); \
> + __ret;
> \
> + })
> +
> +# define lll_futex_syscall_time64(nargs, futexp, op, val, timeout,
> ...)\
> + ({
> \
> + struct timespec __ts32;
> \
> + if (timeout != NULL)
> \
> + __ts32 = valid_timespec64_to_timespec (*((struct
> __timespec64*) timeout)); \
> + long int __ret =
> \
> + __lll_futex_syscall64(nargs, futexp, op, val, timeout,\
> + __VA_ARGS__);
> \
> + __ret = __lll_futex_syscall_time_check (__ret, timeout);
> \
> + if (__ret != 0)
> \
> + __ret = __lll_futex_syscall(nargs, futexp, op, val, \
> + timeout != NULL ? &__ts32 : NULL,
> \
> + __VA_ARGS__);
> \
> + __ret;
> \
> + })
> +
> +# define lll_futex_syscall_time64_4(nargs, futexp, op, val,
> timeout)\
> + ({
> \
> + struct timespec __ts32;
> \
> + if (timeout != NULL)
> \
> + __ts32 = valid_timespec64_to_timespec (*((struct
> __timespec64*) timeout)); \
> + long int __ret =
> \
> + __lll_futex_syscall64(nargs, futexp, op, val, timeout);
> \
> + __ret = __lll_futex_syscall_time_check (__ret, timeout);
> \
> + if (__ret != 0)
> \
> + __ret = __lll_futex_syscall(nargs, futexp, op, val,
> \
> + timeout != NULL ? &__ts32 :
> NULL); \
> + __ret;
> \
> + })
> +
> +# endif
> /* For most of these macros, the return value is never really used.
> Nevertheless, the protocol is that each one returns a negated
> errno code for failure or zero for success. (Note that the
> corresponding @@ -85,7 +153,7 @@
> lll_futex_timed_wait (futexp, val, NULL, private)
>
> # define lll_futex_timed_wait(futexp, val, timeout, private) \
> - lll_futex_syscall (4, futexp, \
> + lll_futex_syscall_time64_4 (4, futexp,
> \ __lll_private_flag (FUTEX_WAIT, private), \
> val, timeout)
>
> @@ -107,7 +175,7 @@
> const int op =
> \ __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private); \
> \
> - __ret = lll_futex_syscall (6, futexp, op, val,
> \
> + __ret = lll_futex_syscall_time64 (6, futexp, op, val,
> \ timeout, NULL /* Unused. */, \
> FUTEX_BITSET_MATCH_ANY);
> \ }
> \ @@ -118,7 +186,7 @@
>
> /* Wake up up to NR waiters on FUTEXP. */
> # define lll_futex_wake(futexp, nr, private)
> \
> - lll_futex_syscall (4, futexp,
> \
> + lll_futex_syscall_time64_4 (4, futexp,
> \ __lll_private_flag (FUTEX_WAKE, private), nr, 0)
>
> /* Wake up up to NR_WAKE waiters on FUTEXP. Move up to NR_MOVE of
> the @@ -159,7 +227,7 @@
> /* Like lll_futex_wait_requeue_pi, but with a timeout. */
> # define lll_futex_timed_wait_requeue_pi(futexp, val, timeout,
> clockbit, \ mutex, private) \
> - lll_futex_syscall (5, futexp,
> \
> + lll_futex_syscall_time64 (5, futexp,
> \ __lll_private_flag (FUTEX_WAIT_REQUEUE_PI \
> | (clockbit), private),
> \ val, timeout, mutex)
> diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
> index 68b3be8819..864152e609 100644
> --- a/sysdeps/nptl/lowlevellock.h
> +++ b/sysdeps/nptl/lowlevellock.h
> @@ -123,7 +123,7 @@ extern void __lll_lock_wait (int *futex, int
> private) attribute_hidden;
>
> extern int __lll_clocklock_wait (int *futex, int val, clockid_t,
> - const struct timespec *,
> + const struct __timespec64 *,
> int private) attribute_hidden;
>
> #define lll_timedwait(futex, val, clockid, abstime, private)
> \
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 03/10] y2038: Convert __lll_futex* functions to use futex_time64 when available
2020-07-12 13:43 ` Lukasz Majewski
@ 2020-07-13 21:08 ` Joseph Myers
2020-07-14 7:23 ` Lukasz Majewski
0 siblings, 1 reply; 30+ messages in thread
From: Joseph Myers @ 2020-07-13 21:08 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Paul Eggert, Adhemerval Zanella, Florian Weimer, GNU C Library,
Andreas Schwab, Stepan Golosunov, Alistair Francis
On Sun, 12 Jul 2020, Lukasz Majewski wrote:
> Dear Community,
>
> > As the 'futex' syscall is wrapped with a C preprocessor macros, it was
> > necessary to provide new set of those to replace call it with Y2038
> > safe 'futex_time64' syscall.
> >
> > The current code expand wrapper's arguments as __VA_ARGS__ when more
> > than three arguments are passed. For the conversion it was necessary
> > to also have 'timeout' explicitly passed, and hence the introduction
> > of lll_futex_syscall_time64_4 and lll_futex_syscall_time64 functions.
> > The former expects exactly 4 arguments - timeout is the last one.
> >
>
> Comments are more than welcome.
There are some formatting issues in this patch series. This patch
contains function / macro calls missing the space before '(', for example.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 03/10] y2038: Convert __lll_futex* functions to use futex_time64 when available
2020-07-13 21:08 ` Joseph Myers
@ 2020-07-14 7:23 ` Lukasz Majewski
0 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-14 7:23 UTC (permalink / raw)
To: Joseph Myers
Cc: Paul Eggert, Adhemerval Zanella, Florian Weimer, GNU C Library,
Andreas Schwab, Stepan Golosunov, Alistair Francis
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
Hi Joseph,
> On Sun, 12 Jul 2020, Lukasz Majewski wrote:
>
> > Dear Community,
> >
> > > As the 'futex' syscall is wrapped with a C preprocessor macros,
> > > it was necessary to provide new set of those to replace call it
> > > with Y2038 safe 'futex_time64' syscall.
> > >
> > > The current code expand wrapper's arguments as __VA_ARGS__ when
> > > more than three arguments are passed. For the conversion it was
> > > necessary to also have 'timeout' explicitly passed, and hence the
> > > introduction of lll_futex_syscall_time64_4 and
> > > lll_futex_syscall_time64 functions. The former expects exactly 4
> > > arguments - timeout is the last one.
> >
> > Comments are more than welcome.
>
> There are some formatting issues in this patch series. This patch
> contains function / macro calls missing the space before '(', for
> example.
>
Thank you for your comment.
Do you have any comments regarding the proposed approach to convert
futex to futex_time64 (i.e. to provide 64 bit time support to nptl) ?
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 04/10] y2038: Replace struct timespec with __timespec64 in futex-internal.h
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (2 preceding siblings ...)
2020-07-07 15:08 ` [RFC 03/10] y2038: Convert __lll_futex* functions to use futex_time64 when available Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-07 15:08 ` [RFC 05/10] y2038: Convert pthread_* functions to support 64 bit time Lukasz Majewski
` (7 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
This patch set replaces all occurences of struct timespec with
__timespec64 in futex-internal.h header.
---
sysdeps/nptl/futex-internal.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index d622122ddc..e23a954e73 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -211,7 +211,7 @@ futex_wait_cancelable (unsigned int *futex_word, unsigned int expected,
*/
static __always_inline int
futex_reltimed_wait (unsigned int* futex_word, unsigned int expected,
- const struct timespec* reltime, int private)
+ const struct __timespec64* reltime, int private)
{
int err = lll_futex_timed_wait (futex_word, expected, reltime, private);
switch (err)
@@ -237,7 +237,7 @@ futex_reltimed_wait (unsigned int* futex_word, unsigned int expected,
static __always_inline int
futex_reltimed_wait_cancelable (unsigned int* futex_word,
unsigned int expected,
- const struct timespec* reltime, int private)
+ const struct __timespec64* reltime, int private)
{
int oldtype;
oldtype = LIBC_CANCEL_ASYNC ();
@@ -275,7 +275,7 @@ futex_abstimed_supported_clockid (clockid_t clockid)
static __always_inline int
futex_abstimed_wait (unsigned int* futex_word, unsigned int expected,
clockid_t clockid,
- const struct timespec* abstime, int private)
+ const struct __timespec64* abstime, int private)
{
/* Work around the fact that the kernel rejects negative timeout values
despite them being valid. */
@@ -309,7 +309,7 @@ static __always_inline int
futex_abstimed_wait_cancelable (unsigned int* futex_word,
unsigned int expected,
clockid_t clockid,
- const struct timespec* abstime, int private)
+ const struct __timespec64* abstime, int private)
{
/* Work around the fact that the kernel rejects negative timeout values
despite them being valid. */
@@ -406,7 +406,7 @@ futex_wake (unsigned int* futex_word, int processes_to_wake, int private)
- ETIMEDOUT if the ABSTIME expires.
*/
static __always_inline int
-futex_lock_pi (unsigned int *futex_word, const struct timespec *abstime,
+futex_lock_pi (unsigned int *futex_word, const struct __timespec64 *abstime,
int private)
{
int err = lll_futex_timed_lock_pi (futex_word, abstime, private);
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [RFC 05/10] y2038: Convert pthread_* functions to support 64 bit time
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (3 preceding siblings ...)
2020-07-07 15:08 ` [RFC 04/10] y2038: Replace struct timespec with __timespec64 in futex-internal.h Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-07 15:08 ` [RFC 06/10] y2038: Convert sem_{timed|clock}wait to support 64 bit timeout Lukasz Majewski
` (6 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
Following function are converted to support 64 bit abstime parameter on
archs with __TIMESIZE != 64 and __WORDSIZE == 32:
pthread_cond_timedwait
pthread_cond_clockwait
pthread_mutex_clocklock
pthread_mutex_timedlock
pthread_clockjoin_np
pthread_timedjoin_np
pthread_rwlock_clockrdlock
pthread_rwlock_clockwrlock
pthread_rwlock_timedrdlock
pthread_rwlock_timedwrlock
---
nptl/pthreadP.h | 53 ++++++++++++++++++++++++++++++-
nptl/pthread_clockjoin.c | 22 +++++++++++--
nptl/pthread_cond_wait.c | 46 +++++++++++++++++++++++----
nptl/pthread_join_common.c | 11 ++++---
nptl/pthread_mutex_timedlock.c | 39 ++++++++++++++++++-----
nptl/pthread_rwlock_clockrdlock.c | 21 ++++++++++--
nptl/pthread_rwlock_clockwrlock.c | 21 ++++++++++--
nptl/pthread_rwlock_common.c | 4 +--
nptl/pthread_rwlock_timedrdlock.c | 21 ++++++++++--
nptl/pthread_rwlock_timedwrlock.c | 21 ++++++++++--
nptl/pthread_timedjoin.c | 20 ++++++++++--
11 files changed, 245 insertions(+), 34 deletions(-)
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 6f94d6be31..cab4adf4a1 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -458,6 +458,57 @@ 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_cond_timedwait64 __pthread_cond_timedwait
+# define __pthread_cond_clockwait64 __pthread_cond_clockwait
+# define __pthread_mutex_clocklock64 __pthread_mutex_clocklock
+# define __pthread_mutex_timedlock64 __pthread_mutex_timedlock
+# define __pthread_clockjoin_np64 __pthread_clockjoin_np
+# define __pthread_timedjoin_np64 __pthread_timedjoin_np
+# define __pthread_rwlock_clockrdlock64 __pthread_rwlock_clockrdlock
+# define __pthread_rwlock_clockwrlock64 __pthread_rwlock_clockwrlock
+# define __pthread_rwlock_timedrdlock64 __pthread_rwlock_timedrdlock
+# define __pthread_rwlock_timedwrlock64 __pthread_rwlock_timedwrlock
+#else
+extern int __pthread_cond_timedwait64 (pthread_cond_t *cond,
+ pthread_mutex_t *mutex,
+ const struct __timespec64 *abstime);
+libc_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);
+libc_hidden_proto (__pthread_cond_clockwait64)
+extern int __pthread_mutex_clocklock64 (pthread_mutex_t *mutex,
+ clockid_t clockid,
+ const struct __timespec64 *abstime);
+libc_hidden_proto (__pthread_mutex_clocklock64)
+extern int __pthread_mutex_timedlock64 (pthread_mutex_t *mutex,
+ const struct __timespec64 *abstime);
+libc_hidden_proto (__pthread_mutex_timedlock64)
+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)
+extern int __pthread_rwlock_clockrdlock64 (pthread_rwlock_t *rwlock,
+ clockid_t clockid,
+ const struct __timespec64 *abstime);
+libc_hidden_proto (__pthread_rwlock_clockrdlock64)
+extern int __pthread_rwlock_clockwrlock64 (pthread_rwlock_t *rwlock,
+ clockid_t clockid,
+ const struct __timespec64 *abstime);
+libc_hidden_proto (__pthread_rwlock_clockwrlock64)
+extern int __pthread_rwlock_timedrdlock64 (pthread_rwlock_t *rwlock,
+ const struct __timespec64 *abstime);
+libc_hidden_proto (__pthread_rwlock_timedrdlock64)
+extern int __pthread_rwlock_timedwrlock64 (pthread_rwlock_t *rwlock,
+ const struct __timespec64 *abstime);
+libc_hidden_proto (__pthread_rwlock_timedwrlock64)
+#endif
extern int __pthread_cond_timedwait (pthread_cond_t *cond,
pthread_mutex_t *mutex,
const struct timespec *abstime);
@@ -488,7 +539,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..c3a92e5e0b 100644
--- a/nptl/pthread_clockjoin.c
+++ b/nptl/pthread_clockjoin.c
@@ -16,14 +16,32 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
+#include <time.h>
#include "pthreadP.h"
+int
+__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)
{
- return __pthread_clockjoin_ex (threadid, thread_return,
- clockid, abstime, true);
+ struct __timespec64 ts64;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __pthread_clockjoin_np64 (threadid, thread_return,
+ clockid, abstime != NULL ? &ts64 : NULL);
}
+#endif
weak_alias (__pthread_clockjoin_np, pthread_clockjoin_np)
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 85ddbc1011..69b18b1316 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -379,7 +379,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)
+ const struct __timespec64 *abstime)
{
const int maxspin = 0;
int err;
@@ -640,8 +640,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 +655,23 @@ __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
+libc_hidden_def (__pthread_cond_timedwait64)
+
+int
+__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
+ const struct timespec *abstime)
+{
+ struct __timespec64 ts64;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __pthread_cond_timedwait64 (cond, mutex,
+ abstime != NULL ? &ts64 : NULL);
+}
+#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 +679,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 +693,21 @@ __pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
}
+
+#if __TIMESIZE != 64
+libc_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;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __pthread_cond_clockwait64(cond, mutex, clockid,
+ abstime != NULL ? &ts64 : NULL);
+}
+#endif
weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait);
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
index a96ceafde4..38a078fd23 100644
--- a/nptl/pthread_join_common.c
+++ b/nptl/pthread_join_common.c
@@ -37,7 +37,8 @@ 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;
@@ -47,11 +48,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. */
@@ -80,8 +81,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_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index 8ae814b984..2438c61aa4 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -45,7 +45,7 @@
int
__pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
clockid_t clockid,
- const struct timespec *abstime)
+ const struct __timespec64 *abstime)
{
int oldval;
pid_t id = THREAD_GETMEM (THREAD_SELF, tid);
@@ -543,10 +543,10 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
goto failpp;
}
- struct timespec rt;
+ struct __timespec64 rt;
/* Get the current time. */
- __clock_gettime (CLOCK_REALTIME, &rt);
+ __clock_gettime64 (CLOCK_REALTIME, &rt);
/* Compute relative timeout. */
rt.tv_sec = abstime->tv_sec - rt.tv_sec;
@@ -599,9 +599,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
}
int
-__pthread_mutex_clocklock (pthread_mutex_t *mutex,
- clockid_t clockid,
- const struct timespec *abstime)
+__pthread_mutex_clocklock64 (pthread_mutex_t *mutex,
+ clockid_t clockid,
+ const struct __timespec64 *abstime)
{
if (__glibc_unlikely (!lll_futex_supported_clockid (clockid)))
return EINVAL;
@@ -609,13 +609,36 @@ __pthread_mutex_clocklock (pthread_mutex_t *mutex,
LIBC_PROBE (mutex_clocklock_entry, 3, mutex, clockid, abstime);
return __pthread_mutex_clocklock_common (mutex, clockid, abstime);
}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__pthread_mutex_clocklock64)
+int
+__pthread_mutex_clocklock (pthread_mutex_t *mutex,
+ clockid_t clockid,
+ const struct timespec *abstime)
+{
+ struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
+ return __pthread_mutex_clocklock64 (mutex, clockid, &ts64);
+}
+#endif
weak_alias (__pthread_mutex_clocklock, pthread_mutex_clocklock)
int
-__pthread_mutex_timedlock (pthread_mutex_t *mutex,
- const struct timespec *abstime)
+__pthread_mutex_timedlock64 (pthread_mutex_t *mutex,
+ const struct __timespec64 *abstime)
{
LIBC_PROBE (mutex_timedlock_entry, 2, mutex, abstime);
return __pthread_mutex_clocklock_common (mutex, CLOCK_REALTIME, abstime);
}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__pthread_mutex_timedlock64)
+int
+__pthread_mutex_timedlock (pthread_mutex_t *mutex,
+ const struct timespec *abstime)
+{
+ struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
+ return __pthread_mutex_timedlock64 (mutex, &ts64);
+}
+#endif
weak_alias (__pthread_mutex_timedlock, pthread_mutex_timedlock)
diff --git a/nptl/pthread_rwlock_clockrdlock.c b/nptl/pthread_rwlock_clockrdlock.c
index 4cedfd1dcd..bb0a0b0d16 100644
--- a/nptl/pthread_rwlock_clockrdlock.c
+++ b/nptl/pthread_rwlock_clockrdlock.c
@@ -21,8 +21,25 @@
/* See pthread_rwlock_common.c. */
int
-pthread_rwlock_clockrdlock (pthread_rwlock_t *rwlock, clockid_t clockid,
- const struct timespec *abstime)
+__pthread_rwlock_clockrdlock64 (pthread_rwlock_t *rwlock, clockid_t clockid,
+ const struct __timespec64 *abstime)
{
return __pthread_rwlock_rdlock_full (rwlock, clockid, abstime);
}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__pthread_rwlock_clockrdlock64)
+
+int
+__pthread_rwlock_clockrdlock (pthread_rwlock_t *rwlock, clockid_t clockid,
+ const struct timespec *abstime)
+{
+ struct __timespec64 ts64;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __pthread_rwlock_clockrdlock64 (rwlock, clockid,
+ abstime != NULL ? &ts64 : NULL);
+}
+#endif
+weak_alias (__pthread_rwlock_clockrdlock, pthread_rwlock_clockrdlock)
diff --git a/nptl/pthread_rwlock_clockwrlock.c b/nptl/pthread_rwlock_clockwrlock.c
index 7a954cf529..742fa04dbc 100644
--- a/nptl/pthread_rwlock_clockwrlock.c
+++ b/nptl/pthread_rwlock_clockwrlock.c
@@ -21,8 +21,25 @@
/* See pthread_rwlock_common.c. */
int
-pthread_rwlock_clockwrlock (pthread_rwlock_t *rwlock, clockid_t clockid,
- const struct timespec *abstime)
+__pthread_rwlock_clockwrlock64 (pthread_rwlock_t *rwlock, clockid_t clockid,
+ const struct __timespec64 *abstime)
{
return __pthread_rwlock_wrlock_full (rwlock, clockid, abstime);
}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__pthread_rwlock_clockwrlock64)
+
+int
+__pthread_rwlock_clockwrlock (pthread_rwlock_t *rwlock, clockid_t clockid,
+ const struct timespec *abstime)
+{
+ struct __timespec64 ts64;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __pthread_rwlock_clockwrlock64 (rwlock, clockid,
+ abstime != NULL ? &ts64 : NULL);
+}
+#endif
+weak_alias (__pthread_rwlock_clockwrlock, pthread_rwlock_clockwrlock)
diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
index 3fbc66ded2..438cf2f74c 100644
--- a/nptl/pthread_rwlock_common.c
+++ b/nptl/pthread_rwlock_common.c
@@ -280,7 +280,7 @@ __pthread_rwlock_rdunlock (pthread_rwlock_t *rwlock)
static __always_inline int
__pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
clockid_t clockid,
- const struct timespec *abstime)
+ const struct __timespec64 *abstime)
{
unsigned int r;
@@ -587,7 +587,7 @@ __pthread_rwlock_wrunlock (pthread_rwlock_t *rwlock)
static __always_inline int
__pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
clockid_t clockid,
- const struct timespec *abstime)
+ const struct __timespec64 *abstime)
{
/* Make sure any passed in clockid and timeout value are valid. Note that
the previous implementation assumed that this check *must* not be
diff --git a/nptl/pthread_rwlock_timedrdlock.c b/nptl/pthread_rwlock_timedrdlock.c
index c5d8aee909..7e808439a6 100644
--- a/nptl/pthread_rwlock_timedrdlock.c
+++ b/nptl/pthread_rwlock_timedrdlock.c
@@ -20,8 +20,25 @@
/* See pthread_rwlock_common.c. */
int
-pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock,
- const struct timespec *abstime)
+__pthread_rwlock_timedrdlock64 (pthread_rwlock_t *rwlock,
+ const struct __timespec64 *abstime)
{
return __pthread_rwlock_rdlock_full (rwlock, CLOCK_REALTIME, abstime);
}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__pthread_rwlock_timedrdlock64)
+
+int
+__pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock,
+ const struct timespec *abstime)
+{
+ struct __timespec64 ts64;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __pthread_rwlock_timedrdlock64 (rwlock,
+ abstime != NULL ? &ts64 : NULL);
+}
+#endif
+weak_alias (__pthread_rwlock_timedrdlock, pthread_rwlock_timedrdlock)
diff --git a/nptl/pthread_rwlock_timedwrlock.c b/nptl/pthread_rwlock_timedwrlock.c
index ccee8b77d9..8160c18819 100644
--- a/nptl/pthread_rwlock_timedwrlock.c
+++ b/nptl/pthread_rwlock_timedwrlock.c
@@ -20,8 +20,25 @@
/* See pthread_rwlock_common.c. */
int
-pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock,
- const struct timespec *abstime)
+__pthread_rwlock_timedwrlock64 (pthread_rwlock_t *rwlock,
+ const struct __timespec64 *abstime)
{
return __pthread_rwlock_wrlock_full (rwlock, CLOCK_REALTIME, abstime);
}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__pthread_rwlock_timedwrlock64)
+
+int
+__pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock,
+ const struct timespec *abstime)
+{
+ struct __timespec64 ts64;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __pthread_rwlock_timedwrlock64 (rwlock,
+ abstime != NULL ? &ts64 : NULL);
+}
+#endif
+weak_alias (__pthread_rwlock_timedwrlock, pthread_rwlock_timedwrlock)
diff --git a/nptl/pthread_timedjoin.c b/nptl/pthread_timedjoin.c
index dd7038dcf7..a6c2bbae53 100644
--- a/nptl/pthread_timedjoin.c
+++ b/nptl/pthread_timedjoin.c
@@ -19,10 +19,26 @@
#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;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __pthread_timedjoin_np64 (threadid, thread_return,
+ abstime != NULL ? &ts64 : NULL);
+}
+#endif
weak_alias (__pthread_timedjoin_np, pthread_timedjoin_np)
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [RFC 06/10] y2038: Convert sem_{timed|clock}wait to support 64 bit timeout
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (4 preceding siblings ...)
2020-07-07 15:08 ` [RFC 05/10] y2038: Convert pthread_* functions to support 64 bit time Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-07 15:08 ` [RFC 07/10] y2038: Convert __lll_clocklock_wait function to support 64 bit time Lukasz Majewski
` (5 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
The conversion was to:
- Replace struct timespec with struct __timespec64
- Provide aliases to __sem_* functions
- Introduce Y2038 safe versions of __sem_timedwait64 and __sem_clockwait64
---
nptl/sem_clockwait.c | 21 +++++++++++++++++++--
nptl/sem_timedwait.c | 18 +++++++++++++++++-
nptl/sem_waitcommon.c | 4 ++--
nptl/semaphoreP.h | 12 ++++++++++++
4 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/nptl/sem_clockwait.c b/nptl/sem_clockwait.c
index b9bae75183..86ce18ee2b 100644
--- a/nptl/sem_clockwait.c
+++ b/nptl/sem_clockwait.c
@@ -20,10 +20,11 @@
#include <time.h>
#include "sem_waitcommon.c"
+#include "semaphoreP.h"
int
-sem_clockwait (sem_t *sem, clockid_t clockid,
- const struct timespec *abstime)
+__sem_clockwait64 (sem_t *sem, clockid_t clockid,
+ const struct __timespec64 *abstime)
{
/* Check that supplied clockid is one we support, even if we don't end up
waiting. */
@@ -44,3 +45,19 @@ sem_clockwait (sem_t *sem, clockid_t clockid,
else
return __new_sem_wait_slow ((struct new_sem *) sem, clockid, abstime);
}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__sem_timedwait64)
+
+int
+__sem_clockwait (sem_t *sem, clockid_t clockid,
+ const struct timespec *abstime)
+{
+ struct __timespec64 ts64;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __sem_clockwait64 (sem, clockid, abstime != NULL ? &ts64 : NULL);
+}
+#endif
+weak_alias (__sem_clockwait, sem_clockwait)
diff --git a/nptl/sem_timedwait.c b/nptl/sem_timedwait.c
index 99c11bf20e..0ffb56ec67 100644
--- a/nptl/sem_timedwait.c
+++ b/nptl/sem_timedwait.c
@@ -19,11 +19,12 @@
#include <time.h>
#include "sem_waitcommon.c"
+#include "semaphoreP.h"
/* This is in a separate file because because sem_timedwait is only provided
if __USE_XOPEN2K is defined. */
int
-sem_timedwait (sem_t *sem, const struct timespec *abstime)
+__sem_timedwait64 (sem_t *sem, const struct __timespec64 *abstime)
{
if (! valid_nanoseconds (abstime->tv_nsec))
{
@@ -40,3 +41,18 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
return __new_sem_wait_slow ((struct new_sem *) sem,
CLOCK_REALTIME, abstime);
}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__sem_timedwait64)
+
+int
+__sem_timedwait (sem_t *sem, const struct timespec *abstime)
+{
+ struct __timespec64 ts64;
+ if (abstime != NULL)
+ ts64 = valid_timespec_to_timespec64 (*abstime);
+
+ return __sem_timedwait64 (sem, abstime != NULL ? &ts64 : NULL);
+}
+#endif
+weak_alias (__sem_timedwait, sem_timedwait)
diff --git a/nptl/sem_waitcommon.c b/nptl/sem_waitcommon.c
index 5a6d2643ee..7fdc084d53 100644
--- a/nptl/sem_waitcommon.c
+++ b/nptl/sem_waitcommon.c
@@ -104,7 +104,7 @@ __sem_wait_cleanup (void *arg)
static int
__attribute__ ((noinline))
do_futex_wait (struct new_sem *sem, clockid_t clockid,
- const struct timespec *abstime)
+ const struct __timespec64 *abstime)
{
int err;
@@ -163,7 +163,7 @@ __new_sem_wait_fast (struct new_sem *sem, int definitive_result)
static int
__attribute__ ((noinline))
__new_sem_wait_slow (struct new_sem *sem, clockid_t clockid,
- const struct timespec *abstime)
+ const struct __timespec64 *abstime)
{
int err = 0;
diff --git a/nptl/semaphoreP.h b/nptl/semaphoreP.h
index e7e1c9763f..9023ea5573 100644
--- a/nptl/semaphoreP.h
+++ b/nptl/semaphoreP.h
@@ -17,6 +17,7 @@
<https://www.gnu.org/licenses/>. */
#include <semaphore.h>
+#include <struct___timespec64.h>
#include "pthreadP.h"
#define SEM_SHM_PREFIX "sem."
@@ -52,3 +53,14 @@ extern int __new_sem_wait (sem_t *sem);
extern int __old_sem_wait (sem_t *sem);
extern int __new_sem_trywait (sem_t *sem);
extern int __new_sem_getvalue (sem_t *sem, int *sval);
+
+#if __TIMESIZE == 64
+# define __sem_timedwait64 __sem_timedwait
+# define __sem_clockwait64 __sem_clockwait
+#else
+extern int __sem_timedwait64 (sem_t *sem, const struct __timespec64 *abstime);
+libc_hidden_proto (__sem_timedwait64)
+extern int __sem_clockwait64 (sem_t *sem, clockid_t clockid,
+ const struct __timespec64 *abstime);
+libc_hidden_proto (__sem_clockwait64)
+#endif
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [RFC 07/10] y2038: Convert __lll_clocklock_wait function to support 64 bit time
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (5 preceding siblings ...)
2020-07-07 15:08 ` [RFC 06/10] y2038: Convert sem_{timed|clock}wait to support 64 bit timeout Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-07 15:08 ` [RFC 08/10] y2038: Convert aio_suspend to support 64 bit timeout Lukasz Majewski
` (4 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
Prepare the internal __lll_clocklock_wait function to support 64 bit time
on architectures, with __TIMESIZE != 64 and __WORDSIZE == 32
by replacing struct timespec with struct __timespec64 and __clock_gettime
with __clock_gettime64.
---
nptl/lll_timedlock_wait.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/nptl/lll_timedlock_wait.c b/nptl/lll_timedlock_wait.c
index eabdca70c8..82cd76e557 100644
--- a/nptl/lll_timedlock_wait.c
+++ b/nptl/lll_timedlock_wait.c
@@ -26,9 +26,9 @@
int
__lll_clocklock_wait (int *futex, int val, clockid_t clockid,
- const struct timespec *abstime, int private)
+ const struct __timespec64 *abstime, int private)
{
- struct timespec ts, *tsp = NULL;
+ struct __timespec64 ts, *tsp = NULL;
if (abstime != NULL)
{
@@ -37,7 +37,7 @@ __lll_clocklock_wait (int *futex, int val, clockid_t clockid,
return EINVAL;
/* Get the current time. This can only fail if clockid is not valid. */
- if (__glibc_unlikely (__clock_gettime (clockid, &ts) != 0))
+ if (__glibc_unlikely (__clock_gettime64 (clockid, &ts) != 0))
return EINVAL;
/* Compute relative timeout. */
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [RFC 08/10] y2038: Convert aio_suspend to support 64 bit timeout
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (6 preceding siblings ...)
2020-07-07 15:08 ` [RFC 07/10] y2038: Convert __lll_clocklock_wait function to support 64 bit time Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-07 15:08 ` [RFC 09/10] y2038: Convert gai_suspend " Lukasz Majewski
` (3 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
This patch converts aio_suspend function to support 64 bit
timeout. This function internally uses futex syscall.
---
sysdeps/nptl/aio_misc.h | 11 ++++++++++-
sysdeps/pthread/aio_suspend.c | 36 ++++++++++++++++++++++++++++-------
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/sysdeps/nptl/aio_misc.h b/sysdeps/nptl/aio_misc.h
index 3f195f4794..5f0ed58217 100644
--- a/sysdeps/nptl/aio_misc.h
+++ b/sysdeps/nptl/aio_misc.h
@@ -23,6 +23,7 @@
#include <assert.h>
#include <nptl/pthreadP.h>
#include <futex-internal.h>
+#include <struct___timespec64.h>
#define DONT_NEED_AIO_MISC_COND 1
@@ -49,7 +50,7 @@
(unsigned int *) futexaddr, oldval, timeout, FUTEX_PRIVATE); \
else \
status = futex_reltimed_wait ((unsigned int *) futexaddr, \
- oldval, timeout, FUTEX_PRIVATE); \
+ oldval, timeout, FUTEX_PRIVATE); \
if (status != EAGAIN) \
break; \
\
@@ -69,3 +70,11 @@
} while (0)
#include_next <aio_misc.h>
+
+#if __TIMESIZE == 64
+# define __aio_suspend64 __aio_suspend
+#else
+extern int __aio_suspend64 (const struct aiocb *const list[], int nent,
+ const struct __timespec64 *timeout);
+libc_hidden_proto (__aio_suspend64)
+#endif
diff --git a/sysdeps/pthread/aio_suspend.c b/sysdeps/pthread/aio_suspend.c
index ad03f13558..71e0dbceb1 100644
--- a/sysdeps/pthread/aio_suspend.c
+++ b/sysdeps/pthread/aio_suspend.c
@@ -94,7 +94,7 @@ cleanup (void *arg)
#ifdef DONT_NEED_AIO_MISC_COND
static int
__attribute__ ((noinline))
-do_aio_misc_wait (unsigned int *cntr, const struct timespec *timeout)
+do_aio_misc_wait (unsigned int *cntr, const struct __timespec64 *timeout)
{
int result = 0;
@@ -104,9 +104,9 @@ do_aio_misc_wait (unsigned int *cntr, const struct timespec *timeout)
}
#endif
-int
-aio_suspend (const struct aiocb *const list[], int nent,
- const struct timespec *timeout)
+static int
+__aio_suspend_common (const struct aiocb *const list[], int nent,
+ const struct __timespec64 *timeout)
{
if (__glibc_unlikely (nent < 0))
{
@@ -183,10 +183,10 @@ aio_suspend (const struct aiocb *const list[], int nent,
{
/* We have to convert the relative timeout value into an
absolute time value with pthread_cond_timedwait expects. */
- struct timespec now;
- struct timespec abstime;
+ struct __timespec64 now;
+ struct __timespec64 abstime;
- __clock_gettime (CLOCK_REALTIME, &now);
+ __clock_gettime64 (CLOCK_REALTIME, &now);
abstime.tv_nsec = timeout->tv_nsec + now.tv_nsec;
abstime.tv_sec = timeout->tv_sec + now.tv_sec;
if (abstime.tv_nsec >= 1000000000)
@@ -250,4 +250,26 @@ aio_suspend (const struct aiocb *const list[], int nent,
return result;
}
+int
+__aio_suspend64 (const struct aiocb *const list[], int nent,
+ const struct __timespec64 *timeout)
+{
+ return __aio_suspend_common (list, nent, timeout);
+}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__aio_suspend64)
+
+int
+__aio_suspend (const struct aiocb *const list[], int nent,
+ const struct timespec *timeout)
+{
+ struct __timespec64 ts64;
+ if (timeout != NULL)
+ ts64 = valid_timespec_to_timespec64 (*timeout);
+
+ return __aio_suspend64 (list, nent, timeout != NULL ? &ts64 : NULL);
+}
+#endif
+weak_alias (__aio_suspend, aio_suspend)
weak_alias (aio_suspend, aio_suspend64)
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [RFC 09/10] y2038: Convert gai_suspend to support 64 bit timeout
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (7 preceding siblings ...)
2020-07-07 15:08 ` [RFC 08/10] y2038: Convert aio_suspend to support 64 bit timeout Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-07 15:08 ` [RFC 10/10] y2038: x86: Fix __lll_clocklock_elision to support 64 bit time Lukasz Majewski
` (2 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
This patch converts gai_suspend to support 64 bit timeout.
It uses internally futex syscall to achieve this goal.
---
resolv/gai_misc.h | 7 +++++++
resolv/gai_suspend.c | 33 ++++++++++++++++++++++++++++-----
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/resolv/gai_misc.h b/resolv/gai_misc.h
index 008d6a4ad6..4ac2a24859 100644
--- a/resolv/gai_misc.h
+++ b/resolv/gai_misc.h
@@ -97,4 +97,11 @@ extern int __gai_notify_only (struct sigevent *sigev, pid_t caller_pid)
/* Send the signal. */
extern int __gai_sigqueue (int sig, const union sigval val, pid_t caller_pid);
+# if __TIMESIZE == 64
+# define __gai_suspend64 __gai_suspend
+# else
+extern int __gai_suspend64 (const struct gaicb *const list[], int ent,
+ const struct __timespec64 *timeout);
+libc_hidden_proto (__gai_suspend64)
+# endif
#endif /* gai_misc.h */
diff --git a/resolv/gai_suspend.c b/resolv/gai_suspend.c
index 734f9b4500..79b183681d 100644
--- a/resolv/gai_suspend.c
+++ b/resolv/gai_suspend.c
@@ -26,8 +26,8 @@
int
-gai_suspend (const struct gaicb *const list[], int ent,
- const struct timespec *timeout)
+__gai_suspend_common (const struct gaicb *const list[], int ent,
+ const struct __timespec64 *timeout)
{
struct waitlist waitlist[ent];
struct requestlist *requestlist[ent];
@@ -91,10 +91,10 @@ gai_suspend (const struct gaicb *const list[], int ent,
{
/* We have to convert the relative timeout value into an
absolute time value with pthread_cond_timedwait expects. */
- struct timespec now;
- struct timespec abstime;
+ struct __timespec64 now;
+ struct __timespec64 abstime;
- __clock_gettime (CLOCK_REALTIME, &now);
+ __clock_gettime64 (CLOCK_REALTIME, &now);
abstime.tv_nsec = timeout->tv_nsec + now.tv_nsec;
abstime.tv_sec = timeout->tv_sec + now.tv_sec;
if (abstime.tv_nsec >= 1000000000)
@@ -155,3 +155,26 @@ gai_suspend (const struct gaicb *const list[], int ent,
return result;
}
+
+int
+__gai_suspend64 (const struct gaicb *const list[], int ent,
+ const struct __timespec64 *timeout)
+{
+ return __gai_suspend_common (list, ent, timeout);
+}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__gai_suspend64)
+
+int
+__gai_suspend (const struct gaicb *const list[], int ent,
+ const struct timespec *timeout)
+{
+ struct __timespec64 ts64;
+ if (timeout != NULL)
+ ts64 = valid_timespec_to_timespec64 (*timeout);
+
+ return __gai_suspend64 (list, ent, timeout != NULL ? &ts64 : NULL);
+}
+#endif
+weak_alias (__gai_suspend, gai_suspend)
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [RFC 10/10] y2038: x86: Fix __lll_clocklock_elision to support 64 bit time
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (8 preceding siblings ...)
2020-07-07 15:08 ` [RFC 09/10] y2038: Convert gai_suspend " Lukasz Majewski
@ 2020-07-07 15:08 ` Lukasz Majewski
2020-07-12 13:42 ` [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
2020-07-13 17:15 ` Adhemerval Zanella
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-07 15:08 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg, Lukasz Majewski
Adjust x86 specific code to use 64 bit struct __timespec64.
---
sysdeps/unix/sysv/linux/x86/elision-timed.c | 2 +-
sysdeps/unix/sysv/linux/x86/lowlevellock.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/x86/elision-timed.c b/sysdeps/unix/sysv/linux/x86/elision-timed.c
index 87e5c788c6..ac4d5f7858 100644
--- a/sysdeps/unix/sysv/linux/x86/elision-timed.c
+++ b/sysdeps/unix/sysv/linux/x86/elision-timed.c
@@ -20,7 +20,7 @@
#include <elision-conf.h>
#include "lowlevellock.h"
#define __lll_lock_elision __lll_clocklock_elision
-#define EXTRAARG clockid_t clockid, const struct timespec *t,
+#define EXTRAARG clockid_t clockid, const struct __timespec64 *t,
#undef LLL_LOCK
#define LLL_LOCK(a, b) lll_clocklock (a, clockid, t, b)
#include "elision-lock.c"
diff --git a/sysdeps/unix/sysv/linux/x86/lowlevellock.h b/sysdeps/unix/sysv/linux/x86/lowlevellock.h
index 27d62c9301..d0ea71b105 100644
--- a/sysdeps/unix/sysv/linux/x86/lowlevellock.h
+++ b/sysdeps/unix/sysv/linux/x86/lowlevellock.h
@@ -84,7 +84,7 @@ __lll_cas_lock (int *futex)
extern int __lll_clocklock_elision (int *futex, short *adapt_count,
clockid_t clockid,
- const struct timespec *timeout,
+ const struct __timespec64 *timeout,
int private) attribute_hidden;
#define lll_clocklock_elision(futex, adapt_count, clockid, timeout, private) \
--
2.20.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (9 preceding siblings ...)
2020-07-07 15:08 ` [RFC 10/10] y2038: x86: Fix __lll_clocklock_elision to support 64 bit time Lukasz Majewski
@ 2020-07-12 13:42 ` Lukasz Majewski
2020-07-13 17:15 ` Adhemerval Zanella
11 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-12 13:42 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg
[-- Attachment #1: Type: text/plain, Size: 3641 bytes --]
Dear Community,
> Please find this early RFC for converting 'futex' syscall based code
> (pthreads/nptl/sem/gai) to support 64 bit time.
> When applicable the futex_time64 syscall is used.
>
> The main purpose of this RFC is to assess if taken approach for
> conversion is correct and acceptable by the glibc community.
>
> Quesitons/issues:
>
> 1. This whole patch set shall be squashed into a single patch,
> otherwise, the glibc will not build between separate commits. I've
> divided it to separate patches on the purpose - to facilitate review.
>
> 2. Question about rewritting lll_* macros in lowlevel-*.h - I'm
> wondering if there is maybe a better way to do it. Please pay
> attention to the *_4 suffix.
>
> 3. What would be the best point in the glibc release cycle to apply
> this patch set as it convets the core functionality of the library?
>
> Just after the stable release?
Any feedback on this RFC patch series? Comments are more than welcome.
>
>
> Lukasz Majewski (10):
> doc: Fix wording and formatting in ./support/README
> y2038: Convert timespec_* from posix-timer.h to be Y2038 safe
> y2038: Convert __lll_futex* functions to use futex_time64 when
> available
> y2038: Replace struct timespec with __timespec64 in futex-internal.h
> y2038: Convert pthread_* functions to support 64 bit time
> y2038: Convert sem_{timed|clock}wait to support 64 bit timeout
> y2038: Convert __lll_clocklock_wait function to support 64 bit time
> y2038: Convert aio_suspend to support 64 bit timeout
> y2038: Convert gai_suspend to support 64 bit timeout
> y2038: x86: Fix __lll_clocklock_elision to support 64 bit time
>
> nptl/lll_timedlock_wait.c | 6 +-
> nptl/pthreadP.h | 53 +++++++++++++-
> nptl/pthread_clockjoin.c | 22 +++++-
> nptl/pthread_cond_wait.c | 46 ++++++++++--
> nptl/pthread_join_common.c | 11 +--
> nptl/pthread_mutex_timedlock.c | 39 +++++++---
> nptl/pthread_rwlock_clockrdlock.c | 21 +++++-
> nptl/pthread_rwlock_clockwrlock.c | 21 +++++-
> nptl/pthread_rwlock_common.c | 4 +-
> nptl/pthread_rwlock_timedrdlock.c | 21 +++++-
> nptl/pthread_rwlock_timedwrlock.c | 21 +++++-
> nptl/pthread_timedjoin.c | 20 +++++-
> nptl/sem_clockwait.c | 21 +++++-
> nptl/sem_timedwait.c | 18 ++++-
> nptl/sem_waitcommon.c | 4 +-
> nptl/semaphoreP.h | 12 ++++
> resolv/gai_misc.h | 7 ++
> resolv/gai_suspend.c | 33 +++++++--
> support/README | 4 +-
> sysdeps/nptl/aio_misc.h | 11 ++-
> sysdeps/nptl/futex-internal.h | 10 +--
> sysdeps/nptl/lowlevellock-futex.h | 80
> +++++++++++++++++++-- sysdeps/nptl/lowlevellock.h |
> 2 +- sysdeps/pthread/aio_suspend.c | 36 ++++++++--
> sysdeps/pthread/posix-timer.h | 11 +--
> sysdeps/unix/sysv/linux/x86/elision-timed.c | 2 +-
> sysdeps/unix/sysv/linux/x86/lowlevellock.h | 2 +-
> 27 files changed, 462 insertions(+), 76 deletions(-)
>
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-07 15:08 [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
` (10 preceding siblings ...)
2020-07-12 13:42 ` [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64 Lukasz Majewski
@ 2020-07-13 17:15 ` Adhemerval Zanella
2020-07-14 7:56 ` Lukasz Majewski
11 siblings, 1 reply; 30+ messages in thread
From: Adhemerval Zanella @ 2020-07-13 17:15 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
Florian Weimer, Carlos O'Donell, Stepan Golosunov,
Andreas Schwab, Zack Weinberg
On 07/07/2020 12:08, Lukasz Majewski wrote:
> Please find this early RFC for converting 'futex' syscall based code
> (pthreads/nptl/sem/gai) to support 64 bit time.
> When applicable the futex_time64 syscall is used.
>
> The main purpose of this RFC is to assess if taken approach for conversion is
> correct and acceptable by the glibc community.
>
> Quesitons/issues:
>
> 1. This whole patch set shall be squashed into a single patch, otherwise, the
> glibc will not build between separate commits. I've divided it to separate
> patches on the purpose - to facilitate review.
Another possibility could to work by adjusting each symbol and the required
futex_* / lll_lock machinery instead. For instance, add 64-bit time_t
support pthread_mutex_{timed,clock}lock, which in turn required to adjust
futex_lock_pi/lll_futex_timed_wait/lll_futex_clock_wait_bitset.
In this way we can tests the change better since they are incremental.
>
> 2. Question about rewritting lll_* macros in lowlevel-*.h - I'm wondering if
> there is maybe a better way to do it. Please pay attention to the *_4 suffix.
For lll_* I really think we should convert them to proper inline function
instead, the required code change to adjust the macro is becoming really
convoluted. I can help you on refactoring to code so the time64 changes
should be simpler.
Also, futex is a syscall used extensively and I think we should optimize
the fallback code to avoid issue the 64-bit time one if the kernel
does not support it (as we do for clock_gettime).
I have send a patchset with some y2038 fixes and I added a generic support
to simplify it [1]. We will probably need some adjustments to make it
work on libpthread.
[1] https://sourceware.org/pipermail/libc-alpha/2020-July/116259.html
>
> 3. What would be the best point in the glibc release cycle to apply this patch
> set as it convets the core functionality of the library?
>
> Just after the stable release?
I think it is late for 2.32, we should postpone it to 2.33.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-13 17:15 ` Adhemerval Zanella
@ 2020-07-14 7:56 ` Lukasz Majewski
2020-07-14 15:10 ` Adhemerval Zanella
0 siblings, 1 reply; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-14 7:56 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Arnd Bergmann,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1: Type: text/plain, Size: 4308 bytes --]
Hi Adhemerval,
> On 07/07/2020 12:08, Lukasz Majewski wrote:
> > Please find this early RFC for converting 'futex' syscall based code
> > (pthreads/nptl/sem/gai) to support 64 bit time.
> > When applicable the futex_time64 syscall is used.
> >
> > The main purpose of this RFC is to assess if taken approach for
> > conversion is correct and acceptable by the glibc community.
> >
> > Quesitons/issues:
> >
> > 1. This whole patch set shall be squashed into a single patch,
> > otherwise, the glibc will not build between separate commits. I've
> > divided it to separate patches on the purpose - to facilitate
> > review.
>
> Another possibility could to work by adjusting each symbol and the
> required futex_* / lll_lock machinery instead. For instance, add
> 64-bit time_t support pthread_mutex_{timed,clock}lock, which in turn
> required to adjust
> futex_lock_pi/lll_futex_timed_wait/lll_futex_clock_wait_bitset.
I've looked for such possibility, but the real issue IMHO is the
need to convert struct timespec to struct __timespec64 in functions
definitions/declarations.
In the end you would need to convert lll_futex_syscall() which need to
support __ASSUME_TIME64_SYSCALLS and futex_time64.
After this you are forced to convert all other pthread syscalls, which
use timeout argument.
>
> In this way we can tests the change better since they are incremental.
Please see above comment - lowlevellock-futex.h written with C
preprocessor macros is the real issue here.
>
> >
> > 2. Question about rewritting lll_* macros in lowlevel-*.h - I'm
> > wondering if there is maybe a better way to do it. Please pay
> > attention to the *_4 suffix.
>
> For lll_* I really think we should convert them to proper inline
> function instead, the required code change to adjust the macro is
> becoming really convoluted.
I fully agree here - the code as proposed[1] - is rather not clean and
safe.
> I can help you on refactoring to code so
> the time64 changes should be simpler.
Ok. Thanks.
Now for instance we do have:
__pthread_mutex_clocklock_common (funcion) (pthread_mutex_timedlock.c)
|
\|/
lll_timedwait (macro) __lll_clocklock
| |
\|/ \|/
__lll_clocklock_wait -> eligible for "conversion" Y2038 (function!)
lll_futex_timed_wait -> (macro)
lll_futex_syscall -> here is the call to futex syscall (macro)
The code itself has many levels with inline functions convoluted with
macros.
>
> Also, futex is a syscall used extensively and I think we should
> optimize the fallback code to avoid issue the 64-bit time one if the
> kernel does not support it (as we do for clock_gettime).
I think that this is not the case for most systems.
The 64 bit call for futex_time64 (and other 64 bit syscalls) were
introduced in Linux 5.1 - which is now more than a year ago.
The newest LTS Linux (5.4) now supports it - so it is likely that
new BSPs will use it. Especially, yocto LTS - dunfell (3.1) supports by
default 5.4 kernel.
>
> I have send a patchset with some y2038 fixes and I added a generic
> support to simplify it [1]. We will probably need some adjustments to
> make it work on libpthread.
>
I will share my comments on it in the patch itself.
> [1] https://sourceware.org/pipermail/libc-alpha/2020-July/116259.html
>
> >
> > 3. What would be the best point in the glibc release cycle to apply
> > this patch set as it convets the core functionality of the library?
> >
> > Just after the stable release?
>
> I think it is late for 2.32, we should postpone it to 2.33.
I'm fine with postponing it as long as some work will be done in
parallel - the futex system call is crucial in many parts of glibc
library. Sooner we convert it and pull patches into glibc master, then
sooner it matures.
Links:
[1]
-https://patchwork.ozlabs.org/project/glibc/patch/20200707150827.20899-4-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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-14 7:56 ` Lukasz Majewski
@ 2020-07-14 15:10 ` Adhemerval Zanella
2020-07-15 8:47 ` Lukasz Majewski
0 siblings, 1 reply; 30+ messages in thread
From: Adhemerval Zanella @ 2020-07-14 15:10 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Arnd Bergmann,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1.1: Type: text/plain, Size: 7975 bytes --]
On 14/07/2020 04:56, Lukasz Majewski wrote:
> Hi Adhemerval,
>
>> On 07/07/2020 12:08, Lukasz Majewski wrote:
>>> Please find this early RFC for converting 'futex' syscall based code
>>> (pthreads/nptl/sem/gai) to support 64 bit time.
>>> When applicable the futex_time64 syscall is used.
>>>
>>> The main purpose of this RFC is to assess if taken approach for
>>> conversion is correct and acceptable by the glibc community.
>>>
>>> Quesitons/issues:
>>>
>>> 1. This whole patch set shall be squashed into a single patch,
>>> otherwise, the glibc will not build between separate commits. I've
>>> divided it to separate patches on the purpose - to facilitate
>>> review.
>>
>> Another possibility could to work by adjusting each symbol and the
>> required futex_* / lll_lock machinery instead. For instance, add
>> 64-bit time_t support pthread_mutex_{timed,clock}lock, which in turn
>> required to adjust
>> futex_lock_pi/lll_futex_timed_wait/lll_futex_clock_wait_bitset.
>
> I've looked for such possibility, but the real issue IMHO is the
> need to convert struct timespec to struct __timespec64 in functions
> definitions/declarations.
>
> In the end you would need to convert lll_futex_syscall() which need to
> support __ASSUME_TIME64_SYSCALLS and futex_time64.
> After this you are forced to convert all other pthread syscalls, which
> use timeout argument.
My idea would be to implement the internal pthread_mutex_{timed,clock}_time64
and make pthread_mutex_{timed,clock} call them. For the internal time64
variant the idea would to add new time64 helper functions, in this case
futex_lock_pi_time64, lll_futex_timed_wait_time64, and
lll_futex_clock_wait_bitset_time64.
What I mean is we would incrementally add new functions without touching
to old lll/futex definitions. Once we move all the internal implementation
to time64 we can then remove them altogether.
>
>>
>> In this way we can tests the change better since they are incremental.
>
> Please see above comment - lowlevellock-futex.h written with C
> preprocessor macros is the real issue here.
>
>>
>>>
>>> 2. Question about rewritting lll_* macros in lowlevel-*.h - I'm
>>> wondering if there is maybe a better way to do it. Please pay
>>> attention to the *_4 suffix.
>>
>> For lll_* I really think we should convert them to proper inline
>> function instead, the required code change to adjust the macro is
>> becoming really convoluted.
>
> I fully agree here - the code as proposed[1] - is rather not clean and
> safe.
>
>> I can help you on refactoring to code so
>> the time64 changes should be simpler.
>
> Ok. Thanks.
>
> Now for instance we do have:
>
> __pthread_mutex_clocklock_common (funcion) (pthread_mutex_timedlock.c)
> |
> \|/
> lll_timedwait (macro) __lll_clocklock
> | |
> \|/ \|/
> __lll_clocklock_wait -> eligible for "conversion" Y2038 (function!)
> lll_futex_timed_wait -> (macro)
> lll_futex_syscall -> here is the call to futex syscall (macro)
>
> The code itself has many levels with inline functions convoluted with
> macros.
Yeah, this is messy indeed. In fact now that we don't have a NaCL
port anymore I don't seem much point in the extra lll_* indirections.
For instance, on pthread_mutex_{timed,clock} I think we can move and
rename both time64 lll_futex_timed_wait and lll_futex_clock_wait_bitset
to sysdeps/nptl/futex-internal.h and call INTERNAL_SYSCALL_* directly.
Something like:
static inline int
futex_private_flag (int fl, int priv)
{
#if IS_IN (libc) || IS_IN (rtld)
return fl | FUTEX_PRIVATE_FLAG;
#else
return (fl | FUTEX_PRIVATE_FLAG) ^ priv;
#endif
}
static __always_inline int
futex_reltimed_wait_time64 (unsigned int* futex_word, unsigned int expected,
const struct __timespec64* reltime, int priv)
{
#ifndef __NR_futex_time64
# define __NR_futex_time64 __NR_futex
#endif
int r = INTERNAL_SYSCALL_CALL (futex, futex_word,
futex_private_flag (FUTEX_WAIT, priv),
expected, reltime);
#ifndef __ASSUME_TIME64_SYSCALLS
if (r == -ENOSYS)
{
struct timespec ts32, *pts32 = NULL;
if (timeout != NULL)
{
if (! in_time_t_range (timeout->tv_sec))
return -EINVAL;
ts32 = valid_timespec64_to_timespec (ts64);
pts32 = &ts32;
}
r = INTERNAL_SYSCALL_CALL (futex, futex_word,
futex_private_flag (FUTEX_WAIT, priv),
expected, pts32);
}
#endif
switch (r)
{
case 0:
case -EAGAIN:
case -EINTR:
case -ETIMEDOUT:
return -r;
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 ();
}
}
>
>>
>> Also, futex is a syscall used extensively and I think we should
>> optimize the fallback code to avoid issue the 64-bit time one if the
>> kernel does not support it (as we do for clock_gettime).
>
> I think that this is not the case for most systems.
>
> The 64 bit call for futex_time64 (and other 64 bit syscalls) were
> introduced in Linux 5.1 - which is now more than a year ago.
>
> The newest LTS Linux (5.4) now supports it - so it is likely that
> new BSPs will use it. Especially, yocto LTS - dunfell (3.1) supports by
> default 5.4 kernel.
It really depends of the kind of deployment and time32 applications
will stick for a while, so IMHO we need to not penalize them too much
with the move to use the time64 syscalls as default.
For riscv32 and other ABI wuth time64 as default ABI it should not be a
problem, but at least for i686 I foresee it might indeed impact 32-bit
application that relies heavily on libpthread (specially on patterns
where locks are used for highly granulated contention). And this might
happen most with virtualized environments where the host kernel is not
updated as the client one (I think last conversation with Florian he say
it is a common scenario for RHEL).
In any case I think we can incrementally add such optimizations.
>
>>
>> I have send a patchset with some y2038 fixes and I added a generic
>> support to simplify it [1]. We will probably need some adjustments to
>> make it work on libpthread.
>>
>
> I will share my comments on it in the patch itself.
>
>> [1] https://sourceware.org/pipermail/libc-alpha/2020-July/116259.html
Thanks.
>>
>>>
>>> 3. What would be the best point in the glibc release cycle to apply
>>> this patch set as it convets the core functionality of the library?
>>>
>>> Just after the stable release?
>>
>> I think it is late for 2.32, we should postpone it to 2.33.
>
> I'm fine with postponing it as long as some work will be done in
> parallel - the futex system call is crucial in many parts of glibc
> library. Sooner we convert it and pull patches into glibc master, then
> sooner it matures.
>
>
> Links:
> [1]
> -https://patchwork.ozlabs.org/project/glibc/patch/20200707150827.20899-4-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
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-14 15:10 ` Adhemerval Zanella
@ 2020-07-15 8:47 ` Lukasz Majewski
2020-07-16 19:02 ` Adhemerval Zanella
0 siblings, 1 reply; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-15 8:47 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Arnd Bergmann,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1: Type: text/plain, Size: 10302 bytes --]
Hi Adhemerval,
> On 14/07/2020 04:56, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >
> >> On 07/07/2020 12:08, Lukasz Majewski wrote:
> >>> Please find this early RFC for converting 'futex' syscall based
> >>> code (pthreads/nptl/sem/gai) to support 64 bit time.
> >>> When applicable the futex_time64 syscall is used.
> >>>
> >>> The main purpose of this RFC is to assess if taken approach for
> >>> conversion is correct and acceptable by the glibc community.
> >>>
> >>> Quesitons/issues:
> >>>
> >>> 1. This whole patch set shall be squashed into a single patch,
> >>> otherwise, the glibc will not build between separate commits. I've
> >>> divided it to separate patches on the purpose - to facilitate
> >>> review.
> >>
> >> Another possibility could to work by adjusting each symbol and the
> >> required futex_* / lll_lock machinery instead. For instance, add
> >> 64-bit time_t support pthread_mutex_{timed,clock}lock, which in
> >> turn required to adjust
> >> futex_lock_pi/lll_futex_timed_wait/lll_futex_clock_wait_bitset.
> >
> > I've looked for such possibility, but the real issue IMHO is the
> > need to convert struct timespec to struct __timespec64 in functions
> > definitions/declarations.
> >
> > In the end you would need to convert lll_futex_syscall() which need
> > to support __ASSUME_TIME64_SYSCALLS and futex_time64.
> > After this you are forced to convert all other pthread syscalls,
> > which use timeout argument.
>
> My idea would be to implement the internal
> pthread_mutex_{timed,clock}_time64 and make
> pthread_mutex_{timed,clock} call them.
Let's consider __pthread_mutex_timedlock() from
./nptl/pthread_mutex_timedlock.c
Its conversion to 64 bit time has been proposed in RFC:
https://patchwork.ozlabs.org/project/glibc/patch/20200707150827.20899-6-lukma@denx.de/
> For the internal time64
> variant the idea would to add new time64 helper functions, in this
> case futex_lock_pi_time64, lll_futex_timed_wait_time64, and
> lll_futex_clock_wait_bitset_time64.
I've written some comments down below regarding this issue.
>
> What I mean is we would incrementally add new functions without
> touching to old lll/futex definitions. Once we move all the internal
> implementation to time64 we can then remove them altogether.\
My concern is that we will introduce "helper" functions (with 64 or
time64 suffix) for internal functions - a lot of new code in the
situation where we shall use __timespe64 anyway as an internal glibc
representation of time.
>
> >
> >>
> >> In this way we can tests the change better since they are
> >> incremental.
> >
> > Please see above comment - lowlevellock-futex.h written with C
> > preprocessor macros is the real issue here.
> >
> >>
> >>>
> >>> 2. Question about rewritting lll_* macros in lowlevel-*.h - I'm
> >>> wondering if there is maybe a better way to do it. Please pay
> >>> attention to the *_4 suffix.
> >>
> >> For lll_* I really think we should convert them to proper inline
> >> function instead, the required code change to adjust the macro is
> >> becoming really convoluted.
> >
> > I fully agree here - the code as proposed[1] - is rather not clean
> > and safe.
> >
> >> I can help you on refactoring to code so
> >> the time64 changes should be simpler.
> >
> > Ok. Thanks.
> >
> > Now for instance we do have:
> >
> > __pthread_mutex_clocklock_common (funcion)
> > (pthread_mutex_timedlock.c) |
> > \|/
> > lll_timedwait (macro) __lll_clocklock
> > | |
> > \|/ \|/
> > __lll_clocklock_wait -> eligible for "conversion" Y2038
> > (function!) lll_futex_timed_wait -> (macro)
> > lll_futex_syscall -> here is the call to futex syscall (macro)
> >
> > The code itself has many levels with inline functions convoluted
> > with macros.
>
> Yeah, this is messy indeed. In fact now that we don't have a NaCL
> port anymore I don't seem much point in the extra lll_* indirections.
Just wondering - what was the rationale for extra lll_* indirection for
the NaCL port?
>
> For instance, on pthread_mutex_{timed,clock} I think we can move and
> rename both time64 lll_futex_timed_wait and
> lll_futex_clock_wait_bitset to sysdeps/nptl/futex-internal.h and call
> INTERNAL_SYSCALL_* directly.
>
> Something like:
>
> static inline int
> futex_private_flag (int fl, int priv)
> {
> #if IS_IN (libc) || IS_IN (rtld)
> return fl | FUTEX_PRIVATE_FLAG;
> #else
> return (fl | FUTEX_PRIVATE_FLAG) ^ priv;
> #endif
> }
>
> static __always_inline int
> futex_reltimed_wait_time64 (unsigned int* futex_word, unsigned int
> expected, const struct __timespec64* reltime, int priv)
> {
> #ifndef __NR_futex_time64
> # define __NR_futex_time64 __NR_futex
> #endif
> int r = INTERNAL_SYSCALL_CALL (futex, futex_word,
> futex_private_flag (FUTEX_WAIT,
> priv), expected, reltime);
> #ifndef __ASSUME_TIME64_SYSCALLS
> if (r == -ENOSYS)
> {
> struct timespec ts32, *pts32 = NULL;
> if (timeout != NULL)
> {
> if (! in_time_t_range (timeout->tv_sec))
> return -EINVAL;
> ts32 = valid_timespec64_to_timespec (ts64);
> pts32 = &ts32;
> }
>
> r = INTERNAL_SYSCALL_CALL (futex, futex_word,
> futex_private_flag (FUTEX_WAIT,
> priv), expected, pts32);
> }
> #endif
>
> switch (r)
> {
> case 0:
> case -EAGAIN:
> case -EINTR:
> case -ETIMEDOUT:
> return -r;
>
> 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 ();
> }
> }
If having the invocations to futex and futex_time64 syscalls is not the
problem in many places - like futex-internal.h and lowlevel-futex.h and
also if removing the lll_* indirection is welcome, then I'm fine with
it.
With the above patch we can also rename struct timespec to __timespec64
for eligible functions - like futex_lock_pi64.
Will you have time to prepare the cleanup patch for
lowlevellock-futex.h in the near future?
Or maybe to prepare the patch with the code you wrote above?
>
> >
> >>
> >> Also, futex is a syscall used extensively and I think we should
> >> optimize the fallback code to avoid issue the 64-bit time one if
> >> the kernel does not support it (as we do for clock_gettime).
> >
> > I think that this is not the case for most systems.
> >
> > The 64 bit call for futex_time64 (and other 64 bit syscalls) were
> > introduced in Linux 5.1 - which is now more than a year ago.
> >
> > The newest LTS Linux (5.4) now supports it - so it is likely that
> > new BSPs will use it. Especially, yocto LTS - dunfell (3.1)
> > supports by default 5.4 kernel.
>
> It really depends of the kind of deployment and time32 applications
> will stick for a while, so IMHO we need to not penalize them too much
> with the move to use the time64 syscalls as default.
I'm wondering what are the numbers - i.e. what is the exact penalty for
this?
>
> For riscv32 and other ABI wuth time64 as default ABI it should not be
> a problem,
Yes. I do agree. It is also not a problem for new ports for ARM.
> but at least for i686 I foresee it might indeed impact
> 32-bit application that relies heavily on libpthread (specially on
> patterns where locks are used for highly granulated contention). And
> this might happen most with virtualized environments where the host
> kernel is not updated as the client one (I think last conversation
> with Florian he say it is a common scenario for RHEL).
Could you elaborate on this?
Please correct me if I'm wrong, but if the client is updated to kernel
5.1+ and recent glibc, then it shall support 64 bit time no matter how
old is the host VM system.
Problem with performance - with missing syscalls - would start when the
old kernel is left in place and only glibc with rootfs is updated.
>
> In any case I think we can incrementally add such optimizations.
Yes, if it solves the real issue.
>
> >
> >>
> >> I have send a patchset with some y2038 fixes and I added a generic
> >> support to simplify it [1]. We will probably need some adjustments
> >> to make it work on libpthread.
> >>
> >
> > I will share my comments on it in the patch itself.
> >
> >> [1]
> >> https://sourceware.org/pipermail/libc-alpha/2020-July/116259.html
>
> Thanks.
>
> >>
> >>>
> >>> 3. What would be the best point in the glibc release cycle to
> >>> apply this patch set as it convets the core functionality of the
> >>> library?
> >>>
> >>> Just after the stable release?
> >>
> >> I think it is late for 2.32, we should postpone it to 2.33.
> >
> > I'm fine with postponing it as long as some work will be done in
> > parallel - the futex system call is crucial in many parts of glibc
> > library. Sooner we convert it and pull patches into glibc master,
> > then sooner it matures.
> >
> >
> > Links:
> > [1]
> > -https://patchwork.ozlabs.org/project/glibc/patch/20200707150827.20899-4-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
>
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-15 8:47 ` Lukasz Majewski
@ 2020-07-16 19:02 ` Adhemerval Zanella
2020-07-17 7:27 ` Lukasz Majewski
0 siblings, 1 reply; 30+ messages in thread
From: Adhemerval Zanella @ 2020-07-16 19:02 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Arnd Bergmann,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1.1: Type: text/plain, Size: 11662 bytes --]
On 15/07/2020 05:47, Lukasz Majewski wrote:
> Hi Adhemerval,
>
>> On 14/07/2020 04:56, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>
>>>> On 07/07/2020 12:08, Lukasz Majewski wrote:
>>>>> Please find this early RFC for converting 'futex' syscall based
>>>>> code (pthreads/nptl/sem/gai) to support 64 bit time.
>>>>> When applicable the futex_time64 syscall is used.
>>>>>
>>>>> The main purpose of this RFC is to assess if taken approach for
>>>>> conversion is correct and acceptable by the glibc community.
>>>>>
>>>>> Quesitons/issues:
>>>>>
>>>>> 1. This whole patch set shall be squashed into a single patch,
>>>>> otherwise, the glibc will not build between separate commits. I've
>>>>> divided it to separate patches on the purpose - to facilitate
>>>>> review.
>>>>
>>>> Another possibility could to work by adjusting each symbol and the
>>>> required futex_* / lll_lock machinery instead. For instance, add
>>>> 64-bit time_t support pthread_mutex_{timed,clock}lock, which in
>>>> turn required to adjust
>>>> futex_lock_pi/lll_futex_timed_wait/lll_futex_clock_wait_bitset.
>>>
>>> I've looked for such possibility, but the real issue IMHO is the
>>> need to convert struct timespec to struct __timespec64 in functions
>>> definitions/declarations.
>>>
>>> In the end you would need to convert lll_futex_syscall() which need
>>> to support __ASSUME_TIME64_SYSCALLS and futex_time64.
>>> After this you are forced to convert all other pthread syscalls,
>>> which use timeout argument.
>>
>> My idea would be to implement the internal
>> pthread_mutex_{timed,clock}_time64 and make
>> pthread_mutex_{timed,clock} call them.
>
> Let's consider __pthread_mutex_timedlock() from
> ./nptl/pthread_mutex_timedlock.c
>
> Its conversion to 64 bit time has been proposed in RFC:
> https://patchwork.ozlabs.org/project/glibc/patch/20200707150827.20899-6-lukma@denx.de/
>
>> For the internal time64
>> variant the idea would to add new time64 helper functions, in this
>> case futex_lock_pi_time64, lll_futex_timed_wait_time64, and
>> lll_futex_clock_wait_bitset_time64.
>
> I've written some comments down below regarding this issue.
>
>>
>> What I mean is we would incrementally add new functions without
>> touching to old lll/futex definitions. Once we move all the internal
>> implementation to time64 we can then remove them altogether.\
>
> My concern is that we will introduce "helper" functions (with 64 or
> time64 suffix) for internal functions - a lot of new code in the
> situation where we shall use __timespe64 anyway as an internal glibc
> representation of time.
Indeed it will add some extra code to handle the conversion to and
from the 32-bit time to 64-bit time types, but once all the affected
function are converted we can cleanup and consolidate the unused code
(as you did for alpha for instance).
>
>>
>>>
>>>>
>>>> In this way we can tests the change better since they are
>>>> incremental.
>>>
>>> Please see above comment - lowlevellock-futex.h written with C
>>> preprocessor macros is the real issue here.
>>>
>>>>
>>>>>
>>>>> 2. Question about rewritting lll_* macros in lowlevel-*.h - I'm
>>>>> wondering if there is maybe a better way to do it. Please pay
>>>>> attention to the *_4 suffix.
>>>>
>>>> For lll_* I really think we should convert them to proper inline
>>>> function instead, the required code change to adjust the macro is
>>>> becoming really convoluted.
>>>
>>> I fully agree here - the code as proposed[1] - is rather not clean
>>> and safe.
>>>
>>>> I can help you on refactoring to code so
>>>> the time64 changes should be simpler.
>>>
>>> Ok. Thanks.
>>>
>>> Now for instance we do have:
>>>
>>> __pthread_mutex_clocklock_common (funcion)
>>> (pthread_mutex_timedlock.c) |
>>> \|/
>>> lll_timedwait (macro) __lll_clocklock
>>> | |
>>> \|/ \|/
>>> __lll_clocklock_wait -> eligible for "conversion" Y2038
>>> (function!) lll_futex_timed_wait -> (macro)
>>> lll_futex_syscall -> here is the call to futex syscall (macro)
>>>
>>> The code itself has many levels with inline functions convoluted
>>> with macros.
>>
>> Yeah, this is messy indeed. In fact now that we don't have a NaCL
>> port anymore I don't seem much point in the extra lll_* indirections.
>
> Just wondering - what was the rationale for extra lll_* indirection for
> the NaCL port?
Afaik NaCL was lousily based on Linux kABI, but each syscall and atomic
operation (and thus all lll_* code) was in fact a libcall to the NaCL
runtime.
>
>>
>> For instance, on pthread_mutex_{timed,clock} I think we can move and
>> rename both time64 lll_futex_timed_wait and
>> lll_futex_clock_wait_bitset to sysdeps/nptl/futex-internal.h and call
>> INTERNAL_SYSCALL_* directly.
>>
>> Something like:
>>
>> static inline int
>> futex_private_flag (int fl, int priv)
>> {
>> #if IS_IN (libc) || IS_IN (rtld)
>> return fl | FUTEX_PRIVATE_FLAG;
>> #else
>> return (fl | FUTEX_PRIVATE_FLAG) ^ priv;
>> #endif
>> }
>>
>> static __always_inline int
>> futex_reltimed_wait_time64 (unsigned int* futex_word, unsigned int
>> expected, const struct __timespec64* reltime, int priv)
>> {
>> #ifndef __NR_futex_time64
>> # define __NR_futex_time64 __NR_futex
>> #endif
>> int r = INTERNAL_SYSCALL_CALL (futex, futex_word,
>> futex_private_flag (FUTEX_WAIT,
>> priv), expected, reltime);
>> #ifndef __ASSUME_TIME64_SYSCALLS
>> if (r == -ENOSYS)
>> {
>> struct timespec ts32, *pts32 = NULL;
>> if (timeout != NULL)
>> {
>> if (! in_time_t_range (timeout->tv_sec))
>> return -EINVAL;
>> ts32 = valid_timespec64_to_timespec (ts64);
>> pts32 = &ts32;
>> }
>>
>> r = INTERNAL_SYSCALL_CALL (futex, futex_word,
>> futex_private_flag (FUTEX_WAIT,
>> priv), expected, pts32);
>> }
>> #endif
>>
>> switch (r)
>> {
>> case 0:
>> case -EAGAIN:
>> case -EINTR:
>> case -ETIMEDOUT:
>> return -r;
>>
>> 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 ();
>> }
>> }
>
> If having the invocations to futex and futex_time64 syscalls is not the
> problem in many places - like futex-internal.h and lowlevel-futex.h and
> also if removing the lll_* indirection is welcome, then I'm fine with
> it.
> With the above patch we can also rename struct timespec to __timespec64
> for eligible functions - like futex_lock_pi64.
>
>
> Will you have time to prepare the cleanup patch for
> lowlevellock-futex.h in the near future?
Yes, it is on my list. I am trying to handle the mess of stat functions
before starting on the lowlevellock-futex.h.
>
> Or maybe to prepare the patch with the code you wrote above?
>
>>
>>>
>>>>
>>>> Also, futex is a syscall used extensively and I think we should
>>>> optimize the fallback code to avoid issue the 64-bit time one if
>>>> the kernel does not support it (as we do for clock_gettime).
>>>
>>> I think that this is not the case for most systems.
>>>
>>> The 64 bit call for futex_time64 (and other 64 bit syscalls) were
>>> introduced in Linux 5.1 - which is now more than a year ago.
>>>
>>> The newest LTS Linux (5.4) now supports it - so it is likely that
>>> new BSPs will use it. Especially, yocto LTS - dunfell (3.1)
>>> supports by default 5.4 kernel.
>>
>> It really depends of the kind of deployment and time32 applications
>> will stick for a while, so IMHO we need to not penalize them too much
>> with the move to use the time64 syscalls as default.
>
> I'm wondering what are the numbers - i.e. what is the exact penalty for
> this?
It is mainly trading a ENOSYS syscall error, which would be cheaper in
terms of syscall operation but even though it has all the syscall extra
overhead; with a relaxed atomic operation. I think it is a good tradeoff.
>
>>
>> For riscv32 and other ABI wuth time64 as default ABI it should not be
>> a problem,
>
> Yes. I do agree. It is also not a problem for new ports for ARM.
>
>> but at least for i686 I foresee it might indeed impact
>> 32-bit application that relies heavily on libpthread (specially on
>> patterns where locks are used for highly granulated contention). And
>> this might happen most with virtualized environments where the host
>> kernel is not updated as the client one (I think last conversation
>> with Florian he say it is a common scenario for RHEL).
>
> Could you elaborate on this?
>
> Please correct me if I'm wrong, but if the client is updated to kernel
> 5.1+ and recent glibc, then it shall support 64 bit time no matter how
> old is the host VM system.
>
> Problem with performance - with missing syscalls - would start when the
> old kernel is left in place and only glibc with rootfs is updated.
I am trying to recall which was the original issue, it was brought when
we discussed raising the minimal supported kernel to 3.2. But I do
agree with you that when time is applicable this issue should be
observable and I am just including it on this patchset because I see
that the code complexity and runtime overhead is quite small.
>
>>
>> In any case I think we can incrementally add such optimizations.
>
> Yes, if it solves the real issue.
>
>>
>>>
>>>>
>>>> I have send a patchset with some y2038 fixes and I added a generic
>>>> support to simplify it [1]. We will probably need some adjustments
>>>> to make it work on libpthread.
>>>>
>>>
>>> I will share my comments on it in the patch itself.
>>>
>>>> [1]
>>>> https://sourceware.org/pipermail/libc-alpha/2020-July/116259.html
>>
>> Thanks.
>>
>>>>
>>>>>
>>>>> 3. What would be the best point in the glibc release cycle to
>>>>> apply this patch set as it convets the core functionality of the
>>>>> library?
>>>>>
>>>>> Just after the stable release?
>>>>
>>>> I think it is late for 2.32, we should postpone it to 2.33.
>>>
>>> I'm fine with postponing it as long as some work will be done in
>>> parallel - the futex system call is crucial in many parts of glibc
>>> library. Sooner we convert it and pull patches into glibc master,
>>> then sooner it matures.
>>>
>>>
>>> Links:
>>> [1]
>>> -https://patchwork.ozlabs.org/project/glibc/patch/20200707150827.20899-4-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
>>
>
>
>
>
> 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
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-16 19:02 ` Adhemerval Zanella
@ 2020-07-17 7:27 ` Lukasz Majewski
2020-07-17 8:11 ` Arnd Bergmann
` (2 more replies)
0 siblings, 3 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-17 7:27 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Arnd Bergmann,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1: Type: text/plain, Size: 13671 bytes --]
Hi Adhemerval,
> On 15/07/2020 05:47, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >
> >> On 14/07/2020 04:56, Lukasz Majewski wrote:
> >>> Hi Adhemerval,
> >>>
> >>>> On 07/07/2020 12:08, Lukasz Majewski wrote:
> >>>>> Please find this early RFC for converting 'futex' syscall based
> >>>>> code (pthreads/nptl/sem/gai) to support 64 bit time.
> >>>>> When applicable the futex_time64 syscall is used.
> >>>>>
> >>>>> The main purpose of this RFC is to assess if taken approach for
> >>>>> conversion is correct and acceptable by the glibc community.
> >>>>>
> >>>>> Quesitons/issues:
> >>>>>
> >>>>> 1. This whole patch set shall be squashed into a single patch,
> >>>>> otherwise, the glibc will not build between separate commits.
> >>>>> I've divided it to separate patches on the purpose - to
> >>>>> facilitate review.
> >>>>
> >>>> Another possibility could to work by adjusting each symbol and
> >>>> the required futex_* / lll_lock machinery instead. For
> >>>> instance, add 64-bit time_t support
> >>>> pthread_mutex_{timed,clock}lock, which in turn required to adjust
> >>>> futex_lock_pi/lll_futex_timed_wait/lll_futex_clock_wait_bitset.
> >>>>
> >>>
> >>> I've looked for such possibility, but the real issue IMHO is the
> >>> need to convert struct timespec to struct __timespec64 in
> >>> functions definitions/declarations.
> >>>
> >>> In the end you would need to convert lll_futex_syscall() which
> >>> need to support __ASSUME_TIME64_SYSCALLS and futex_time64.
> >>> After this you are forced to convert all other pthread syscalls,
> >>> which use timeout argument.
> >>
> >> My idea would be to implement the internal
> >> pthread_mutex_{timed,clock}_time64 and make
> >> pthread_mutex_{timed,clock} call them.
> >
> > Let's consider __pthread_mutex_timedlock() from
> > ./nptl/pthread_mutex_timedlock.c
> >
> > Its conversion to 64 bit time has been proposed in RFC:
> > https://patchwork.ozlabs.org/project/glibc/patch/20200707150827.20899-6-lukma@denx.de/
> >
> >> For the internal time64
> >> variant the idea would to add new time64 helper functions, in this
> >> case futex_lock_pi_time64, lll_futex_timed_wait_time64, and
> >> lll_futex_clock_wait_bitset_time64.
> >
> > I've written some comments down below regarding this issue.
> >
> >>
> >> What I mean is we would incrementally add new functions without
> >> touching to old lll/futex definitions. Once we move all the
> >> internal implementation to time64 we can then remove them
> >> altogether.\
> >
> > My concern is that we will introduce "helper" functions (with 64 or
> > time64 suffix) for internal functions - a lot of new code in the
> > situation where we shall use __timespe64 anyway as an internal glibc
> > representation of time.
>
> Indeed it will add some extra code to handle the conversion to and
> from the 32-bit time to 64-bit time types, but once all the affected
> function are converted we can cleanup and consolidate the unused code
> (as you did for alpha for instance).
>
> >
> >>
> >>>
> >>>>
> >>>> In this way we can tests the change better since they are
> >>>> incremental.
> >>>
> >>> Please see above comment - lowlevellock-futex.h written with C
> >>> preprocessor macros is the real issue here.
> >>>
> >>>>
> >>>>>
> >>>>> 2. Question about rewritting lll_* macros in lowlevel-*.h - I'm
> >>>>> wondering if there is maybe a better way to do it. Please pay
> >>>>> attention to the *_4 suffix.
> >>>>
> >>>> For lll_* I really think we should convert them to proper inline
> >>>> function instead, the required code change to adjust the macro is
> >>>> becoming really convoluted.
> >>>
> >>> I fully agree here - the code as proposed[1] - is rather not clean
> >>> and safe.
> >>>
> >>>> I can help you on refactoring to code so
> >>>> the time64 changes should be simpler.
> >>>
> >>> Ok. Thanks.
> >>>
> >>> Now for instance we do have:
> >>>
> >>> __pthread_mutex_clocklock_common (funcion)
> >>> (pthread_mutex_timedlock.c) |
> >>> \|/
> >>> lll_timedwait (macro) __lll_clocklock
> >>> | |
> >>> \|/ \|/
> >>> __lll_clocklock_wait -> eligible for "conversion" Y2038
> >>> (function!) lll_futex_timed_wait -> (macro)
> >>> lll_futex_syscall -> here is the call to futex syscall
> >>> (macro)
> >>>
> >>> The code itself has many levels with inline functions convoluted
> >>> with macros.
> >>
> >> Yeah, this is messy indeed. In fact now that we don't have a NaCL
> >> port anymore I don't seem much point in the extra lll_*
> >> indirections.
> >
> > Just wondering - what was the rationale for extra lll_* indirection
> > for the NaCL port?
>
> Afaik NaCL was lousily based on Linux kABI, but each syscall and
> atomic operation (and thus all lll_* code) was in fact a libcall to
> the NaCL runtime.
>
> >
> >>
> >> For instance, on pthread_mutex_{timed,clock} I think we can move
> >> and rename both time64 lll_futex_timed_wait and
> >> lll_futex_clock_wait_bitset to sysdeps/nptl/futex-internal.h and
> >> call INTERNAL_SYSCALL_* directly.
> >>
> >> Something like:
> >>
> >> static inline int
> >> futex_private_flag (int fl, int priv)
> >> {
> >> #if IS_IN (libc) || IS_IN (rtld)
> >> return fl | FUTEX_PRIVATE_FLAG;
> >> #else
> >> return (fl | FUTEX_PRIVATE_FLAG) ^ priv;
> >> #endif
> >> }
> >>
> >> static __always_inline int
> >> futex_reltimed_wait_time64 (unsigned int* futex_word, unsigned
> >> int expected, const struct __timespec64* reltime, int priv)
> >> {
> >> #ifndef __NR_futex_time64
> >> # define __NR_futex_time64 __NR_futex
> >> #endif
> >> int r = INTERNAL_SYSCALL_CALL (futex, futex_word,
> >> futex_private_flag (FUTEX_WAIT,
> >> priv), expected, reltime);
> >> #ifndef __ASSUME_TIME64_SYSCALLS
> >> if (r == -ENOSYS)
> >> {
> >> struct timespec ts32, *pts32 = NULL;
> >> if (timeout != NULL)
> >> {
> >> if (! in_time_t_range (timeout->tv_sec))
> >> return -EINVAL;
> >> ts32 = valid_timespec64_to_timespec (ts64);
> >> pts32 = &ts32;
> >> }
> >>
> >> r = INTERNAL_SYSCALL_CALL (futex, futex_word,
> >> futex_private_flag (FUTEX_WAIT,
> >> priv), expected, pts32);
> >> }
> >> #endif
> >>
> >> switch (r)
> >> {
> >> case 0:
> >> case -EAGAIN:
> >> case -EINTR:
> >> case -ETIMEDOUT:
> >> return -r;
> >>
> >> 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 ();
> >> }
> >> }
> >
> > If having the invocations to futex and futex_time64 syscalls is not
> > the problem in many places - like futex-internal.h and
> > lowlevel-futex.h and also if removing the lll_* indirection is
> > welcome, then I'm fine with it.
> > With the above patch we can also rename struct timespec to
> > __timespec64 for eligible functions - like futex_lock_pi64.
> >
> >
> > Will you have time to prepare the cleanup patch for
> > lowlevellock-futex.h in the near future?
>
> Yes, it is on my list. I am trying to handle the mess of stat
> functions
Regarding stat - it is indeed a mess - many versions of the same
functions for generic, LFS, and architectures (like ARM). Conversion to
64 bit time support is challenging there as well.
But as fair as I can tell - more than "code base" cleanup - the raise
of glibc minimal kernel supported version would help much more in this
situation as code for many "use cases" would be just removed.
A side remark - now the oldest LTS kernel supported is 4.4 and oldest
supported kernel for glibc is 3.2.
Maybe it is a good time to bump the minimal supported kernel for glibc?
> before starting on the lowlevellock-futex.h.
If you don't have time to do the conversion in next two weeks then
- I can in the meantime do a conversion for the above code snippet -
futex_reltimed_wait_time64() (and up to pthread_mutex_{timed,clock}).
This work shall be orthogonal to lowlevellock-futex.h cleanup.
What do you think?
>
> >
> > Or maybe to prepare the patch with the code you wrote above?
> >
> >>
> >>>
> >>>>
> >>>> Also, futex is a syscall used extensively and I think we should
> >>>> optimize the fallback code to avoid issue the 64-bit time one if
> >>>> the kernel does not support it (as we do for clock_gettime).
> >>>
> >>> I think that this is not the case for most systems.
> >>>
> >>> The 64 bit call for futex_time64 (and other 64 bit syscalls) were
> >>> introduced in Linux 5.1 - which is now more than a year ago.
> >>>
> >>> The newest LTS Linux (5.4) now supports it - so it is likely that
> >>> new BSPs will use it. Especially, yocto LTS - dunfell (3.1)
> >>> supports by default 5.4 kernel.
> >>
> >> It really depends of the kind of deployment and time32 applications
> >> will stick for a while, so IMHO we need to not penalize them too
> >> much with the move to use the time64 syscalls as default.
> >
> > I'm wondering what are the numbers - i.e. what is the exact penalty
> > for this?
>
> It is mainly trading a ENOSYS syscall error, which would be cheaper in
> terms of syscall operation but even though it has all the syscall
> extra overhead; with a relaxed atomic operation. I think it is a good
> tradeoff.
Yes, I do agree.
>
> >
> >>
> >> For riscv32 and other ABI wuth time64 as default ABI it should not
> >> be a problem,
> >
> > Yes. I do agree. It is also not a problem for new ports for ARM.
> >
> >> but at least for i686 I foresee it might indeed impact
> >> 32-bit application that relies heavily on libpthread (specially on
> >> patterns where locks are used for highly granulated contention).
> >> And this might happen most with virtualized environments where the
> >> host kernel is not updated as the client one (I think last
> >> conversation with Florian he say it is a common scenario for
> >> RHEL).
> >
> > Could you elaborate on this?
> >
> > Please correct me if I'm wrong, but if the client is updated to
> > kernel 5.1+ and recent glibc, then it shall support 64 bit time no
> > matter how old is the host VM system.
> >
> > Problem with performance - with missing syscalls - would start when
> > the old kernel is left in place and only glibc with rootfs is
> > updated.
>
> I am trying to recall which was the original issue, it was brought
> when we discussed raising the minimal supported kernel to 3.2. But I
> do agree with you that when time is applicable this issue should be
> observable and I am just including it on this patchset because I see
> that the code complexity and runtime overhead is quite small.
Ok.
>
> >
> >>
> >> In any case I think we can incrementally add such optimizations.
> >
> > Yes, if it solves the real issue.
> >
> >>
> >>>
> >>>>
> >>>> I have send a patchset with some y2038 fixes and I added a
> >>>> generic support to simplify it [1]. We will probably need some
> >>>> adjustments to make it work on libpthread.
> >>>>
> >>>
> >>> I will share my comments on it in the patch itself.
> >>>
> >>>> [1]
> >>>> https://sourceware.org/pipermail/libc-alpha/2020-July/116259.html
> >>>>
> >>
> >> Thanks.
> >>
> >>>>
> >>>>>
> >>>>> 3. What would be the best point in the glibc release cycle to
> >>>>> apply this patch set as it convets the core functionality of the
> >>>>> library?
> >>>>>
> >>>>> Just after the stable release?
> >>>>
> >>>> I think it is late for 2.32, we should postpone it to 2.33.
> >>>
> >>> I'm fine with postponing it as long as some work will be done in
> >>> parallel - the futex system call is crucial in many parts of glibc
> >>> library. Sooner we convert it and pull patches into glibc master,
> >>> then sooner it matures.
> >>>
> >>>
> >>> Links:
> >>> [1]
> >>> -https://patchwork.ozlabs.org/project/glibc/patch/20200707150827.20899-4-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
> >>
> >
> >
> >
> >
> > 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
>
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-17 7:27 ` Lukasz Majewski
@ 2020-07-17 8:11 ` Arnd Bergmann
2020-07-17 10:17 ` Lukasz Majewski
2020-07-17 15:04 ` Adhemerval Zanella
2020-07-17 14:32 ` Joseph Myers
2020-07-17 15:04 ` Adhemerval Zanella
2 siblings, 2 replies; 30+ messages in thread
From: Arnd Bergmann @ 2020-07-17 8:11 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Adhemerval Zanella, Joseph Myers, Paul Eggert, Alistair Francis,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
On Fri, Jul 17, 2020 at 9:27 AM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Yes, it is on my list. I am trying to handle the mess of stat
> > functions
>
> Regarding stat - it is indeed a mess - many versions of the same
> functions for generic, LFS, and architectures (like ARM). Conversion to
> 64 bit time support is challenging there as well.
>
> But as fair as I can tell - more than "code base" cleanup - the raise
> of glibc minimal kernel supported version would help much more in this
> situation as code for many "use cases" would be just removed.
>
> A side remark - now the oldest LTS kernel supported is 4.4 and oldest
> supported kernel for glibc is 3.2.
> Maybe it is a good time to bump the minimal supported kernel for glibc?
>
> > before starting on the lowlevellock-futex.h.
>
> If you don't have time to do the conversion in next two weeks then
> - I can in the meantime do a conversion for the above code snippet -
> futex_reltimed_wait_time64() (and up to pthread_mutex_{timed,clock}).
> This work shall be orthogonal to lowlevellock-futex.h cleanup.
>
> What do you think?
I don't think it makes any difference for stat: linux-3.2 and linux-5.0
appear to have the exact same kernel interface for newstat/stat64
and newfstatat/fstatat64 on all architectures, though there are two
special cases that need to be handled:
- mips-n32 and ix86-x32 are lacking the stat64 version, so they only
have the non-LFS newstat/newfstatat version but not stat64/fstatat64.
The x32 version is wide enough to support 64-bit offsets but uses
the non-LFS name for it.
- alpha, parisc and sparc are 64-bit architecture with stat64/fstatat64,
but they lack newstat/newfstatat.
While statx() was added in in linux-4.11, it was not enabled in ia64
until linux-5.1 when we sanitized the syscall tables across architectures,
so you can't rely on that until v5.1 is the minimum kernel for glibc,
and that will still be a long time.
There may be other reasons to increase the minimum runtimer kernel
level or the minimum kernel header version for glibc in the meantime.
Arnd
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-17 8:11 ` Arnd Bergmann
@ 2020-07-17 10:17 ` Lukasz Majewski
2020-07-17 15:04 ` Adhemerval Zanella
1 sibling, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-17 10:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Adhemerval Zanella, Joseph Myers, Paul Eggert, Alistair Francis,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1: Type: text/plain, Size: 2889 bytes --]
On Fri, 17 Jul 2020 10:11:50 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Jul 17, 2020 at 9:27 AM Lukasz Majewski <lukma@denx.de> wrote:
> > >
> > > Yes, it is on my list. I am trying to handle the mess of stat
> > > functions
> >
> > Regarding stat - it is indeed a mess - many versions of the same
> > functions for generic, LFS, and architectures (like ARM).
> > Conversion to 64 bit time support is challenging there as well.
> >
> > But as fair as I can tell - more than "code base" cleanup - the
> > raise of glibc minimal kernel supported version would help much
> > more in this situation as code for many "use cases" would be just
> > removed.
> >
> > A side remark - now the oldest LTS kernel supported is 4.4 and
> > oldest supported kernel for glibc is 3.2.
> > Maybe it is a good time to bump the minimal supported kernel for
> > glibc?
> > > before starting on the lowlevellock-futex.h.
> >
> > If you don't have time to do the conversion in next two weeks then
> > - I can in the meantime do a conversion for the above code snippet -
> > futex_reltimed_wait_time64() (and up to
> > pthread_mutex_{timed,clock}). This work shall be orthogonal to
> > lowlevellock-futex.h cleanup.
> >
> > What do you think?
>
> I don't think it makes any difference for stat: linux-3.2 and
> linux-5.0 appear to have the exact same kernel interface for
> newstat/stat64 and newfstatat/fstatat64 on all architectures, though
> there are two special cases that need to be handled:
>
> - mips-n32 and ix86-x32 are lacking the stat64 version, so they only
> have the non-LFS newstat/newfstatat version but not
> stat64/fstatat64. The x32 version is wide enough to support 64-bit
> offsets but uses the non-LFS name for it.
>
> - alpha, parisc and sparc are 64-bit architecture with
> stat64/fstatat64, but they lack newstat/newfstatat.
>
> While statx() was added in in linux-4.11, it was not enabled in ia64
> until linux-5.1 when we sanitized the syscall tables across
> architectures, so you can't rely on that until v5.1 is the minimum
> kernel for glibc, and that will still be a long time.
Yes, I was thinking about the statx availability since 4.11 - it
supports 64 bit time out of the box even on 32 bit machines, so can be
easily used to internally (i.e. in-glibc) implement stat calls for them.
Unfortunately, I've overlooked the case for ia64. Thanks for pointing
this out.
>
> There may be other reasons to increase the minimum runtimer kernel
> level or the minimum kernel header version for glibc in the meantime.
>
> Arnd
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-17 8:11 ` Arnd Bergmann
2020-07-17 10:17 ` Lukasz Majewski
@ 2020-07-17 15:04 ` Adhemerval Zanella
2020-07-17 16:41 ` Lukasz Majewski
2020-07-17 17:18 ` Joseph Myers
1 sibling, 2 replies; 30+ messages in thread
From: Adhemerval Zanella @ 2020-07-17 15:04 UTC (permalink / raw)
To: Arnd Bergmann, Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Florian Weimer, Carlos O'Donell,
Stepan Golosunov, Andreas Schwab, Zack Weinberg
On 17/07/2020 05:11, Arnd Bergmann wrote:
> On Fri, Jul 17, 2020 at 9:27 AM Lukasz Majewski <lukma@denx.de> wrote:
>>>
>>> Yes, it is on my list. I am trying to handle the mess of stat
>>> functions
>>
>> Regarding stat - it is indeed a mess - many versions of the same
>> functions for generic, LFS, and architectures (like ARM). Conversion to
>> 64 bit time support is challenging there as well.
>>
>> But as fair as I can tell - more than "code base" cleanup - the raise
>> of glibc minimal kernel supported version would help much more in this
>> situation as code for many "use cases" would be just removed.
>>
>> A side remark - now the oldest LTS kernel supported is 4.4 and oldest
>> supported kernel for glibc is 3.2.
>> Maybe it is a good time to bump the minimal supported kernel for glibc?
>>
>>> before starting on the lowlevellock-futex.h.
>>
>> If you don't have time to do the conversion in next two weeks then
>> - I can in the meantime do a conversion for the above code snippet -
>> futex_reltimed_wait_time64() (and up to pthread_mutex_{timed,clock}).
>> This work shall be orthogonal to lowlevellock-futex.h cleanup.
>>
>> What do you think?
>
> I don't think it makes any difference for stat: linux-3.2 and linux-5.0
> appear to have the exact same kernel interface for newstat/stat64
> and newfstatat/fstatat64 on all architectures, though there are two
> special cases that need to be handled:
>
> - mips-n32 and ix86-x32 are lacking the stat64 version, so they only
> have the non-LFS newstat/newfstatat version but not stat64/fstatat64.
> The x32 version is wide enough to support 64-bit offsets but uses
> the non-LFS name for it.
>
> - alpha, parisc and sparc are 64-bit architecture with stat64/fstatat64,
> but they lack newstat/newfstatat.
>
> While statx() was added in in linux-4.11, it was not enabled in ia64
> until linux-5.1 when we sanitized the syscall tables across architectures,
> so you can't rely on that until v5.1 is the minimum kernel for glibc,
> and that will still be a long time.
>
> There may be other reasons to increase the minimum runtimer kernel
> level or the minimum kernel header version for glibc in the meantime.
Indeed raising the kernel does not help us much here, as you pointed out
we need to handle some outliers. My current work to simplify the code base
and keep me sane while adding y2038 support is to:
1. Consolidate the {f}xstat{at}{64} routines. With some ifdef checks
to use the right syscall the only architectures that require special
implementations are alpha and mips64 (which support either specific
glibc _STAT_VER version or special function conversions.
2. Remove internal usage of the {f}xstat{at}{64} routines and replace
with {f}stat{at}64.
3. Move the {f}xstat{at}{64} to compat symbols and add {f}stat{at}64
symbol for 2.33. My plan is also do decouple the struct stat
definition from the kernel and define a glibc generic one for
*all* architectures (we can do it because there are new symbols).
4. Implement fstat, lstat, and fstat on fstatat basis. This will simplify
the y2038 support since only the all syscall selection logic will
at fstatat.
5. Add y2038 support by adding a new internal struct __stat{64}_time{64}.
These will require some for symbol for non-LFS architectures, but with
the implementation consolidation their implementation should be way
simpler.
For the generic fstatat{64} my plan is to adjust the current struct
kernel_stat to defined for *all* architectures (not only when it is required,
such as mips). It will decouple the glibc from kernel, although it will
require some routines to copy out the result (which we are do for some
architectures anyway). In a high level what I have devised so far is:
---
* fstatat.c
/* XSTAT_IS_XSTAT64 defines whether architecture support LFS or not. */
#if !XSTAT_IS_XSTAT64
/* Copies the fields from the kernel struct stat KST to glibc struct stat
ST, returning EOVERFLOW if st_ino, st_size, or st_blocks does not fit
in the non-LFS ST. */
extern int __kstat_to_stat (struct kernel_stat *kst, struct stat *st);
/* This is build only for ABI with non-LFS support: arm, csky, i386,
hppa, m68k, mips32, microblaze, nios2, s390, sh, powerpc32, and
sparc32. */
int
__fxstatat (int vers, int fd, const char *file, struct stat *st, int flag)
{
struct kernel_stat kst;
int r = INLINE_SYSCALL_CALL (fstatat64, fd, file, &kst, flag);
return r ?: __kstat_to_stat (&kst, st);
}
#endif /* XSTAT_IS_XSTAT64 */
--
* fstatat64.c
int
__fxstatat64 (int vers, int fd, const char *file, struct stat64 *st, int flag)
{
#ifdef __NR_newfstatat
/* 64-bit kABI, e.g. aarch64, ia64, mips64*, powerpc64*, s390x, riscv64,
and x86_64. */
r = INLINE_SYSCALL_CALL (newfstatat, fd, file, &kst, flag);
#if defined __NR_fstatat64
/* 64-bit kABI outlier, e.g. alpha and sparc64. */
r = INLINE_SYSCALL_CALL (fstatat64, fd, file, &kst, flag)
#else
struct statx tmp;
int r = INLINE_SYSCALL_CALL (statx, fd, file, AT_NO_AUTOMOUNT | flag,
STATX_BASIC_STATS, &tmp);
if (r == 0 || errno != ENOSYS)
{
if (r == 0)
__cp_stat64_statx (st, &tmp);
return r;
}
/* Maybe I need to handle mips64-n32 here... */
r = INLINE_SYSCALL_CALL (fstatat64, fd, file, &kst, flag);
#endif
return r ?: __kstat_to_stat64 (&kst, st);
}
--
I am currently working on the 4. item on the list above and this is just the
part to consolidate the implementations and make it sane. The y2038 part should
be way simpler than trying to work with current implementation: the __fxstatat64
would be the same, but for the statx fallback to use fstatat64 we will need an
extra convention to check for overflow.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-17 15:04 ` Adhemerval Zanella
@ 2020-07-17 16:41 ` Lukasz Majewski
2020-07-17 17:18 ` Joseph Myers
1 sibling, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-17 16:41 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Arnd Bergmann, Joseph Myers, Paul Eggert, Alistair Francis,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1: Type: text/plain, Size: 7034 bytes --]
Hi Adhemerval,
> On 17/07/2020 05:11, Arnd Bergmann wrote:
> > On Fri, Jul 17, 2020 at 9:27 AM Lukasz Majewski <lukma@denx.de>
> > wrote:
> >>>
> >>> Yes, it is on my list. I am trying to handle the mess of stat
> >>> functions
> >>
> >> Regarding stat - it is indeed a mess - many versions of the same
> >> functions for generic, LFS, and architectures (like ARM).
> >> Conversion to 64 bit time support is challenging there as well.
> >>
> >> But as fair as I can tell - more than "code base" cleanup - the
> >> raise of glibc minimal kernel supported version would help much
> >> more in this situation as code for many "use cases" would be just
> >> removed.
> >>
> >> A side remark - now the oldest LTS kernel supported is 4.4 and
> >> oldest supported kernel for glibc is 3.2.
> >> Maybe it is a good time to bump the minimal supported kernel for
> >> glibc?
> >>> before starting on the lowlevellock-futex.h.
> >>
> >> If you don't have time to do the conversion in next two weeks then
> >> - I can in the meantime do a conversion for the above code snippet
> >> - futex_reltimed_wait_time64() (and up to
> >> pthread_mutex_{timed,clock}). This work shall be orthogonal to
> >> lowlevellock-futex.h cleanup.
> >>
> >> What do you think?
> >
> > I don't think it makes any difference for stat: linux-3.2 and
> > linux-5.0 appear to have the exact same kernel interface for
> > newstat/stat64 and newfstatat/fstatat64 on all architectures,
> > though there are two special cases that need to be handled:
> >
> > - mips-n32 and ix86-x32 are lacking the stat64 version, so they only
> > have the non-LFS newstat/newfstatat version but not
> > stat64/fstatat64. The x32 version is wide enough to support 64-bit
> > offsets but uses the non-LFS name for it.
> >
> > - alpha, parisc and sparc are 64-bit architecture with
> > stat64/fstatat64, but they lack newstat/newfstatat.
> >
> > While statx() was added in in linux-4.11, it was not enabled in ia64
> > until linux-5.1 when we sanitized the syscall tables across
> > architectures, so you can't rely on that until v5.1 is the minimum
> > kernel for glibc, and that will still be a long time.
> >
> > There may be other reasons to increase the minimum runtimer kernel
> > level or the minimum kernel header version for glibc in the
> > meantime.
>
> Indeed raising the kernel does not help us much here, as you pointed
> out we need to handle some outliers. My current work to simplify the
> code base and keep me sane while adding y2038 support is to:
>
> 1. Consolidate the {f}xstat{at}{64} routines. With some ifdef
> checks to use the right syscall the only architectures that require
> special implementations are alpha and mips64 (which support either
> specific glibc _STAT_VER version or special function conversions.
>
> 2. Remove internal usage of the {f}xstat{at}{64} routines and
> replace with {f}stat{at}64.
>
> 3. Move the {f}xstat{at}{64} to compat symbols and add {f}stat{at}64
> symbol for 2.33. My plan is also do decouple the struct stat
> definition from the kernel and define a glibc generic one for
> *all* architectures (we can do it because there are new symbols).
>
> 4. Implement fstat, lstat, and fstat on fstatat basis. This will
> simplify the y2038 support since only the all syscall selection logic
> will at fstatat.
>
> 5. Add y2038 support by adding a new internal struct
> __stat{64}_time{64}. These will require some for symbol for non-LFS
> architectures, but with the implementation consolidation their
> implementation should be way simpler.
>
> For the generic fstatat{64} my plan is to adjust the current struct
> kernel_stat to defined for *all* architectures (not only when it is
> required, such as mips). It will decouple the glibc from kernel,
> although it will require some routines to copy out the result (which
> we are do for some architectures anyway). In a high level what I
> have devised so far is:
>
> ---
> * fstatat.c
>
> /* XSTAT_IS_XSTAT64 defines whether architecture support LFS or not.
> */ #if !XSTAT_IS_XSTAT64
>
> /* Copies the fields from the kernel struct stat KST to glibc struct
> stat ST, returning EOVERFLOW if st_ino, st_size, or st_blocks does
> not fit in the non-LFS ST. */
> extern int __kstat_to_stat (struct kernel_stat *kst, struct stat *st);
>
> /* This is build only for ABI with non-LFS support: arm, csky, i386,
> hppa, m68k, mips32, microblaze, nios2, s390, sh, powerpc32, and
> sparc32. */
> int
> __fxstatat (int vers, int fd, const char *file, struct stat *st, int
> flag) {
> struct kernel_stat kst;
> int r = INLINE_SYSCALL_CALL (fstatat64, fd, file, &kst, flag);
> return r ?: __kstat_to_stat (&kst, st);
> }
>
> #endif /* XSTAT_IS_XSTAT64 */
>
> --
> * fstatat64.c
>
> int
> __fxstatat64 (int vers, int fd, const char *file, struct stat64 *st,
> int flag) {
> #ifdef __NR_newfstatat
> /* 64-bit kABI, e.g. aarch64, ia64, mips64*, powerpc64*, s390x,
> riscv64, and x86_64. */
> r = INLINE_SYSCALL_CALL (newfstatat, fd, file, &kst, flag);
> #if defined __NR_fstatat64
> /* 64-bit kABI outlier, e.g. alpha and sparc64. */
> r = INLINE_SYSCALL_CALL (fstatat64, fd, file, &kst, flag)
> #else
> struct statx tmp;
> int r = INLINE_SYSCALL_CALL (statx, fd, file, AT_NO_AUTOMOUNT |
> flag, STATX_BASIC_STATS, &tmp);
> if (r == 0 || errno != ENOSYS)
> {
> if (r == 0)
> __cp_stat64_statx (st, &tmp);
> return r;
> }
> /* Maybe I need to handle mips64-n32 here... */
> r = INLINE_SYSCALL_CALL (fstatat64, fd, file, &kst, flag);
> #endif
> return r ?: __kstat_to_stat64 (&kst, st);
> }
> --
>
> I am currently working on the 4. item on the list above and this is
> just the part to consolidate the implementations and make it sane.
> The y2038 part should be way simpler than trying to work with current
> implementation: the __fxstatat64 would be the same, but for the statx
> fallback to use fstatat64 we will need an extra convention to check
> for overflow.
It would be great to have the "syscall decision logic" in one place.
Some of my "hackish" attempts to convert *stat* [1][2][3] to support
64 bit time were using statx as it is already available and supported
in glibc.
The only issue was to convert/emulate data for stat output.
Links:
[1] -
https://github.com/lmajewski/y2038_glibc/commit/57841b674d3fc007e31e7551a66963834cf096ac
[2] -
https://github.com/lmajewski/y2038_glibc/commit/d79192c729562f5734a4fbc058e933c2a715d893
[3] -
https://github.com/lmajewski/y2038_glibc/commit/c74c19ab74eef4dcba8bd48d5306d4b38c55648e
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-17 15:04 ` Adhemerval Zanella
2020-07-17 16:41 ` Lukasz Majewski
@ 2020-07-17 17:18 ` Joseph Myers
1 sibling, 0 replies; 30+ messages in thread
From: Joseph Myers @ 2020-07-17 17:18 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Arnd Bergmann, Lukasz Majewski, Florian Weimer, GNU C Library,
Andreas Schwab, Stepan Golosunov, Alistair Francis
On Fri, 17 Jul 2020, Adhemerval Zanella via Libc-alpha wrote:
> 3. Move the {f}xstat{at}{64} to compat symbols and add {f}stat{at}64
> symbol for 2.33. My plan is also do decouple the struct stat
> definition from the kernel and define a glibc generic one for
> *all* architectures (we can do it because there are new symbols).
That's the sort of change that, although it wouldn't break the ABI as seen
by an existing set of binaries, would be problematic when any existing
shared library has struct stat or struct stat64 in its ABI and people try
to link to it with a new binary without recompiling the shared library, or
recompile the shared library with new libc without recompiling all
binaries linked against it. I don't think such a change to struct stat or
struct stat64 would be a good idea - but a generic definition certainly
makes sense for the case of 64-bit time on architectures that don't
currently have 64-bit time.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-17 7:27 ` Lukasz Majewski
2020-07-17 8:11 ` Arnd Bergmann
@ 2020-07-17 14:32 ` Joseph Myers
2020-07-17 15:04 ` Adhemerval Zanella
2 siblings, 0 replies; 30+ messages in thread
From: Joseph Myers @ 2020-07-17 14:32 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Adhemerval Zanella, Paul Eggert, Alistair Francis, Arnd Bergmann,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
On Fri, 17 Jul 2020, Lukasz Majewski wrote:
> But as fair as I can tell - more than "code base" cleanup - the raise
> of glibc minimal kernel supported version would help much more in this
> situation as code for many "use cases" would be just removed.
>
> A side remark - now the oldest LTS kernel supported is 4.4 and oldest
> supported kernel for glibc is 3.2.
> Maybe it is a good time to bump the minimal supported kernel for glibc?
Carlos was proposing to remove the runtime check that prevents glibc from
running if the kernel version is too old, on the basis that often new
features may have been backported to an older kernel or may not actually
be used in glibc for many programs, so a newer glibc could often work in a
container under a kernel reporting an older version number. I think it
makes sense to do that before increasing the minimum supported kernel
version.
An increase to 4.4 allows very little cleanup relevant for e.g. x86_64.
The main cleanup allowed is removing support for socketcall for many
socket operations (and thus probably allowing some of them to be
implemented through syscalls.list for all architectures rather than
needing custom C implementations). That requires careful checks on the
status of each such syscall for each architecture in 4.4, however - note
the comment in sparc/kernel-features.h about some syscalls having been
added for 32-bit in that version only for 32-bit kernels and not in the
compat syscall table for 64-bit kernels, so we'd need to keep support for
at least getpeername and getsockname using socketcall until 4.20 (Linux
kernel commit 1f2b5b8e2df4591fbca430aff9c5a072dcc0f408) or later is the
minimum version.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-17 7:27 ` Lukasz Majewski
2020-07-17 8:11 ` Arnd Bergmann
2020-07-17 14:32 ` Joseph Myers
@ 2020-07-17 15:04 ` Adhemerval Zanella
2020-07-17 16:42 ` Lukasz Majewski
2 siblings, 1 reply; 30+ messages in thread
From: Adhemerval Zanella @ 2020-07-17 15:04 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Arnd Bergmann,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1.1: Type: text/plain, Size: 1179 bytes --]
On 17/07/2020 04:27, Lukasz Majewski wrote:
>>
>> Yes, it is on my list. I am trying to handle the mess of stat
>> functions
>
> Regarding stat - it is indeed a mess - many versions of the same
> functions for generic, LFS, and architectures (like ARM). Conversion to
> 64 bit time support is challenging there as well.
>
> But as fair as I can tell - more than "code base" cleanup - the raise
> of glibc minimal kernel supported version would help much more in this
> situation as code for many "use cases" would be just removed.
>
> A side remark - now the oldest LTS kernel supported is 4.4 and oldest
> supported kernel for glibc is 3.2.
> Maybe it is a good time to bump the minimal supported kernel for glibc?
>
>> before starting on the lowlevellock-futex.h.
>
> If you don't have time to do the conversion in next two weeks then
> - I can in the meantime do a conversion for the above code snippet -
> futex_reltimed_wait_time64() (and up to pthread_mutex_{timed,clock}).
> This work shall be orthogonal to lowlevellock-futex.h cleanup.
>
> What do you think?
Sure, I am not sure if I will be able to start the lowlevellock work.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC 00/10] y2038: nptl: futex: Provide support for futex_time64
2020-07-17 15:04 ` Adhemerval Zanella
@ 2020-07-17 16:42 ` Lukasz Majewski
0 siblings, 0 replies; 30+ messages in thread
From: Lukasz Majewski @ 2020-07-17 16:42 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Arnd Bergmann,
Alistair Francis, GNU C Library, Florian Weimer,
Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
Zack Weinberg
[-- Attachment #1: Type: text/plain, Size: 1576 bytes --]
Hi Adhemerval,
> On 17/07/2020 04:27, Lukasz Majewski wrote:
> >>
> >> Yes, it is on my list. I am trying to handle the mess of stat
> >> functions
> >
> > Regarding stat - it is indeed a mess - many versions of the same
> > functions for generic, LFS, and architectures (like ARM).
> > Conversion to 64 bit time support is challenging there as well.
> >
> > But as fair as I can tell - more than "code base" cleanup - the
> > raise of glibc minimal kernel supported version would help much
> > more in this situation as code for many "use cases" would be just
> > removed.
> >
> > A side remark - now the oldest LTS kernel supported is 4.4 and
> > oldest supported kernel for glibc is 3.2.
> > Maybe it is a good time to bump the minimal supported kernel for
> > glibc?
> >> before starting on the lowlevellock-futex.h.
> >
> > If you don't have time to do the conversion in next two weeks then
> > - I can in the meantime do a conversion for the above code snippet -
> > futex_reltimed_wait_time64() (and up to
> > pthread_mutex_{timed,clock}). This work shall be orthogonal to
> > lowlevellock-futex.h cleanup.
> >
> > What do you think?
>
> Sure, I am not sure if I will be able to start the lowlevellock work.
>
Ok, Then I will prepare proper patch. Thanks.
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread