[RFC,v1,01/16] sysdeps/nanosleep: Use clock_nanosleep_time64 instead of nanosleep

Message ID cf7ecd7aa844bf65b1f1426f8a2153c78ee72281.1561177967.git.alistair.francis@wdc.com
State New, archived
Headers

Commit Message

Alistair Francis June 22, 2019, 4:37 a.m. UTC
  The nanosleep syscall is not supported on newer 32-bit platforms (such
as RV32). To fix this issue let's change to using clock_nanosleep_time64
instead.

Let's use CLOCK_REALTIME when calling clock_nanosleep_time64 as the
Linux specification says:
  "POSIX.1 specifies that nanosleep() should measure time against the
   CLOCK_REALTIME clock. However, Linux measures the time using the
   CLOCK_MONOTONIC clock. This probably does not matter, since the POSIX.1
   specification for clock_settime(2) says that discontinuous changes in
   CLOCK_REALTIME should not affect nanosleep()"

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 ChangeLog                                    | 6 ++++++
 nptl/thrd_sleep.c                            | 3 ++-
 sysdeps/unix/sysv/linux/nanosleep.c          | 3 ++-
 sysdeps/unix/sysv/linux/nanosleep_nocancel.c | 3 ++-
 4 files changed, 12 insertions(+), 3 deletions(-)
  

Comments

Florian Weimer June 23, 2019, 6:34 a.m. UTC | #1
* Alistair Francis:

> +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
> +
> +	* nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
> +	* sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> +	* sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.

Sorry, how is this supposed to work for all the other architectures
which don't have this system call with current kernel versions?

Thanks,
Florian
  
Alistair Francis June 23, 2019, 3:12 p.m. UTC | #2
On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Alistair Francis:
>
> > +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
> > +
> > +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
> > +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> > +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
>
> Sorry, how is this supposed to work for all the other architectures
> which don't have this system call with current kernel versions?

Good question, I'm sure it doesn't.

If we put this around #if defined(clock_nanosleep_time64) or #ifndef
clock_nanosleep and then keep the current code as the #else would that
be acceptable to upstream?

The main goal of this RFC series was to get feedback if this is in the
right direction, or maybe I'm just missing something much simpler.

Alistair

>
> Thanks,
> Florian
  
Florian Weimer June 23, 2019, 4:48 p.m. UTC | #3
* Alistair Francis:

> On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Alistair Francis:
>>
>> > +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
>> > +
>> > +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
>> > +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
>> > +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
>>
>> Sorry, how is this supposed to work for all the other architectures
>> which don't have this system call with current kernel versions?
>
> Good question, I'm sure it doesn't.
>
> If we put this around #if defined(clock_nanosleep_time64) or #ifndef
> clock_nanosleep and then keep the current code as the #else would that
> be acceptable to upstream?

Does this work in <sysdep.h> for the port?

#define __NR_clock_nanosleep  __NR_clock_nanosleep_time64

This is somewhat hackish, but it's reasonably accurate for architectures
that provide standard system calls under non-standard names for some
reason.

Obviously, this assume that for these new ports, the UAPI headers do the
right thing and use 64-bit time_t for the old structs as well.

Thanks,
Florian
  
Alistair Francis June 23, 2019, 5 p.m. UTC | #4
On Sun, Jun 23, 2019 at 9:48 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Alistair Francis:
>
> > On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * Alistair Francis:
> >>
> >> > +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
> >> > +
> >> > +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
> >> > +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> >> > +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
> >>
> >> Sorry, how is this supposed to work for all the other architectures
> >> which don't have this system call with current kernel versions?
> >
> > Good question, I'm sure it doesn't.
> >
> > If we put this around #if defined(clock_nanosleep_time64) or #ifndef
> > clock_nanosleep and then keep the current code as the #else would that
> > be acceptable to upstream?
>
> Does this work in <sysdep.h> for the port?
>
> #define __NR_clock_nanosleep  __NR_clock_nanosleep_time64

I'm at home today so I can't check, but I suspect that will work.

