[v5] y2038: Introduce __ASSUME_TIME64_SYSCALLS define

Message ID 20190515142723.20182-1-lukma@denx.de
State New
Headers show
Series
  • [v5] y2038: Introduce __ASSUME_TIME64_SYSCALLS define
Related show

Commit Message

Lukasz Majewski May 15, 2019, 2:27 p.m.
This define indicates if the Linux kernel (5.1+) provides syscalls supporting
64 bit versions of struct timespec and timeval.

For architectures with __WORDSIZE==64 and __TIMESIZE==64 (e.g. x86_64, aarch64)
this flag is never defined (as those already use 64 bit versions of struct
timespec and timeval).

The __ASSUME_TIME64_SYSCALLS shall be only defined on systems with
__WORDSIZE==32.

For x32 this flag is explicitly undefined as this architecture has
__WORDSIZE==32 with __TIMESIZE==64. Despite having __WORDSIZE==32 the x32
has support for 64 bit time values and hence needs to undefine
__ASSUME_TIME64_SYSCALLS flag.


* sysdeps/unix/sysv/linux/kernel-features.h: (__ASSUME_TIME64_SYSCALLS):
[__LINUX_KERNEL_VERSION >= 0x050100]: Define.
* sysdeps/unix/sysv/linux/x86_64/kernel-features.h (__ASSUME_TIME64_SYSCALLS):
#undef the __ASSUME_TIME64_SYSCALLS for x32 architecture

---
Changes for v5:
- Rewrite the in-code comment (x32 description more precise)
- Change patch description (for x32)

Changes for v4:
- Exclude this patch from the clock_settime64 patch series
- Rewrite the in-code comment
- Change patch description

Changes for v3:
- Provide more detailed and verbose description
- Change name to __ASSUME_TIME64_SYSCALLS
- Undefine __ASSUME_TIME64_SYSCALLS on x32

Changes for v2:
- New patch
---
 sysdeps/unix/sysv/linux/kernel-features.h        | 38 ++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/x86_64/kernel-features.h |  7 +++++
 2 files changed, 45 insertions(+)

-- 
2.11.0

Comments

Lukasz Majewski May 21, 2019, 3:15 p.m. | #1
Dear All,

> This define indicates if the Linux kernel (5.1+) provides syscalls

> supporting 64 bit versions of struct timespec and timeval.

> 

> For architectures with __WORDSIZE==64 and __TIMESIZE==64 (e.g.

> x86_64, aarch64) this flag is never defined (as those already use 64

> bit versions of struct timespec and timeval).

> 

> The __ASSUME_TIME64_SYSCALLS shall be only defined on systems with

> __WORDSIZE==32.

> 

> For x32 this flag is explicitly undefined as this architecture has

> __WORDSIZE==32 with __TIMESIZE==64. Despite having __WORDSIZE==32 the

> x32 has support for 64 bit time values and hence needs to undefine

> __ASSUME_TIME64_SYSCALLS flag.

> 

> 

> * sysdeps/unix/sysv/linux/kernel-features.h:

> (__ASSUME_TIME64_SYSCALLS): [__LINUX_KERNEL_VERSION >= 0x050100]:

> Define.

> * sysdeps/unix/sysv/linux/x86_64/kernel-features.h

> (__ASSUME_TIME64_SYSCALLS): #undef the __ASSUME_TIME64_SYSCALLS for

> x32 architecture


Gentle ping on this patch. Do you have any more comments?

> 

> ---

> Changes for v5:

> - Rewrite the in-code comment (x32 description more precise)

> - Change patch description (for x32)

> 

> Changes for v4:

> - Exclude this patch from the clock_settime64 patch series

> - Rewrite the in-code comment

> - Change patch description

> 

> Changes for v3:

> - Provide more detailed and verbose description

> - Change name to __ASSUME_TIME64_SYSCALLS

> - Undefine __ASSUME_TIME64_SYSCALLS on x32

> 

