[RFC v4 00/24] RISC-V glibc port for the 32-bit

classic Classic list List threaded Threaded
73 messages Options
1234
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 02/24] sysdeps/nanosleep: Use clock_nanosleep_time64 if avaliable

Stepan Golosunov
14.08.2019 в 11:20:54 -0700 Alistair Francis написал:

>    On Mon, Aug 12, 2019 at 10:22 AM Joseph Myers
> <[hidden email]> wrote:
> > The pattern we have previously discussed for 64-bit time support is that
> > the code should (a) define the function (__thrd_sleep_time64, say) for
> > 64-bit time (which then only needs conversions in the reverse direction -
> > if the 64-bit syscall is not in fact available, but the 32-bit one is),
> > (b) if __TIMESIZE != 64, defines the 32-bit function as a thin wrapper
> > round that, (c) in the appropriate internal header, has a #define of the
>
> Doesn't having a thing wrapper around __thrd_sleep64() result in
> unnecessary conversions? When __NR_clock_nanosleep_time64 is not
> defined we will end up converting a 32-bit time_t to a 64-bit time_t
> just to convert it back to a 32-bit time_t.

The only purpose of handling __NR_clock_nanosleep_time64 being not
defined (when __ASSUME_TIME64_SYSCALLS is not defined too) is just to
avoid compilation failure with old kernel headers.  There is no point
to optimize this case.  And Florian proposed patches that remove it
altogether.

> It seems simpler to me to just keep the structure here and fix the
> 32-bit time_t when __ASSUME_TIME64_SYSCALLS is defined. That will
> probably result in a helper function for
> defined(__ASSUME_TIME64_SYSCALLS) ||
> __NR_clock_nanosleep_time64 as they are very similar.

If you add support for a 64-bit time version of a function on
__TIMESIZE==32 architectures, you'll end up with 2 functions.  And it
is much simpler to have 32-bit-time one as a thin wrapper around
64-bit-time one.  Code will be more tested this way.

And if you do not add such support it should be sufficient to do just

#ifdef __NR_nanosleep
  int ret = INTERNAL_SYSCALL_CANCEL (nanosleep, err, time_point, remaining);
#else
  int ret = INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, CLOCK_MONOTONIC,
                                     0, time_point, remaining);
#endif

plus
#define __NR_clock_nanosleep __NR_clock_nanosleep64
in sysdep.h for rv32.

(I am not sure whether the #ifdef __NR_nanosleep part is needed.)
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Alistair Francis-2
In reply to this post by Joseph Myers
On Mon, Aug 12, 2019 at 12:46 PM Joseph Myers <[hidden email]> wrote:

>
> On Fri, 9 Aug 2019, Alistair Francis wrote:
>
> > diff --git a/sysdeps/unix/sysv/linux/timespec_get.c b/sysdeps/unix/sysv/linux/timespec_get.c
> > index 52080ddf08a..97ef9c5f799 100644
> > --- a/sysdeps/unix/sysv/linux/timespec_get.c
> > +++ b/sysdeps/unix/sysv/linux/timespec_get.c
> > @@ -24,6 +24,58 @@
> >  #endif
> >  #include <sysdep-vdso.h>
> >
> > +int
> > +__timespec_get (struct timespec *ts, int base)
> > +{
> > +#ifdef __ASSUME_TIME64_SYSCALLS
> > +  return INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
> > +#else
>
> This has the usual problems with missing conversions.

Is this the type of layout you are looking for? (untested, just a general idea)

int
__timespec_get64 (struct __timespec64 *ts, int base)
{
#ifdef __ASSUME_TIME64_SYSCALLS
  return INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
#else
  long int ret;
# ifdef __NR_clock_gettime64
  ret = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);

  if (ret == 0 || errno != ENOSYS)
    {
      return ret;
    }
# endif /* __NR_clock_gettime64 */
  struct timespec ts32;

  if (! in_time_t_range (ts->tv_sec))
    {
      __set_errno (EOVERFLOW);
      return -1;
    }

  valid_timespec64_to_timespec (ts, &ts32);
  ret = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);

  if ((ret == 0 || errno != ENOSYS) && ts)
    {
      ts->tv_sec = ts32.tv_sec;
      ts->tv_nsec = ts32.tv_nsec;
    }
  return ret;
#endif
}