>
> This is somewhat hackish, but it's reasonably accurate for architectures
> that provide standard system calls under non-standard names for some
> reason.

For the RV32 case I think this will provide a workable solution. I am
under the impression though that all 32-bit architectures are
eventually removing __ARCH_WANT_TIME32_SYSCALLS from Linux so they
will also hit this issue. Which is why I was going for a more generic
approach. In saying that, whatever gets the RV32 port in works for me
:)

>
> Obviously, this assume that for these new ports, the UAPI headers do the
> right thing and use 64-bit time_t for the old structs as well.

AFAIK they should be, at least in this case.

Alistair

>
> Thanks,
> Florian
  
Arnd Bergmann June 23, 2019, 7:34 p.m. UTC | #5
On Sun, Jun 23, 2019 at 7:00 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Sun, Jun 23, 2019 at 9:48 AM Florian Weimer <fweimer@redhat.com> wrote:
> > * Alistair Francis:
> > > On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
> > >>
> > >> * Alistair Francis:
> > >>
> > >> > +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
> > >> > +
> > >> > +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
> > >> > +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> > >> > +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
> > >>
> > >> Sorry, how is this supposed to work for all the other architectures
> > >> which don't have this system call with current kernel versions?
> > >
> > > Good question, I'm sure it doesn't.
> > >
> > > If we put this around #if defined(clock_nanosleep_time64) or #ifndef
> > > clock_nanosleep and then keep the current code as the #else would that
> > > be acceptable to upstream?
> >
> > Does this work in <sysdep.h> for the port?
> >
> > #define __NR_clock_nanosleep  __NR_clock_nanosleep_time64
>
> I'm at home today so I can't check, but I suspect that will work.
>
> >
> > This is somewhat hackish, but it's reasonably accurate for architectures
> > that provide standard system calls under non-standard names for some
> > reason.
>
> For the RV32 case I think this will provide a workable solution. I am
> under the impression though that all 32-bit architectures are
> eventually removing __ARCH_WANT_TIME32_SYSCALLS from Linux so they
> will also hit this issue. Which is why I was going for a more generic
> approach. In saying that, whatever gets the RV32 port in works for me
> :)

Generally speaking we never remove system calls from the kernel, rv32
was the exception to this rule because there was no upstream C library
port. __ARCH_WANT_TIME32_SYSCALLS will probably hang around for
shortly before year 2038 -- afterwards there is no point any more as all
binaries relying on it will be broken then.

We will however allow building kernels that omit the time32 syscalls
before then, as they are only adding baggage to the kernel size and
make it harder to verify that user space doesn't try calling them.

I think for internal libc functions, you need to try both functions:
first the time64 version for kernels that leave out the 32-bit one,
and then the time32 version for running on pre-5.1 kernels.

For user-visible glibc interfaces (e.g. nanosleep()), user space with
a 64-bit time_t will have to try the time64 syscall first and then fall
back on the time32 version for pre-5.1 kernels, while user space
with 32-bit time_t should only call the old time32 syscall and fail
on any kernel that doesn't support that, either because
__ARCH_WANT_TIME32_SYSCALLS is not set at compile
time, or because the kernel was configured without these.

     Arnd
  
