[PATCH v2 1/3] nptl: futex: Move __NR_futex_time64 alias to beginning of futex-internal.h

[PATCH v2 1/3] nptl: futex: Move __NR_futex_time64 alias to beginning of futex-internal.h

[Lukasz Majewski]

This alias macro shall be moved to the beginning of the futex-internal.h
to be easily reused by other functions, which would support 64 bit time.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

---
Changes for v2:
- None
---
 sysdeps/nptl/futex-internal.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index 84ab3f3853..7f3910ad98 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -74,6 +74,10 @@
 # error FUTEX_PRIVATE must be equal to 0
 #endif
 
+#ifndef __NR_futex_time64
+# define __NR_futex_time64 __NR_futex
+#endif
+
 /* Calls __libc_fatal with an error message.  Convenience function for
    concrete implementations of the futex interface.  */
 static __always_inline __attribute__ ((__noreturn__)) void
@@ -467,10 +471,6 @@ futex_unlock_pi (unsigned int *futex_word, int private)
     }
 }
 
-#ifndef __NR_futex_time64
-# define __NR_futex_time64 __NR_futex
-#endif
-
 static __always_inline int
 futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
                            const struct __timespec64 *timeout, int private)
-- 
2.20.1

[PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Lukasz Majewski]

This is the helper function, which uses struct __timespec64
to provide 64 bit absolute time to futex syscalls.

The aim of this function is to move convoluted pre-processor
macro code from sysdeps/nptl/lowlevellock-futex.h to C
function in futex-internal.c

The futex_abstimed_wait64 function has been put into a separate
file on the purpose - to avoid issues apparent on the m68k
architecture related to small number of available registers (there
is not enough registers to put all necessary arguments in them if
the above function would be added to futex-internal.h with
__always_inline attribute).

Additional precautions for m68k port have been taken - the
futex-internal.c file will be compiled with -fno-inline flag.

---
Changes for v2:
- Handle the case when *abstime pointer is NULL
---
 sysdeps/nptl/futex-internal.c         | 70 +++++++++++++++++++++++++++
 sysdeps/nptl/futex-internal.h         |  6 +++
 sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
 3 files changed, 78 insertions(+)

diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
index 3366aac162..3211b4c94f 100644
--- a/sysdeps/nptl/futex-internal.c
+++ b/sysdeps/nptl/futex-internal.c
@@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
                                   abstime != NULL ? &ts32 : NULL,
                                   NULL /* Unused.  */, FUTEX_BITSET_MATCH_ANY);
 }
+
+static int
+__futex_abstimed_wait32 (unsigned int* futex_word,
+                         unsigned int expected, clockid_t clockid,
+                         const struct __timespec64* abstime,
+                         int private)
+{
+  struct timespec ts32;
+
+  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
+    return -EOVERFLOW;
+
+  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
+    FUTEX_CLOCK_REALTIME : 0;
+  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
+
+  if (abstime != NULL)
+    ts32 = valid_timespec64_to_timespec (*abstime);
+
+  return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
+                                abstime != NULL ? &ts32 : NULL,
+                                NULL /* Unused.  */, FUTEX_BITSET_MATCH_ANY);
+}
 #endif
 
 int
@@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
       futex_fatal_error ();
     }
 }
+
+int
+__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
+                         clockid_t clockid,
+                         const struct __timespec64* abstime, int private)
+{
+  unsigned int clockbit;
+  int err;
+
+  /* Work around the fact that the kernel rejects negative timeout values
+     despite them being valid.  */
+  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec < 0)))
+    return ETIMEDOUT;
+
+  if (! lll_futex_supported_clockid(clockid))
+    return EINVAL;
+
+  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
+  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
+
+  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, expected,
+                               abstime, NULL /* Unused.  */,
+                               FUTEX_BITSET_MATCH_ANY);
+#ifndef __ASSUME_TIME64_SYSCALLS
+  if (err == -ENOSYS)
+    err = __futex_abstimed_wait32 (futex_word, expected,
+                                   clockid, abstime, private);
+#endif
+  switch (err)
+    {
+    case 0:
+    case -EAGAIN:
+    case -EINTR:
+    case -ETIMEDOUT:
+      return -err;
+
+    case -EFAULT: /* Must have been caused by a glibc or application bug.  */
+    case -EINVAL: /* Either due to wrong alignment, unsupported
+		     clockid 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 ();
+    }
+}
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index 7f3910ad98..1ba0d61938 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
                                     const struct __timespec64* abstime,
                                     int private) attribute_hidden;
 
+int
+__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
+                         clockid_t clockid,
+                         const struct __timespec64* abstime,
+                         int private) attribute_hidden;
+
 #endif  /* futex-internal.h */
diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile b/sysdeps/unix/sysv/linux/m68k/Makefile
index be40fae68a..65164c5752 100644
--- a/sysdeps/unix/sysv/linux/m68k/Makefile
+++ b/sysdeps/unix/sysv/linux/m68k/Makefile
@@ -21,3 +21,5 @@ sysdep-dl-routines += dl-static
 sysdep-others += lddlibc4
 install-bin += lddlibc4
 endif
+
+CFLAGS-futex-internal.c += -fno-inline
-- 
2.20.1

[PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time

[Lukasz Majewski]

The pthread_rwlock_clockrdlock, pthread_rwlock_clockwrlock,
pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock have been converted
to support 64 bit time.

This change uses new futex_abstimed_wait64 function in
./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where possible.

The pthread_rwlock_{clock|timed}{rd|wr}lock only accepts absolute time.
Moreover, there is no need to check for NULL passed as *abstime pointer to the
syscalls as those calls have exported symbols marked with __nonull attribute
for abstime.

For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
- Conversions between 64 bit time to 32 bit are necessary
- Redirection to pthread_rwlock_{clock|timed}{rd|wr}lock will provide support
  for 64 bit time

Build tests:
./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Above tests were performed with Y2038 redirection applied as well as without
to test the proper usage of both __pthread_rwlock_{clock|timed}{rd|wr}lock64
and __pthread_rwlock_{clock|timed}{rd|wr}lock.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

---
Changes for v2:
- None
---
 nptl/pthreadP.h                   | 18 +++++++++++++++++
 nptl/pthread_rwlock_clockrdlock.c | 19 +++++++++++++++---
 nptl/pthread_rwlock_clockwrlock.c | 19 +++++++++++++++---
 nptl/pthread_rwlock_common.c      | 33 +++++++++++++++----------------
 nptl/pthread_rwlock_rdlock.c      |  2 +-
 nptl/pthread_rwlock_timedrdlock.c | 19 +++++++++++++++---
 nptl/pthread_rwlock_timedwrlock.c | 19 +++++++++++++++---
 nptl/pthread_rwlock_wrlock.c      |  2 +-
 8 files changed, 100 insertions(+), 31 deletions(-)

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 9bb44c8535..5bcc8a2db5 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -464,6 +464,10 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
 # define __pthread_timedjoin_np64 __pthread_timedjoin_np
 # define __pthread_cond_timedwait64 __pthread_cond_timedwait
 # define __pthread_cond_clockwait64 __pthread_cond_clockwait
+# 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_clockjoin_np64 (pthread_t threadid, void **thread_return,
                                      clockid_t clockid,
@@ -481,6 +485,20 @@ extern int __pthread_cond_clockwait64 (pthread_cond_t *cond,
                                        clockid_t clockid,
                                        const struct __timespec64 *abstime);
 libpthread_hidden_proto (__pthread_cond_clockwait64)
+extern int __pthread_rwlock_clockrdlock64 (pthread_rwlock_t *rwlock,
+                                           clockid_t clockid,
+                                           const struct __timespec64 *abstime);
+libpthread_hidden_proto (__pthread_rwlock_clockrdlock64)
+extern int __pthread_rwlock_clockwrlock64 (pthread_rwlock_t *rwlock,
+                                           clockid_t clockid,
+                                           const struct __timespec64 *abstime);
+libpthread_hidden_proto (__pthread_rwlock_clockwrlock64)
+extern int __pthread_rwlock_timedrdlock64 (pthread_rwlock_t *rwlock,
+                                           const struct __timespec64 *abstime);
+libpthread_hidden_proto (__pthread_rwlock_timedrdlock64)
+extern int __pthread_rwlock_timedwrlock64 (pthread_rwlock_t *rwlock,
+                                           const struct __timespec64 *abstime);
+libpthread_hidden_proto (__pthread_rwlock_timedwrlock64)
 #endif
 
 extern int __pthread_cond_timedwait (pthread_cond_t *cond,
diff --git a/nptl/pthread_rwlock_clockrdlock.c b/nptl/pthread_rwlock_clockrdlock.c
index 4cedfd1dcd..d93b133c9e 100644
--- a/nptl/pthread_rwlock_clockrdlock.c
+++ b/nptl/pthread_rwlock_clockrdlock.c
@@ -21,8 +21,21 @@
 
 /* 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);
+  return __pthread_rwlock_rdlock_full64 (rwlock, clockid, abstime);
 }
+
+#if __TIMESIZE != 64
+libpthread_hidden_def (__pthread_rwlock_clockrdlock64)
+int
+__pthread_rwlock_clockrdlock (pthread_rwlock_t *rwlock, clockid_t clockid,
+                              const struct timespec *abstime)
+{
+  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
+
+  return __pthread_rwlock_clockrdlock64 (rwlock, clockid, &ts64);
+}
+#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..10a314c76d 100644
--- a/nptl/pthread_rwlock_clockwrlock.c
+++ b/nptl/pthread_rwlock_clockwrlock.c
@@ -21,8 +21,21 @@
 
 /* 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);
+  return __pthread_rwlock_wrlock_full64 (rwlock, clockid, abstime);
 }
+
+#if __TIMESIZE != 64
+libpthread_hidden_def (__pthread_rwlock_clockwrlock64)
+int
+__pthread_rwlock_clockwrlock (pthread_rwlock_t *rwlock, clockid_t clockid,
+                              const struct timespec *abstime)
+{
+  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
+
+  return __pthread_rwlock_clockwrlock64 (rwlock, clockid, &ts64);
+}
+#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..4c9f582d3d 100644
--- a/nptl/pthread_rwlock_common.c
+++ b/nptl/pthread_rwlock_common.c
@@ -278,9 +278,8 @@ __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)
+__pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
+                                const struct __timespec64 *abstime)
 {
   unsigned int r;
 
@@ -330,8 +329,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
 		      & PTHREAD_RWLOCK_RWAITING) != 0)
 		{
 		  int private = __pthread_rwlock_get_private (rwlock);
-		  int err = futex_abstimed_wait (&rwlock->__data.__readers,
-						 r, clockid, abstime, private);
+		  int err = __futex_abstimed_wait64 (&rwlock->__data.__readers,
+		                                     r, clockid, abstime,
+		                                     private);
 		  /* We ignore EAGAIN and EINTR.  On time-outs, we can just
 		     return because we don't need to clean up anything.  */
 		  if (err == ETIMEDOUT)
@@ -457,9 +457,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
 		  (&rwlock->__data.__wrphase_futex,
 		   &wpf, wpf | PTHREAD_RWLOCK_FUTEX_USED)))
 	    continue;
-	  int err = futex_abstimed_wait (&rwlock->__data.__wrphase_futex,
-					 1 | PTHREAD_RWLOCK_FUTEX_USED,
-					 clockid, abstime, private);
+	  int err = __futex_abstimed_wait64 (&rwlock->__data.__wrphase_futex,
+					     1 | PTHREAD_RWLOCK_FUTEX_USED,
+					     clockid, abstime, private);
 	  if (err == ETIMEDOUT)
 	    {
 	      /* If we timed out, we need to unregister.  If no read phase
@@ -585,9 +585,8 @@ __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)
+__pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
+                                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
@@ -728,9 +727,9 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
 	     share the flag, and another writer will wake one of the writers
 	     in this group.  */
 	  may_share_futex_used_flag = true;
-	  int err = futex_abstimed_wait (&rwlock->__data.__writers_futex,
-					 1 | PTHREAD_RWLOCK_FUTEX_USED,
-					 clockid, abstime, private);
+	  int err = __futex_abstimed_wait64 (&rwlock->__data.__writers_futex,
+					     1 | PTHREAD_RWLOCK_FUTEX_USED,
+					     clockid, abstime, private);
 	  if (err == ETIMEDOUT)
 	    {
 	      if (prefer_writer)
@@ -827,9 +826,9 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
 		  (&rwlock->__data.__wrphase_futex, &wpf,
 		   PTHREAD_RWLOCK_FUTEX_USED)))
 	    continue;
-	  int err = futex_abstimed_wait (&rwlock->__data.__wrphase_futex,
-					 PTHREAD_RWLOCK_FUTEX_USED,
-					 clockid, abstime, private);
+	  int err = __futex_abstimed_wait64 (&rwlock->__data.__wrphase_futex,
+					     PTHREAD_RWLOCK_FUTEX_USED,
+					     clockid, abstime, private);
 	  if (err == ETIMEDOUT)
 	    {
 	      if (rwlock->__data.__flags != PTHREAD_RWLOCK_PREFER_READER_NP)
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 04cecad395..2b8509a49c 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t *rwlock)
 {
   LIBC_PROBE (rdlock_entry, 1, rwlock);
 
-  int result = __pthread_rwlock_rdlock_full (rwlock, CLOCK_REALTIME, NULL);
+  int result = __pthread_rwlock_rdlock_full64 (rwlock, CLOCK_REALTIME, NULL);
   LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
   return result;
 }
diff --git a/nptl/pthread_rwlock_timedrdlock.c b/nptl/pthread_rwlock_timedrdlock.c
index c5d8aee909..d5999a1419 100644
--- a/nptl/pthread_rwlock_timedrdlock.c
+++ b/nptl/pthread_rwlock_timedrdlock.c
@@ -20,8 +20,21 @@
 
 /* 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);
+  return __pthread_rwlock_rdlock_full64 (rwlock, CLOCK_REALTIME, abstime);
 }
+
+#if __TIMESIZE != 64
+libpthread_hidden_def (__pthread_rwlock_timedrdlock64)
+int
+__pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock,
+                              const struct timespec *abstime)
+{
+  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
+
+  return __pthread_rwlock_timedrdlock64 (rwlock, &ts64);
+}
+#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..6c5284a172 100644
--- a/nptl/pthread_rwlock_timedwrlock.c
+++ b/nptl/pthread_rwlock_timedwrlock.c
@@ -20,8 +20,21 @@
 
 /* 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);
+  return __pthread_rwlock_wrlock_full64 (rwlock, CLOCK_REALTIME, abstime);
 }
+
+#if __TIMESIZE != 64
+libpthread_hidden_def (__pthread_rwlock_timedwrlock64)
+int
+__pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock,
+                              const struct timespec *abstime)
+{
+  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
+
+  return __pthread_rwlock_timedwrlock64 (rwlock, &ts64);
+}
+#endif
+weak_alias (__pthread_rwlock_timedwrlock, pthread_rwlock_timedwrlock)
diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
index a42aa626f0..210e6cffdc 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -24,7 +24,7 @@ __pthread_rwlock_wrlock (pthread_rwlock_t *rwlock)
 {
   LIBC_PROBE (wrlock_entry, 1, rwlock);
 
-  int result = __pthread_rwlock_wrlock_full (rwlock, CLOCK_REALTIME, NULL);
+  int result = __pthread_rwlock_wrlock_full64 (rwlock, CLOCK_REALTIME, NULL);
   LIBC_PROBE (wrlock_acquire_write, 1, rwlock);
   return result;
 }
-- 
2.20.1

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Alistair Francis]

On Sat, Sep 19, 2020 at 6:08 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> This is the helper function, which uses struct __timespec64
> to provide 64 bit absolute time to futex syscalls.
>
> The aim of this function is to move convoluted pre-processor
> macro code from sysdeps/nptl/lowlevellock-futex.h to C
> function in futex-internal.c
>
> The futex_abstimed_wait64 function has been put into a separate
> file on the purpose - to avoid issues apparent on the m68k
> architecture related to small number of available registers (there
> is not enough registers to put all necessary arguments in them if
> the above function would be added to futex-internal.h with
> __always_inline attribute).
>
> Additional precautions for m68k port have been taken - the
> futex-internal.c file will be compiled with -fno-inline flag.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

>
> ---
> Changes for v2:
> - Handle the case when *abstime pointer is NULL
> ---
>  sysdeps/nptl/futex-internal.c         | 70 +++++++++++++++++++++++++++
>  sysdeps/nptl/futex-internal.h         |  6 +++
>  sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
>  3 files changed, 78 insertions(+)
>
> diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
> index 3366aac162..3211b4c94f 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
>                                    abstime != NULL ? &ts32 : NULL,
>                                    NULL /* Unused.  */, FUTEX_BITSET_MATCH_ANY);
>  }
> +
> +static int
> +__futex_abstimed_wait32 (unsigned int* futex_word,
> +                         unsigned int expected, clockid_t clockid,
> +                         const struct __timespec64* abstime,
> +                         int private)
> +{
> +  struct timespec ts32;
> +
> +  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> +    return -EOVERFLOW;
> +
> +  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
> +    FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
> +
> +  if (abstime != NULL)
> +    ts32 = valid_timespec64_to_timespec (*abstime);
> +
> +  return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
> +                                abstime != NULL ? &ts32 : NULL,
> +                                NULL /* Unused.  */, FUTEX_BITSET_MATCH_ANY);
> +}
>  #endif
>
>  int
> @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
>        futex_fatal_error ();
>      }
>  }
> +
> +int
> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
> +                         clockid_t clockid,
> +                         const struct __timespec64* abstime, int private)
> +{
> +  unsigned int clockbit;
> +  int err;
> +
> +  /* Work around the fact that the kernel rejects negative timeout values
> +     despite them being valid.  */
> +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec < 0)))
> +    return ETIMEDOUT;
> +
> +  if (! lll_futex_supported_clockid(clockid))
> +    return EINVAL;
> +
> +  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
> +
> +  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, expected,
> +                               abstime, NULL /* Unused.  */,
> +                               FUTEX_BITSET_MATCH_ANY);
> +#ifndef __ASSUME_TIME64_SYSCALLS
> +  if (err == -ENOSYS)
> +    err = __futex_abstimed_wait32 (futex_word, expected,
> +                                   clockid, abstime, private);
> +#endif
> +  switch (err)
> +    {
> +    case 0:
> +    case -EAGAIN:
> +    case -EINTR:
> +    case -ETIMEDOUT:
> +      return -err;
> +
> +    case -EFAULT: /* Must have been caused by a glibc or application bug.  */
> +    case -EINVAL: /* Either due to wrong alignment, unsupported
> +                    clockid 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 ();
> +    }
> +}
> diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
> index 7f3910ad98..1ba0d61938 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
>                                      const struct __timespec64* abstime,
>                                      int private) attribute_hidden;
>
> +int
> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
> +                         clockid_t clockid,
> +                         const struct __timespec64* abstime,
> +                         int private) attribute_hidden;
> +
>  #endif  /* futex-internal.h */
> diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile b/sysdeps/unix/sysv/linux/m68k/Makefile
> index be40fae68a..65164c5752 100644
> --- a/sysdeps/unix/sysv/linux/m68k/Makefile
> +++ b/sysdeps/unix/sysv/linux/m68k/Makefile
> @@ -21,3 +21,5 @@ sysdep-dl-routines += dl-static
>  sysdep-others += lddlibc4
>  install-bin += lddlibc4
>  endif
> +
> +CFLAGS-futex-internal.c += -fno-inline
> --
> 2.20.1
>

Re: [PATCH v2 1/3] nptl: futex: Move __NR_futex_time64 alias to beginning of futex-internal.h

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]

