public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: Joseph Myers <joseph@codesourcery.com>,
	Paul Eggert <eggert@cs.ucla.edu>,
	Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Alistair Francis <alistair23@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Alistair Francis <alistair.francis@wdc.com>,
	GNU C Library <libc-alpha@sourceware.org>,
	Florian Weimer <fweimer@redhat.com>,
	Carlos O'Donell <carlos@redhat.com>,
	Stepan Golosunov <stepan@golosunov.pp.ru>,
	Andreas Schwab <schwab@suse.de>, Zack Weinberg <zackw@panix.com>,
	Jeff Law <law@redhat.com>, Lukasz Majewski <lukma@denx.de>
Subject: [PATCH] [RFC] y2038: nptl: Convert pthread_cond_{clock|timed}wait to support 64 bit time
Date: Mon, 24 Aug 2020 18:40:00 +0200	[thread overview]
Message-ID: <20200824164000.1619-1-lukma@denx.de> (raw)

The pthread_cond_clockwait and pthread_cond_timedwait have been converted
to support 64 bit time.

This change introduces new futex_abstimed_wait_cancelable64 function in
./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where possible
and tries to replace low-level preprocessor macros from
lowlevellock-futex.h
The pthread_cond_{clock|timed}wait only accepts absolute time. Moreover,
there is no need to check for NULL passed as *abstime pointer as
__pthread_cond_wait_common() always passes non-NULL struct __timespec64
pointer to futex_abstimed_wait_cancellable64().

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

The futex_abstimed_wait_cancelable64 function has been put into a separate
file on the purpose - to avoid issues apparent on m68k architecture related
to small number of available registers (there is not enough registers to
put all necessary arguments in them when inlining).
In fact - the futex_helper.c is reused, but extra flag "-fno-inline" is set
when it is build for this architecture. Such approach fixes the build issue.


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_cond_{clock|timed}wait64 and
__pthread_cond_{clock|timed}wait.
---
 nptl/pthreadP.h                              | 11 +++
 nptl/pthread_cond_wait.c                     | 46 ++++++++--
 sysdeps/nptl/Makefile                        |  2 +-
 sysdeps/nptl/futex-helpers.c                 | 89 ++++++++++++++++++++
 sysdeps/nptl/futex-internal.h                | 11 +++
 sysdeps/unix/sysv/linux/m68k/Makefile        |  5 ++
 sysdeps/unix/sysv/linux/m68k/futex-helpers.c | 19 +++++
 7 files changed, 173 insertions(+), 10 deletions(-)
 create mode 100644 sysdeps/nptl/futex-helpers.c
 create mode 100644 sysdeps/unix/sysv/linux/m68k/futex-helpers.c

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 99713c8447..e288c7e778 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -462,6 +462,8 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
 #if __TIMESIZE == 64
 # define __pthread_clockjoin_np64 __pthread_clockjoin_np
 # define __pthread_timedjoin_np64 __pthread_timedjoin_np
+# define __pthread_cond_timedwait64 __pthread_cond_timedwait
+# define __pthread_cond_clockwait64 __pthread_cond_clockwait
 #else
 extern int __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
                                      clockid_t clockid,
@@ -470,6 +472,15 @@ libc_hidden_proto (__pthread_clockjoin_np64)
 extern int __pthread_timedjoin_np64 (pthread_t threadid, void **thread_return,
                                      const struct __timespec64 *abstime);
 libc_hidden_proto (__pthread_timedjoin_np64)