Alistair Francis June 23, 2019, 11:42 p.m. UTC | #6
On Sun, Jun 23, 2019 at 12:35 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sun, Jun 23, 2019 at 7:00 PM Alistair Francis <alistair23@gmail.com> wrote:
> > On Sun, Jun 23, 2019 at 9:48 AM Florian Weimer <fweimer@redhat.com> wrote:
> > > * Alistair Francis:
> > > > On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
> > > >>
> > > >> * Alistair Francis:
> > > >>
> > > >> > +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
> > > >> > +
> > > >> > +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
> > > >> > +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> > > >> > +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
> > > >>
> > > >> Sorry, how is this supposed to work for all the other architectures
> > > >> which don't have this system call with current kernel versions?
> > > >
> > > > Good question, I'm sure it doesn't.
> > > >
> > > > If we put this around #if defined(clock_nanosleep_time64) or #ifndef
> > > > clock_nanosleep and then keep the current code as the #else would that
> > > > be acceptable to upstream?
> > >
> > > Does this work in <sysdep.h> for the port?
> > >
> > > #define __NR_clock_nanosleep  __NR_clock_nanosleep_time64
> >
> > I'm at home today so I can't check, but I suspect that will work.
> >
> > >
> > > This is somewhat hackish, but it's reasonably accurate for architectures
> > > that provide standard system calls under non-standard names for some
> > > reason.
> >
> > For the RV32 case I think this will provide a workable solution. I am
> > under the impression though that all 32-bit architectures are
> > eventually removing __ARCH_WANT_TIME32_SYSCALLS from Linux so they
> > will also hit this issue. Which is why I was going for a more generic
> > approach. In saying that, whatever gets the RV32 port in works for me
> > :)
>
> Generally speaking we never remove system calls from the kernel, rv32
> was the exception to this rule because there was no upstream C library
> port. __ARCH_WANT_TIME32_SYSCALLS will probably hang around for
> shortly before year 2038 -- afterwards there is no point any more as all
> binaries relying on it will be broken then.
>
> We will however allow building kernels that omit the time32 syscalls
> before then, as they are only adding baggage to the kernel size and
> make it harder to verify that user space doesn't try calling them.
>
> I think for internal libc functions, you need to try both functions:
> first the time64 version for kernels that leave out the 32-bit one,
> and then the time32 version for running on pre-5.1 kernels.

Ok, so if I re-jig the series to #ifdef the 64-bit syscall first and
then fall back to the standard (32-bit) syscall that will be the
correct solution?


>
> For user-visible glibc interfaces (e.g. nanosleep()), user space with
> a 64-bit time_t will have to try the time64 syscall first and then fall
> back on the time32 version for pre-5.1 kernels, while user space
> with 32-bit time_t should only call the old time32 syscall and fail
> on any kernel that doesn't support that, either because
> __ARCH_WANT_TIME32_SYSCALLS is not set at compile
> time, or because the kernel was configured without these.

Ok, so this seems to just be changing the #ifdef logic to something like this:

#ifdef time_t == 64
# ifdef __NR_syscall64
    __NR_syscall64()
# else
    __NR_syscall()
# endif
#else /* time_t == 64*/
    __NR_syscall()
#endif

Although that just simplifies to:

# ifdef __NR_syscall64
    __NR_syscall64()
# else
    __NR_syscall()
# endif

Alistair

>
>      Arnd
  
