Re: [PATCH 2/3] Refactor nanosleep in terms of clock_nanosleep

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

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;

Attachment: signature.asc
Description: OpenPGP digital signature