[v2] sysdeps/nanosleep: Use clock_nanosleep_time64 if avaliable

Message ID 6e51cc8e-f6cd-ebbe-36d6-95df96199ad9@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Netto Nov. 4, 2019, 7:30 p.m. UTC
  On 04/11/2019 15:16, Alistair Francis wrote:
> On Tue, Oct 29, 2019 at 1:16 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 24/10/2019 20:41, Alistair Francis wrote:

>>
>> There is no need to replicate all the syscall logic, nanosleep can be implemented
>> with __clock_nanosleep.  You can do:
>>
>> int
>> __nanosleep (const struct timespec *requested_time,
>>              struct timespec *remaining)
>> {
>>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
>>   if (ret != 0)
>>     {
>>       __set_errno (-ret);
>>       return -1;
>>     }
>>   return ret;
>> }
> 
> This doesn't work as __clock_nanosleep() isn't avaliable in nptl so it
> fails to compile. My v3 patch attempted to fix this, but that also
> doesn't work and ends up squishing some patches together.

For this case you will need to add __clock_nanosleep as a GLIBC_PRIVATE
exported symbol from libc and add a hidden_proto/hidden_def for libc 
internal plt avoidance (as below).

However, I agree with Joseph and Florian that for this case it would be
better to move the symbol from libpthread to libc. I will send a patch
to refactor it.
  

Comments

Alistair Francis Nov. 4, 2019, 10:57 p.m. UTC | #1
On Mon, Nov 4, 2019 at 11:30 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 04/11/2019 15:16, Alistair Francis wrote:
> > On Tue, Oct 29, 2019 at 1:16 PM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 24/10/2019 20:41, Alistair Francis wrote:
>
> >>
> >> There is no need to replicate all the syscall logic, nanosleep can be implemented
> >> with __clock_nanosleep.  You can do:
> >>
> >> int
> >> __nanosleep (const struct timespec *requested_time,
> >>              struct timespec *remaining)
> >> {
> >>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
> >>   if (ret != 0)
> >>     {
> >>       __set_errno (-ret);
> >>       return -1;
> >>     }
> >>   return ret;
> >> }
> >
> > This doesn't work as __clock_nanosleep() isn't avaliable in nptl so it
> > fails to compile. My v3 patch attempted to fix this, but that also
> > doesn't work and ends up squishing some patches together.
>
> For this case you will need to add __clock_nanosleep as a GLIBC_PRIVATE
> exported symbol from libc and add a hidden_proto/hidden_def for libc
> internal plt avoidance (as below).

Thanks for explaining this.

>
> However, I agree with Joseph and Florian that for this case it would be
> better to move the symbol from libpthread to libc. I will send a patch
> to refactor it.

I am going to include the diff below and convert __nanosleep to call
__clock_nanosleep(), but I won't change the file structure or move
anything else around. I'll leave that to you in a follow up patch.

Alistair

>
> diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
> index d385085c61..475abb5004 100644
> --- a/sysdeps/unix/sysv/linux/Versions
> +++ b/sysdeps/unix/sysv/linux/Versions
> @@ -187,5 +187,6 @@ libc {
>      __sigtimedwait;
>      # functions used by nscd
>      __netlink_assert_response;
> +    __clock_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
> index 6787909248..1e2481847b 100644
> --- a/sysdeps/unix/sysv/linux/nanosleep.c
> +++ b/sysdeps/unix/sysv/linux/nanosleep.c
> @@ -25,7 +25,14 @@ int
>  __nanosleep (const struct timespec *requested_time,
>              struct timespec *remaining)
>  {
> -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
> +  //return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
> +  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
> +  if (ret != 0)
> +    {
> +      __set_errno (-ret);
> +      return -1;
> +    }
> +  return ret;
>  }
>  hidden_def (__nanosleep)
>  weak_alias (__nanosleep, nanosleep)
  

Patch

diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
index d385085c61..475abb5004 100644
--- a/sysdeps/unix/sysv/linux/Versions
+++ b/sysdeps/unix/sysv/linux/Versions
@@ -187,5 +187,6 @@  libc {
     __sigtimedwait;
     # functions used by nscd
     __netlink_assert_response;
+    __clock_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
index 6787909248..1e2481847b 100644
--- a/sysdeps/unix/sysv/linux/nanosleep.c
+++ b/sysdeps/unix/sysv/linux/nanosleep.c
@@ -25,7 +25,14 @@  int
 __nanosleep (const struct timespec *requested_time,
             struct timespec *remaining)
 {
-  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
+  //return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
+  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
+  if (ret != 0)
+    {
+      __set_errno (-ret);
+      return -1;
+    }
+  return ret;
 }
 hidden_def (__nanosleep)
 weak_alias (__nanosleep, nanosleep)