Adhemerval Zanella June 24, 2019, 11:52 a.m. UTC | #7
On 23/06/2019 20:42, Alistair Francis wrote:
> On Sun, Jun 23, 2019 at 12:35 PM Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> On Sun, Jun 23, 2019 at 7:00 PM Alistair Francis <alistair23@gmail.com> wrote:
>>> On Sun, Jun 23, 2019 at 9:48 AM Florian Weimer <fweimer@redhat.com> wrote:
>>>> * Alistair Francis:
>>>>> On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
>>>>>>
>>>>>> * Alistair Francis:
>>>>>>
>>>>>>> +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
>>>>>>> +
>>>>>>> +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
>>>>>>> +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
>>>>>>> +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
>>>>>>
>>>>>> Sorry, how is this supposed to work for all the other architectures
>>>>>> which don't have this system call with current kernel versions?
>>>>>
>>>>> Good question, I'm sure it doesn't.
>>>>>
>>>>> If we put this around #if defined(clock_nanosleep_time64) or #ifndef
>>>>> clock_nanosleep and then keep the current code as the #else would that
>>>>> be acceptable to upstream?
>>>>
>>>> Does this work in <sysdep.h> for the port?
>>>>
>>>> #define __NR_clock_nanosleep  __NR_clock_nanosleep_time64
>>>
>>> I'm at home today so I can't check, but I suspect that will work.
>>>
>>>>
>>>> This is somewhat hackish, but it's reasonably accurate for architectures
>>>> that provide standard system calls under non-standard names for some
>>>> reason.
>>>
>>> For the RV32 case I think this will provide a workable solution. I am
>>> under the impression though that all 32-bit architectures are
>>> eventually removing __ARCH_WANT_TIME32_SYSCALLS from Linux so they
>>> will also hit this issue. Which is why I was going for a more generic
>>> approach. In saying that, whatever gets the RV32 port in works for me
>>> :)
>>
>> Generally speaking we never remove system calls from the kernel, rv32
>> was the exception to this rule because there was no upstream C library
>> port. __ARCH_WANT_TIME32_SYSCALLS will probably hang around for
>> shortly before year 2038 -- afterwards there is no point any more as all
>> binaries relying on it will be broken then.
>>
>> We will however allow building kernels that omit the time32 syscalls
>> before then, as they are only adding baggage to the kernel size and
>> make it harder to verify that user space doesn't try calling them.
>>
>> I think for internal libc functions, you need to try both functions:
>> first the time64 version for kernels that leave out the 32-bit one,
>> and then the time32 version for running on pre-5.1 kernels.
> 
> Ok, so if I re-jig the series to #ifdef the 64-bit syscall first and
> then fall back to the standard (32-bit) syscall that will be the
> correct solution?
> 
> 
>>
>> For user-visible glibc interfaces (e.g. nanosleep()), user space with
>> a 64-bit time_t will have to try the time64 syscall first and then fall
>> back on the time32 version for pre-5.1 kernels, while user space
>> with 32-bit time_t should only call the old time32 syscall and fail
>> on any kernel that doesn't support that, either because
>> __ARCH_WANT_TIME32_SYSCALLS is not set at compile
>> time, or because the kernel was configured without these.
> 
> Ok, so this seems to just be changing the #ifdef logic to something like this:
> 
> #ifdef time_t == 64
> # ifdef __NR_syscall64
>     __NR_syscall64()
> # else
>     __NR_syscall()
> # endif
> #else /* time_t == 64*/
>     __NR_syscall()
> #endif
> 
> Although that just simplifies to:
> 
> # ifdef __NR_syscall64
>     __NR_syscall64()
> # else
>     __NR_syscall()
> # endif
> 

Kernel seems to allow builds with disabled time32 syscalls and if it were indeed 
used on deployments we will need to do change the glibc implementation to do:

--
#ifdef __NR_syscall64
  long int ret = __NR_syscall64 ();
  if (ret == 0 || errno != ENOSYS)
    return ret;
#endif
  return __NR_syscall ();
--

We can optimize by adding __ASSUME macros on kernel-features to explicit make 
syscall64 is always used, which will turn to previous logic to:

--
#ifdef __ASSUME_syscall64
  return __NR_syscall64 ();
#else
# ifdef __NR_syscall64
  long int ret = __NR_syscall64 ();
  if (ret == 0 || errno != ENOSYS)
    return ret;
# endif
  return __NR_syscall ();
#endif
--
  