#if __TIMESIZE != 64
int
__timespec_get32 (struct timespec *ts, int base)
{
  struct __timespec64 ts64;

  ret = __timespec_get64 (ts64, base);

  if ((ret == 0 || errno != ENOSYS) && ts)
    {
      ts->tv_sec = ts64.tv_sec;
      ts->tv_nsec = ts64.tv_nsec;
    }

  return ret;
}
#endif

#if __TIMESIZE == 64
# define __timespec_get __timespec_get64
#else
# define __timespec_get __timespec_get32
#endif

/* Set TS to calendar time based in time base BASE.  */
int
timespec_get (struct timespec *ts, int base)
{
  switch (base)
    {
      int res;
      INTERNAL_SYSCALL_DECL (err);
    case TIME_UTC:
      res = __timespec_get (ts, base);
      if (INTERNAL_SYSCALL_ERROR_P (res, err))
        return 0;
      break;

    default:
      return 0;
    }

  return base;
}

Alistair

>
> --
> Joseph S. Myers
> [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Joseph Myers
On Thu, 15 Aug 2019, Alistair Francis wrote:

> Is this the type of layout you are looking for? (untested, just a general idea)

I'm not convinced making timespec_get into a wrapper round another
function is a good thing.

You need to end up, in the __TIMESIZE == 32 case, with two functions that
have the public timespec_get semantics - not two internal functions and a
single wrapper - in preparation for when we support _TIME_BITS == 64.  So
I'd expect defining __timespec_get64 as a function with the public
semantics (not as an internal function that only calls the syscall), and
conditionally defining timespec_get as a thin wrapper in the case of
32-bit time_t, and having a #define of __timespec_get64 to timespec_get in
an internal header in the case of 64-bit time_t.

> int
> __timespec_get64 (struct __timespec64 *ts, int base)
> {
> #ifdef __ASSUME_TIME64_SYSCALLS
>   return INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
> #else

You need

# ifndef __NR_clock_gettime64
#  define __NR_clock_gettime64 __NR_clock_gettime
# endif

here, because 64-bit platforms define __ASSUME_TIME64_SYSCALLS but with
unsuffixed syscall names.

>   long int ret;
> # ifdef __NR_clock_gettime64
>   ret = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
>
>   if (ret == 0 || errno != ENOSYS)
>     {
>       return ret;
>     }

You need to check INTERNAL_SYSCALL_ERRNO (...) not errno; errno isn't set
by INTERNAL_*, only by INLINE_*.

No braces when only a single statement is inside them.

>   ret = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);
>
>   if ((ret == 0 || errno != ENOSYS) && ts)

No need to consider ENOSYS here.  NULL ts isn't valid so no need to check
for it being non-NULL.

> /* Set TS to calendar time based in time base BASE.  */
> int
> timespec_get (struct timespec *ts, int base)
> {
>   switch (base)
>     {
>       int res;
>       INTERNAL_SYSCALL_DECL (err);
>     case TIME_UTC:
>       res = __timespec_get (ts, base);
>       if (INTERNAL_SYSCALL_ERROR_P (res, err))
>         return 0;

This wrapper is using an uninitialized variable err.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Alistair Francis-2
On Thu, Aug 15, 2019 at 12:46 PM Joseph Myers <[hidden email]> wrote:

>
> On Thu, 15 Aug 2019, Alistair Francis wrote:
>
> > Is this the type of layout you are looking for? (untested, just a general idea)
>
> I'm not convinced making timespec_get into a wrapper round another
> function is a good thing.
>
> You need to end up, in the __TIMESIZE == 32 case, with two functions that
> have the public timespec_get semantics - not two internal functions and a
> single wrapper - in preparation for when we support _TIME_BITS == 64.  So
> I'd expect defining __timespec_get64 as a function with the public
> semantics (not as an internal function that only calls the syscall), and
> conditionally defining timespec_get as a thin wrapper in the case of
> 32-bit time_t, and having a #define of __timespec_get64 to timespec_get in
> an internal header in the case of 64-bit time_t.

Ok, so more like this?

#if __TIMESIZE == 64
# define timespec_get __timespec_get64
#else
# define timespec_get __timespec_get32
#endif

/* Set TS to calendar time based in time base BASE.  */
int
__timespec_get64 (struct timespec *ts, int base)
{
  switch (base)
    {
      int res;
      INTERNAL_SYSCALL_DECL (err);
    case TIME_UTC:
#ifdef __ASSUME_TIME64_SYSCALLS
  res = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
#else
# ifdef __NR_clock_gettime64
  res = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
# endif /* __NR_clock_gettime64 */
  struct timespec ts32;

  if (! in_time_t_range (ts->tv_sec))
    {
      __set_errno (EOVERFLOW);
      return -1;
    }

  valid_timespec64_to_timespec (ts, &ts32);
  res = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);

  if (res == 0 || !INTERNAL_SYSCALL_ERROR_P (res, err))
    {
      ts->tv_sec = ts32.tv_sec;
      ts->tv_nsec = ts32.tv_nsec;
    }
#endif

      if (INTERNAL_SYSCALL_ERROR_P (res, err))
        return 0;
      break;

    default:
      return 0;
    }

  return base;
}

#if __TIMESIZE != 64
int
__timespec_get32 (struct timespec *ts, int base)
{
  struct __timespec64 ts64;

  ret = __timespec_get64 (ts64, base);

  if (res == 0 || !INTERNAL_SYSCALL_ERROR_P (res, err))
    {
      ts->tv_sec = ts64.tv_sec;
      ts->tv_nsec = ts64.tv_nsec;
    }

  return ret;
}
#endif

>
> > int
> > __timespec_get64 (struct __timespec64 *ts, int base)
> > {
> > #ifdef __ASSUME_TIME64_SYSCALLS
> >   return INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
> > #else
>
> You need
>
> # ifndef __NR_clock_gettime64
> #  define __NR_clock_gettime64 __NR_clock_gettime
> # endif
>
> here, because 64-bit platforms define __ASSUME_TIME64_SYSCALLS but with
> unsuffixed syscall names.

The kernel defines 64 suffixed syscalls, is this required because
older kernels don't do this?

>
> >   long int ret;
> > # ifdef __NR_clock_gettime64
> >   ret = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
> >
> >   if (ret == 0 || errno != ENOSYS)
> >     {
> >       return ret;
> >     }
>
> You need to check INTERNAL_SYSCALL_ERRNO (...) not errno; errno isn't set
> by INTERNAL_*, only by INLINE_*.

Fixed! Thanks

>
> No braces when only a single statement is inside them.
>
> >   ret = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);
> >
> >   if ((ret == 0 || errno != ENOSYS) && ts)
>
> No need to consider ENOSYS here.  NULL ts isn't valid so no need to check
> for it being non-NULL.

Fixed

>
> > /* Set TS to calendar time based in time base BASE.  */
> > int
> > timespec_get (struct timespec *ts, int base)
> > {
> >   switch (base)
> >     {
> >       int res;
> >       INTERNAL_SYSCALL_DECL (err);
> >     case TIME_UTC:
> >       res = __timespec_get (ts, base);
> >       if (INTERNAL_SYSCALL_ERROR_P (res, err))
> >         return 0;
>
> This wrapper is using an uninitialized variable err.

Good point, fixed.

Alistair

>
> --
> Joseph S. Myers
> [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Joseph Myers
On Thu, 15 Aug 2019, Alistair Francis wrote:

> Ok, so more like this?
>
> #if __TIMESIZE == 64
> # define timespec_get __timespec_get64
> #else
> # define timespec_get __timespec_get32
> #endif

No.  Please see what's done for mktime, for example (but it's simpler here
because mktime supports being built outside of glibc, which is irrelevant
for timespec_get).

* The function __mktime64 is defined, unconditionally.

* The function mktime is defined as a thin wrapper, conditionally (only
when 32-bit time is supported).

* There's no __mktime32 anywhere.

* mktime-internal.h deals with defining __mktime64 back to mktime in the
case where __TIMESIZE == 64 and so only a single function is needed with
no wrapper.

> > # ifndef __NR_clock_gettime64
> > #  define __NR_clock_gettime64 __NR_clock_gettime
> > # endif
> >
> > here, because 64-bit platforms define __ASSUME_TIME64_SYSCALLS but with
> > unsuffixed syscall names.
>
> The kernel defines 64 suffixed syscalls, is this required because
> older kernels don't do this?

The kernel does *not* define 64-suffixed syscalls on platforms where
__SYSCALL_WORDSIZE == 64.  It defined unsuffixed syscalls with the same
semantics.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Alistair Francis-2
On Thu, Aug 15, 2019 at 1:59 PM Joseph Myers <[hidden email]> wrote:

>
> On Thu, 15 Aug 2019, Alistair Francis wrote:
>
> > Ok, so more like this?
> >
> > #if __TIMESIZE == 64
> > # define timespec_get __timespec_get64
> > #else
> > # define timespec_get __timespec_get32
> > #endif
>
> No.  Please see what's done for mktime, for example (but it's simpler here
> because mktime supports being built outside of glibc, which is irrelevant
> for timespec_get).
>
> * The function __mktime64 is defined, unconditionally.
>
> * The function mktime is defined as a thin wrapper, conditionally (only
> when 32-bit time is supported).
>
> * There's no __mktime32 anywhere.
>
> * mktime-internal.h deals with defining __mktime64 back to mktime in the
> case where __TIMESIZE == 64 and so only a single function is needed with
> no wrapper.

There is no internal header for timespec_get, would you prefer me to
create one or put the define in time/time.h?

>
> > > # ifndef __NR_clock_gettime64
> > > #  define __NR_clock_gettime64 __NR_clock_gettime
> > > # endif
> > >
> > > here, because 64-bit platforms define __ASSUME_TIME64_SYSCALLS but with
> > > unsuffixed syscall names.
> >
> > The kernel defines 64 suffixed syscalls, is this required because
> > older kernels don't do this?
>
> The kernel does *not* define 64-suffixed syscalls on platforms where
> __SYSCALL_WORDSIZE == 64.  It defined unsuffixed syscalls with the same
> semantics.

Ah, you are right. I have added these defines.

Alistair

>
> --
> Joseph S. Myers
> [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Joseph Myers
On Thu, 15 Aug 2019, Alistair Francis wrote:

> There is no internal header for timespec_get, would you prefer me to
> create one or put the define in time/time.h?

time/time.h is an installed header, so it mustn't go there.  
include/time.h would be fine (it has several such defines, for
__localtime64 etc.).

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Alistair Francis-2
On Thu, Aug 15, 2019 at 2:19 PM Joseph Myers <[hidden email]> wrote:
>
> On Thu, 15 Aug 2019, Alistair Francis wrote:
>
> > There is no internal header for timespec_get, would you prefer me to
> > create one or put the define in time/time.h?
>
> time/time.h is an installed header, so it mustn't go there.
> include/time.h would be fine (it has several such defines, for
> __localtime64 etc.).

Ok, I have this in include/time.h:

#if __TIMESIZE == 64
#define timespec_get __timespec_get64
#else

and this in the c file:

/* Set TS to calendar time based in time base BASE.  */
int
__timespec_get64 (struct timespec *ts, int base)
{
  switch (base)
    {
      int res;
      INTERNAL_SYSCALL_DECL (err);
    case TIME_UTC:
#ifdef __ASSUME_TIME64_SYSCALLS
# ifndef __NR_clock_gettime64
#  define __NR_clock_gettime64 __NR_clock_gettime
# endif
  res = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
#else
# ifdef __NR_clock_gettime64
  res = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
# endif /* __NR_clock_gettime64 */
  struct timespec ts32;

  if (! in_time_t_range (ts->tv_sec))
    {
      __set_errno (EOVERFLOW);
      return -1;
    }

  valid_timespec64_to_timespec (ts, &ts32);
  res = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);

  if (res == 0 || !INTERNAL_SYSCALL_ERROR_P (res, err))
    {
      ts->tv_sec = ts32.tv_sec;
      ts->tv_nsec = ts32.tv_nsec;
    }
#endif

      if (INTERNAL_SYSCALL_ERROR_P (res, err))
        return 0;
      break;

    default:
      return 0;
    }

  return base;
}

#if __TIMESIZE != 64
int
timespec_get (struct timespec *ts, int base)
{
  struct __timespec64 ts64;

  ret = __timespec_get64 (ts64, base);

  if (res == 0 || !INTERNAL_SYSCALL_ERROR_P (res, err))
    {
      ts->tv_sec = ts64.tv_sec;
      ts->tv_nsec = ts64.tv_nsec;
    }

  return ret;
}
#endif

Alistair

>
> --
> Joseph S. Myers
> [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Joseph Myers
On Thu, 15 Aug 2019, Alistair Francis wrote:

> On Thu, Aug 15, 2019 at 2:19 PM Joseph Myers <[hidden email]> wrote:
> >
> > On Thu, 15 Aug 2019, Alistair Francis wrote:
> >
> > > There is no internal header for timespec_get, would you prefer me to
> > > create one or put the define in time/time.h?
> >
> > time/time.h is an installed header, so it mustn't go there.
> > include/time.h would be fine (it has several such defines, for
> > __localtime64 etc.).
>
> Ok, I have this in include/time.h:
>
> #if __TIMESIZE == 64
> #define timespec_get __timespec_get64
> #else

That should be the other way round (defining __timespec_get64 to
timespec_get so that timespec_get gets properly exported from libc).

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

Alistair Francis-2
On Thu, Aug 15, 2019 at 2:28 PM Joseph Myers <[hidden email]> wrote:

>
> On Thu, 15 Aug 2019, Alistair Francis wrote:
>
> > On Thu, Aug 15, 2019 at 2:19 PM Joseph Myers <[hidden email]> wrote:
> > >
> > > On Thu, 15 Aug 2019, Alistair Francis wrote:
> > >
> > > > There is no internal header for timespec_get, would you prefer me to
> > > > create one or put the define in time/time.h?
> > >
> > > time/time.h is an installed header, so it mustn't go there.
> > > include/time.h would be fine (it has several such defines, for
> > > __localtime64 etc.).
> >
> > Ok, I have this in include/time.h:
> >
> > #if __TIMESIZE == 64
> > #define timespec_get __timespec_get64
> > #else
>
> That should be the other way round (defining __timespec_get64 to
> timespec_get so that timespec_get gets properly exported from libc).

Ok.

Thanks for your help with this. I'll update the other patches with a
similar style and look at sending a full patch series after running
tests.

Alistair

>
> --
> Joseph S. Myers
> [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 23/24] WIP: syscall.list: Call 64-bit versions of syscalls

Alistair Francis-2
In reply to this post by Florian Weimer-5
On Wed, Aug 14, 2019 at 11:57 AM Florian Weimer <[hidden email]> wrote:

>
> * Alistair Francis:
>
> > On Fri, Aug 9, 2019 at 6:04 PM Alistair Francis
> > <[hidden email]> wrote:
> >>
> >> Signed-off-by: Alistair Francis <[hidden email]>
> >> ---
> >>  sysdeps/unix/sysv/linux/syscalls.list | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> >> index e374f97b5f8..4844d1a9a3b 100644
> >> --- a/sysdeps/unix/sysv/linux/syscalls.list
> >> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> >> @@ -5,7 +5,7 @@ alarm           -       alarm           i:i     alarm
> >>  bdflush                EXTRA   bdflush         i:ii    __compat_bdflush        bdflush@GLIBC_2.0:GLIBC_2.23
> >>  capget         EXTRA   capget          i:pp    capget
> >>  capset         EXTRA   capset          i:pp    capset
> >> -clock_adjtime  EXTRA   clock_adjtime   i:ip    clock_adjtime
> >> +clock_adjtime  EXTRA   clock_adjtime64 i:ip    clock_adjtime
> >>  create_module  EXTRA   create_module   3       __compat_create_module  create_module@GLIBC_2.0:GLIBC_2.23
> >>  delete_module  EXTRA   delete_module   3       delete_module
> >>  epoll_create   EXTRA   epoll_create    i:i     epoll_create
> >> @@ -52,7 +52,7 @@ sched_getp    -       sched_getparam  i:ip    __sched_getparam        sched_getparam
> >>  sched_gets     -       sched_getscheduler      i:i     __sched_getscheduler    sched_getscheduler
> >>  sched_primax   -       sched_get_priority_max  i:i     __sched_get_priority_max        sched_get_priority_max
> >>  sched_primin   -       sched_get_priority_min  i:i     __sched_get_priority_min        sched_get_priority_min
> >> -sched_rr_gi    -       sched_rr_get_interval   i:ip    __sched_rr_get_interval sched_rr_get_interval
> >> +sched_rr_gi    -       sched_rr_get_interval_time64    i:ip    __sched_rr_get_interval sched_rr_get_interval
> >>  sched_setp     -       sched_setparam  i:ip    __sched_setparam        sched_setparam
> >>  sched_sets     -       sched_setscheduler      i:iip   __sched_setscheduler    sched_setscheduler
> >>  sched_yield    -       sched_yield     i:      __sched_yield   sched_yield
> >> @@ -96,8 +96,8 @@ fremovexattr  -       fremovexattr    i:is    fremovexattr
> >>  mq_setattr     -       mq_getsetattr   i:ipp   mq_setattr
> >>
> >>  timerfd_create EXTRA   timerfd_create  i:ii    timerfd_create
> >> -timerfd_settime        EXTRA   timerfd_settime i:iipp  timerfd_settime
> >> -timerfd_gettime        EXTRA   timerfd_gettime i:ip    timerfd_gettime
> >> +timerfd_settime        EXTRA   timerfd_settime64       i:iipp  timerfd_settime
> >> +timerfd_gettime        EXTRA   timerfd_gettime64       i:ip    timerfd_gettime
> >
> > Does anyone have ideas/opinions on how to handle this correctly?
>
> As in many of the other cases, you can add this to <sysdep.h>:
>
> #define __NR_timerfd_settime __NR_timerfd_settime64
> #define __NR_timerfd_gettime __NR_timerfd_gettime64
>
> Once a second such port arrives, we can factor out these common system
> call renamings into a generic-64 subdirectory.

Great! That's a straightforward fix.

Alistair

>
> I still think it's just wrong that the kernel doesn't provide these
> names as part of the UAPI headers.
>
> Thanks,
> Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 23/24] WIP: syscall.list: Call 64-bit versions of syscalls

Florian Weimer-5
* Alistair Francis:

> On Wed, Aug 14, 2019 at 11:57 AM Florian Weimer <[hidden email]> wrote:
>>
>> * Alistair Francis:
>>
>> > On Fri, Aug 9, 2019 at 6:04 PM Alistair Francis
>> > <[hidden email]> wrote:
>> >>
>> >> Signed-off-by: Alistair Francis <[hidden email]>
>> >> ---
>> >>  sysdeps/unix/sysv/linux/syscalls.list | 8 ++++----
>> >>  1 file changed, 4 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
>> >> index e374f97b5f8..4844d1a9a3b 100644
>> >> --- a/sysdeps/unix/sysv/linux/syscalls.list
>> >> +++ b/sysdeps/unix/sysv/linux/syscalls.list
>> >> @@ -5,7 +5,7 @@ alarm           -       alarm           i:i     alarm
>> >>  bdflush                EXTRA   bdflush         i:ii    __compat_bdflush        bdflush@GLIBC_2.0:GLIBC_2.23
>> >>  capget         EXTRA   capget          i:pp    capget
>> >>  capset         EXTRA   capset          i:pp    capset
>> >> -clock_adjtime  EXTRA   clock_adjtime   i:ip    clock_adjtime
>> >> +clock_adjtime  EXTRA   clock_adjtime64 i:ip    clock_adjtime
>> >>  create_module  EXTRA   create_module   3       __compat_create_module  create_module@GLIBC_2.0:GLIBC_2.23
>> >>  delete_module  EXTRA   delete_module   3       delete_module
>> >>  epoll_create   EXTRA   epoll_create    i:i     epoll_create
>> >> @@ -52,7 +52,7 @@ sched_getp    -       sched_getparam  i:ip    __sched_getparam        sched_getparam
>> >>  sched_gets     -       sched_getscheduler      i:i     __sched_getscheduler    sched_getscheduler
>> >>  sched_primax   -       sched_get_priority_max  i:i     __sched_get_priority_max        sched_get_priority_max
>> >>  sched_primin   -       sched_get_priority_min  i:i     __sched_get_priority_min        sched_get_priority_min
>> >> -sched_rr_gi    -       sched_rr_get_interval   i:ip    __sched_rr_get_interval sched_rr_get_interval
>> >> +sched_rr_gi    -       sched_rr_get_interval_time64    i:ip    __sched_rr_get_interval sched_rr_get_interval
>> >>  sched_setp     -       sched_setparam  i:ip    __sched_setparam        sched_setparam
>> >>  sched_sets     -       sched_setscheduler      i:iip   __sched_setscheduler    sched_setscheduler
>> >>  sched_yield    -       sched_yield     i:      __sched_yield   sched_yield
>> >> @@ -96,8 +96,8 @@ fremovexattr  -       fremovexattr    i:is    fremovexattr
>> >>  mq_setattr     -       mq_getsetattr   i:ipp   mq_setattr
>> >>
>> >>  timerfd_create EXTRA   timerfd_create  i:ii    timerfd_create
>> >> -timerfd_settime        EXTRA   timerfd_settime i:iipp  timerfd_settime
>> >> -timerfd_gettime        EXTRA   timerfd_gettime i:ip    timerfd_gettime
>> >> +timerfd_settime        EXTRA   timerfd_settime64       i:iipp  timerfd_settime
>> >> +timerfd_gettime        EXTRA   timerfd_gettime64       i:ip    timerfd_gettime
>> >
>> > Does anyone have ideas/opinions on how to handle this correctly?
>>
>> As in many of the other cases, you can add this to <sysdep.h>:
>>
>> #define __NR_timerfd_settime __NR_timerfd_settime64
>> #define __NR_timerfd_gettime __NR_timerfd_gettime64
>>
>> Once a second such port arrives, we can factor out these common system
>> call renamings into a generic-64 subdirectory.
>
> Great! That's a straightforward fix.

Note that getting this to work on legacy architectures (which have a
32-bit time_t) today obviously requires vastly different steps.

I do not know if completing this work is a requirement for acceptance of
the RV32 port (I don't think it should be, but others might disagree).

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v4 24/24] timerfd_settime: Use 64-bit call if avaliable

Maciej W. Rozycki-5
In reply to this post by Alistair Francis-2
On Fri, 9 Aug 2019, Alistair Francis wrote:

> This commit was clearly not tested very well (it's missing
> semicolons), just see it more as pseudo code.
[...]
> > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> > index 1ab6bcbfc81..89128e80868 100644
> > --- a/sysdeps/unix/sysv/linux/Makefile
> > +++ b/sysdeps/unix/sysv/linux/Makefile
> > @@ -19,6 +19,8 @@ sysdep_routines += clone umount umount2 readahead \
> >                    eventfd eventfd_read eventfd_write prlimit \
> >                    personality epoll_wait tee vmsplice splice \
> >                    open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get

 Missing trailing backslash here too, which breaks building.

> > +                  timerfd_settime

  Maciej
1234