+extern int __pthread_cond_timedwait64 (pthread_cond_t *cond,
+                                       pthread_mutex_t *mutex,
+                                       const struct __timespec64 *abstime);
+libc_hidden_proto (__pthread_cond_timedwait64)
+extern int __pthread_cond_clockwait64 (pthread_cond_t *cond,
+                                       pthread_mutex_t *mutex,
+                                       clockid_t clockid,
+                                       const struct __timespec64 *abstime);
+libc_hidden_proto (__pthread_cond_clockwait64)
 #endif
 
 extern int __pthread_cond_timedwait (pthread_cond_t *cond,
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 85ddbc1011..560b30129b 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -31,6 +31,7 @@
 #include <stap-probe.h>
 #include <time.h>
 
+
 #include "pthread_cond_common.c"
 
 
@@ -378,8 +379,7 @@ __condvar_cleanup_waiting (void *arg)
 */
 static __always_inline int
 __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
-    clockid_t clockid,
-    const struct timespec *abstime)
+    clockid_t clockid, const struct __timespec64 *abstime)
 {
   const int maxspin = 0;
   int err;
@@ -517,7 +517,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
 	        err = ETIMEDOUT;
 	      else
 		{
-		  err = futex_abstimed_wait_cancelable
+		  err = futex_abstimed_wait_cancelable64
                     (cond->__data.__g_signals + g, 0, clockid, abstime,
                      private);
 		}
@@ -640,8 +640,8 @@ __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex)
 
 /* See __pthread_cond_wait_common.  */
 int
-__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
-    const struct timespec *abstime)
+__pthread_cond_timedwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex,
+                            const struct __timespec64 *abstime)
 {
   /* Check parameter validity.  This should also tell the compiler that
      it can assume that abstime is not NULL.  */
@@ -655,6 +655,20 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
                     ? CLOCK_MONOTONIC : CLOCK_REALTIME;
   return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
 }
+
+#if __TIMESIZE != 64
+libc_hidden_def (__pthread_cond_timedwait64)
+
+int
+__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
+                          const struct timespec *abstime)
+{
+  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
+
+  return __pthread_cond_timedwait64 (cond, mutex, &ts64);
+}
+#endif
+
 versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait,
 		  GLIBC_2_3_2);
 versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,
@@ -662,18 +676,32 @@ versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,
 
 /* See __pthread_cond_wait_common.  */
 int
-__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
-			  clockid_t clockid,
-			  const struct timespec *abstime)
+__pthread_cond_clockwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex,
+                            clockid_t clockid,
+                            const struct __timespec64 *abstime)
 {
   /* Check parameter validity.  This should also tell the compiler that
      it can assume that abstime is not NULL.  */
   if (! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
-  if (!futex_abstimed_supported_clockid (clockid))
+  if (! futex_abstimed_supported_clockid (clockid))
     return EINVAL;
 
   return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
 }
+
+#if __TIMESIZE != 64
+libc_hidden_def (__pthread_cond_clockwait64)
+
+int
+__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
+                          clockid_t clockid,
+                          const struct timespec *abstime)
+{
+  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
+
+  return __pthread_cond_clockwait64 (cond, mutex, clockid, &ts64);
+}
+#endif
 weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait);
diff --git a/sysdeps/nptl/Makefile b/sysdeps/nptl/Makefile
index 0631a870c8..6d649a94a2 100644
--- a/sysdeps/nptl/Makefile
+++ b/sysdeps/nptl/Makefile
@@ -17,7 +17,7 @@
 # <https://www.gnu.org/licenses/>.
 
 ifeq ($(subdir),nptl)
-libpthread-sysdep_routines += errno-loc
+libpthread-sysdep_routines += errno-loc futex-helpers
 endif
 
 ifeq ($(subdir),rt)
diff --git a/sysdeps/nptl/futex-helpers.c b/sysdeps/nptl/futex-helpers.c
new file mode 100644
index 0000000000..dfd8870d35
--- /dev/null
+++ b/sysdeps/nptl/futex-helpers.c
@@ -0,0 +1,89 @@
+/* futex helper functions for glibc-internal use.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <sysdep.h>
+#include <time.h>
+#include <futex-internal.h>
+#include <kernel-features.h>
+
+int
+futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
+                                  unsigned int expected, clockid_t clockid,
+                                  const struct __timespec64* abstime,
+                                  int private)
+{
+  unsigned int clockbit;
+  int oldtype, err, op;
+
+  /* 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;
+
+  oldtype = __pthread_enable_asynccancel ();
+  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
+  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)
+  {
+    struct timespec ts32;
+      if (in_time_t_range (abstime->tv_sec))
+        {
+          ts32 = valid_timespec64_to_timespec (*abstime);
+
+          err = INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
+                                       &ts32, NULL /* Unused.  */,
+                                       FUTEX_BITSET_MATCH_ANY);
+        }
+      else
+        err = -EOVERFLOW;
+  }
+#endif
+  __pthread_disable_asynccancel (oldtype);
+
+  switch (err)
+    {
+    case 0:
+    case -EAGAIN:
+    case -EINTR:
+    case -ETIMEDOUT:
+    case -EOVERFLOW:  /* Passed absolute timeout uses 64 bit time_t type, but
+                         underlying kernel does not support 64 bit time_t futex
+                         syscalls.  */
+      return -err;
+
+    case -EFAULT: /* Must have been caused by a glibc or application bug.  */
+    case -EINVAL: /* Either due to wrong alignment or due to the timeout not
+		     being normalized.  Must have been caused by a glibc or
+		     application bug.  */
+    case -ENOSYS: /* Must have been caused by a glibc bug.  */
+    /* No other errors are documented at this time.  */
+    default:
+      futex_fatal_error ();
+    }
+}
+
+libpthread_hidden_def (futex_abstimed_wait_cancelable64)
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index 159aae82dc..aea01e5bcd 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -519,4 +519,15 @@ futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
       futex_fatal_error ();
     }
 }