Alistair Francis June 24, 2019, 5:12 p.m. UTC | #8
On Mon, Jun 24, 2019 at 4:52 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 23/06/2019 20:42, Alistair Francis wrote:
> > On Sun, Jun 23, 2019 at 12:35 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >>
> >> On Sun, Jun 23, 2019 at 7:00 PM Alistair Francis <alistair23@gmail.com> wrote:
> >>> On Sun, Jun 23, 2019 at 9:48 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>>> * Alistair Francis:
> >>>>> On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
> >>>>>>
> >>>>>> * Alistair Francis:
> >>>>>>
> >>>>>>> +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
> >>>>>>> +
> >>>>>>> +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
> >>>>>>> +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> >>>>>>> +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
> >>>>>>
> >>>>>> Sorry, how is this supposed to work for all the other architectures
> >>>>>> which don't have this system call with current kernel versions?
> >>>>>
> >>>>> Good question, I'm sure it doesn't.
> >>>>>
> >>>>> If we put this around #if defined(clock_nanosleep_time64) or #ifndef
> >>>>> clock_nanosleep and then keep the current code as the #else would that
> >>>>> be acceptable to upstream?
> >>>>
> >>>> Does this work in <sysdep.h> for the port?
> >>>>
> >>>> #define __NR_clock_nanosleep  __NR_clock_nanosleep_time64
> >>>
> >>> I'm at home today so I can't check, but I suspect that will work.
> >>>
> >>>>
> >>>> This is somewhat hackish, but it's reasonably accurate for architectures
> >>>> that provide standard system calls under non-standard names for some
> >>>> reason.
> >>>
> >>> For the RV32 case I think this will provide a workable solution. I am
> >>> under the impression though that all 32-bit architectures are
> >>> eventually removing __ARCH_WANT_TIME32_SYSCALLS from Linux so they
> >>> will also hit this issue. Which is why I was going for a more generic
> >>> approach. In saying that, whatever gets the RV32 port in works for me
> >>> :)
> >>
> >> Generally speaking we never remove system calls from the kernel, rv32
> >> was the exception to this rule because there was no upstream C library
> >> port. __ARCH_WANT_TIME32_SYSCALLS will probably hang around for
> >> shortly before year 2038 -- afterwards there is no point any more as all
> >> binaries relying on it will be broken then.
> >>
> >> We will however allow building kernels that omit the time32 syscalls
> >> before then, as they are only adding baggage to the kernel size and
> >> make it harder to verify that user space doesn't try calling them.
> >>
> >> I think for internal libc functions, you need to try both functions:
> >> first the time64 version for kernels that leave out the 32-bit one,
> >> and then the time32 version for running on pre-5.1 kernels.
> >
> > Ok, so if I re-jig the series to #ifdef the 64-bit syscall first and
> > then fall back to the standard (32-bit) syscall that will be the
> > correct solution?
> >
> >
> >>
> >> For user-visible glibc interfaces (e.g. nanosleep()), user space with
> >> a 64-bit time_t will have to try the time64 syscall first and then fall
> >> back on the time32 version for pre-5.1 kernels, while user space
> >> with 32-bit time_t should only call the old time32 syscall and fail
> >> on any kernel that doesn't support that, either because
> >> __ARCH_WANT_TIME32_SYSCALLS is not set at compile
> >> time, or because the kernel was configured without these.
> >
> > Ok, so this seems to just be changing the #ifdef logic to something like this:
> >
> > #ifdef time_t == 64
> > # ifdef __NR_syscall64
> >     __NR_syscall64()
> > # else
> >     __NR_syscall()
> > # endif
> > #else /* time_t == 64*/
> >     __NR_syscall()
> > #endif
> >
> > Although that just simplifies to:
> >
> > # ifdef __NR_syscall64
> >     __NR_syscall64()
> > # else
> >     __NR_syscall()
> > # endif
> >
>
> Kernel seems to allow builds with disabled time32 syscalls and if it were indeed
> used on deployments we will need to do change the glibc implementation to do:
>
> --
> #ifdef __NR_syscall64
>   long int ret = __NR_syscall64 ();
>   if (ret == 0 || errno != ENOSYS)
>     return ret;
> #endif
>   return __NR_syscall ();
> --
>
> We can optimize by adding __ASSUME macros on kernel-features to explicit make
> syscall64 is always used, which will turn to previous logic to:
>
> --
> #ifdef __ASSUME_syscall64
>   return __NR_syscall64 ();
> #else
> # ifdef __NR_syscall64
>   long int ret = __NR_syscall64 ();
>   if (ret == 0 || errno != ENOSYS)
>     return ret;
> # endif
>   return __NR_syscall ();
> #endif

Ok, so it sounds like the correct solution is to do the above
#ifdef-ing for all the problematic syscalls. I'll do that and send an
v2.

Alistair

> --
  
Alistair Francis June 24, 2019, 11:05 p.m. UTC | #9
On Mon, Jun 24, 2019 at 4:52 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 23/06/2019 20:42, Alistair Francis wrote:
> > On Sun, Jun 23, 2019 at 12:35 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >>
> >> On Sun, Jun 23, 2019 at 7:00 PM Alistair Francis <alistair23@gmail.com> wrote:
> >>> On Sun, Jun 23, 2019 at 9:48 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>>> * Alistair Francis:
> >>>>> On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
> >>>>>>
> >>>>>> * Alistair Francis:
> >>>>>>
> >>>>>>> +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
> >>>>>>> +
> >>>>>>> +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
> >>>>>>> +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> >>>>>>> +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
> >>>>>>
> >>>>>> Sorry, how is this supposed to work for all the other architectures
> >>>>>> which don't have this system call with current kernel versions?
> >>>>>
> >>>>> Good question, I'm sure it doesn't.
> >>>>>
> >>>>> If we put this around #if defined(clock_nanosleep_time64) or #ifndef
> >>>>> clock_nanosleep and then keep the current code as the #else would that
> >>>>> be acceptable to upstream?
> >>>>
> >>>> Does this work in <sysdep.h> for the port?
> >>>>
> >>>> #define __NR_clock_nanosleep  __NR_clock_nanosleep_time64
> >>>
> >>> I'm at home today so I can't check, but I suspect that will work.
> >>>
> >>>>
> >>>> This is somewhat hackish, but it's reasonably accurate for architectures
> >>>> that provide standard system calls under non-standard names for some
> >>>> reason.
> >>>
> >>> For the RV32 case I think this will provide a workable solution. I am
> >>> under the impression though that all 32-bit architectures are
> >>> eventually removing __ARCH_WANT_TIME32_SYSCALLS from Linux so they
> >>> will also hit this issue. Which is why I was going for a more generic
> >>> approach. In saying that, whatever gets the RV32 port in works for me
> >>> :)
> >>
> >> Generally speaking we never remove system calls from the kernel, rv32
> >> was the exception to this rule because there was no upstream C library
> >> port. __ARCH_WANT_TIME32_SYSCALLS will probably hang around for
> >> shortly before year 2038 -- afterwards there is no point any more as all
> >> binaries relying on it will be broken then.
> >>
> >> We will however allow building kernels that omit the time32 syscalls
> >> before then, as they are only adding baggage to the kernel size and
> >> make it harder to verify that user space doesn't try calling them.
> >>
> >> I think for internal libc functions, you need to try both functions:
> >> first the time64 version for kernels that leave out the 32-bit one,
> >> and then the time32 version for running on pre-5.1 kernels.
> >
> > Ok, so if I re-jig the series to #ifdef the 64-bit syscall first and
> > then fall back to the standard (32-bit) syscall that will be the
> > correct solution?
> >
> >
> >>
> >> For user-visible glibc interfaces (e.g. nanosleep()), user space with
> >> a 64-bit time_t will have to try the time64 syscall first and then fall
> >> back on the time32 version for pre-5.1 kernels, while user space
> >> with 32-bit time_t should only call the old time32 syscall and fail
> >> on any kernel that doesn't support that, either because
> >> __ARCH_WANT_TIME32_SYSCALLS is not set at compile
> >> time, or because the kernel was configured without these.
> >
> > Ok, so this seems to just be changing the #ifdef logic to something like this:
> >
> > #ifdef time_t == 64
> > # ifdef __NR_syscall64
> >     __NR_syscall64()
> > # else
> >     __NR_syscall()
> > # endif
> > #else /* time_t == 64*/
> >     __NR_syscall()
> > #endif
> >
> > Although that just simplifies to:
> >
> > # ifdef __NR_syscall64
> >     __NR_syscall64()
> > # else
> >     __NR_syscall()
> > # endif
> >
>
> Kernel seems to allow builds with disabled time32 syscalls and if it were indeed
> used on deployments we will need to do change the glibc implementation to do:
>
> --
> #ifdef __NR_syscall64
>   long int ret = __NR_syscall64 ();
>   if (ret == 0 || errno != ENOSYS)
>     return ret;
> #endif
>   return __NR_syscall ();
> --
>
> We can optimize by adding __ASSUME macros on kernel-features to explicit make
> syscall64 is always used, which will turn to previous logic to:
>
> --
> #ifdef __ASSUME_syscall64
>   return __NR_syscall64 ();
> #else
> # ifdef __NR_syscall64
>   long int ret = __NR_syscall64 ();
>   if (ret == 0 || errno != ENOSYS)
>     return ret;
> # endif
>   return __NR_syscall ();
> #endif