> Changes for v2:

> - New patch

> ---

>  sysdeps/unix/sysv/linux/kernel-features.h        | 38

> ++++++++++++++++++++++++

> sysdeps/unix/sysv/linux/x86_64/kernel-features.h |  7 +++++ 2 files

> changed, 45 insertions(+)

> 

> diff --git a/sysdeps/unix/sysv/linux/kernel-features.h

> b/sysdeps/unix/sysv/linux/kernel-features.h index

> bc5c959f58..30e77dd213 100644 ---

> a/sysdeps/unix/sysv/linux/kernel-features.h +++

> b/sysdeps/unix/sysv/linux/kernel-features.h @@ -143,3 +143,41 @@

>     */

>  

>  #define __ASSUME_CLONE_DEFAULT 1

> +

> +#include <bits/wordsize.h>

> +#if __WORDSIZE != 64

> +/* Support for Linux kernel syscalls, which are able to handle 64 bit

> +   time on 32 bit systems (with 'long' and __WORDSIZE equal to 32

> bits). +

> +   Linux kernel, as of version 5.1, provides following set of

> syscalls,

> +   which accept data based on struct timespec and timeval with 64 bit

> +   tv_sec:

> +

> +   clock_gettime64

> +   clock_settime64

> +   clock_adjtime64

> +   clock_getres_time64

> +   clock_nanosleep_time64

> +   timer_gettime64

> +   timer_settime64

> +   timerfd_gettime64

> +   timerfd_settime64

> +   utimensat_time64

> +   pselect6_time64

> +   ppoll_time64

> +   io_pgetevents_time64

> +   recvmmsg_time64

> +   mq_timedsend_time64

> +   mq_timedreceive_time64

> +   semtimedop_time64

> +   rt_sigtimedwait_time64

> +   futex_time64

> +   sched_rr_get_interval_time64

> +

> +   Above syscalls are supposed to replace legacy ones, which handle

> 32

> +   bit version of struct timespec and timeval (i.e. without the '64'

> +   suffix).  */

> +# if __LINUX_KERNEL_VERSION >= 0x050100

> +#  define __ASSUME_TIME64_SYSCALLS 1

> +# endif

> +#endif

> diff --git a/sysdeps/unix/sysv/linux/x86_64/kernel-features.h

> b/sysdeps/unix/sysv/linux/x86_64/kernel-features.h index

> 26280f57ec..179a9ae932 100644 ---

> a/sysdeps/unix/sysv/linux/x86_64/kernel-features.h +++

> b/sysdeps/unix/sysv/linux/x86_64/kernel-features.h @@ -24,3 +24,10 @@

>  #endif

>  

>  #include_next <kernel-features.h>

> +

> +/* For x32, which is a special case in respect to 64 bit time support

> +   (it has __WORDSIZE==32 but __TIMESIZE==64), the

> +   __ASSUME_TIME64_SYSCALLS flag needs to be explicitly undefined.

> */ +#ifdef __ILP32__

> +# undef __ASSUME_TIME64_SYSCALLS

> +#endif





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
Stepan Golosunov May 23, 2019, 7:34 a.m. | #2
15.05.2019 в 16:27:23 +0200 Lukasz Majewski написал:
> This define indicates if the Linux kernel (5.1+) provides syscalls supporting

> 64 bit versions of struct timespec and timeval.

> 

> For architectures with __WORDSIZE==64 and __TIMESIZE==64 (e.g. x86_64, aarch64)

> this flag is never defined (as those already use 64 bit versions of struct

> timespec and timeval).

> 

> The __ASSUME_TIME64_SYSCALLS shall be only defined on systems with

> __WORDSIZE==32.

> 

> For x32 this flag is explicitly undefined as this architecture has

> __WORDSIZE==32 with __TIMESIZE==64. Despite having __WORDSIZE==32 the x32

> has support for 64 bit time values and hence needs to undefine

> __ASSUME_TIME64_SYSCALLS flag.


