diff mbox series

[2/3] Refactor nanosleep in terms of clock_nanosleep

Message ID 20191106125245.28102-2-adhemerval.zanella@linaro.org
State New
Headers show
Series [1/3] nptl: Move nanosleep implementation to libc | expand

Commit Message

Adhemerval Zanella Nov. 6, 2019, 12:52 p.m. UTC
The generic version is straightforward.  For Hurd, its nanosleep
implementation is moved to clock_nanosleep with adjustments from
generic unix implementation.

The generic clock_nanosleep unix version is also removed since
it calls nanosleep.

Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
---
 include/time.h                            |  3 +
 posix/nanosleep.c                         | 13 ++--
 sysdeps/{unix => mach}/clock_nanosleep.c  | 79 +++++++++++++++++------
 sysdeps/mach/nanosleep.c                  | 79 -----------------------
 sysdeps/unix/sysv/linux/clock_nanosleep.c |  2 +-
 sysdeps/unix/sysv/linux/nanosleep.c       | 31 ---------
 time/clock_nanosleep.c                    |  2 +-
 7 files changed, 71 insertions(+), 138 deletions(-)
 rename sysdeps/{unix => mach}/clock_nanosleep.c (59%)
 delete mode 100644 sysdeps/mach/nanosleep.c
 delete mode 100644 sysdeps/unix/sysv/linux/nanosleep.c

Comments

Florian Weimer Nov. 6, 2019, 1:08 p.m. UTC | #1
* Adhemerval Zanella:

> +++ b/sysdeps/mach/clock_nanosleep.c

> +static struct timespec
> +timespec_sub (struct timespec a, struct timespec b)
> +{
> +  struct timespec result = { a.tv_sec - b.tv_sec,
> +			     a.tv_nsec - b.tv_nsec };
> +  if (result.tv_nsec < 0)
> +    {
> +      --result.tv_sec;
> +      result.tv_nsec += 1000000000ul;
> +    }
> +  return result;
> +}

Can you use the version in <posix-timers.h>?

Thanks,
Florian
Adhemerval Zanella Nov. 6, 2019, 2:17 p.m. UTC | #2
On 06/11/2019 10:08, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> +++ b/sysdeps/mach/clock_nanosleep.c
> 
>> +static struct timespec
>> +timespec_sub (struct timespec a, struct timespec b)
>> +{
>> +  struct timespec result = { a.tv_sec - b.tv_sec,
>> +			     a.tv_nsec - b.tv_nsec };
>> +  if (result.tv_nsec < 0)
>> +    {
>> +      --result.tv_sec;
>> +      result.tv_nsec += 1000000000ul;
>> +    }
>> +  return result;
>> +}
> 
> Can you use the version in <posix-timers.h>?

Ack.
Dmitry V. Levin Nov. 28, 2019, 11:44 p.m. UTC | #3
On Wed, Nov 06, 2019 at 09:52:44AM -0300, Adhemerval Zanella wrote:
> The generic version is straightforward.  For Hurd, its nanosleep
> implementation is moved to clock_nanosleep with adjustments from
> generic unix implementation.
> 
> The generic clock_nanosleep unix version is also removed since
> it calls nanosleep.
> 
> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
> ---
>  include/time.h                            |  3 +
>  posix/nanosleep.c                         | 13 ++--
>  sysdeps/{unix => mach}/clock_nanosleep.c  | 79 +++++++++++++++++------
>  sysdeps/mach/nanosleep.c                  | 79 -----------------------
>  sysdeps/unix/sysv/linux/clock_nanosleep.c |  2 +-
>  sysdeps/unix/sysv/linux/nanosleep.c       | 31 ---------
>  time/clock_nanosleep.c                    |  2 +-
>  7 files changed, 71 insertions(+), 138 deletions(-)
>  rename sysdeps/{unix => mach}/clock_nanosleep.c (59%)
>  delete mode 100644 sysdeps/mach/nanosleep.c
>  delete mode 100644 sysdeps/unix/sysv/linux/nanosleep.c
> 
> diff --git a/include/time.h b/include/time.h
> index 8ac58e891b..b3e635395d 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -25,6 +25,9 @@ libc_hidden_proto (__clock_gettime)
>  extern __typeof (clock_settime) __clock_settime;
>  libc_hidden_proto (__clock_settime)
>  
> +extern __typeof (clock_nanosleep) __clock_nanosleep;
> +libc_hidden_proto (__clock_nanosleep);
> +
>  #ifdef __linux__
>  extern __typeof (clock_adjtime) __clock_adjtime;
>  libc_hidden_proto (__clock_adjtime);
> diff --git a/posix/nanosleep.c b/posix/nanosleep.c
> index d8564c7119..ed41c8cce7 100644
> --- a/posix/nanosleep.c
> +++ b/posix/nanosleep.c
> @@ -24,10 +24,13 @@ int
>  __nanosleep (const struct timespec *requested_time,
>  	     struct timespec *remaining)
>  {
> -  __set_errno (ENOSYS);
> -  return -1;
> +  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);