Neither of these work as in this case __NR_syscall() isn't defiend. We
can't include it at all for glibc to compile.

Alistair

> --
  
Alistair Francis June 24, 2019, 11:56 p.m. UTC | #10
On Mon, Jun 24, 2019 at 4:05 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, Jun 24, 2019 at 4:52 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> >
> >
> >
> > On 23/06/2019 20:42, Alistair Francis wrote:
> > > On Sun, Jun 23, 2019 at 12:35 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >>
> > >> On Sun, Jun 23, 2019 at 7:00 PM Alistair Francis <alistair23@gmail.com> wrote:
> > >>> On Sun, Jun 23, 2019 at 9:48 AM Florian Weimer <fweimer@redhat.com> wrote:
> > >>>> * Alistair Francis:
> > >>>>> On Sat, Jun 22, 2019 at 11:34 PM Florian Weimer <fweimer@redhat.com> wrote:
> > >>>>>>
> > >>>>>> * Alistair Francis:
> > >>>>>>
> > >>>>>>> +2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
> > >>>>>>> +
> > >>>>>>> +     * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
> > >>>>>>> +     * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> > >>>>>>> +     * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
> > >>>>>>
> > >>>>>> Sorry, how is this supposed to work for all the other architectures
> > >>>>>> which don't have this system call with current kernel versions?
> > >>>>>
> > >>>>> Good question, I'm sure it doesn't.
> > >>>>>
> > >>>>> If we put this around #if defined(clock_nanosleep_time64) or #ifndef
> > >>>>> clock_nanosleep and then keep the current code as the #else would that
> > >>>>> be acceptable to upstream?
> > >>>>
> > >>>> Does this work in <sysdep.h> for the port?
> > >>>>
> > >>>> #define __NR_clock_nanosleep  __NR_clock_nanosleep_time64
> > >>>
> > >>> I'm at home today so I can't check, but I suspect that will work.
> > >>>
> > >>>>
> > >>>> This is somewhat hackish, but it's reasonably accurate for architectures
> > >>>> that provide standard system calls under non-standard names for some
> > >>>> reason.
> > >>>
> > >>> For the RV32 case I think this will provide a workable solution. I am
> > >>> under the impression though that all 32-bit architectures are
> > >>> eventually removing __ARCH_WANT_TIME32_SYSCALLS from Linux so they
> > >>> will also hit this issue. Which is why I was going for a more generic
> > >>> approach. In saying that, whatever gets the RV32 port in works for me
> > >>> :)
> > >>
> > >> Generally speaking we never remove system calls from the kernel, rv32
> > >> was the exception to this rule because there was no upstream C library
> > >> port. __ARCH_WANT_TIME32_SYSCALLS will probably hang around for
> > >> shortly before year 2038 -- afterwards there is no point any more as all
> > >> binaries relying on it will be broken then.
> > >>
> > >> We will however allow building kernels that omit the time32 syscalls
> > >> before then, as they are only adding baggage to the kernel size and
> > >> make it harder to verify that user space doesn't try calling them.
> > >>
> > >> I think for internal libc functions, you need to try both functions:
> > >> first the time64 version for kernels that leave out the 32-bit one,
> > >> and then the time32 version for running on pre-5.1 kernels.
> > >
> > > Ok, so if I re-jig the series to #ifdef the 64-bit syscall first and
> > > then fall back to the standard (32-bit) syscall that will be the
> > > correct solution?
> > >
> > >
> > >>
> > >> For user-visible glibc interfaces (e.g. nanosleep()), user space with
> > >> a 64-bit time_t will have to try the time64 syscall first and then fall
> > >> back on the time32 version for pre-5.1 kernels, while user space
> > >> with 32-bit time_t should only call the old time32 syscall and fail
> > >> on any kernel that doesn't support that, either because
> > >> __ARCH_WANT_TIME32_SYSCALLS is not set at compile
> > >> time, or because the kernel was configured without these.
> > >
> > > Ok, so this seems to just be changing the #ifdef logic to something like this:
> > >
> > > #ifdef time_t == 64
> > > # ifdef __NR_syscall64
> > >     __NR_syscall64()
> > > # else
> > >     __NR_syscall()
> > > # endif
> > > #else /* time_t == 64*/
> > >     __NR_syscall()
> > > #endif
> > >
> > > Although that just simplifies to:
> > >
> > > # ifdef __NR_syscall64
> > >     __NR_syscall64()
> > > # else
> > >     __NR_syscall()
> > > # endif
> > >
> >
> > Kernel seems to allow builds with disabled time32 syscalls and if it were indeed
> > used on deployments we will need to do change the glibc implementation to do:
> >
> > --
> > #ifdef __NR_syscall64
> >   long int ret = __NR_syscall64 ();
> >   if (ret == 0 || errno != ENOSYS)
> >     return ret;
> > #endif
> >   return __NR_syscall ();
> > --
> >
> > We can optimize by adding __ASSUME macros on kernel-features to explicit make
> > syscall64 is always used, which will turn to previous logic to:
> >
> > --
> > #ifdef __ASSUME_syscall64
> >   return __NR_syscall64 ();
> > #else
> > # ifdef __NR_syscall64
> >   long int ret = __NR_syscall64 ();
> >   if (ret == 0 || errno != ENOSYS)
> >     return ret;
> > # endif
> >   return __NR_syscall ();
> > #endif
>
> Neither of these work as in this case __NR_syscall() isn't defiend. We
> can't include it at all for glibc to compile.