+
+/* The futex_abstimed_wait_cancelable64 has been moved to a separate file
+   to avoid problems with exhausting available registers on some architectures
+   - e.g. on m68k architecture.  */
+int
+futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
+                                  unsigned int expected, clockid_t clockid,
+                                  const struct __timespec64* abstime,
+                                  int private);
+libpthread_hidden_proto (futex_abstimed_wait_cancelable64)
+
 #endif  /* futex-internal.h */
diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile b/sysdeps/unix/sysv/linux/m68k/Makefile
index be40fae68a..f19c8c825d 100644
--- a/sysdeps/unix/sysv/linux/m68k/Makefile
+++ b/sysdeps/unix/sysv/linux/m68k/Makefile
@@ -21,3 +21,8 @@ sysdep-dl-routines += dl-static
 sysdep-others += lddlibc4
 install-bin += lddlibc4
 endif
+
+ifeq ($(subdir),nptl)
+libpthread-sysdep_routines += futex-helpers
+CFLAGS-futex-helpers.c += -fno-inline
+endif
diff --git a/sysdeps/unix/sysv/linux/m68k/futex-helpers.c b/sysdeps/unix/sysv/linux/m68k/futex-helpers.c
new file mode 100644
index 0000000000..fb03b5d174
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/m68k/futex-helpers.c
@@ -0,0 +1,19 @@
+/* futex helper functions for glibc-internal use for m68k architecture
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sysdeps/nptl/futex-helpers.c>
-- 
2.20.1


             reply	other threads:[~2020-08-24 16:40 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-24 16:40 Lukasz Majewski [this message]
2020-08-24 16:59 ` Lukasz Majewski
2020-08-24 17:15   ` Florian Weimer
2020-08-24 17:50     ` Lukasz Majewski
2020-08-25  8:08 ` Andreas Schwab
2020-08-25 10:03   ` Lukasz Majewski
2020-08-25 10:34     ` Andreas Schwab
2020-08-25 14:29       ` Adhemerval Zanella
2020-08-25 14:30       ` Lukasz Majewski
2020-08-25 14:47         ` Andreas Schwab
2020-08-25 14:53           ` Adhemerval Zanella
2020-08-25 15:26             ` Andreas Schwab
2020-08-25 15:31               ` Adhemerval Zanella
2020-08-25 15:36                 ` Adhemerval Zanella
2020-08-25 14:43 ` Adhemerval Zanella
2020-08-26 17:59   ` Lukasz Majewski
2020-08-26 18:06     ` Florian Weimer
2020-08-26 19:38     ` Adhemerval Zanella
2020-08-27  6:20       ` Lukasz Majewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200824164000.1619-1-lukma@denx.de \
    --to=lukma@denx.de \
    --cc=adhemerval.zanella@linaro.org \
    --cc=alistair.francis@wdc.com \
    --cc=alistair23@gmail.com \
    --cc=arnd@arndb.de \
    --cc=carlos@redhat.com \
    --cc=eggert@cs.ucla.edu \
    --cc=fweimer@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=law@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=schwab@suse.de \
    --cc=stepan@golosunov.pp.ru \
    --cc=zackw@panix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).