What is not clear is how architectures where syscalls like
clock_settime are already using 64-bit time_t are supposed to be
identified.  Last patch for clock_settime seems to be using

#if __WORDSIZE != 32 || !defined __NR_clock_settime64 && defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64

to select code for these architectures.

This seems too complicated and potentially buggy.  Why not just
define __ASSUME_TIME64_SYSCALLS for this case too and then use

#if defined __ASSUME_TIME64_SYSCALLS && !defined __NR_clock_settime64

?

Especially given that in most cases the only difference in resulting
code with __ASSUME_TIME64_SYSCALLS defined would be the name of the
constant used (__NR_clock_settime64 when it's defined,
__NR_clock_settime otherwise).
Lukasz Majewski May 23, 2019, 9:35 a.m. | #3
Hi Stepan,

> 15.05.2019 в 16:27:23 +0200 Lukasz Majewski написал:

> > This define indicates if the Linux kernel (5.1+) provides syscalls

> > supporting 64 bit versions of struct timespec and timeval.

> > 

> > For architectures with __WORDSIZE==64 and __TIMESIZE==64 (e.g.

> > x86_64, aarch64) this flag is never defined (as those already use

> > 64 bit versions of struct timespec and timeval).

> > 

> > The __ASSUME_TIME64_SYSCALLS shall be only defined on systems with

> > __WORDSIZE==32.

> > 

> > For x32 this flag is explicitly undefined as this architecture has

> > __WORDSIZE==32 with __TIMESIZE==64. Despite having __WORDSIZE==32

> > the x32 has support for 64 bit time values and hence needs to

> > undefine __ASSUME_TIME64_SYSCALLS flag.  

> 

> What is not clear is how architectures where syscalls like

> clock_settime are already using 64-bit time_t are supposed to be

> identified.  Last patch for clock_settime seems to be using

> 

> #if __WORDSIZE != 32 || !defined __NR_clock_settime64 && defined

> __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64

> 


The check has been taken from:
sysdeps/unix/sysv/linux/bits/statvfs.h (line 24).


Considering your above comment - we would need to introduce two
flags:

1. New, glibc global - __ARCH_HAVE_TIME64_SYSCALLS (other names
possible: __ARCH_SUPPORT_TIME64_SYSCALLS, __ARCH_TIME64_SUPPORT)

#if (__WORDSIZE == 32 \
     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
#define __ARCH_HAVE_TIME64_SYSCALLS
#endif

2. __ASSUME_TIME64_SYSCALLS as it is in this patch (to indicate
kernel support after 5.1+).


The __clock_settime64() pseudo code:

...
tv_nsec check
...

#if __ARCH_HAVE_TIME64_SYSCALLS
# ifdef __NR_clock_settime64
  int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp);
#  ifdef __ASSUME_TIME64_SYSCALLS
  return ret;
#  else
  if (ret == 0 || errno != ENOSYS)
    return ret;
#  endif
# endif
  if (! in_time_t_range (tp->tv_sec))
    {
      __set_errno (EOVERFLOW);
      return -1;
    }  
  struct timespec ts32;
  valid_timespec64_to_timespec (tp, &ts32);
  return INLINE_SYSCALL_CALL (clock_settime, clock_id, &ts32);
#else
  /* x32, x86_64 */
  return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp);
#endif

> to select code for these architectures.

> 

> This seems too complicated and potentially buggy.  Why not just

> define __ASSUME_TIME64_SYSCALLS for this case too and then use

> 

> #if defined __ASSUME_TIME64_SYSCALLS && !defined __NR_clock_settime64

> 


As fair as I understood the __ASSUME_TIME64_SYSCALLS was supposed to
indicate in-kernel support for syscalls (similar to __ASSUME_STATX) -
which would result in simpler execution paths. 

> ?

> 

> Especially given that in most cases the only difference in resulting

> code with __ASSUME_TIME64_SYSCALLS defined would be the name of the

> constant used (__NR_clock_settime64 when it's defined,

> __NR_clock_settime otherwise).


And also the case where __clock_settime64() is called from
__clock_settime().


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Joseph Myers May 23, 2019, 9:17 p.m. | #4
On Thu, 23 May 2019, Stepan Golosunov wrote:

> This seems too complicated and potentially buggy.  Why not just

> define __ASSUME_TIME64_SYSCALLS for this case too and then use


There are multiple reasonable choices that could be made for the semantics 
of __ASSUME_TIME64_SYSCALLS.

One is the choice in this patch series at present, that it relates to the 
new syscalls *with their given names*.  Another possible choice would be 
the one you suggest, that it relates to *any syscalls whose semantics 
match those of the new syscalls, possibly under different names*.

If we use the latter choice, I think it would be best also to have 
#defines of the new to the old names (such as #define __NR_clock_settime64 
__NR_clock_settime) so the various places that use 
__ASSUME_TIME64_SYSCALLS don't all need their own conditionals.

Also, if we use the latter choice, we need to be very careful about 
ensuring the old syscalls really do have the right semantics, and really 
do exist on all kernels with at least the configured minimum version.  For 
example, we must not claim that semtimedop_time64 is available simply 
because a platform's syscall interface always had 64-bit time, if there 
wasn't an equivalent older syscall, or the equivalent older syscall had 
different semantics or was introduced more recently than the minimum 
kernel version for that glibc build.  This might well mean defining the 
macro to relate to a smaller set of syscalls for which this property can 
be more readily verified.

-- 
Joseph S. Myers
joseph@codesourcery.com

Patch

diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
index bc5c959f58..30e77dd213 100644
--- a/sysdeps/unix/sysv/linux/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/kernel-features.h
@@ -143,3 +143,41 @@ 
    */
 
 #define __ASSUME_CLONE_DEFAULT 1
+
+#include <bits/wordsize.h>
+#if __WORDSIZE != 64
+/* Support for Linux kernel syscalls, which are able to handle 64 bit
+   time on 32 bit systems (with 'long' and __WORDSIZE equal to 32 bits).
+
+   Linux kernel, as of version 5.1, provides following set of syscalls,
+   which accept data based on struct timespec and timeval with 64 bit
+   tv_sec:
+
+   clock_gettime64
+   clock_settime64
+   clock_adjtime64
+   clock_getres_time64
+   clock_nanosleep_time64
+   timer_gettime64
+   timer_settime64
+   timerfd_gettime64
+   timerfd_settime64
+   utimensat_time64
+   pselect6_time64
+   ppoll_time64
+   io_pgetevents_time64
+   recvmmsg_time64
+   mq_timedsend_time64
+   mq_timedreceive_time64
+   semtimedop_time64
+   rt_sigtimedwait_time64
+   futex_time64
+   sched_rr_get_interval_time64
+
+   Above syscalls are supposed to replace legacy ones, which handle 32
+   bit version of struct timespec and timeval (i.e. without the '64'
+   suffix).  */
+# if __LINUX_KERNEL_VERSION >= 0x050100
+#  define __ASSUME_TIME64_SYSCALLS 1
+# endif
+#endif
diff --git a/sysdeps/unix/sysv/linux/x86_64/kernel-features.h b/sysdeps/unix/sysv/linux/x86_64/kernel-features.h
index 26280f57ec..179a9ae932 100644
--- a/sysdeps/unix/sysv/linux/x86_64/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/x86_64/kernel-features.h
@@ -24,3 +24,10 @@ 
 #endif
 
 #include_next <kernel-features.h>
+
+/* For x32, which is a special case in respect to 64 bit time support
+   (it has __WORDSIZE==32 but __TIMESIZE==64), the
+   __ASSUME_TIME64_SYSCALLS flag needs to be explicitly undefined.  */
+#ifdef __ILP32__
+# undef __ASSUME_TIME64_SYSCALLS
+#endif