Nevermind, I think I figured it out. I'll send a v2.

Alistair

>
> Alistair
>
> > --
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 05291d7825..b90c5ab60c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@ 
+2019-06-21  Alistair Francis  <alistair.francis@wdc.com>
+
+	* nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep.
+	* sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
+	* sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
+
 2019-06-20  Dmitry V. Levin  <ldv@altlinux.org>
 	    Florian Weimer  <fweimer@redhat.com>
 
diff --git a/nptl/thrd_sleep.c b/nptl/thrd_sleep.c
index 07a51808df..82a3b84366 100644
--- a/nptl/thrd_sleep.c
+++ b/nptl/thrd_sleep.c
@@ -25,7 +25,8 @@  int
 thrd_sleep (const struct timespec* time_point, struct timespec* remaining)
 {
   INTERNAL_SYSCALL_DECL (err);
-  int ret = INTERNAL_SYSCALL_CANCEL (nanosleep, err, time_point, remaining);
+  int ret = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err,
+                                     CLOCK_REALTIME, 0, time_point, remaining);
   if (INTERNAL_SYSCALL_ERROR_P (ret, err))
     {
       /* C11 states thrd_sleep function returns -1 if it has been interrupted
diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c
index f14ae565af..bb40121fb6 100644
--- a/sysdeps/unix/sysv/linux/nanosleep.c
+++ b/sysdeps/unix/sysv/linux/nanosleep.c
@@ -25,7 +25,8 @@  int
 __nanosleep (const struct timespec *requested_time,
 	     struct timespec *remaining)
 {
-  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
+  return SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,
+                         requested_time, remaining);
 }
 hidden_def (__nanosleep)
 weak_alias (__nanosleep, nanosleep)
diff --git a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c
index 122ba627ff..8cc56f42d0 100644
--- a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c
+++ b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c
@@ -24,6 +24,7 @@  int
 __nanosleep_nocancel (const struct timespec *requested_time,
 		      struct timespec *remaining)
 {
-  return INLINE_SYSCALL_CALL (nanosleep, requested_time, remaining);
+  return INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,
+                              requested_time, remaining);
 }
 hidden_def (__nanosleep_nocancel)