Dear Community,

> This alias macro shall be moved to the beginning of the
> futex-internal.h to be easily reused by other functions, which would
> support 64 bit time.
> 

Do we have more comments regarding this patch? Or shall I pull it to
-master?

> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> ---
> Changes for v2:
> - None
> ---
>  sysdeps/nptl/futex-internal.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index 84ab3f3853..7f3910ad98 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -74,6 +74,10 @@
>  # error FUTEX_PRIVATE must be equal to 0
>  #endif
>  
> +#ifndef __NR_futex_time64
> +# define __NR_futex_time64 __NR_futex
> +#endif
> +
>  /* Calls __libc_fatal with an error message.  Convenience function
> for concrete implementations of the futex interface.  */
>  static __always_inline __attribute__ ((__noreturn__)) void
> @@ -467,10 +471,6 @@ futex_unlock_pi (unsigned int *futex_word, int
> private) }
>  }
>  
> -#ifndef __NR_futex_time64
> -# define __NR_futex_time64 __NR_futex
> -#endif
> -
>  static __always_inline int
>  futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
>                             const struct __timespec64 *timeout, int
> private)




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 --]

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 5729 bytes --]

Dear Community,

> This is the helper function, which uses struct __timespec64
> to provide 64 bit absolute time to futex syscalls.
> 
> The aim of this function is to move convoluted pre-processor
> macro code from sysdeps/nptl/lowlevellock-futex.h to C
> function in futex-internal.c
> 
> The futex_abstimed_wait64 function has been put into a separate
> file on the purpose - to avoid issues apparent on the m68k
> architecture related to small number of available registers (there
> is not enough registers to put all necessary arguments in them if
> the above function would be added to futex-internal.h with
> __always_inline attribute).
> 
> Additional precautions for m68k port have been taken - the
> futex-internal.c file will be compiled with -fno-inline flag.
> 

Do we have more comments regarding this patch? Or shall I pull it to
-master?

> ---
> Changes for v2:
> - Handle the case when *abstime pointer is NULL
> ---
>  sysdeps/nptl/futex-internal.c         | 70
> +++++++++++++++++++++++++++ sysdeps/nptl/futex-internal.h         |
> 6 +++ sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
>  3 files changed, 78 insertions(+)
> 
> diff --git a/sysdeps/nptl/futex-internal.c
> b/sysdeps/nptl/futex-internal.c index 3366aac162..3211b4c94f 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned int*
> futex_word, abstime != NULL ? &ts32 : NULL,
>                                    NULL /* Unused.  */,
> FUTEX_BITSET_MATCH_ANY); }
> +
> +static int
> +__futex_abstimed_wait32 (unsigned int* futex_word,
> +                         unsigned int expected, clockid_t clockid,
> +                         const struct __timespec64* abstime,
> +                         int private)
> +{
> +  struct timespec ts32;
> +
> +  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> +    return -EOVERFLOW;
> +
> +  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
> +    FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> private); +
> +  if (abstime != NULL)
> +    ts32 = valid_timespec64_to_timespec (*abstime);
> +
> +  return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
> +                                abstime != NULL ? &ts32 : NULL,
> +                                NULL /* Unused.  */,
> FUTEX_BITSET_MATCH_ANY); +}
>  #endif
>  
>  int
> @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned int*
> futex_word, futex_fatal_error ();
>      }
>  }
> +
> +int
> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> expected,
> +                         clockid_t clockid,
> +                         const struct __timespec64* abstime, int
> private) +{
> +  unsigned int clockbit;
> +  int err;
> +
> +  /* Work around the fact that the kernel rejects negative timeout
> values
> +     despite them being valid.  */
> +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec < 0)))
> +    return ETIMEDOUT;
> +
> +  if (! lll_futex_supported_clockid(clockid))
> +    return EINVAL;
> +
> +  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> private); +
> +  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> expected,
> +                               abstime, NULL /* Unused.  */,
> +                               FUTEX_BITSET_MATCH_ANY);
> +#ifndef __ASSUME_TIME64_SYSCALLS
> +  if (err == -ENOSYS)
> +    err = __futex_abstimed_wait32 (futex_word, expected,
> +                                   clockid, abstime, private);
> +#endif
> +  switch (err)
> +    {
> +    case 0:
> +    case -EAGAIN:
> +    case -EINTR:
> +    case -ETIMEDOUT:
> +      return -err;
> +
> +    case -EFAULT: /* Must have been caused by a glibc or application
> bug.  */
> +    case -EINVAL: /* Either due to wrong alignment, unsupported
> +		     clockid 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 ();
> +    }
> +}
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index 7f3910ad98..1ba0d61938 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned
> int* futex_word, const struct __timespec64* abstime,
>                                      int private) attribute_hidden;
>  
> +int
> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> expected,
> +                         clockid_t clockid,
> +                         const struct __timespec64* abstime,
> +                         int private) attribute_hidden;
> +
>  #endif  /* futex-internal.h */
> diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile
> b/sysdeps/unix/sysv/linux/m68k/Makefile index be40fae68a..65164c5752
> 100644 --- a/sysdeps/unix/sysv/linux/m68k/Makefile
> +++ b/sysdeps/unix/sysv/linux/m68k/Makefile
> @@ -21,3 +21,5 @@ sysdep-dl-routines += dl-static
>  sysdep-others += lddlibc4
>  install-bin += lddlibc4
>  endif
> +
> +CFLAGS-futex-internal.c += -fno-inline




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 --]

Re: [PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 13916 bytes --]

Dear Community,

> The pthread_rwlock_clockrdlock, pthread_rwlock_clockwrlock,
> pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock have been
> converted to support 64 bit time.
> 
> This change uses new futex_abstimed_wait64 function in
> ./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where
> possible.
> 
> The pthread_rwlock_{clock|timed}{rd|wr}lock only accepts absolute
> time. Moreover, there is no need to check for NULL passed as *abstime
> pointer to the syscalls as those calls have exported symbols marked
> with __nonull attribute for abstime.
> 
> For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
> - Conversions between 64 bit time to 32 bit are necessary
> - Redirection to pthread_rwlock_{clock|timed}{rd|wr}lock will provide
> support for 64 bit time

Do we have more comments regarding this patch? Or shall I pull it to
-master?

> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Above tests were performed with Y2038 redirection applied as well as
> without to test the proper usage of both
> __pthread_rwlock_{clock|timed}{rd|wr}lock64 and
> __pthread_rwlock_{clock|timed}{rd|wr}lock.
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> ---
> Changes for v2:
> - None
> ---
>  nptl/pthreadP.h                   | 18 +++++++++++++++++
>  nptl/pthread_rwlock_clockrdlock.c | 19 +++++++++++++++---
>  nptl/pthread_rwlock_clockwrlock.c | 19 +++++++++++++++---
>  nptl/pthread_rwlock_common.c      | 33
> +++++++++++++++---------------- nptl/pthread_rwlock_rdlock.c      |
> 2 +- nptl/pthread_rwlock_timedrdlock.c | 19 +++++++++++++++---
>  nptl/pthread_rwlock_timedwrlock.c | 19 +++++++++++++++---
>  nptl/pthread_rwlock_wrlock.c      |  2 +-
>  8 files changed, 100 insertions(+), 31 deletions(-)
> 
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 9bb44c8535..5bcc8a2db5 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -464,6 +464,10 @@ extern int __pthread_cond_wait (pthread_cond_t
> *cond, pthread_mutex_t *mutex); # define __pthread_timedjoin_np64
> __pthread_timedjoin_np # define __pthread_cond_timedwait64
> __pthread_cond_timedwait # define __pthread_cond_clockwait64
> __pthread_cond_clockwait +# 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_clockjoin_np64 (pthread_t threadid, void
> **thread_return, clockid_t clockid,
> @@ -481,6 +485,20 @@ extern int __pthread_cond_clockwait64
> (pthread_cond_t *cond, clockid_t clockid,
>                                         const struct __timespec64
> *abstime); libpthread_hidden_proto (__pthread_cond_clockwait64)
> +extern int __pthread_rwlock_clockrdlock64 (pthread_rwlock_t *rwlock,
> +                                           clockid_t clockid,
> +                                           const struct __timespec64
> *abstime); +libpthread_hidden_proto (__pthread_rwlock_clockrdlock64)
> +extern int __pthread_rwlock_clockwrlock64 (pthread_rwlock_t *rwlock,
> +                                           clockid_t clockid,
> +                                           const struct __timespec64
> *abstime); +libpthread_hidden_proto (__pthread_rwlock_clockwrlock64)
> +extern int __pthread_rwlock_timedrdlock64 (pthread_rwlock_t *rwlock,
> +                                           const struct __timespec64
> *abstime); +libpthread_hidden_proto (__pthread_rwlock_timedrdlock64)
> +extern int __pthread_rwlock_timedwrlock64 (pthread_rwlock_t *rwlock,
> +                                           const struct __timespec64
> *abstime); +libpthread_hidden_proto (__pthread_rwlock_timedwrlock64)
>  #endif
>  
>  extern int __pthread_cond_timedwait (pthread_cond_t *cond,
> diff --git a/nptl/pthread_rwlock_clockrdlock.c
> b/nptl/pthread_rwlock_clockrdlock.c index 4cedfd1dcd..d93b133c9e
> 100644 --- a/nptl/pthread_rwlock_clockrdlock.c
> +++ b/nptl/pthread_rwlock_clockrdlock.c
> @@ -21,8 +21,21 @@
>  
>  /* 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);
> +  return __pthread_rwlock_rdlock_full64 (rwlock, clockid, abstime);
>  }
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_rwlock_clockrdlock64)
> +int
> +__pthread_rwlock_clockrdlock (pthread_rwlock_t *rwlock, clockid_t
> clockid,
> +                              const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_rwlock_clockrdlock64 (rwlock, clockid, &ts64);
> +}
> +#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..10a314c76d
> 100644 --- a/nptl/pthread_rwlock_clockwrlock.c
> +++ b/nptl/pthread_rwlock_clockwrlock.c
> @@ -21,8 +21,21 @@
>  
>  /* 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);
> +  return __pthread_rwlock_wrlock_full64 (rwlock, clockid, abstime);
>  }
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_rwlock_clockwrlock64)
> +int
> +__pthread_rwlock_clockwrlock (pthread_rwlock_t *rwlock, clockid_t
> clockid,
> +                              const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_rwlock_clockwrlock64 (rwlock, clockid, &ts64);
> +}
> +#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..4c9f582d3d 100644
> --- a/nptl/pthread_rwlock_common.c
> +++ b/nptl/pthread_rwlock_common.c
> @@ -278,9 +278,8 @@ __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)
> +__pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t
> clockid,
> +                                const struct __timespec64 *abstime)
>  {
>    unsigned int r;
>  
> @@ -330,8 +329,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t
> *rwlock, & PTHREAD_RWLOCK_RWAITING) != 0)
>  		{
>  		  int private = __pthread_rwlock_get_private
> (rwlock);
> -		  int err = futex_abstimed_wait
> (&rwlock->__data.__readers,
> -						 r, clockid,
> abstime, private);
> +		  int err = __futex_abstimed_wait64
> (&rwlock->__data.__readers,
> +		                                     r, clockid,
> abstime,
> +		                                     private);
>  		  /* We ignore EAGAIN and EINTR.  On time-outs, we
> can just return because we don't need to clean up anything.  */
>  		  if (err == ETIMEDOUT)
> @@ -457,9 +457,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t
> *rwlock, (&rwlock->__data.__wrphase_futex,
>  		   &wpf, wpf | PTHREAD_RWLOCK_FUTEX_USED)))
>  	    continue;
> -	  int err = futex_abstimed_wait
> (&rwlock->__data.__wrphase_futex,
> -					 1 |
> PTHREAD_RWLOCK_FUTEX_USED,
> -					 clockid, abstime, private);
> +	  int err = __futex_abstimed_wait64
> (&rwlock->__data.__wrphase_futex,
> +					     1 |
> PTHREAD_RWLOCK_FUTEX_USED,
> +					     clockid, abstime,
> private); if (err == ETIMEDOUT)
>  	    {
>  	      /* If we timed out, we need to unregister.  If no read
> phase @@ -585,9 +585,8 @@ __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)
> +__pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t
> clockid,
> +                                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 @@ -728,9 +727,9 @@ __pthread_rwlock_wrlock_full
> (pthread_rwlock_t *rwlock, share the flag, and another writer will
> wake one of the writers in this group.  */
>  	  may_share_futex_used_flag = true;
> -	  int err = futex_abstimed_wait
> (&rwlock->__data.__writers_futex,
> -					 1 |
> PTHREAD_RWLOCK_FUTEX_USED,
> -					 clockid, abstime, private);
> +	  int err = __futex_abstimed_wait64
> (&rwlock->__data.__writers_futex,
> +					     1 |
> PTHREAD_RWLOCK_FUTEX_USED,
> +					     clockid, abstime,
> private); if (err == ETIMEDOUT)
>  	    {
>  	      if (prefer_writer)
> @@ -827,9 +826,9 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t
> *rwlock, (&rwlock->__data.__wrphase_futex, &wpf,
>  		   PTHREAD_RWLOCK_FUTEX_USED)))
>  	    continue;
> -	  int err = futex_abstimed_wait
> (&rwlock->__data.__wrphase_futex,
> -					 PTHREAD_RWLOCK_FUTEX_USED,
> -					 clockid, abstime, private);
> +	  int err = __futex_abstimed_wait64
> (&rwlock->__data.__wrphase_futex,
> +
> PTHREAD_RWLOCK_FUTEX_USED,
> +					     clockid, abstime,
> private); if (err == ETIMEDOUT)
>  	    {
>  	      if (rwlock->__data.__flags !=
> PTHREAD_RWLOCK_PREFER_READER_NP) diff --git
> a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c index
> 04cecad395..2b8509a49c 100644 --- a/nptl/pthread_rwlock_rdlock.c
> +++ b/nptl/pthread_rwlock_rdlock.c
> @@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t *rwlock)
>  {
>    LIBC_PROBE (rdlock_entry, 1, rwlock);
>  
> -  int result = __pthread_rwlock_rdlock_full (rwlock, CLOCK_REALTIME,
> NULL);
> +  int result = __pthread_rwlock_rdlock_full64 (rwlock,
> CLOCK_REALTIME, NULL); LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
>    return result;
>  }
> diff --git a/nptl/pthread_rwlock_timedrdlock.c
> b/nptl/pthread_rwlock_timedrdlock.c index c5d8aee909..d5999a1419
> 100644 --- a/nptl/pthread_rwlock_timedrdlock.c
> +++ b/nptl/pthread_rwlock_timedrdlock.c
> @@ -20,8 +20,21 @@
>  
>  /* 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);
> +  return __pthread_rwlock_rdlock_full64 (rwlock, CLOCK_REALTIME,
> abstime); }
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_rwlock_timedrdlock64)
> +int
> +__pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock,
> +                              const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_rwlock_timedrdlock64 (rwlock, &ts64);
> +}
> +#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..6c5284a172
> 100644 --- a/nptl/pthread_rwlock_timedwrlock.c
> +++ b/nptl/pthread_rwlock_timedwrlock.c
> @@ -20,8 +20,21 @@
>  
>  /* 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);
> +  return __pthread_rwlock_wrlock_full64 (rwlock, CLOCK_REALTIME,
> abstime); }
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_rwlock_timedwrlock64)
> +int
> +__pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock,
> +                              const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_rwlock_timedwrlock64 (rwlock, &ts64);
> +}
> +#endif
> +weak_alias (__pthread_rwlock_timedwrlock, pthread_rwlock_timedwrlock)
> diff --git a/nptl/pthread_rwlock_wrlock.c
> b/nptl/pthread_rwlock_wrlock.c index a42aa626f0..210e6cffdc 100644
> --- a/nptl/pthread_rwlock_wrlock.c
> +++ b/nptl/pthread_rwlock_wrlock.c
> @@ -24,7 +24,7 @@ __pthread_rwlock_wrlock (pthread_rwlock_t *rwlock)
>  {
>    LIBC_PROBE (wrlock_entry, 1, rwlock);
>  
> -  int result = __pthread_rwlock_wrlock_full (rwlock, CLOCK_REALTIME,
> NULL);
> +  int result = __pthread_rwlock_wrlock_full64 (rwlock,
> CLOCK_REALTIME, NULL); LIBC_PROBE (wrlock_acquire_write, 1, rwlock);
>    return result;
>  }




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 --]

Re: [PATCH v2 1/3] nptl: futex: Move __NR_futex_time64 alias to beginning of futex-internal.h

[Adhemerval Zanella]

On 19/09/2020 10:07, Lukasz Majewski wrote:
> This alias macro shall be moved to the beginning of the futex-internal.h
> to be easily reused by other functions, which would support 64 bit time.
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> ---
> Changes for v2:
> - None

LGTM, thanks.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/nptl/futex-internal.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
> index 84ab3f3853..7f3910ad98 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -74,6 +74,10 @@
>  # error FUTEX_PRIVATE must be equal to 0
>  #endif
>  
> +#ifndef __NR_futex_time64
> +# define __NR_futex_time64 __NR_futex
> +#endif
> +
>  /* Calls __libc_fatal with an error message.  Convenience function for
>     concrete implementations of the futex interface.  */
>  static __always_inline __attribute__ ((__noreturn__)) void
> @@ -467,10 +471,6 @@ futex_unlock_pi (unsigned int *futex_word, int private)
>      }
>  }
>  
> -#ifndef __NR_futex_time64
> -# define __NR_futex_time64 __NR_futex
> -#endif
> -
>  static __always_inline int
>  futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
>                             const struct __timespec64 *timeout, int private)
> 

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Adhemerval Zanella]

On 19/09/2020 10:07, Lukasz Majewski wrote:
> This is the helper function, which uses struct __timespec64
> to provide 64 bit absolute time to futex syscalls.
> 
> The aim of this function is to move convoluted pre-processor
> macro code from sysdeps/nptl/lowlevellock-futex.h to C
> function in futex-internal.c
> 
> The futex_abstimed_wait64 function has been put into a separate
> file on the purpose - to avoid issues apparent on the m68k
> architecture related to small number of available registers (there
> is not enough registers to put all necessary arguments in them if
> the above function would be added to futex-internal.h with
> __always_inline attribute).
> 
> Additional precautions for m68k port have been taken - the
> futex-internal.c file will be compiled with -fno-inline flag.

LGTM with a nit regarding style and a clarification about 
'-fno-inline'.

> 
> ---
> Changes for v2:
> - Handle the case when *abstime pointer is NULL
> ---
>  sysdeps/nptl/futex-internal.c         | 70 +++++++++++++++++++++++++++
>  sysdeps/nptl/futex-internal.h         |  6 +++
>  sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
>  3 files changed, 78 insertions(+)
> 
> diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
> index 3366aac162..3211b4c94f 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
>                                    abstime != NULL ? &ts32 : NULL,
>                                    NULL /* Unused.  */, FUTEX_BITSET_MATCH_ANY);
>  }
> +
> +static int
> +__futex_abstimed_wait32 (unsigned int* futex_word,
> +                         unsigned int expected, clockid_t clockid,
> +                         const struct __timespec64* abstime,
> +                         int private)
> +{
> +  struct timespec ts32;
> +
> +  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> +    return -EOVERFLOW;
> +
> +  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
> +    FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
> +
> +  if (abstime != NULL)
> +    ts32 = valid_timespec64_to_timespec (*abstime);
> +
> +  return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
> +                                abstime != NULL ? &ts32 : NULL,
> +                                NULL /* Unused.  */, FUTEX_BITSET_MATCH_ANY);
> +}
>  #endif
>  
>  int

Ok.

> @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
>        futex_fatal_error ();
>      }
>  }
> +
> +int
> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
> +                         clockid_t clockid,
> +                         const struct __timespec64* abstime, int private)
> +{
> +  unsigned int clockbit;
> +  int err;
> +
> +  /* Work around the fact that the kernel rejects negative timeout values
> +     despite them being valid.  */
> +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec < 0)))
> +    return ETIMEDOUT;
> +
> +  if (! lll_futex_supported_clockid(clockid))
> +    return EINVAL;

Space before parentheses. 

> +
> +  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
> +
> +  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, expected,
> +                               abstime, NULL /* Unused.  */,
> +                               FUTEX_BITSET_MATCH_ANY);
> +#ifndef __ASSUME_TIME64_SYSCALLS
> +  if (err == -ENOSYS)
> +    err = __futex_abstimed_wait32 (futex_word, expected,
> +                                   clockid, abstime, private);
> +#endif
> +  switch (err)
> +    {
> +    case 0:
> +    case -EAGAIN:
> +    case -EINTR:
> +    case -ETIMEDOUT:
> +      return -err;
> +
> +    case -EFAULT: /* Must have been caused by a glibc or application bug.  */
> +    case -EINVAL: /* Either due to wrong alignment, unsupported
> +		     clockid 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 ();
> +    }
> +}

Ok.

> diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
> index 7f3910ad98..1ba0d61938 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
>                                      const struct __timespec64* abstime,
>                                      int private) attribute_hidden;
>  
> +int
> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
> +                         clockid_t clockid,
> +                         const struct __timespec64* abstime,
> +                         int private) attribute_hidden;
> +
>  #endif  /* futex-internal.h */

Ok.

> diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile b/sysdeps/unix/sysv/linux/m68k/Makefile
> index be40fae68a..65164c5752 100644
> --- a/sysdeps/unix/sysv/linux/m68k/Makefile
> +++ b/sysdeps/unix/sysv/linux/m68k/Makefile
> @@ -21,3 +21,5 @@ sysdep-dl-routines += dl-static
>  sysdep-others += lddlibc4
>  install-bin += lddlibc4
>  endif
> +
> +CFLAGS-futex-internal.c += -fno-inline
> 

Do we still need it?

Re: [PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time

[Adhemerval Zanella]

On 19/09/2020 10:07, Lukasz Majewski wrote:
> The pthread_rwlock_clockrdlock, pthread_rwlock_clockwrlock,
> pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock have been converted
> to support 64 bit time.
> 
> This change uses new futex_abstimed_wait64 function in
> ./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where possible.
> 
> The pthread_rwlock_{clock|timed}{rd|wr}lock only accepts absolute time.
> Moreover, there is no need to check for NULL passed as *abstime pointer to the
> syscalls as those calls have exported symbols marked with __nonull attribute
> for abstime.
> 
> For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
> - Conversions between 64 bit time to 32 bit are necessary
> - Redirection to pthread_rwlock_{clock|timed}{rd|wr}lock will provide support
>   for 64 bit time
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Above tests were performed with Y2038 redirection applied as well as without
> to test the proper usage of both __pthread_rwlock_{clock|timed}{rd|wr}lock64
> and __pthread_rwlock_{clock|timed}{rd|wr}lock.
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Looks good, there is only a issue regarding LIBC_PROBE (below).


> 
> ---
> Changes for v2:
> - None
> ---
>  nptl/pthreadP.h                   | 18 +++++++++++++++++
>  nptl/pthread_rwlock_clockrdlock.c | 19 +++++++++++++++---
>  nptl/pthread_rwlock_clockwrlock.c | 19 +++++++++++++++---
>  nptl/pthread_rwlock_common.c      | 33 +++++++++++++++----------------
>  nptl/pthread_rwlock_rdlock.c      |  2 +-
>  nptl/pthread_rwlock_timedrdlock.c | 19 +++++++++++++++---
>  nptl/pthread_rwlock_timedwrlock.c | 19 +++++++++++++++---
>  nptl/pthread_rwlock_wrlock.c      |  2 +-
>  8 files changed, 100 insertions(+), 31 deletions(-)
> 
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 9bb44c8535..5bcc8a2db5 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -464,6 +464,10 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
>  # define __pthread_timedjoin_np64 __pthread_timedjoin_np
>  # define __pthread_cond_timedwait64 __pthread_cond_timedwait
>  # define __pthread_cond_clockwait64 __pthread_cond_clockwait
> +# 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_clockjoin_np64 (pthread_t threadid, void **thread_return,
>                                       clockid_t clockid,
> @@ -481,6 +485,20 @@ extern int __pthread_cond_clockwait64 (pthread_cond_t *cond,
>                                         clockid_t clockid,
>                                         const struct __timespec64 *abstime);
>  libpthread_hidden_proto (__pthread_cond_clockwait64)
> +extern int __pthread_rwlock_clockrdlock64 (pthread_rwlock_t *rwlock,
> +                                           clockid_t clockid,
> +                                           const struct __timespec64 *abstime);
> +libpthread_hidden_proto (__pthread_rwlock_clockrdlock64)
> +extern int __pthread_rwlock_clockwrlock64 (pthread_rwlock_t *rwlock,
> +                                           clockid_t clockid,
> +                                           const struct __timespec64 *abstime);
> +libpthread_hidden_proto (__pthread_rwlock_clockwrlock64)
> +extern int __pthread_rwlock_timedrdlock64 (pthread_rwlock_t *rwlock,
> +                                           const struct __timespec64 *abstime);
> +libpthread_hidden_proto (__pthread_rwlock_timedrdlock64)
> +extern int __pthread_rwlock_timedwrlock64 (pthread_rwlock_t *rwlock,
> +                                           const struct __timespec64 *abstime);
> +libpthread_hidden_proto (__pthread_rwlock_timedwrlock64)
>  #endif
>  
>  extern int __pthread_cond_timedwait (pthread_cond_t *cond,

Ok.

> diff --git a/nptl/pthread_rwlock_clockrdlock.c b/nptl/pthread_rwlock_clockrdlock.c
> index 4cedfd1dcd..d93b133c9e 100644
> --- a/nptl/pthread_rwlock_clockrdlock.c
> +++ b/nptl/pthread_rwlock_clockrdlock.c
> @@ -21,8 +21,21 @@
>  
>  /* 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);
> +  return __pthread_rwlock_rdlock_full64 (rwlock, clockid, abstime);
>  }
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_rwlock_clockrdlock64)

Maybe an extra space here?

> +int
> +__pthread_rwlock_clockrdlock (pthread_rwlock_t *rwlock, clockid_t clockid,
> +                              const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_rwlock_clockrdlock64 (rwlock, clockid, &ts64);
> +}
> +#endif
> +weak_alias (__pthread_rwlock_clockrdlock, pthread_rwlock_clockrdlock)

Ok.

> diff --git a/nptl/pthread_rwlock_clockwrlock.c b/nptl/pthread_rwlock_clockwrlock.c
> index 7a954cf529..10a314c76d 100644
> --- a/nptl/pthread_rwlock_clockwrlock.c
> +++ b/nptl/pthread_rwlock_clockwrlock.c
> @@ -21,8 +21,21 @@
>  
>  /* 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);
> +  return __pthread_rwlock_wrlock_full64 (rwlock, clockid, abstime);
>  }
> +

Ok.

> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_rwlock_clockwrlock64)

Maybe an extra space here?

> +int
> +__pthread_rwlock_clockwrlock (pthread_rwlock_t *rwlock, clockid_t clockid,
> +                              const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_rwlock_clockwrlock64 (rwlock, clockid, &ts64);
> +}
> +#endif
> +weak_alias (__pthread_rwlock_clockwrlock, pthread_rwlock_clockwrlock)

Ok.

> diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
> index 3fbc66ded2..4c9f582d3d 100644
> --- a/nptl/pthread_rwlock_common.c
> +++ b/nptl/pthread_rwlock_common.c
> @@ -278,9 +278,8 @@ __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)
> +__pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
> +                                const struct __timespec64 *abstime)
>  {
>    unsigned int r;
>  

Ok.

> @@ -330,8 +329,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
>  		      & PTHREAD_RWLOCK_RWAITING) != 0)
>  		{
>  		  int private = __pthread_rwlock_get_private (rwlock);
> -		  int err = futex_abstimed_wait (&rwlock->__data.__readers,
> -						 r, clockid, abstime, private);
> +		  int err = __futex_abstimed_wait64 (&rwlock->__data.__readers,
> +		                                     r, clockid, abstime,
> +		                                     private);
>  		  /* We ignore EAGAIN and EINTR.  On time-outs, we can just
>  		     return because we don't need to clean up anything.  */
>  		  if (err == ETIMEDOUT)

Ok.

> @@ -457,9 +457,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
>  		  (&rwlock->__data.__wrphase_futex,
>  		   &wpf, wpf | PTHREAD_RWLOCK_FUTEX_USED)))
>  	    continue;
> -	  int err = futex_abstimed_wait (&rwlock->__data.__wrphase_futex,
> -					 1 | PTHREAD_RWLOCK_FUTEX_USED,
> -					 clockid, abstime, private);
> +	  int err = __futex_abstimed_wait64 (&rwlock->__data.__wrphase_futex,
> +					     1 | PTHREAD_RWLOCK_FUTEX_USED,
> +					     clockid, abstime, private);
>  	  if (err == ETIMEDOUT)
>  	    {
>  	      /* If we timed out, we need to unregister.  If no read phase

Ok.

> @@ -585,9 +585,8 @@ __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)
> +__pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
> +                                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
> @@ -728,9 +727,9 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
>  	     share the flag, and another writer will wake one of the writers
>  	     in this group.  */
>  	  may_share_futex_used_flag = true;
> -	  int err = futex_abstimed_wait (&rwlock->__data.__writers_futex,
> -					 1 | PTHREAD_RWLOCK_FUTEX_USED,
> -					 clockid, abstime, private);
> +	  int err = __futex_abstimed_wait64 (&rwlock->__data.__writers_futex,
> +					     1 | PTHREAD_RWLOCK_FUTEX_USED,
> +					     clockid, abstime, private);
>  	  if (err == ETIMEDOUT)
>  	    {
>  	      if (prefer_writer)

Ok.

> @@ -827,9 +826,9 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
>  		  (&rwlock->__data.__wrphase_futex, &wpf,
>  		   PTHREAD_RWLOCK_FUTEX_USED)))
>  	    continue;
> -	  int err = futex_abstimed_wait (&rwlock->__data.__wrphase_futex,
> -					 PTHREAD_RWLOCK_FUTEX_USED,
> -					 clockid, abstime, private);
> +	  int err = __futex_abstimed_wait64 (&rwlock->__data.__wrphase_futex,
> +					     PTHREAD_RWLOCK_FUTEX_USED,
> +					     clockid, abstime, private);
>  	  if (err == ETIMEDOUT)
>  	    {
>  	      if (rwlock->__data.__flags != PTHREAD_RWLOCK_PREFER_READER_NP)

Ok.

> diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
> index 04cecad395..2b8509a49c 100644
> --- a/nptl/pthread_rwlock_rdlock.c
> +++ b/nptl/pthread_rwlock_rdlock.c
> @@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t *rwlock)
>  {
>    LIBC_PROBE (rdlock_entry, 1, rwlock);

We need to move the systemtap probe to the 64-bit variant, so the 64-bit time
will still trigger it.

>  
> -  int result = __pthread_rwlock_rdlock_full (rwlock, CLOCK_REALTIME, NULL);
> +  int result = __pthread_rwlock_rdlock_full64 (rwlock, CLOCK_REALTIME, NULL);
>    LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
>    return result;
>  }

Ok.

> diff --git a/nptl/pthread_rwlock_timedrdlock.c b/nptl/pthread_rwlock_timedrdlock.c
> index c5d8aee909..d5999a1419 100644
> --- a/nptl/pthread_rwlock_timedrdlock.c
> +++ b/nptl/pthread_rwlock_timedrdlock.c
> @@ -20,8 +20,21 @@
>  
>  /* 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);
> +  return __pthread_rwlock_rdlock_full64 (rwlock, CLOCK_REALTIME, abstime);
>  }
> +

Ok.

> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_rwlock_timedrdlock64)

Maybe an extra space here?

> +int
> +__pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock,
> +                              const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_rwlock_timedrdlock64 (rwlock, &ts64);
> +}
> +#endif
> +weak_alias (__pthread_rwlock_timedrdlock, pthread_rwlock_timedrdlock)

Ok.

> diff --git a/nptl/pthread_rwlock_timedwrlock.c b/nptl/pthread_rwlock_timedwrlock.c
> index ccee8b77d9..6c5284a172 100644
> --- a/nptl/pthread_rwlock_timedwrlock.c
> +++ b/nptl/pthread_rwlock_timedwrlock.c
> @@ -20,8 +20,21 @@
>  
>  /* 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);
> +  return __pthread_rwlock_wrlock_full64 (rwlock, CLOCK_REALTIME, abstime);
>  }
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_rwlock_timedwrlock64)


Maybe an extra space here?

> +int
> +__pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock,
> +                              const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_rwlock_timedwrlock64 (rwlock, &ts64);
> +}
> +#endif
> +weak_alias (__pthread_rwlock_timedwrlock, pthread_rwlock_timedwrlock)

Ok.

> diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
> index a42aa626f0..210e6cffdc 100644
> --- a/nptl/pthread_rwlock_wrlock.c
> +++ b/nptl/pthread_rwlock_wrlock.c
> @@ -24,7 +24,7 @@ __pthread_rwlock_wrlock (pthread_rwlock_t *rwlock)
>  {
>    LIBC_PROBE (wrlock_entry, 1, rwlock);

We need to move the systemtap probe to the 64-bit variant, so the 64-bit time
will still trigger it.

>  
> -  int result = __pthread_rwlock_wrlock_full (rwlock, CLOCK_REALTIME, NULL);
> +  int result = __pthread_rwlock_wrlock_full64 (rwlock, CLOCK_REALTIME, NULL);
>    LIBC_PROBE (wrlock_acquire_write, 1, rwlock);
>    return result;
>  }
> 

Ok.

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 6891 bytes --]

Hi Adhemerval,

> On 19/09/2020 10:07, Lukasz Majewski wrote:
> > This is the helper function, which uses struct __timespec64
> > to provide 64 bit absolute time to futex syscalls.
> > 
> > The aim of this function is to move convoluted pre-processor
> > macro code from sysdeps/nptl/lowlevellock-futex.h to C
> > function in futex-internal.c
> > 
> > The futex_abstimed_wait64 function has been put into a separate
> > file on the purpose - to avoid issues apparent on the m68k
> > architecture related to small number of available registers (there
> > is not enough registers to put all necessary arguments in them if
> > the above function would be added to futex-internal.h with
> > __always_inline attribute).
> > 
> > Additional precautions for m68k port have been taken - the
> > futex-internal.c file will be compiled with -fno-inline flag.  
> 
> LGTM with a nit regarding style and a clarification about 
> '-fno-inline'.

Please find the explanation below.

> 
> > 
> > ---
> > Changes for v2:
> > - Handle the case when *abstime pointer is NULL
> > ---
> >  sysdeps/nptl/futex-internal.c         | 70
> > +++++++++++++++++++++++++++ sysdeps/nptl/futex-internal.h         |
> >  6 +++ sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
> >  3 files changed, 78 insertions(+)
> > 
> > diff --git a/sysdeps/nptl/futex-internal.c
> > b/sysdeps/nptl/futex-internal.c index 3366aac162..3211b4c94f 100644
> > --- a/sysdeps/nptl/futex-internal.c
> > +++ b/sysdeps/nptl/futex-internal.c
> > @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned
> > int* futex_word, abstime != NULL ? &ts32 : NULL,
> >                                    NULL /* Unused.  */,
> > FUTEX_BITSET_MATCH_ANY); }
> > +
> > +static int
> > +__futex_abstimed_wait32 (unsigned int* futex_word,
> > +                         unsigned int expected, clockid_t clockid,
> > +                         const struct __timespec64* abstime,
> > +                         int private)
> > +{
> > +  struct timespec ts32;
> > +
> > +  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> > +    return -EOVERFLOW;
> > +
> > +  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
> > +    FUTEX_CLOCK_REALTIME : 0;
> > +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> > private); +
> > +  if (abstime != NULL)
> > +    ts32 = valid_timespec64_to_timespec (*abstime);
> > +
> > +  return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
> > +                                abstime != NULL ? &ts32 : NULL,
> > +                                NULL /* Unused.  */,
> > FUTEX_BITSET_MATCH_ANY); +}
> >  #endif
> >  
> >  int  
> 
> Ok.
> 
> > @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned
> > int* futex_word, futex_fatal_error ();
> >      }
> >  }
> > +
> > +int
> > +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> > expected,
> > +                         clockid_t clockid,
> > +                         const struct __timespec64* abstime, int
> > private) +{
> > +  unsigned int clockbit;
> > +  int err;
> > +
> > +  /* Work around the fact that the kernel rejects negative timeout
> > values
> > +     despite them being valid.  */
> > +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec <
> > 0)))
> > +    return ETIMEDOUT;
> > +
> > +  if (! lll_futex_supported_clockid(clockid))
> > +    return EINVAL;  
> 
> Space before parentheses. 

Ok. Fixed.

> 
> > +
> > +  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME :
> > 0;
> > +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> > private); +
> > +  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> > expected,
> > +                               abstime, NULL /* Unused.  */,
> > +                               FUTEX_BITSET_MATCH_ANY);
> > +#ifndef __ASSUME_TIME64_SYSCALLS
> > +  if (err == -ENOSYS)
> > +    err = __futex_abstimed_wait32 (futex_word, expected,
> > +                                   clockid, abstime, private);
> > +#endif
> > +  switch (err)
> > +    {
> > +    case 0:
> > +    case -EAGAIN:
> > +    case -EINTR:
> > +    case -ETIMEDOUT:
> > +      return -err;
> > +
> > +    case -EFAULT: /* Must have been caused by a glibc or
> > application bug.  */
> > +    case -EINVAL: /* Either due to wrong alignment, unsupported
> > +		     clockid 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 ();
> > +    }
> > +}  
> 
> Ok.
> 
> > diff --git a/sysdeps/nptl/futex-internal.h
> > b/sysdeps/nptl/futex-internal.h index 7f3910ad98..1ba0d61938 100644
> > --- a/sysdeps/nptl/futex-internal.h
> > +++ b/sysdeps/nptl/futex-internal.h
> > @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned
> > int* futex_word, const struct __timespec64* abstime,
> >                                      int private) attribute_hidden;
> >  
> > +int
> > +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> > expected,
> > +                         clockid_t clockid,
> > +                         const struct __timespec64* abstime,
> > +                         int private) attribute_hidden;
> > +
> >  #endif  /* futex-internal.h */  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile
> > b/sysdeps/unix/sysv/linux/m68k/Makefile index
> > be40fae68a..65164c5752 100644 ---
> > a/sysdeps/unix/sysv/linux/m68k/Makefile +++
> > b/sysdeps/unix/sysv/linux/m68k/Makefile @@ -21,3 +21,5 @@
> > sysdep-dl-routines += dl-static sysdep-others += lddlibc4
> >  install-bin += lddlibc4
> >  endif
> > +
> > +CFLAGS-futex-internal.c += -fno-inline
> >   
> 
> Do we still need it?

Unfortunately, yes. The m68k architecture has the issue with properly
compiling this code (in futex-internal.c) as it has very limited number
of registers (8 x 32 bit IIRC).

I did some investigation and it looks like static inline
in_time_t_range() function affects the compiler capabilities to
generate correct code.

It can be easily inlined on any architecture but m68k.

As m68k has issues with this code compilation - it is IMHO better to
disable inlining (-fno-inline) on this particular port.

As a result we would have slower execution only on relevant functions,
but 64 bit time support would be provided. 


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 --]

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Andreas Schwab]

On Sep 30 2020, Lukasz Majewski wrote:

> Unfortunately, yes. The m68k architecture has the issue with properly
> compiling this code (in futex-internal.c) as it has very limited number
> of registers (8 x 32 bit IIRC).

The m68k has 16 registers, which is plenty (x86 has only 8).

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 15965 bytes --]

Hi Adhemerval,

> On 19/09/2020 10:07, Lukasz Majewski wrote:
> > The pthread_rwlock_clockrdlock, pthread_rwlock_clockwrlock,
> > pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock have been
> > converted to support 64 bit time.
> > 
> > This change uses new futex_abstimed_wait64 function in
> > ./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where
> > possible.
> > 
> > The pthread_rwlock_{clock|timed}{rd|wr}lock only accepts absolute
> > time. Moreover, there is no need to check for NULL passed as
> > *abstime pointer to the syscalls as those calls have exported
> > symbols marked with __nonull attribute for abstime.
> > 
> > For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
> > - Conversions between 64 bit time to 32 bit are necessary
> > - Redirection to pthread_rwlock_{clock|timed}{rd|wr}lock will
> > provide support for 64 bit time
> > 
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> > 
> > Run-time tests:
> > - Run specific tests on ARM/x86 32bit systems (qemu):
> >   https://github.com/lmajewski/meta-y2038 and run tests:
> >   https://github.com/lmajewski/y2038-tests/commits/master
> > 
> > Above tests were performed with Y2038 redirection applied as well
> > as without to test the proper usage of both
> > __pthread_rwlock_{clock|timed}{rd|wr}lock64 and
> > __pthread_rwlock_{clock|timed}{rd|wr}lock.
> > 
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>  
> 
> Looks good, there is only a issue regarding LIBC_PROBE (below).
> 
> 
> > 
> > ---
> > Changes for v2:
> > - None
> > ---
> >  nptl/pthreadP.h                   | 18 +++++++++++++++++
> >  nptl/pthread_rwlock_clockrdlock.c | 19 +++++++++++++++---
> >  nptl/pthread_rwlock_clockwrlock.c | 19 +++++++++++++++---
> >  nptl/pthread_rwlock_common.c      | 33
> > +++++++++++++++---------------- nptl/pthread_rwlock_rdlock.c      |
> >  2 +- nptl/pthread_rwlock_timedrdlock.c | 19 +++++++++++++++---
> >  nptl/pthread_rwlock_timedwrlock.c | 19 +++++++++++++++---
> >  nptl/pthread_rwlock_wrlock.c      |  2 +-
> >  8 files changed, 100 insertions(+), 31 deletions(-)
> > 
> > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> > index 9bb44c8535..5bcc8a2db5 100644
> > --- a/nptl/pthreadP.h
> > +++ b/nptl/pthreadP.h
> > @@ -464,6 +464,10 @@ extern int __pthread_cond_wait (pthread_cond_t
> > *cond, pthread_mutex_t *mutex); # define __pthread_timedjoin_np64
> > __pthread_timedjoin_np # define __pthread_cond_timedwait64
> > __pthread_cond_timedwait # define __pthread_cond_clockwait64
> > __pthread_cond_clockwait +# 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_clockjoin_np64 (pthread_t threadid, void **thread_return,
> > clockid_t clockid, @@ -481,6 +485,20 @@ extern int
> > __pthread_cond_clockwait64 (pthread_cond_t *cond, clockid_t clockid,
> >                                         const struct __timespec64
> > *abstime); libpthread_hidden_proto (__pthread_cond_clockwait64)
> > +extern int __pthread_rwlock_clockrdlock64 (pthread_rwlock_t
> > *rwlock,
> > +                                           clockid_t clockid,
> > +                                           const struct
> > __timespec64 *abstime); +libpthread_hidden_proto
> > (__pthread_rwlock_clockrdlock64) +extern int
> > __pthread_rwlock_clockwrlock64 (pthread_rwlock_t *rwlock,
> > +                                           clockid_t clockid,
> > +                                           const struct
> > __timespec64 *abstime); +libpthread_hidden_proto
> > (__pthread_rwlock_clockwrlock64) +extern int
> > __pthread_rwlock_timedrdlock64 (pthread_rwlock_t *rwlock,
> > +                                           const struct
> > __timespec64 *abstime); +libpthread_hidden_proto
> > (__pthread_rwlock_timedrdlock64) +extern int
> > __pthread_rwlock_timedwrlock64 (pthread_rwlock_t *rwlock,
> > +                                           const struct
> > __timespec64 *abstime); +libpthread_hidden_proto
> > (__pthread_rwlock_timedwrlock64) #endif
> >  
> >  extern int __pthread_cond_timedwait (pthread_cond_t *cond,  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_rwlock_clockrdlock.c
> > b/nptl/pthread_rwlock_clockrdlock.c index 4cedfd1dcd..d93b133c9e
> > 100644 --- a/nptl/pthread_rwlock_clockrdlock.c
> > +++ b/nptl/pthread_rwlock_clockrdlock.c
> > @@ -21,8 +21,21 @@
> >  
> >  /* 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);
> > +  return __pthread_rwlock_rdlock_full64 (rwlock, clockid, abstime);
> >  }
> > +
> > +#if __TIMESIZE != 64
> > +libpthread_hidden_def (__pthread_rwlock_clockrdlock64)  
> 
> Maybe an extra space here?

Ok. Fixed.

> 
> > +int
> > +__pthread_rwlock_clockrdlock (pthread_rwlock_t *rwlock, clockid_t
> > clockid,
> > +                              const struct timespec *abstime)
> > +{
> > +  struct __timespec64 ts64 = valid_timespec_to_timespec64
> > (*abstime); +
> > +  return __pthread_rwlock_clockrdlock64 (rwlock, clockid, &ts64);
> > +}
> > +#endif
> > +weak_alias (__pthread_rwlock_clockrdlock,
> > pthread_rwlock_clockrdlock)  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_rwlock_clockwrlock.c
> > b/nptl/pthread_rwlock_clockwrlock.c index 7a954cf529..10a314c76d
> > 100644 --- a/nptl/pthread_rwlock_clockwrlock.c
> > +++ b/nptl/pthread_rwlock_clockwrlock.c
> > @@ -21,8 +21,21 @@
> >  
> >  /* 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);
> > +  return __pthread_rwlock_wrlock_full64 (rwlock, clockid, abstime);
> >  }
> > +  
> 
> Ok.
> 
> > +#if __TIMESIZE != 64
> > +libpthread_hidden_def (__pthread_rwlock_clockwrlock64)  
> 
> Maybe an extra space here?

Ok. Fixed.

> 
> > +int
> > +__pthread_rwlock_clockwrlock (pthread_rwlock_t *rwlock, clockid_t
> > clockid,
> > +                              const struct timespec *abstime)
> > +{
> > +  struct __timespec64 ts64 = valid_timespec_to_timespec64
> > (*abstime); +
> > +  return __pthread_rwlock_clockwrlock64 (rwlock, clockid, &ts64);
> > +}
> > +#endif
> > +weak_alias (__pthread_rwlock_clockwrlock,
> > pthread_rwlock_clockwrlock)  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_rwlock_common.c
> > b/nptl/pthread_rwlock_common.c index 3fbc66ded2..4c9f582d3d 100644
> > --- a/nptl/pthread_rwlock_common.c
> > +++ b/nptl/pthread_rwlock_common.c
> > @@ -278,9 +278,8 @@ __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)
> > +__pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock,
> > clockid_t clockid,
> > +                                const struct __timespec64 *abstime)
> >  {
> >    unsigned int r;
> >    
> 
> Ok.
> 
> > @@ -330,8 +329,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t
> > *rwlock, & PTHREAD_RWLOCK_RWAITING) != 0)
> >  		{
> >  		  int private = __pthread_rwlock_get_private
> > (rwlock);
> > -		  int err = futex_abstimed_wait
> > (&rwlock->__data.__readers,
> > -						 r, clockid,
> > abstime, private);
> > +		  int err = __futex_abstimed_wait64
> > (&rwlock->__data.__readers,
> > +		                                     r, clockid,
> > abstime,
> > +		                                     private);
> >  		  /* We ignore EAGAIN and EINTR.  On time-outs, we
> > can just return because we don't need to clean up anything.  */
> >  		  if (err == ETIMEDOUT)  
> 
> Ok.
> 
> > @@ -457,9 +457,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t
> > *rwlock, (&rwlock->__data.__wrphase_futex,
> >  		   &wpf, wpf | PTHREAD_RWLOCK_FUTEX_USED)))
> >  	    continue;
> > -	  int err = futex_abstimed_wait
> > (&rwlock->__data.__wrphase_futex,
> > -					 1 |
> > PTHREAD_RWLOCK_FUTEX_USED,
> > -					 clockid, abstime,
> > private);
> > +	  int err = __futex_abstimed_wait64
> > (&rwlock->__data.__wrphase_futex,
> > +					     1 |
> > PTHREAD_RWLOCK_FUTEX_USED,
> > +					     clockid, abstime,
> > private); if (err == ETIMEDOUT)
> >  	    {
> >  	      /* If we timed out, we need to unregister.  If no
> > read phase  
> 
> Ok.
> 
> > @@ -585,9 +585,8 @@ __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)
> > +__pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock,
> > clockid_t clockid,
> > +                                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 @@ -728,9 +727,9 @@ __pthread_rwlock_wrlock_full
> > (pthread_rwlock_t *rwlock, share the flag, and another writer will
> > wake one of the writers in this group.  */
> >  	  may_share_futex_used_flag = true;
> > -	  int err = futex_abstimed_wait
> > (&rwlock->__data.__writers_futex,
> > -					 1 |
> > PTHREAD_RWLOCK_FUTEX_USED,
> > -					 clockid, abstime,
> > private);
> > +	  int err = __futex_abstimed_wait64
> > (&rwlock->__data.__writers_futex,
> > +					     1 |
> > PTHREAD_RWLOCK_FUTEX_USED,
> > +					     clockid, abstime,
> > private); if (err == ETIMEDOUT)
> >  	    {
> >  	      if (prefer_writer)  
> 
> Ok.
> 
> > @@ -827,9 +826,9 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t
> > *rwlock, (&rwlock->__data.__wrphase_futex, &wpf,
> >  		   PTHREAD_RWLOCK_FUTEX_USED)))
> >  	    continue;
> > -	  int err = futex_abstimed_wait
> > (&rwlock->__data.__wrphase_futex,
> > -					 PTHREAD_RWLOCK_FUTEX_USED,
> > -					 clockid, abstime,
> > private);
> > +	  int err = __futex_abstimed_wait64
> > (&rwlock->__data.__wrphase_futex,
> > +
> > PTHREAD_RWLOCK_FUTEX_USED,
> > +					     clockid, abstime,
> > private); if (err == ETIMEDOUT)
> >  	    {
> >  	      if (rwlock->__data.__flags !=
> > PTHREAD_RWLOCK_PREFER_READER_NP)  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_rwlock_rdlock.c
> > b/nptl/pthread_rwlock_rdlock.c index 04cecad395..2b8509a49c 100644
> > --- a/nptl/pthread_rwlock_rdlock.c
> > +++ b/nptl/pthread_rwlock_rdlock.c
> > @@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t *rwlock)
> >  {
> >    LIBC_PROBE (rdlock_entry, 1, rwlock);  
> 
> We need to move the systemtap probe to the 64-bit variant, so the
> 64-bit time will still trigger it.

Those are syscalls, which don't require timespec time.

The LIBC_PROBE() macros were there before, so I left them as is and
only changed __pthread_rwlock_rdlock_full() to
__pthread_rwlock_rdlock_full64().


On the other hand - for example:
__pthread_rwlock_clockwrlock() and __pthread_rwlock_clockwrlock64()
are only calling __pthread_rwlock_wrlock_full64(), and there were no
LIBC_PROBE() macros before.

Do I understand correctly that you propose to add LIBC_PROBE() macros
to e.g. __pthread_rwlock_clockwrlock64() ?

> 
> >  
> > -  int result = __pthread_rwlock_rdlock_full (rwlock,
> > CLOCK_REALTIME, NULL);
> > +  int result = __pthread_rwlock_rdlock_full64 (rwlock,
> > CLOCK_REALTIME, NULL); LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
> >    return result;
> >  }  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_rwlock_timedrdlock.c
> > b/nptl/pthread_rwlock_timedrdlock.c index c5d8aee909..d5999a1419
> > 100644 --- a/nptl/pthread_rwlock_timedrdlock.c
> > +++ b/nptl/pthread_rwlock_timedrdlock.c
> > @@ -20,8 +20,21 @@
> >  
> >  /* 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);
> > +  return __pthread_rwlock_rdlock_full64 (rwlock, CLOCK_REALTIME,
> > abstime); }
> > +  
> 
> Ok.
> 
> > +#if __TIMESIZE != 64
> > +libpthread_hidden_def (__pthread_rwlock_timedrdlock64)  
> 
> Maybe an extra space here?

Ok. Fixed.

> 
> > +int
> > +__pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock,
> > +                              const struct timespec *abstime)
> > +{
> > +  struct __timespec64 ts64 = valid_timespec_to_timespec64
> > (*abstime); +
> > +  return __pthread_rwlock_timedrdlock64 (rwlock, &ts64);
> > +}
> > +#endif
> > +weak_alias (__pthread_rwlock_timedrdlock,
> > pthread_rwlock_timedrdlock)  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_rwlock_timedwrlock.c
> > b/nptl/pthread_rwlock_timedwrlock.c index ccee8b77d9..6c5284a172
> > 100644 --- a/nptl/pthread_rwlock_timedwrlock.c
> > +++ b/nptl/pthread_rwlock_timedwrlock.c
> > @@ -20,8 +20,21 @@
> >  
> >  /* 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);
> > +  return __pthread_rwlock_wrlock_full64 (rwlock, CLOCK_REALTIME,
> > abstime); }
> > +
> > +#if __TIMESIZE != 64
> > +libpthread_hidden_def (__pthread_rwlock_timedwrlock64)  
> 
> 
> Maybe an extra space here?

Ok. Fixed.

> 
> > +int
> > +__pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock,
> > +                              const struct timespec *abstime)
> > +{
> > +  struct __timespec64 ts64 = valid_timespec_to_timespec64
> > (*abstime); +
> > +  return __pthread_rwlock_timedwrlock64 (rwlock, &ts64);
> > +}
> > +#endif
> > +weak_alias (__pthread_rwlock_timedwrlock,
> > pthread_rwlock_timedwrlock)  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_rwlock_wrlock.c
> > b/nptl/pthread_rwlock_wrlock.c index a42aa626f0..210e6cffdc 100644
> > --- a/nptl/pthread_rwlock_wrlock.c
> > +++ b/nptl/pthread_rwlock_wrlock.c
> > @@ -24,7 +24,7 @@ __pthread_rwlock_wrlock (pthread_rwlock_t *rwlock)
> >  {
> >    LIBC_PROBE (wrlock_entry, 1, rwlock);  
> 
> We need to move the systemtap probe to the 64-bit variant, so the
> 64-bit time will still trigger it.

The same comment as above apply here.

> 
> >  
> > -  int result = __pthread_rwlock_wrlock_full (rwlock,
> > CLOCK_REALTIME, NULL);
> > +  int result = __pthread_rwlock_wrlock_full64 (rwlock,
> > CLOCK_REALTIME, NULL); LIBC_PROBE (wrlock_acquire_write, 1, rwlock);
> >    return result;
> >  }
> >   
> 
> Ok.


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 --]

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Adhemerval Zanella]

On 30/09/2020 09:47, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 19/09/2020 10:07, Lukasz Majewski wrote:
>>> This is the helper function, which uses struct __timespec64
>>> to provide 64 bit absolute time to futex syscalls.
>>>
>>> The aim of this function is to move convoluted pre-processor
>>> macro code from sysdeps/nptl/lowlevellock-futex.h to C
>>> function in futex-internal.c
>>>
>>> The futex_abstimed_wait64 function has been put into a separate
>>> file on the purpose - to avoid issues apparent on the m68k
>>> architecture related to small number of available registers (there
>>> is not enough registers to put all necessary arguments in them if
>>> the above function would be added to futex-internal.h with
>>> __always_inline attribute).
>>>
>>> Additional precautions for m68k port have been taken - the
>>> futex-internal.c file will be compiled with -fno-inline flag.  
>>
>> LGTM with a nit regarding style and a clarification about 
>> '-fno-inline'.
> 
> Please find the explanation below.
> 
>>
>>>
>>> ---
>>> Changes for v2:
>>> - Handle the case when *abstime pointer is NULL
>>> ---
>>>  sysdeps/nptl/futex-internal.c         | 70
>>> +++++++++++++++++++++++++++ sysdeps/nptl/futex-internal.h         |
>>>  6 +++ sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
>>>  3 files changed, 78 insertions(+)
>>>
>>> diff --git a/sysdeps/nptl/futex-internal.c
>>> b/sysdeps/nptl/futex-internal.c index 3366aac162..3211b4c94f 100644
>>> --- a/sysdeps/nptl/futex-internal.c
>>> +++ b/sysdeps/nptl/futex-internal.c
>>> @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned
>>> int* futex_word, abstime != NULL ? &ts32 : NULL,
>>>                                    NULL /* Unused.  */,
>>> FUTEX_BITSET_MATCH_ANY); }
>>> +
>>> +static int
>>> +__futex_abstimed_wait32 (unsigned int* futex_word,
>>> +                         unsigned int expected, clockid_t clockid,
>>> +                         const struct __timespec64* abstime,
>>> +                         int private)
>>> +{
>>> +  struct timespec ts32;
>>> +
>>> +  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
>>> +    return -EOVERFLOW;
>>> +
>>> +  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
>>> +    FUTEX_CLOCK_REALTIME : 0;
>>> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
>>> private); +
>>> +  if (abstime != NULL)
>>> +    ts32 = valid_timespec64_to_timespec (*abstime);
>>> +
>>> +  return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
>>> +                                abstime != NULL ? &ts32 : NULL,
>>> +                                NULL /* Unused.  */,
>>> FUTEX_BITSET_MATCH_ANY); +}
>>>  #endif
>>>  
>>>  int  
>>
>> Ok.
>>
>>> @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned
>>> int* futex_word, futex_fatal_error ();
>>>      }
>>>  }
>>> +
>>> +int
>>> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
>>> expected,
>>> +                         clockid_t clockid,
>>> +                         const struct __timespec64* abstime, int
>>> private) +{
>>> +  unsigned int clockbit;
>>> +  int err;
>>> +
>>> +  /* Work around the fact that the kernel rejects negative timeout
>>> values
>>> +     despite them being valid.  */
>>> +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec <
>>> 0)))
>>> +    return ETIMEDOUT;
>>> +
>>> +  if (! lll_futex_supported_clockid(clockid))
>>> +    return EINVAL;  
>>
>> Space before parentheses. 
> 
> Ok. Fixed.
> 
>>
>>> +
>>> +  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME :
>>> 0;
>>> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
>>> private); +
>>> +  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
>>> expected,
>>> +                               abstime, NULL /* Unused.  */,
>>> +                               FUTEX_BITSET_MATCH_ANY);
>>> +#ifndef __ASSUME_TIME64_SYSCALLS
>>> +  if (err == -ENOSYS)
>>> +    err = __futex_abstimed_wait32 (futex_word, expected,
>>> +                                   clockid, abstime, private);
>>> +#endif
>>> +  switch (err)
>>> +    {
>>> +    case 0:
>>> +    case -EAGAIN:
>>> +    case -EINTR:
>>> +    case -ETIMEDOUT:
>>> +      return -err;
>>> +
>>> +    case -EFAULT: /* Must have been caused by a glibc or
>>> application bug.  */
>>> +    case -EINVAL: /* Either due to wrong alignment, unsupported
>>> +		     clockid 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 ();
>>> +    }
>>> +}  
>>
>> Ok.
>>
>>> diff --git a/sysdeps/nptl/futex-internal.h
>>> b/sysdeps/nptl/futex-internal.h index 7f3910ad98..1ba0d61938 100644
>>> --- a/sysdeps/nptl/futex-internal.h
>>> +++ b/sysdeps/nptl/futex-internal.h
>>> @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned
>>> int* futex_word, const struct __timespec64* abstime,
>>>                                      int private) attribute_hidden;
>>>  
>>> +int
>>> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
>>> expected,
>>> +                         clockid_t clockid,
>>> +                         const struct __timespec64* abstime,
>>> +                         int private) attribute_hidden;
>>> +
>>>  #endif  /* futex-internal.h */  
>>
>> Ok.
>>
>>> diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile
>>> b/sysdeps/unix/sysv/linux/m68k/Makefile index
>>> be40fae68a..65164c5752 100644 ---
>>> a/sysdeps/unix/sysv/linux/m68k/Makefile +++
>>> b/sysdeps/unix/sysv/linux/m68k/Makefile @@ -21,3 +21,5 @@
>>> sysdep-dl-routines += dl-static sysdep-others += lddlibc4
>>>  install-bin += lddlibc4
>>>  endif
>>> +
>>> +CFLAGS-futex-internal.c += -fno-inline
>>>   
>>
>> Do we still need it?
> 
> Unfortunately, yes. The m68k architecture has the issue with properly
> compiling this code (in futex-internal.c) as it has very limited number
> of registers (8 x 32 bit IIRC).
> 
> I did some investigation and it looks like static inline
> in_time_t_range() function affects the compiler capabilities to
> generate correct code.
> 
> It can be easily inlined on any architecture but m68k.
> 
> As m68k has issues with this code compilation - it is IMHO better to
> disable inlining (-fno-inline) on this particular port.
> 
> As a result we would have slower execution only on relevant functions,
> but 64 bit time support would be provided. 

I recall this was need on first patch for the cancellable 64-bit
futex_abstimed_wait where it embedded the 32-bit fallback in the 
__futex_abstimed_wait_cancelable64. And my suggestion to move it to 
an external function seems to avoid the extra compiler flag.

This patchset uses the same strategy and I checked both patches
and it seems that -fno-inline is not required to build m68k.

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

Hi Andreas,

> On Sep 30 2020, Lukasz Majewski wrote:
> 
> > Unfortunately, yes. The m68k architecture has the issue with
> > properly compiling this code (in futex-internal.c) as it has very
> > limited number of registers (8 x 32 bit IIRC).  
> 
> The m68k has 16 registers, which is plenty (x86 has only 8).

This is what I got from wiki (and no I'm not the expert with m68k, so
corrections are welcome):
https://en.wikipedia.org/wiki/Motorola_68000_series (8 x 32 bit data
registers)

And I cannot say why it is not able to generate the correct code...

The proposed flag (-fno-inline) seems to be the most pragmatic solution
(the code in question is in futex-interna.c and the flag only affects
this port).

> 
> Andreas.
> 




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 --]

Re: [PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time

[Adhemerval Zanella]

On 30/09/2020 10:12, Lukasz Majewski wrote:
>>> diff --git a/nptl/pthread_rwlock_rdlock.c
>>> b/nptl/pthread_rwlock_rdlock.c index 04cecad395..2b8509a49c 100644
>>> --- a/nptl/pthread_rwlock_rdlock.c
>>> +++ b/nptl/pthread_rwlock_rdlock.c
>>> @@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t *rwlock)
>>>  {
>>>    LIBC_PROBE (rdlock_entry, 1, rwlock);  
>>
>> We need to move the systemtap probe to the 64-bit variant, so the
>> 64-bit time will still trigger it.
> 
> Those are syscalls, which don't require timespec time.

These are in fact systemtap markers [1] and they are not related to 
syscalls.

> 
> The LIBC_PROBE() macros were there before, so I left them as is and
> only changed __pthread_rwlock_rdlock_full() to
> __pthread_rwlock_rdlock_full64().
> 
> 
> On the other hand - for example:
> __pthread_rwlock_clockwrlock() and __pthread_rwlock_clockwrlock64()
> are only calling __pthread_rwlock_wrlock_full64(), and there were no
> LIBC_PROBE() macros before.
> 
> Do I understand correctly that you propose to add LIBC_PROBE() macros
> to e.g. __pthread_rwlock_clockwrlock64() ?

The idea of the LIBC_PROBE is to trigger an systemtap even when the
function is called so systemtap can act accordingly (and the markers
are enable only if --enable-systemtap is used on configure).

Without moving the LIBC_PROBE, 64-bit time architectures won't see the 
probes anymore since they will call the 64-bit time version instead.

[1] https://sourceware.org/systemtap/wiki/glibcMarkers

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 8109 bytes --]

Hi Adhemerval,

> On 30/09/2020 09:47, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 19/09/2020 10:07, Lukasz Majewski wrote:  
> >>> This is the helper function, which uses struct __timespec64
> >>> to provide 64 bit absolute time to futex syscalls.
> >>>
> >>> The aim of this function is to move convoluted pre-processor
> >>> macro code from sysdeps/nptl/lowlevellock-futex.h to C
> >>> function in futex-internal.c
> >>>
> >>> The futex_abstimed_wait64 function has been put into a separate
> >>> file on the purpose - to avoid issues apparent on the m68k
> >>> architecture related to small number of available registers (there
> >>> is not enough registers to put all necessary arguments in them if
> >>> the above function would be added to futex-internal.h with
> >>> __always_inline attribute).
> >>>
> >>> Additional precautions for m68k port have been taken - the
> >>> futex-internal.c file will be compiled with -fno-inline flag.    
> >>
> >> LGTM with a nit regarding style and a clarification about 
> >> '-fno-inline'.  
> > 
> > Please find the explanation below.
> >   
> >>  
> >>>
> >>> ---
> >>> Changes for v2:
> >>> - Handle the case when *abstime pointer is NULL
> >>> ---
> >>>  sysdeps/nptl/futex-internal.c         | 70
> >>> +++++++++++++++++++++++++++ sysdeps/nptl/futex-internal.h
> >>> | 6 +++ sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
> >>>  3 files changed, 78 insertions(+)
> >>>
> >>> diff --git a/sysdeps/nptl/futex-internal.c
> >>> b/sysdeps/nptl/futex-internal.c index 3366aac162..3211b4c94f
> >>> 100644 --- a/sysdeps/nptl/futex-internal.c
> >>> +++ b/sysdeps/nptl/futex-internal.c
> >>> @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned
> >>> int* futex_word, abstime != NULL ? &ts32 : NULL,
> >>>                                    NULL /* Unused.  */,
> >>> FUTEX_BITSET_MATCH_ANY); }
> >>> +
> >>> +static int
> >>> +__futex_abstimed_wait32 (unsigned int* futex_word,
> >>> +                         unsigned int expected, clockid_t
> >>> clockid,
> >>> +                         const struct __timespec64* abstime,
> >>> +                         int private)
> >>> +{
> >>> +  struct timespec ts32;
> >>> +
> >>> +  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> >>> +    return -EOVERFLOW;
> >>> +
> >>> +  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
> >>> +    FUTEX_CLOCK_REALTIME : 0;
> >>> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> >>> private); +
> >>> +  if (abstime != NULL)
> >>> +    ts32 = valid_timespec64_to_timespec (*abstime);
> >>> +
> >>> +  return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
> >>> +                                abstime != NULL ? &ts32 : NULL,
> >>> +                                NULL /* Unused.  */,
> >>> FUTEX_BITSET_MATCH_ANY); +}
> >>>  #endif
> >>>  
> >>>  int    
> >>
> >> Ok.
> >>  
> >>> @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned
> >>> int* futex_word, futex_fatal_error ();
> >>>      }
> >>>  }
> >>> +
> >>> +int
> >>> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> >>> expected,
> >>> +                         clockid_t clockid,
> >>> +                         const struct __timespec64* abstime, int
> >>> private) +{
> >>> +  unsigned int clockbit;
> >>> +  int err;
> >>> +
> >>> +  /* Work around the fact that the kernel rejects negative
> >>> timeout values
> >>> +     despite them being valid.  */
> >>> +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec <
> >>> 0)))
> >>> +    return ETIMEDOUT;
> >>> +
> >>> +  if (! lll_futex_supported_clockid(clockid))
> >>> +    return EINVAL;    
> >>
> >> Space before parentheses.   
> > 
> > Ok. Fixed.
> >   
> >>  
> >>> +
> >>> +  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME :
> >>> 0;
> >>> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> >>> private); +
> >>> +  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> >>> expected,
> >>> +                               abstime, NULL /* Unused.  */,
> >>> +                               FUTEX_BITSET_MATCH_ANY);
> >>> +#ifndef __ASSUME_TIME64_SYSCALLS
> >>> +  if (err == -ENOSYS)
> >>> +    err = __futex_abstimed_wait32 (futex_word, expected,
> >>> +                                   clockid, abstime, private);
> >>> +#endif
> >>> +  switch (err)
> >>> +    {
> >>> +    case 0:
> >>> +    case -EAGAIN:
> >>> +    case -EINTR:
> >>> +    case -ETIMEDOUT:
> >>> +      return -err;
> >>> +
> >>> +    case -EFAULT: /* Must have been caused by a glibc or
> >>> application bug.  */
> >>> +    case -EINVAL: /* Either due to wrong alignment, unsupported
> >>> +		     clockid 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 ();
> >>> +    }
> >>> +}    
> >>
> >> Ok.
> >>  
> >>> diff --git a/sysdeps/nptl/futex-internal.h
> >>> b/sysdeps/nptl/futex-internal.h index 7f3910ad98..1ba0d61938
> >>> 100644 --- a/sysdeps/nptl/futex-internal.h
> >>> +++ b/sysdeps/nptl/futex-internal.h
> >>> @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned
> >>> int* futex_word, const struct __timespec64* abstime,
> >>>                                      int private)
> >>> attribute_hidden; 
> >>> +int
> >>> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> >>> expected,
> >>> +                         clockid_t clockid,
> >>> +                         const struct __timespec64* abstime,
> >>> +                         int private) attribute_hidden;
> >>> +
> >>>  #endif  /* futex-internal.h */    
> >>
> >> Ok.
> >>  
> >>> diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile
> >>> b/sysdeps/unix/sysv/linux/m68k/Makefile index
> >>> be40fae68a..65164c5752 100644 ---
> >>> a/sysdeps/unix/sysv/linux/m68k/Makefile +++
> >>> b/sysdeps/unix/sysv/linux/m68k/Makefile @@ -21,3 +21,5 @@
> >>> sysdep-dl-routines += dl-static sysdep-others += lddlibc4
> >>>  install-bin += lddlibc4
> >>>  endif
> >>> +
> >>> +CFLAGS-futex-internal.c += -fno-inline
> >>>     
> >>
> >> Do we still need it?  
> > 
> > Unfortunately, yes. The m68k architecture has the issue with
> > properly compiling this code (in futex-internal.c) as it has very
> > limited number of registers (8 x 32 bit IIRC).
> > 
> > I did some investigation and it looks like static inline
> > in_time_t_range() function affects the compiler capabilities to
> > generate correct code.
> > 
> > It can be easily inlined on any architecture but m68k.
> > 
> > As m68k has issues with this code compilation - it is IMHO better to
> > disable inlining (-fno-inline) on this particular port.
> > 
> > As a result we would have slower execution only on relevant
> > functions, but 64 bit time support would be provided.   
> 
> I recall this was need on first patch for the cancellable 64-bit
> futex_abstimed_wait where it embedded the 32-bit fallback in the 
> __futex_abstimed_wait_cancelable64. And my suggestion to move it to 
> an external function seems to avoid the extra compiler flag.
> 
> This patchset uses the same strategy and I checked both patches
> and it seems that -fno-inline is not required to build m68k.

As fair as I can tell (at the time I tested it) - it was necessary to
have -fno-inline when this patch was applied on top of the one which is
now in the -master branch (in futex-internal.c).

I will double check it now with:

glibc/glibc-many-build --keep failed glibcs


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 --]

Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 9247 bytes --]

On Wed, 30 Sep 2020 15:39:53 +0200
Lukasz Majewski <lukma@denx.de> wrote:

> Hi Adhemerval,
> 
> > On 30/09/2020 09:47, Lukasz Majewski wrote:  
> > > Hi Adhemerval,
> > >     
> > >> On 19/09/2020 10:07, Lukasz Majewski wrote:    
> > >>> This is the helper function, which uses struct __timespec64
> > >>> to provide 64 bit absolute time to futex syscalls.
> > >>>
> > >>> The aim of this function is to move convoluted pre-processor
> > >>> macro code from sysdeps/nptl/lowlevellock-futex.h to C
> > >>> function in futex-internal.c
> > >>>
> > >>> The futex_abstimed_wait64 function has been put into a separate
> > >>> file on the purpose - to avoid issues apparent on the m68k
> > >>> architecture related to small number of available registers
> > >>> (there is not enough registers to put all necessary arguments
> > >>> in them if the above function would be added to
> > >>> futex-internal.h with __always_inline attribute).
> > >>>
> > >>> Additional precautions for m68k port have been taken - the
> > >>> futex-internal.c file will be compiled with -fno-inline flag.
> > >>>    
> > >>
> > >> LGTM with a nit regarding style and a clarification about 
> > >> '-fno-inline'.    
> > > 
> > > Please find the explanation below.
> > >     
> > >>    
> > >>>
> > >>> ---
> > >>> Changes for v2:
> > >>> - Handle the case when *abstime pointer is NULL
> > >>> ---
> > >>>  sysdeps/nptl/futex-internal.c         | 70
> > >>> +++++++++++++++++++++++++++ sysdeps/nptl/futex-internal.h
> > >>> | 6 +++ sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
> > >>>  3 files changed, 78 insertions(+)
> > >>>
> > >>> diff --git a/sysdeps/nptl/futex-internal.c
> > >>> b/sysdeps/nptl/futex-internal.c index 3366aac162..3211b4c94f
> > >>> 100644 --- a/sysdeps/nptl/futex-internal.c
> > >>> +++ b/sysdeps/nptl/futex-internal.c
> > >>> @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned
> > >>> int* futex_word, abstime != NULL ? &ts32 : NULL,
> > >>>                                    NULL /* Unused.  */,
> > >>> FUTEX_BITSET_MATCH_ANY); }
> > >>> +
> > >>> +static int
> > >>> +__futex_abstimed_wait32 (unsigned int* futex_word,
> > >>> +                         unsigned int expected, clockid_t
> > >>> clockid,
> > >>> +                         const struct __timespec64* abstime,
> > >>> +                         int private)
> > >>> +{
> > >>> +  struct timespec ts32;
> > >>> +
> > >>> +  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> > >>> +    return -EOVERFLOW;
> > >>> +
> > >>> +  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
> > >>> +    FUTEX_CLOCK_REALTIME : 0;
> > >>> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> > >>> private); +
> > >>> +  if (abstime != NULL)
> > >>> +    ts32 = valid_timespec64_to_timespec (*abstime);
> > >>> +
> > >>> +  return INTERNAL_SYSCALL_CALL (futex, futex_word, op,
> > >>> expected,
> > >>> +                                abstime != NULL ? &ts32 : NULL,
> > >>> +                                NULL /* Unused.  */,
> > >>> FUTEX_BITSET_MATCH_ANY); +}
> > >>>  #endif
> > >>>  
> > >>>  int      
> > >>
> > >> Ok.
> > >>    
> > >>> @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned
> > >>> int* futex_word, futex_fatal_error ();
> > >>>      }
> > >>>  }
> > >>> +
> > >>> +int
> > >>> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> > >>> expected,
> > >>> +                         clockid_t clockid,
> > >>> +                         const struct __timespec64* abstime,
> > >>> int private) +{
> > >>> +  unsigned int clockbit;
> > >>> +  int err;
> > >>> +
> > >>> +  /* Work around the fact that the kernel rejects negative
> > >>> timeout values
> > >>> +     despite them being valid.  */
> > >>> +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec <
> > >>> 0)))
> > >>> +    return ETIMEDOUT;
> > >>> +
> > >>> +  if (! lll_futex_supported_clockid(clockid))
> > >>> +    return EINVAL;      
> > >>
> > >> Space before parentheses.     
> > > 
> > > Ok. Fixed.
> > >     
> > >>    
> > >>> +
> > >>> +  clockbit = (clockid == CLOCK_REALTIME) ?
> > >>> FUTEX_CLOCK_REALTIME : 0;
> > >>> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> > >>> private); +
> > >>> +  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> > >>> expected,
> > >>> +                               abstime, NULL /* Unused.  */,
> > >>> +                               FUTEX_BITSET_MATCH_ANY);
> > >>> +#ifndef __ASSUME_TIME64_SYSCALLS
> > >>> +  if (err == -ENOSYS)
> > >>> +    err = __futex_abstimed_wait32 (futex_word, expected,
> > >>> +                                   clockid, abstime, private);
> > >>> +#endif
> > >>> +  switch (err)
> > >>> +    {
> > >>> +    case 0:
> > >>> +    case -EAGAIN:
> > >>> +    case -EINTR:
> > >>> +    case -ETIMEDOUT:
> > >>> +      return -err;
> > >>> +
> > >>> +    case -EFAULT: /* Must have been caused by a glibc or
> > >>> application bug.  */
> > >>> +    case -EINVAL: /* Either due to wrong alignment, unsupported
> > >>> +		     clockid 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 ();
> > >>> +    }
> > >>> +}      
> > >>
> > >> Ok.
> > >>    
> > >>> diff --git a/sysdeps/nptl/futex-internal.h
> > >>> b/sysdeps/nptl/futex-internal.h index 7f3910ad98..1ba0d61938
> > >>> 100644 --- a/sysdeps/nptl/futex-internal.h
> > >>> +++ b/sysdeps/nptl/futex-internal.h
> > >>> @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64
> > >>> (unsigned int* futex_word, const struct __timespec64* abstime,
> > >>>                                      int private)
> > >>> attribute_hidden; 
> > >>> +int
> > >>> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> > >>> expected,
> > >>> +                         clockid_t clockid,
> > >>> +                         const struct __timespec64* abstime,
> > >>> +                         int private) attribute_hidden;
> > >>> +
> > >>>  #endif  /* futex-internal.h */      
> > >>
> > >> Ok.
> > >>    
> > >>> diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile
> > >>> b/sysdeps/unix/sysv/linux/m68k/Makefile index
> > >>> be40fae68a..65164c5752 100644 ---
> > >>> a/sysdeps/unix/sysv/linux/m68k/Makefile +++
> > >>> b/sysdeps/unix/sysv/linux/m68k/Makefile @@ -21,3 +21,5 @@
> > >>> sysdep-dl-routines += dl-static sysdep-others += lddlibc4
> > >>>  install-bin += lddlibc4
> > >>>  endif
> > >>> +
> > >>> +CFLAGS-futex-internal.c += -fno-inline
> > >>>       
> > >>
> > >> Do we still need it?    
> > > 
> > > Unfortunately, yes. The m68k architecture has the issue with
> > > properly compiling this code (in futex-internal.c) as it has very
> > > limited number of registers (8 x 32 bit IIRC).
> > > 
> > > I did some investigation and it looks like static inline
> > > in_time_t_range() function affects the compiler capabilities to
> > > generate correct code.
> > > 
> > > It can be easily inlined on any architecture but m68k.
> > > 
> > > As m68k has issues with this code compilation - it is IMHO better
> > > to disable inlining (-fno-inline) on this particular port.
> > > 
> > > As a result we would have slower execution only on relevant
> > > functions, but 64 bit time support would be provided.     
> > 
> > I recall this was need on first patch for the cancellable 64-bit
> > futex_abstimed_wait where it embedded the 32-bit fallback in the 
> > __futex_abstimed_wait_cancelable64. And my suggestion to move it to 
> > an external function seems to avoid the extra compiler flag.
> > 
> > This patchset uses the same strategy and I checked both patches
> > and it seems that -fno-inline is not required to build m68k.  
> 
> As fair as I can tell (at the time I tested it) - it was necessary to
> have -fno-inline when this patch was applied on top of the one which
> is now in the -master branch (in futex-internal.c).
> 
> I will double check it now with:
> 
> glibc/glibc-many-build --keep failed glibcs

Indeed with current -master the issue is gone:

../src/scripts/build-many-glibcs.py
/home/lukma/work/glibc/glibc-many-build --keep failed glibcs
m68k-linux-gnu m68k-linux-gnu-coldfire m68k-linux-gnu-coldfire-soft
x86_64-linux-gnu

Is all OK with the -fno-inline removed.... Strange.

> 
> 
> 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 --]

Re: [PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

Hi Adhemerval,

> On 30/09/2020 10:12, Lukasz Majewski wrote:
> >>> diff --git a/nptl/pthread_rwlock_rdlock.c
> >>> b/nptl/pthread_rwlock_rdlock.c index 04cecad395..2b8509a49c 100644
> >>> --- a/nptl/pthread_rwlock_rdlock.c
> >>> +++ b/nptl/pthread_rwlock_rdlock.c
> >>> @@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t
> >>> *rwlock) {
> >>>    LIBC_PROBE (rdlock_entry, 1, rwlock);    
> >>
> >> We need to move the systemtap probe to the 64-bit variant, so the
> >> 64-bit time will still trigger it.  
> > 
> > Those are syscalls, which don't require timespec time.  
> 
> These are in fact systemtap markers [1] and they are not related to 
> syscalls.

Ok. Thanks for the info (and link).

> 
> > 
> > The LIBC_PROBE() macros were there before, so I left them as is and
> > only changed __pthread_rwlock_rdlock_full() to
> > __pthread_rwlock_rdlock_full64().
> > 
> > 
> > On the other hand - for example:
> > __pthread_rwlock_clockwrlock() and __pthread_rwlock_clockwrlock64()
> > are only calling __pthread_rwlock_wrlock_full64(), and there were no
> > LIBC_PROBE() macros before.
> > 
> > Do I understand correctly that you propose to add LIBC_PROBE()
> > macros to e.g. __pthread_rwlock_clockwrlock64() ?  
> 
> The idea of the LIBC_PROBE is to trigger an systemtap even when the
> function is called so systemtap can act accordingly (and the markers
> are enable only if --enable-systemtap is used on configure).
> 

Ok.

> Without moving the LIBC_PROBE, 64-bit time architectures won't see
> the probes anymore since they will call the 64-bit time version
> instead.

Please correct me if I'm wrong, but it seems to me that you are
concerned if 

- __pthread_rwlock_rdlock_full64 
- __pthread_rwlock_wrlock_full64

would be called as aliased (for e.g. x86-64) and redirected (for e.g.
arm)?

This is not the case as above functions are local (i.e. helpers) ones
defined in ./nptl/pthread_rwlock_common.c (and are not exported).

For them, the only change was to add '64' suffix.

The LIBC_PROBE() calls have been left untouched, surrounding them, so I
think that we don't need to move them.


Or maybe I do miss something important here? Thanks in advance for your
help and explanation.

> 
> [1] https://sourceware.org/systemtap/wiki/glibcMarkers




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 --]

Re: [PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time

[Adhemerval Zanella]

On 02/10/2020 09:15, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 30/09/2020 10:12, Lukasz Majewski wrote:
>>>>> diff --git a/nptl/pthread_rwlock_rdlock.c
>>>>> b/nptl/pthread_rwlock_rdlock.c index 04cecad395..2b8509a49c 100644
>>>>> --- a/nptl/pthread_rwlock_rdlock.c
>>>>> +++ b/nptl/pthread_rwlock_rdlock.c
>>>>> @@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t
>>>>> *rwlock) {
>>>>>    LIBC_PROBE (rdlock_entry, 1, rwlock);    
>>>>
>>>> We need to move the systemtap probe to the 64-bit variant, so the
>>>> 64-bit time will still trigger it.  
>>>
>>> Those are syscalls, which don't require timespec time.  
>>
>> These are in fact systemtap markers [1] and they are not related to 
>> syscalls.
> 
> Ok. Thanks for the info (and link).
> 
>>
>>>
>>> The LIBC_PROBE() macros were there before, so I left them as is and
>>> only changed __pthread_rwlock_rdlock_full() to
>>> __pthread_rwlock_rdlock_full64().
>>>
>>>
>>> On the other hand - for example:
>>> __pthread_rwlock_clockwrlock() and __pthread_rwlock_clockwrlock64()
>>> are only calling __pthread_rwlock_wrlock_full64(), and there were no
>>> LIBC_PROBE() macros before.
>>>
>>> Do I understand correctly that you propose to add LIBC_PROBE()
>>> macros to e.g. __pthread_rwlock_clockwrlock64() ?  
>>
>> The idea of the LIBC_PROBE is to trigger an systemtap even when the
>> function is called so systemtap can act accordingly (and the markers
>> are enable only if --enable-systemtap is used on configure).
>>
> 
> Ok.
> 
>> Without moving the LIBC_PROBE, 64-bit time architectures won't see
>> the probes anymore since they will call the 64-bit time version
>> instead.
> 
> Please correct me if I'm wrong, but it seems to me that you are
> concerned if 
> 
> - __pthread_rwlock_rdlock_full64 
> - __pthread_rwlock_wrlock_full64
> 
> would be called as aliased (for e.g. x86-64) and redirected (for e.g.
> arm)?
> 
> This is not the case as above functions are local (i.e. helpers) ones
> defined in ./nptl/pthread_rwlock_common.c (and are not exported).
> 
> For them, the only change was to add '64' suffix.
> 
> The LIBC_PROBE() calls have been left untouched, surrounding them, so I
> think that we don't need to move them.
> 
> 
> Or maybe I do miss something important here? Thanks in advance for your
> help and explanation.

In fact this is a wrong assumption from my part, there is no LIBC_PROBE
on the timed variant rwlocks. The patch looks ok in this regard (sorry
for the noise).