Shouldn't it set flags to TIMER_ABSTIME?

In the current form it causes a regression when the syscall is interrupted
by signal - "remaining" is updated by the kernel but __clock_nanosleep
leaves it unchanged.
Adhemerval Zanella Nov. 29, 2019, 11:27 a.m. UTC | #4
On 28/11/2019 20:44, Dmitry V. Levin wrote:
> On Wed, Nov 06, 2019 at 09:52:44AM -0300, Adhemerval Zanella wrote:
>> The generic version is straightforward.  For Hurd, its nanosleep
>> implementation is moved to clock_nanosleep with adjustments from
>> generic unix implementation.
>>
>> The generic clock_nanosleep unix version is also removed since
>> it calls nanosleep.
>>
>> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
>> ---
>>  include/time.h                            |  3 +
>>  posix/nanosleep.c                         | 13 ++--
>>  sysdeps/{unix => mach}/clock_nanosleep.c  | 79 +++++++++++++++++------
>>  sysdeps/mach/nanosleep.c                  | 79 -----------------------
>>  sysdeps/unix/sysv/linux/clock_nanosleep.c |  2 +-
>>  sysdeps/unix/sysv/linux/nanosleep.c       | 31 ---------
>>  time/clock_nanosleep.c                    |  2 +-
>>  7 files changed, 71 insertions(+), 138 deletions(-)
>>  rename sysdeps/{unix => mach}/clock_nanosleep.c (59%)
>>  delete mode 100644 sysdeps/mach/nanosleep.c
>>  delete mode 100644 sysdeps/unix/sysv/linux/nanosleep.c
>>
>> diff --git a/include/time.h b/include/time.h
>> index 8ac58e891b..b3e635395d 100644
>> --- a/include/time.h
>> +++ b/include/time.h
>> @@ -25,6 +25,9 @@ libc_hidden_proto (__clock_gettime)
>>  extern __typeof (clock_settime) __clock_settime;
>>  libc_hidden_proto (__clock_settime)
>>  
>> +extern __typeof (clock_nanosleep) __clock_nanosleep;
>> +libc_hidden_proto (__clock_nanosleep);
>> +
>>  #ifdef __linux__
>>  extern __typeof (clock_adjtime) __clock_adjtime;
>>  libc_hidden_proto (__clock_adjtime);
>> diff --git a/posix/nanosleep.c b/posix/nanosleep.c
>> index d8564c7119..ed41c8cce7 100644
>> --- a/posix/nanosleep.c
>> +++ b/posix/nanosleep.c
>> @@ -24,10 +24,13 @@ int
>>  __nanosleep (const struct timespec *requested_time,
>>  	     struct timespec *remaining)
>>  {
>> -  __set_errno (ENOSYS);
>> -  return -1;
>> +  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
> 
> Shouldn't it set flags to TIMER_ABSTIME?
> 
> In the current form it causes a regression when the syscall is interrupted
> by signal - "remaining" is updated by the kernel but __clock_nanosleep
> leaves it unchanged.

I am not seeing this regression:

$ cat test.c 
#include <stdio.h>
#include <time.h>
#include <unistd.h>
#include <signal.h>
#include <stdint.h>

void sigalrm_handler (int s)
{
}

int main ()
{
  {
    signal (SIGALRM, SIG_IGN);
    alarm (1);
    struct timespec r = { 0, 0 };
    nanosleep (& (struct timespec) { 2, 0 }, &r);
    printf ("SIG_IGN: %ju.%09ju\n", (uintmax_t) r.tv_sec, (uintmax_t) r.tv_nsec);
  }

  {
    signal (SIGALRM, sigalrm_handler);
    alarm (1);
    struct timespec r = { 0, 0 };
    nanosleep (& (struct timespec) { 2, 0 }, &r);
    printf ("handler: %ju.%09ju\n", (uintmax_t) r.tv_sec, (uintmax_t) r.tv_nsec);
  }

  return 0;
}
$ gcc -Wall test.c -o test -m64
$ ldd test
        linux-vdso.so.1 (0x00007ffcd2bee000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fdf26866000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fdf26e59000)
$ /lib/x86_64-linux-gnu/libc.so.6
GNU C Library (Ubuntu GLIBC 2.27-3ubuntu1) stable release version 2.27.
[...]
$ ./test
SIG_IGN: 0.000000000
handler: 1.000018272
$ ./libc.so
GNU C Library (GNU libc) development release version 2.30.9000.
[..]
$ ./testrun.sh ./test
SIG_IGN: 0.000000000
handler: 1.000043369

Same for i686-linux-gnu running on a 4.15 kernel (which calls clock_gettime)
and 5.3 (which calls clock_gettime_time64).

My understanding is TIMER_ABSTIME *does not* update the timer in case of
a interruption:

kernel/time/posix-timers.c

1208 SYSCALL_DEFINE4(clock_nanosleep, const clockid_t, which_clock, int, flags,
1209                 const struct __kernel_timespec __user *, rqtp,
1210                 struct __kernel_timespec __user *, rmtp)
1211 {
1212         const struct k_clock *kc = clockid_to_kclock(which_clock);
1213         struct timespec64 t;
1214 
1215         if (!kc)
1216                 return -EINVAL;
1217         if (!kc->nsleep)
1218                 return -EOPNOTSUPP;
1219 
1220         if (get_timespec64(&t, rqtp))
1221                 return -EFAULT;
1222 
1223         if (!timespec64_valid(&t))
1224                 return -EINVAL;
1225         if (flags & TIMER_ABSTIME)
1226                 rmtp = NULL;
Dmitry V. Levin Nov. 29, 2019, 3:21 p.m. UTC | #5
On Fri, Nov 29, 2019 at 08:27:06AM -0300, Adhemerval Zanella wrote:
> On 28/11/2019 20:44, Dmitry V. Levin wrote:
> > On Wed, Nov 06, 2019 at 09:52:44AM -0300, Adhemerval Zanella wrote:
> >> The generic version is straightforward.  For Hurd, its nanosleep
> >> implementation is moved to clock_nanosleep with adjustments from
> >> generic unix implementation.
> >>
> >> The generic clock_nanosleep unix version is also removed since
> >> it calls nanosleep.
> >>
> >> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
> >> ---
> >>  include/time.h                            |  3 +
> >>  posix/nanosleep.c                         | 13 ++--
> >>  sysdeps/{unix => mach}/clock_nanosleep.c  | 79 +++++++++++++++++------
> >>  sysdeps/mach/nanosleep.c                  | 79 -----------------------
> >>  sysdeps/unix/sysv/linux/clock_nanosleep.c |  2 +-
> >>  sysdeps/unix/sysv/linux/nanosleep.c       | 31 ---------
> >>  time/clock_nanosleep.c                    |  2 +-
> >>  7 files changed, 71 insertions(+), 138 deletions(-)
> >>  rename sysdeps/{unix => mach}/clock_nanosleep.c (59%)
> >>  delete mode 100644 sysdeps/mach/nanosleep.c
> >>  delete mode 100644 sysdeps/unix/sysv/linux/nanosleep.c
> >>
> >> diff --git a/include/time.h b/include/time.h
> >> index 8ac58e891b..b3e635395d 100644
> >> --- a/include/time.h
> >> +++ b/include/time.h
> >> @@ -25,6 +25,9 @@ libc_hidden_proto (__clock_gettime)
> >>  extern __typeof (clock_settime) __clock_settime;
> >>  libc_hidden_proto (__clock_settime)
> >>  
> >> +extern __typeof (clock_nanosleep) __clock_nanosleep;
> >> +libc_hidden_proto (__clock_nanosleep);
> >> +
> >>  #ifdef __linux__
> >>  extern __typeof (clock_adjtime) __clock_adjtime;
> >>  libc_hidden_proto (__clock_adjtime);
> >> diff --git a/posix/nanosleep.c b/posix/nanosleep.c
> >> index d8564c7119..ed41c8cce7 100644
> >> --- a/posix/nanosleep.c
> >> +++ b/posix/nanosleep.c
> >> @@ -24,10 +24,13 @@ int
> >>  __nanosleep (const struct timespec *requested_time,
> >>  	     struct timespec *remaining)
> >>  {
> >> -  __set_errno (ENOSYS);
> >> -  return -1;
> >> +  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
> > 
> > Shouldn't it set flags to TIMER_ABSTIME?
> > 
> > In the current form it causes a regression when the syscall is interrupted
> > by signal - "remaining" is updated by the kernel but __clock_nanosleep
> > leaves it unchanged.
> 
> I am not seeing this regression:

The regression has been detected by the strace test suite on Rawhide,
the log is at
https://kojipkgs.fedoraproject.org/work/tasks/1866/39391866/build.log
(look for "FAIL: restart_syscall" there), the test is
https://github.com/strace/strace/blob/master/tests/restart_syscall.c
Dmitry V. Levin Nov. 29, 2019, 5:49 p.m. UTC | #6
On Fri, Nov 29, 2019 at 06:21:53PM +0300, Dmitry V. Levin wrote:
> On Fri, Nov 29, 2019 at 08:27:06AM -0300, Adhemerval Zanella wrote:
> > On 28/11/2019 20:44, Dmitry V. Levin wrote:
> > > On Wed, Nov 06, 2019 at 09:52:44AM -0300, Adhemerval Zanella wrote:
> > >> The generic version is straightforward.  For Hurd, its nanosleep
> > >> implementation is moved to clock_nanosleep with adjustments from
> > >> generic unix implementation.
> > >>
> > >> The generic clock_nanosleep unix version is also removed since
> > >> it calls nanosleep.
> > >>
> > >> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
> > >> ---
> > >>  include/time.h                            |  3 +
> > >>  posix/nanosleep.c                         | 13 ++--
> > >>  sysdeps/{unix => mach}/clock_nanosleep.c  | 79 +++++++++++++++++------
> > >>  sysdeps/mach/nanosleep.c                  | 79 -----------------------
> > >>  sysdeps/unix/sysv/linux/clock_nanosleep.c |  2 +-
> > >>  sysdeps/unix/sysv/linux/nanosleep.c       | 31 ---------
> > >>  time/clock_nanosleep.c                    |  2 +-
> > >>  7 files changed, 71 insertions(+), 138 deletions(-)
> > >>  rename sysdeps/{unix => mach}/clock_nanosleep.c (59%)
> > >>  delete mode 100644 sysdeps/mach/nanosleep.c
> > >>  delete mode 100644 sysdeps/unix/sysv/linux/nanosleep.c
> > >>
> > >> diff --git a/include/time.h b/include/time.h
> > >> index 8ac58e891b..b3e635395d 100644
> > >> --- a/include/time.h
> > >> +++ b/include/time.h
> > >> @@ -25,6 +25,9 @@ libc_hidden_proto (__clock_gettime)
> > >>  extern __typeof (clock_settime) __clock_settime;
> > >>  libc_hidden_proto (__clock_settime)
> > >>  
> > >> +extern __typeof (clock_nanosleep) __clock_nanosleep;
> > >> +libc_hidden_proto (__clock_nanosleep);
> > >> +
> > >>  #ifdef __linux__
> > >>  extern __typeof (clock_adjtime) __clock_adjtime;
> > >>  libc_hidden_proto (__clock_adjtime);
> > >> diff --git a/posix/nanosleep.c b/posix/nanosleep.c
> > >> index d8564c7119..ed41c8cce7 100644
> > >> --- a/posix/nanosleep.c
> > >> +++ b/posix/nanosleep.c
> > >> @@ -24,10 +24,13 @@ int
> > >>  __nanosleep (const struct timespec *requested_time,
> > >>  	     struct timespec *remaining)
> > >>  {
> > >> -  __set_errno (ENOSYS);
> > >> -  return -1;
> > >> +  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
> > > 
> > > Shouldn't it set flags to TIMER_ABSTIME?
> > > 
> > > In the current form it causes a regression when the syscall is interrupted
> > > by signal - "remaining" is updated by the kernel but __clock_nanosleep
> > > leaves it unchanged.
> > 
> > I am not seeing this regression:
> 
> The regression has been detected by the strace test suite on Rawhide,
> the log is at
> https://kojipkgs.fedoraproject.org/work/tasks/1866/39391866/build.log
> (look for "FAIL: restart_syscall" there), the test is
> https://github.com/strace/strace/blob/master/tests/restart_syscall.c

Here is a stripped down example:
$ cat nanorem.c
#include <err.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/time.h>
#include <time.h>

int
main(void)
{
	if (signal(SIGALRM, SIG_IGN))
		err(1, "signal");

	const sigset_t set = {};
	if (sigprocmask(SIG_SETMASK, &set, NULL))
		err(1, "sigprocmask");

	const struct itimerval itv = { .it_value.tv_usec = 111111 };
	const struct timespec req = { .tv_nsec = 222222222 };
	const struct timespec bad = { 0xbadc0ded, 0xbadc0ded };

	struct timespec rem = bad;
	if (setitimer(ITIMER_REAL, &itv, NULL))
		err(1, "setitimer");
	if (nanosleep(&req, &rem))
		err(1, "nanosleep");
	printf("nanosleep: tv_sec=%lld, tv_nsec=%llu\n",
	       (long long) rem.tv_sec, (unsigned long long) rem.tv_nsec);

	rem = bad;
	if (setitimer(ITIMER_REAL, &itv, NULL))
		err(1, "setitimer");
	if (clock_nanosleep(CLOCK_REALTIME, 0, &req, &rem))
		err(1, "clock_nanosleep");
	printf("clock_nanosleep: tv_sec=%lld, tv_nsec=%llu\n",
	       (long long) rem.tv_sec, (unsigned long long) rem.tv_nsec);

	return 0;
}
$ gcc -Wall -O2 nanorem.c -o nanorem
$ strace -qq -enone -esignal=none ./nanorem
nanosleep: tv_sec=0, tv_nsec=111185098
clock_nanosleep: tv_sec=0, tv_nsec=111173210

Apparently, the output for nanosleep is no longer the same after the
change.
Adhemerval Zanella Nov. 29, 2019, 8:33 p.m. UTC | #7
On 29/11/2019 14:49, Dmitry V. Levin wrote:
> On Fri, Nov 29, 2019 at 06:21:53PM +0300, Dmitry V. Levin wrote:
>> On Fri, Nov 29, 2019 at 08:27:06AM -0300, Adhemerval Zanella wrote:
>>> On 28/11/2019 20:44, Dmitry V. Levin wrote:
>>>> On Wed, Nov 06, 2019 at 09:52:44AM -0300, Adhemerval Zanella wrote:
>>>>> The generic version is straightforward.  For Hurd, its nanosleep
>>>>> implementation is moved to clock_nanosleep with adjustments from
>>>>> generic unix implementation.
>>>>>
>>>>> The generic clock_nanosleep unix version is also removed since
>>>>> it calls nanosleep.
>>>>>
>>>>> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
>>>>> ---
>>>>>  include/time.h                            |  3 +
>>>>>  posix/nanosleep.c                         | 13 ++--
>>>>>  sysdeps/{unix => mach}/clock_nanosleep.c  | 79 +++++++++++++++++------
>>>>>  sysdeps/mach/nanosleep.c                  | 79 -----------------------
>>>>>  sysdeps/unix/sysv/linux/clock_nanosleep.c |  2 +-
>>>>>  sysdeps/unix/sysv/linux/nanosleep.c       | 31 ---------
>>>>>  time/clock_nanosleep.c                    |  2 +-
>>>>>  7 files changed, 71 insertions(+), 138 deletions(-)
>>>>>  rename sysdeps/{unix => mach}/clock_nanosleep.c (59%)
>>>>>  delete mode 100644 sysdeps/mach/nanosleep.c
>>>>>  delete mode 100644 sysdeps/unix/sysv/linux/nanosleep.c
>>>>>
>>>>> diff --git a/include/time.h b/include/time.h
>>>>> index 8ac58e891b..b3e635395d 100644
>>>>> --- a/include/time.h
>>>>> +++ b/include/time.h
>>>>> @@ -25,6 +25,9 @@ libc_hidden_proto (__clock_gettime)
>>>>>  extern __typeof (clock_settime) __clock_settime;
>>>>>  libc_hidden_proto (__clock_settime)
>>>>>  
>>>>> +extern __typeof (clock_nanosleep) __clock_nanosleep;
>>>>> +libc_hidden_proto (__clock_nanosleep);
>>>>> +
>>>>>  #ifdef __linux__
>>>>>  extern __typeof (clock_adjtime) __clock_adjtime;
>>>>>  libc_hidden_proto (__clock_adjtime);
>>>>> diff --git a/posix/nanosleep.c b/posix/nanosleep.c
>>>>> index d8564c7119..ed41c8cce7 100644
>>>>> --- a/posix/nanosleep.c
>>>>> +++ b/posix/nanosleep.c
>>>>> @@ -24,10 +24,13 @@ int
>>>>>  __nanosleep (const struct timespec *requested_time,
>>>>>  	     struct timespec *remaining)
>>>>>  {
>>>>> -  __set_errno (ENOSYS);
>>>>> -  return -1;
>>>>> +  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
>>>>
>>>> Shouldn't it set flags to TIMER_ABSTIME?
>>>>
>>>> In the current form it causes a regression when the syscall is interrupted
>>>> by signal - "remaining" is updated by the kernel but __clock_nanosleep
>>>> leaves it unchanged.
>>>
>>> I am not seeing this regression:
>>
>> The regression has been detected by the strace test suite on Rawhide,
>> the log is at
>> https://kojipkgs.fedoraproject.org/work/tasks/1866/39391866/build.log
>> (look for "FAIL: restart_syscall" there), the test is
>> https://github.com/strace/strace/blob/master/tests/restart_syscall.c
> 
> Here is a stripped down example:
> $ cat nanorem.c
> #include <err.h>
> #include <signal.h>
> #include <stdint.h>
> #include <stdio.h>
> #include <sys/time.h>
> #include <time.h>
> 
> int
> main(void)
> {
> 	if (signal(SIGALRM, SIG_IGN))
> 		err(1, "signal");
> 
> 	const sigset_t set = {};
> 	if (sigprocmask(SIG_SETMASK, &set, NULL))
> 		err(1, "sigprocmask");
> 
> 	const struct itimerval itv = { .it_value.tv_usec = 111111 };
> 	const struct timespec req = { .tv_nsec = 222222222 };
> 	const struct timespec bad = { 0xbadc0ded, 0xbadc0ded };
> 
> 	struct timespec rem = bad;
> 	if (setitimer(ITIMER_REAL, &itv, NULL))
> 		err(1, "setitimer");
> 	if (nanosleep(&req, &rem))
> 		err(1, "nanosleep");
> 	printf("nanosleep: tv_sec=%lld, tv_nsec=%llu\n",
> 	       (long long) rem.tv_sec, (unsigned long long) rem.tv_nsec);
> 
> 	rem = bad;
> 	if (setitimer(ITIMER_REAL, &itv, NULL))
> 		err(1, "setitimer");
> 	if (clock_nanosleep(CLOCK_REALTIME, 0, &req, &rem))
> 		err(1, "clock_nanosleep");
> 	printf("clock_nanosleep: tv_sec=%lld, tv_nsec=%llu\n",
> 	       (long long) rem.tv_sec, (unsigned long long) rem.tv_nsec);
> 
> 	return 0;
> }
> $ gcc -Wall -O2 nanorem.c -o nanorem
> $ strace -qq -enone -esignal=none ./nanorem
> nanosleep: tv_sec=0, tv_nsec=111185098
> clock_nanosleep: tv_sec=0, tv_nsec=111173210
> 
> Apparently, the output for nanosleep is no longer the same after the
> change.

I am not seeing any change here., using the same environment 
as before (ubuntu 18, glibc 2.27, kernel 4.15, x86_64):

$ strace -qq -enone -esignal=none ./nanorem
nanosleep: tv_sec=0, tv_nsec=111162421
clock_nanosleep: tv_sec=0, tv_nsec=111250560
$ strace -qq -enone -esignal=none ./elf/ld-linux-x86-64.so.2 --library-path .:./math:./elf:./dlfcn:./nss:./nis:./rt:./resolv:./mathvec:./support:./crypt:./nptl ./nanorem
nanosleep: tv_sec=0, tv_nsec=111153506
clock_nanosleep: tv_sec=0, tv_nsec=111153154

Neither on i686-linux-gnu (kernel 4.15 and kernel 5.4).  

My guess is what might be characterized as a regression is although POSIX 
states that nanosleep should be measured as CLOCK_REALTIME [1], Linux in the
other hand uses CLOCK_MONOTONIC:

kernel/time/hrtimer.c
1945 SYSCALL_DEFINE2(nanosleep, struct __kernel_timespec __user *, rqtp,
1946                 struct __kernel_timespec __user *, rmtp)
1947 {
[...]
1958         return hrtimer_nanosleep(&tu, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
1959 }

Not sure if this is worth to add a compat symbol.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/nanosleep.html
diff mbox series

Patch

diff --git a/include/time.h b/include/time.h
index 8ac58e891b..b3e635395d 100644
--- a/include/time.h
+++ b/include/time.h
@@ -25,6 +25,9 @@  libc_hidden_proto (__clock_gettime)
 extern __typeof (clock_settime) __clock_settime;
 libc_hidden_proto (__clock_settime)
 
+extern __typeof (clock_nanosleep) __clock_nanosleep;
+libc_hidden_proto (__clock_nanosleep);
+
 #ifdef __linux__
 extern __typeof (clock_adjtime) __clock_adjtime;
 libc_hidden_proto (__clock_adjtime);
diff --git a/posix/nanosleep.c b/posix/nanosleep.c
index d8564c7119..ed41c8cce7 100644
--- a/posix/nanosleep.c
+++ b/posix/nanosleep.c
@@ -24,10 +24,13 @@  int
 __nanosleep (const struct timespec *requested_time,
 	     struct timespec *remaining)
 {
-  __set_errno (ENOSYS);
-  return -1;
+  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
+  if (ret != 0)
+    {
+      __set_errno (ret);
+      return -1;
+    }
+  return 0;
 }
-stub_warning (nanosleep)
-
-hidden_def (__nanosleep)
+libc_hidden_def (__nanosleep)
 weak_alias (__nanosleep, nanosleep)
diff --git a/sysdeps/unix/clock_nanosleep.c b/sysdeps/mach/clock_nanosleep.c
similarity index 59%
rename from sysdeps/unix/clock_nanosleep.c
rename to sysdeps/mach/clock_nanosleep.c
index 8514a439ee..6a0792d684 100644
--- a/sysdeps/unix/clock_nanosleep.c
+++ b/sysdeps/mach/clock_nanosleep.c
@@ -1,5 +1,5 @@ 
-/* High-resolution sleep with the specified clock.
-   Copyright (C) 2000-2019 Free Software Foundation, Inc.
+/* clock_nanosleep - high-resolution sleep with specifiable clock.
+   Copyright (C) 2002-2019 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
@@ -16,28 +16,70 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <assert.h>
 #include <errno.h>
+#include <mach.h>
 #include <time.h>
-#include <sysdep-cancel.h>
+#include <unistd.h>
 #include <shlib-compat.h>
 
-/* This implementation assumes that these is only a `nanosleep' system
-   call.  So we have to remap all other activities.  */
+static struct timespec
+timespec_sub (struct timespec a, struct timespec b)
+{
+  struct timespec result = { a.tv_sec - b.tv_sec,
+			     a.tv_nsec - b.tv_nsec };
+  if (result.tv_nsec < 0)
+    {
+      --result.tv_sec;
+      result.tv_nsec += 1000000000ul;
+    }
+  return result;
+}
+
+static int
+nanosleep_call (const struct timespec *req, struct timespec *rem)
+{
+  mach_port_t recv;
+  struct timespec before;
+  error_t err;
+
+  const mach_msg_timeout_t ms
+    = req->tv_sec * 1000
+    + (req->tv_nsec + 999999) / 1000000;
+
+  recv = __mach_reply_port ();
+
+  if (rem != NULL)
+    __clock_gettime (CLOCK_REALTIME, &before);
+
+  err = __mach_msg (NULL, MACH_RCV_MSG|MACH_RCV_TIMEOUT|MACH_RCV_INTERRUPT,
+                    0, 0, recv, ms, MACH_PORT_NULL);
+  __mach_port_destroy (mach_task_self (), recv);
+  if (err == EMACH_RCV_INTERRUPTED)
+    {
+      if (rem != NULL)
+	{
+	  struct timespec after, elapsed;
+	  __clock_gettime (CLOCK_REALTIME, &after);
+	  elapsed = timespec_sub (after, before);
+	  *rem = timespec_sub (*req, elapsed);
+	}
+
+      return EINTR;
+    }
+
+  return 0;
+}
+
 int
 __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
 		   struct timespec *rem)
 {
-  struct timespec now;
-
-  if (! valid_nanoseconds (req->tv_nsec))
+  if (clock_id != CLOCK_REALTIME
+      || !valid_nanoseconds (req->tv_nsec)
+      || (flags != 0 || flags != TIMER_ABSTIME))
     return EINVAL;
 
-  if (clock_id == CLOCK_THREAD_CPUTIME_ID)
-    return EINVAL;		/* POSIX specifies EINVAL for this case.  */
-
-  if (clock_id < CLOCK_REALTIME || clock_id > CLOCK_THREAD_CPUTIME_ID)
-    return EINVAL;
+  struct timespec now;
 
   /* If we got an absolute time, remap it.  */
   if (flags == TIMER_ABSTIME)
@@ -68,15 +110,10 @@  __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
       /* Make sure we are not modifying the struct pointed to by REM.  */
       rem = NULL;
     }
-  else if (flags != 0)
-    return EINVAL;
-  else if (clock_id != CLOCK_REALTIME)
-    /* Not supported.  */
-    return ENOTSUP;
 
-  return __nanosleep (req, rem), 0 ? errno : 0;
+  return nanosleep_call (req, rem);
 }
-
+libc_hidden_def (__clock_nanosleep)
 versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);
 /* clock_nanosleep moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
diff --git a/sysdeps/mach/nanosleep.c b/sysdeps/mach/nanosleep.c
deleted file mode 100644
index b60a2179f6..0000000000
--- a/sysdeps/mach/nanosleep.c
+++ /dev/null
@@ -1,79 +0,0 @@ 
-/* nanosleep -- sleep for a period specified with a struct timespec
-   Copyright (C) 2002-2019 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 <mach.h>
-#include <time.h>
-#include <unistd.h>
-
-# define timespec_sub(a, b, result)					      \
-  do {									      \
-    (result)->tv_sec = (a)->tv_sec - (b)->tv_sec;			      \
-    (result)->tv_nsec = (a)->tv_nsec - (b)->tv_nsec;			      \
-    if ((result)->tv_nsec < 0) {					      \
-      --(result)->tv_sec;						      \
-      (result)->tv_nsec += 1000000000;					      \
-    }									      \
-  } while (0)
-
-int
-__libc_nanosleep (const struct timespec *requested_time,
-                  struct timespec *remaining)
-{
-  mach_port_t recv;
-  struct timespec before;
-  error_t err;
-
-  if (requested_time->tv_sec < 0
-      || ! valid_nanoseconds (requested_time->tv_nsec))
-    {
-      errno = EINVAL;
-      return -1;
-    }
-
-  const mach_msg_timeout_t ms
-    = requested_time->tv_sec * 1000
-    + (requested_time->tv_nsec + 999999) / 1000000;
-
-  recv = __mach_reply_port ();
-
-  if (remaining != 0)
-    __clock_gettime (CLOCK_REALTIME, &before);
-
-  err = __mach_msg (NULL, MACH_RCV_MSG|MACH_RCV_TIMEOUT|MACH_RCV_INTERRUPT,
-                    0, 0, recv, ms, MACH_PORT_NULL);
-  __mach_port_destroy (mach_task_self (), recv);
-  if (err == EMACH_RCV_INTERRUPTED)
-    {
-      if (remaining != 0)
-	{
-	  struct timespec after, elapsed;
-	  __clock_gettime (CLOCK_REALTIME, &after);
-	  timespec_sub (&after, &before, &elapsed);
-	  timespec_sub (requested_time, &elapsed, remaining);
-	}
-
-      errno = EINTR;
-      return -1;
-    }
-
-  return 0;
-}
-weak_alias(__libc_nanosleep, __nanosleep)
-libc_hidden_def (__nanosleep)
-weak_alias (__libc_nanosleep, nanosleep)
diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c
index 1f240b8720..f3c6fd2d5f 100644
--- a/sysdeps/unix/sysv/linux/clock_nanosleep.c
+++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c
@@ -42,7 +42,7 @@  __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
   return (INTERNAL_SYSCALL_ERROR_P (r, err)
 	  ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
 }
-
+libc_hidden_def (__clock_nanosleep)
 versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);
 /* clock_nanosleep moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c
deleted file mode 100644
index 6787909248..0000000000
--- a/sysdeps/unix/sysv/linux/nanosleep.c
+++ /dev/null
@@ -1,31 +0,0 @@ 
-/* Linux nanosleep syscall implementation.
-   Copyright (C) 2017-2019 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 <time.h>
-#include <sysdep-cancel.h>
-#include <not-cancel.h>
-
-/* Pause execution for a number of nanoseconds.  */
-int
-__nanosleep (const struct timespec *requested_time,
-	     struct timespec *remaining)
-{
-  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
-}
-hidden_def (__nanosleep)
-weak_alias (__nanosleep, nanosleep)
diff --git a/time/clock_nanosleep.c b/time/clock_nanosleep.c
index 5952d29195..d4fc16f6ec 100644
--- a/time/clock_nanosleep.c
+++ b/time/clock_nanosleep.c
@@ -33,7 +33,7 @@  __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
   /* Not implemented.  */
   return ENOSYS;
 }
-
+libc_hidden_def (__clock_nanosleep)
 versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);
 /* clock_nanosleep moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */