[RFC,03/10] y2038: Convert __lll_futex* functions to use futex_time64 when available

Message ID 20200707150827.20899-4-lukma@denx.de
State Dropped
Delegated to: Lukasz Majewski
Headers
Series y2038: nptl: futex: Provide support for futex_time64 |

Commit Message

Lukasz Majewski July 7, 2020, 3:08 p.m. UTC
  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(-)
  

Comments

Lukasz Majewski July 12, 2020, 1:43 p.m. UTC | #1
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
  
Joseph Myers July 13, 2020, 9:08 p.m. UTC | #2
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.
  
Lukasz Majewski July 14, 2020, 7:23 a.m. UTC | #3
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
  

Patch

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)		\