diff mbox series

[v2,5/5] y2038: Replace __clock_gettime with __clock_gettime64

Message ID 20200326080641.10193-6-lukma@denx.de
State Not applicable
Delegated to: Lukasz Majewski
Headers
Series y2038: Replace __clock_gettime with __clock_gettime64 |

Commit Message

Lukasz Majewski March 26, 2020, 8:06 a.m. UTC 
  The __clock_gettime internal function is not supporting 64 bit time on
architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like e.g. ARM 32
bit).

The __clock_gettime64 shall be used instead in the glibc itself as it
supports 64 bit time on those systems.
This change does not bring any change to systems with __WORDSIZE == 64 as
for them the __clock_gettime64 is aliased to __clock_gettime
(in ./include/time.h).

---
Changes for v2:
- Use only TIMESPEC_TO_TIMEVAL instead of valid_timespec64_to_timeval in
  logout.c and logwtmp.c as it is generic enough to also support struct
  __timespec64 conversion to struct timeval
---
 benchtests/bench-timing.h                        | 2 +-
 include/random-bits.h                            | 4 ++--
 login/logout.c                                   | 4 ++--
 login/logwtmp.c                                  | 5 +++--
 nis/nis_call.c                                   | 6 +++---
 sysdeps/generic/hp-timing.h                      | 4 ++--
 sysdeps/generic/memusage.h                       | 4 ++--
 sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c | 4 ++--
 sysdeps/unix/sysv/linux/clock.c                  | 7 ++-----
 9 files changed, 19 insertions(+), 21 deletions(-)

Comments

Florian Weimer April 6, 2020, 4:07 p.m. UTC | #1 
* Lukasz Majewski:

> The __clock_gettime internal function is not supporting 64 bit time on
> architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like e.g. ARM 32
> bit).
>
> The __clock_gettime64 shall be used instead in the glibc itself as it

The __clock_gettime64 function?

> supports 64 bit time on those systems.
> This change does not bring any change to systems with __WORDSIZE == 64 as
> for them the __clock_gettime64 is aliased to __clock_gettime
> (in ./include/time.h).

Should this patch remove the __clock_gettime GLIBC_PRIVATE export?
Lukasz Majewski April 6, 2020, 9:03 p.m. UTC | #2 
Hi Florian,

> * Lukasz Majewski:
> 
> > The __clock_gettime internal function is not supporting 64 bit time
> > on architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like
> > e.g. ARM 32 bit).
> >
> > The __clock_gettime64 shall be used instead in the glibc itself as
> > it  
> 
> The __clock_gettime64 function?

Yes. Correct. Thanks for spotting it.

> 
> > supports 64 bit time on those systems.
> > This change does not bring any change to systems with __WORDSIZE ==
> > 64 as for them the __clock_gettime64 is aliased to __clock_gettime
> > (in ./include/time.h).  
> 
> Should this patch remove the __clock_gettime GLIBC_PRIVATE export?

No. This work is just a first step. The nptl and pthread code still use
__clock_gettime internally. I will prepare __clock_gettime conversion
to __clock_gettime64 for those parts of glibc in latter patches.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Lukasz Majewski April 14, 2020, 12:14 p.m. UTC | #3 
Dear Community, 


> Hi Florian,
> 
> > * Lukasz Majewski:
> >   
> > > The __clock_gettime internal function is not supporting 64 bit
> > > time on architectures with __WORDSIZE == 32 and __TIMESIZE != 64
> > > (like e.g. ARM 32 bit).
> > >
> > > The __clock_gettime64 shall be used instead in the glibc itself as
> > > it    
> > 
> > The __clock_gettime64 function?  
> 
> Yes. Correct. Thanks for spotting it.
> 
> >   
> > > supports 64 bit time on those systems.
> > > This change does not bring any change to systems with __WORDSIZE
> > > == 64 as for them the __clock_gettime64 is aliased to
> > > __clock_gettime (in ./include/time.h).    
> > 
> > Should this patch remove the __clock_gettime GLIBC_PRIVATE export?  
> 
> No. This work is just a first step. The nptl and pthread code still
> use __clock_gettime internally. I will prepare __clock_gettime
> conversion to __clock_gettime64 for those parts of glibc in latter
> patches.

Are there any more comments regarding this patch set? In other word -
is it eligible for pulling?

> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Lukasz Majewski April 21, 2020, 1:48 p.m. UTC | #4 
Dear Community,

> Dear Community, 
> 
> 
> > Hi Florian,
> >   
> > > * Lukasz Majewski:
> > >     
> > > > The __clock_gettime internal function is not supporting 64 bit
> > > > time on architectures with __WORDSIZE == 32 and __TIMESIZE != 64
> > > > (like e.g. ARM 32 bit).
> > > >
> > > > The __clock_gettime64 shall be used instead in the glibc itself
> > > > as it      
> > > 
> > > The __clock_gettime64 function?    
> > 
> > Yes. Correct. Thanks for spotting it.
> >   
> > >     
> > > > supports 64 bit time on those systems.
> > > > This change does not bring any change to systems with __WORDSIZE
> > > > == 64 as for them the __clock_gettime64 is aliased to
> > > > __clock_gettime (in ./include/time.h).      
> > > 
> > > Should this patch remove the __clock_gettime GLIBC_PRIVATE
> > > export?    
> > 
> > No. This work is just a first step. The nptl and pthread code still
> > use __clock_gettime internally. I will prepare __clock_gettime
> > conversion to __clock_gettime64 for those parts of glibc in latter
> > patches.  
> 
> Are there any more comments regarding this patch set? In other word -
> is it eligible for pulling?

Are there any more comments?

> 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma@denx.de  
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Adhemerval Zanella April 30, 2020, 9:03 p.m. UTC | #5 
On 26/03/2020 05:06, Lukasz Majewski wrote:
> The __clock_gettime internal function is not supporting 64 bit time on
> architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like e.g. ARM 32
> bit).
> 
> The __clock_gettime64 shall be used instead in the glibc itself as it
> supports 64 bit time on those systems.
> This change does not bring any change to systems with __WORDSIZE == 64 as
> for them the __clock_gettime64 is aliased to __clock_gettime
> (in ./include/time.h).
> 
> ---
> Changes for v2:
> - Use only TIMESPEC_TO_TIMEVAL instead of valid_timespec64_to_timeval in
>   logout.c and logwtmp.c as it is generic enough to also support struct
>   __timespec64 conversion to struct timeval
> ---
>  benchtests/bench-timing.h                        | 2 +-
>  include/random-bits.h                            | 4 ++--
>  login/logout.c                                   | 4 ++--
>  login/logwtmp.c                                  | 5 +++--
>  nis/nis_call.c                                   | 6 +++---
>  sysdeps/generic/hp-timing.h                      | 4 ++--
>  sysdeps/generic/memusage.h                       | 4 ++--
>  sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c | 4 ++--
>  sysdeps/unix/sysv/linux/clock.c                  | 7 ++-----
>  9 files changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/benchtests/bench-timing.h b/benchtests/bench-timing.h
> index 5b9a8384bb..a0d6f82465 100644
> --- a/benchtests/bench-timing.h
> +++ b/benchtests/bench-timing.h
> @@ -18,7 +18,7 @@
>  
>  #undef attribute_hidden
>  #define attribute_hidden
> -#define __clock_gettime clock_gettime
> +#define __clock_gettime __clock_gettime64
>  #include <hp-timing.h>
>  #include <stdint.h>
>  

Ok, although we might revise it later and build the benchmarks
with __TIMESIZE==64.

> diff --git a/include/random-bits.h b/include/random-bits.h
> index fd3fa01f9b..7561e55ca6 100644
> --- a/include/random-bits.h
> +++ b/include/random-bits.h
> @@ -30,8 +30,8 @@
>  static inline uint32_t
>  random_bits (void)
>  {
> -  struct timespec tv;
> -  __clock_gettime (CLOCK_MONOTONIC, &tv);
> +  struct __timespec64 tv;
> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>    /* Shuffle the lower bits to minimize the clock bias.  */
>    uint32_t ret = tv.tv_nsec ^ tv.tv_sec;
>    ret ^= (ret << 24) | (ret >> 8);

Ok.

> diff --git a/login/logout.c b/login/logout.c
> index 7653fe8886..091312eb1d 100644
> --- a/login/logout.c
> +++ b/login/logout.c
> @@ -47,8 +47,8 @@ logout (const char *line)
>        memset (ut->ut_name, '\0', sizeof ut->ut_name);
>        memset (ut->ut_host, '\0', sizeof ut->ut_host);
>  
> -      struct timespec ts;
> -      __clock_gettime (CLOCK_REALTIME, &ts);
> +      struct __timespec64 ts;
> +      __clock_gettime64 (CLOCK_REALTIME, &ts);
>        TIMESPEC_TO_TIMEVAL (&ut->ut_tv, &ts);
>        ut->ut_type = DEAD_PROCESS;
>  

Ok.

> diff --git a/login/logwtmp.c b/login/logwtmp.c
> index 90406acc3d..050219c153 100644
> --- a/login/logwtmp.c
> +++ b/login/logwtmp.c
> @@ -21,6 +21,7 @@
>  #include <time.h>
>  #include <unistd.h>
>  #include <utmp.h>
> +#include <struct___timespec64.h>
>  
>  
>  void
> @@ -36,8 +37,8 @@ logwtmp (const char *line, const char *name, const char *host)
>    strncpy (ut.ut_name, name, sizeof ut.ut_name);
>    strncpy (ut.ut_host, host, sizeof ut.ut_host);
>  
> -  struct timespec ts;
> -  __clock_gettime (CLOCK_REALTIME, &ts);
> +  struct __timespec64 ts;
> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
>    TIMESPEC_TO_TIMEVAL (&ut.ut_tv, &ts);
>  
>    updwtmp (_PATH_WTMP, &ut);

Ok.

> diff --git a/nis/nis_call.c b/nis/nis_call.c
> index 92c70e97aa..9c6f62a753 100644
> --- a/nis/nis_call.c
> +++ b/nis/nis_call.c
> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int search_parent,
>    nis_error status;
>    directory_obj *obj;
>    struct timeval now;
> -  struct timespec ts;
> +  struct __timespec64 ts;
>    unsigned int server_used = ~0;
>    unsigned int current_ep = ~0;
>  
> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int search_parent,
>    if (*dir != NULL)
>      return NIS_SUCCESS;
>  
> -  __clock_gettime (CLOCK_REALTIME, &ts);
> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> +  now = valid_timespec64_to_timeval (ts);
>  
>    if ((flags & NO_CACHE) == 0)
>      *dir = nis_server_cache_search (name, search_parent, &server_used,

I think it would be simpler to just remove the timeval argument on
nis_server_cache_search and move the __clock_gettime64 call on the
function start. 

Also, it would require to change nis_server_cache to use a
__time64_t for 'expires', otherwise this change won't help in
case of a time_t overflow.


> diff --git a/sysdeps/generic/hp-timing.h b/sysdeps/generic/hp-timing.h
> index e2d7447212..af9d92f7f7 100644
> --- a/sysdeps/generic/hp-timing.h
> +++ b/sysdeps/generic/hp-timing.h
> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
>     vDSO symbol.  */
>  #define HP_TIMING_NOW(var) \
>  ({								\
> -  struct timespec tv;						\
> -  __clock_gettime (CLOCK_MONOTONIC, &tv);			\
> +  struct __timespec64 tv;						\
> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);			\
>    (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);	\
>  })
>  

Ok.

> diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
> index a111864b0b..91e56d24de 100644
> --- a/sysdeps/generic/memusage.h
> +++ b/sysdeps/generic/memusage.h
> @@ -28,9 +28,9 @@
>  #ifndef GETTIME
>  # define GETTIME(low,high)						   \
>    {									   \
> -    struct timespec now;						   \
> +    struct __timespec64 now;						   \
>      uint64_t usecs;							   \
> -    clock_gettime (CLOCK_REALTIME, &now);				   \
> +    __clock_gettime64 (CLOCK_REALTIME, &now);				   \
>      usecs = (uint64_t)now.tv_nsec / 1000 + (uint64_t)now.tv_sec * 1000000; \
>      low = usecs & 0xffffffff;						   \
>      high = usecs >> 32;							   \

Is is the requirement to export __clock_gettime64 as a GLIBC_PRIVATE symbol?

In any case, I think we should try to avoid use internal symbols even for
distributed glibc libraries, so I think this change should go once we
start to export the clock_gettime64 as default symbol.

> diff --git a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
> index 8cf5d303f9..5075ae0444 100644
> --- a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
> @@ -35,8 +35,8 @@ __gettimeofday_tv32 (struct __timeval32 *restrict tv32, void *restrict tz)
>    if (__glibc_unlikely (tz != 0))
>      memset (tz, 0, sizeof (struct timezone));
>  
> -  struct timespec ts;
> -  __clock_gettime (CLOCK_REALTIME, &ts);
> +  struct __timespec64 ts;
> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
>  
>    *tv32 = valid_timespec_to_timeval32 (ts);
>    return 0;

Ok.

> diff --git a/sysdeps/unix/sysv/linux/clock.c b/sysdeps/unix/sysv/linux/clock.c
> index 24a8df0cf5..157ae8eb3f 100644
> --- a/sysdeps/unix/sysv/linux/clock.c
> +++ b/sysdeps/unix/sysv/linux/clock.c
> @@ -23,15 +23,12 @@
>  clock_t
>  clock (void)
>  {
> -  struct timespec ts;
> +  struct __timespec64 ts;
>  
>    _Static_assert (CLOCKS_PER_SEC == 1000000,
>  		  "CLOCKS_PER_SEC should be 1000000");
>  
> -  /* clock_gettime shouldn't fail here since CLOCK_PROCESS_CPUTIME_ID is
> -     supported since 2.6.12.  Check the return value anyway in case the kernel
> -     barfs on us for some reason.  */
> -  if (__glibc_unlikely (__clock_gettime (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0))
> +  if (__glibc_unlikely (__clock_gettime64 (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0))
>      return (clock_t) -1;
>  
>    return (ts.tv_sec * CLOCKS_PER_SEC
> 

Ok.
Lukasz Majewski May 1, 2020, 11:56 a.m. UTC | #6 
Hi Adhemerval,

> On 26/03/2020 05:06, Lukasz Majewski wrote:
> > The __clock_gettime internal function is not supporting 64 bit time
> > on architectures with __WORDSIZE == 32 and __TIMESIZE != 64 (like
> > e.g. ARM 32 bit).
> > 
> > The __clock_gettime64 shall be used instead in the glibc itself as
> > it supports 64 bit time on those systems.
> > This change does not bring any change to systems with __WORDSIZE ==
> > 64 as for them the __clock_gettime64 is aliased to __clock_gettime
> > (in ./include/time.h).
> > 
> > ---
> > Changes for v2:
> > - Use only TIMESPEC_TO_TIMEVAL instead of
> > valid_timespec64_to_timeval in logout.c and logwtmp.c as it is
> > generic enough to also support struct __timespec64 conversion to
> > struct timeval ---
> >  benchtests/bench-timing.h                        | 2 +-
> >  include/random-bits.h                            | 4 ++--
> >  login/logout.c                                   | 4 ++--
> >  login/logwtmp.c                                  | 5 +++--
> >  nis/nis_call.c                                   | 6 +++---
> >  sysdeps/generic/hp-timing.h                      | 4 ++--
> >  sysdeps/generic/memusage.h                       | 4 ++--
> >  sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c | 4 ++--
> >  sysdeps/unix/sysv/linux/clock.c                  | 7 ++-----
> >  9 files changed, 19 insertions(+), 21 deletions(-)
> > 
> > diff --git a/benchtests/bench-timing.h b/benchtests/bench-timing.h
> > index 5b9a8384bb..a0d6f82465 100644
> > --- a/benchtests/bench-timing.h
> > +++ b/benchtests/bench-timing.h
> > @@ -18,7 +18,7 @@
> >  
> >  #undef attribute_hidden
> >  #define attribute_hidden
> > -#define __clock_gettime clock_gettime
> > +#define __clock_gettime __clock_gettime64
> >  #include <hp-timing.h>
> >  #include <stdint.h>
> >    
> 
> Ok, although we might revise it later and build the benchmarks
> with __TIMESIZE==64.
> 
> > diff --git a/include/random-bits.h b/include/random-bits.h
> > index fd3fa01f9b..7561e55ca6 100644
> > --- a/include/random-bits.h
> > +++ b/include/random-bits.h
> > @@ -30,8 +30,8 @@
> >  static inline uint32_t
> >  random_bits (void)
> >  {
> > -  struct timespec tv;
> > -  __clock_gettime (CLOCK_MONOTONIC, &tv);
> > +  struct __timespec64 tv;
> > +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
> >    /* Shuffle the lower bits to minimize the clock bias.  */
> >    uint32_t ret = tv.tv_nsec ^ tv.tv_sec;
> >    ret ^= (ret << 24) | (ret >> 8);  
> 
> Ok.
> 
> > diff --git a/login/logout.c b/login/logout.c
> > index 7653fe8886..091312eb1d 100644
> > --- a/login/logout.c
> > +++ b/login/logout.c
> > @@ -47,8 +47,8 @@ logout (const char *line)
> >        memset (ut->ut_name, '\0', sizeof ut->ut_name);
> >        memset (ut->ut_host, '\0', sizeof ut->ut_host);
> >  
> > -      struct timespec ts;
> > -      __clock_gettime (CLOCK_REALTIME, &ts);
> > +      struct __timespec64 ts;
> > +      __clock_gettime64 (CLOCK_REALTIME, &ts);
> >        TIMESPEC_TO_TIMEVAL (&ut->ut_tv, &ts);
> >        ut->ut_type = DEAD_PROCESS;
> >    
> 
> Ok.
> 
> > diff --git a/login/logwtmp.c b/login/logwtmp.c
> > index 90406acc3d..050219c153 100644
> > --- a/login/logwtmp.c
> > +++ b/login/logwtmp.c
> > @@ -21,6 +21,7 @@
> >  #include <time.h>
> >  #include <unistd.h>
> >  #include <utmp.h>
> > +#include <struct___timespec64.h>
> >  
> >  
> >  void
> > @@ -36,8 +37,8 @@ logwtmp (const char *line, const char *name,
> > const char *host) strncpy (ut.ut_name, name, sizeof ut.ut_name);
> >    strncpy (ut.ut_host, host, sizeof ut.ut_host);
> >  
> > -  struct timespec ts;
> > -  __clock_gettime (CLOCK_REALTIME, &ts);
> > +  struct __timespec64 ts;
> > +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> >    TIMESPEC_TO_TIMEVAL (&ut.ut_tv, &ts);
> >  
> >    updwtmp (_PATH_WTMP, &ut);  
> 
> Ok.
> 
> > diff --git a/nis/nis_call.c b/nis/nis_call.c
> > index 92c70e97aa..9c6f62a753 100644
> > --- a/nis/nis_call.c
> > +++ b/nis/nis_call.c
> > @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
> > search_parent, nis_error status;
> >    directory_obj *obj;
> >    struct timeval now;
> > -  struct timespec ts;
> > +  struct __timespec64 ts;
> >    unsigned int server_used = ~0;
> >    unsigned int current_ep = ~0;
> >  
> > @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
> > search_parent, if (*dir != NULL)
> >      return NIS_SUCCESS;
> >  
> > -  __clock_gettime (CLOCK_REALTIME, &ts);
> > -  TIMESPEC_TO_TIMEVAL (&now, &ts);
> > +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> > +  now = valid_timespec64_to_timeval (ts);
> >  
> >    if ((flags & NO_CACHE) == 0)
> >      *dir = nis_server_cache_search (name, search_parent,
> > &server_used,  
> 
> I think it would be simpler to just remove the timeval argument on
> nis_server_cache_search and move the __clock_gettime64 call on the
> function start. 

Have I understood you correctly that you recommend removing the "now"
struct timeval argument and then call explicitly __clock_gettime64 on
the beginning of nis_server_cache_search function?

> 
> Also, it would require to change nis_server_cache to use a
> __time64_t for 'expires', otherwise this change won't help in
> case of a time_t overflow.
> 

Ok. I will update this. Thanks for pointing this out.

> 
> > diff --git a/sysdeps/generic/hp-timing.h
> > b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
> > --- a/sysdeps/generic/hp-timing.h
> > +++ b/sysdeps/generic/hp-timing.h
> > @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
> >     vDSO symbol.  */
> >  #define HP_TIMING_NOW(var) \
> >  ({								\
> > -  struct timespec tv;
> > 	\
> > -  __clock_gettime (CLOCK_MONOTONIC, &tv);			\
> > +  struct __timespec64 tv;
> > 	\
> > +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
> > \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);	\
> >  })
> >    
> 
> Ok.
> 
> > diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
> > index a111864b0b..91e56d24de 100644
> > --- a/sysdeps/generic/memusage.h
> > +++ b/sysdeps/generic/memusage.h
> > @@ -28,9 +28,9 @@
> >  #ifndef GETTIME
> >  # define GETTIME(low,high)
> > 	   \ {
> > 			   \
> > -    struct timespec now;
> > 	   \
> > +    struct __timespec64 now;
> > 		   \ uint64_t usecs;
> > 			   \
> > -    clock_gettime (CLOCK_REALTIME, &now);
> > 	   \
> > +    __clock_gettime64 (CLOCK_REALTIME, &now);
> > 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
> > (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
> > 					   \ high = usecs >>
> > 32;							   \  
> 
> Is is the requirement to export __clock_gettime64 as a GLIBC_PRIVATE
> symbol?
> 

The __clock_gettime is already exported as GLIBC_PRIVATE at
./time/Versions, so I'm following this pattern.

Moreover, the glibc will not build when __clock_gettime64 is not
exported.

> In any case, I think we should try to avoid use internal symbols even
> for distributed glibc libraries, so I think this change should go
> once we start to export the clock_gettime64 as default symbol.

Am I correct that this is a preprocessor macro, which is in the
exported header?

> 
> > diff --git a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
> > b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c index
> > 8cf5d303f9..5075ae0444 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c +++
> > b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c @@ -35,8 +35,8
> > @@ __gettimeofday_tv32 (struct __timeval32 *restrict tv32, void
> > *restrict tz) if (__glibc_unlikely (tz != 0)) memset (tz, 0, sizeof
> > (struct timezone)); 
> > -  struct timespec ts;
> > -  __clock_gettime (CLOCK_REALTIME, &ts);
> > +  struct __timespec64 ts;
> > +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> >  
> >    *tv32 = valid_timespec_to_timeval32 (ts);
> >    return 0;  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/clock.c
> > b/sysdeps/unix/sysv/linux/clock.c index 24a8df0cf5..157ae8eb3f
> > 100644 --- a/sysdeps/unix/sysv/linux/clock.c
> > +++ b/sysdeps/unix/sysv/linux/clock.c
> > @@ -23,15 +23,12 @@
> >  clock_t
> >  clock (void)
> >  {
> > -  struct timespec ts;
> > +  struct __timespec64 ts;
> >  
> >    _Static_assert (CLOCKS_PER_SEC == 1000000,
> >  		  "CLOCKS_PER_SEC should be 1000000");
> >  
> > -  /* clock_gettime shouldn't fail here since
> > CLOCK_PROCESS_CPUTIME_ID is
> > -     supported since 2.6.12.  Check the return value anyway in
> > case the kernel
> > -     barfs on us for some reason.  */
> > -  if (__glibc_unlikely (__clock_gettime (CLOCK_PROCESS_CPUTIME_ID,
> > &ts) != 0))
> > +  if (__glibc_unlikely (__clock_gettime64
> > (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0)) return (clock_t) -1;
> >  
> >    return (ts.tv_sec * CLOCKS_PER_SEC
> >   
> 
> Ok.




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Adhemerval Zanella May 4, 2020, 2:04 p.m. UTC | #7 
On 01/05/2020 08:56, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 26/03/2020 05:06, Lukasz Majewski wrote:
>>> diff --git a/nis/nis_call.c b/nis/nis_call.c
>>> index 92c70e97aa..9c6f62a753 100644
>>> --- a/nis/nis_call.c
>>> +++ b/nis/nis_call.c
>>> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
>>> search_parent, nis_error status;
>>>    directory_obj *obj;
>>>    struct timeval now;
>>> -  struct timespec ts;
>>> +  struct __timespec64 ts;
>>>    unsigned int server_used = ~0;
>>>    unsigned int current_ep = ~0;
>>>  
>>> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
>>> search_parent, if (*dir != NULL)
>>>      return NIS_SUCCESS;
>>>  
>>> -  __clock_gettime (CLOCK_REALTIME, &ts);
>>> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
>>> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
>>> +  now = valid_timespec64_to_timeval (ts);
>>>  
>>>    if ((flags & NO_CACHE) == 0)
>>>      *dir = nis_server_cache_search (name, search_parent,
>>> &server_used,  
>>
>> I think it would be simpler to just remove the timeval argument on
>> nis_server_cache_search and move the __clock_gettime64 call on the
>> function start. 
> 
> Have I understood you correctly that you recommend removing the "now"
> struct timeval argument and then call explicitly __clock_gettime64 on
> the beginning of nis_server_cache_search function?

Yes, the nis_server_cache_search is a static function used only once
at __nisfind_server.

> 
>>
>> Also, it would require to change nis_server_cache to use a
>> __time64_t for 'expires', otherwise this change won't help in
>> case of a time_t overflow.
>>
> 
> Ok. I will update this. Thanks for pointing this out.
> 
>>
>>> diff --git a/sysdeps/generic/hp-timing.h
>>> b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
>>> --- a/sysdeps/generic/hp-timing.h
>>> +++ b/sysdeps/generic/hp-timing.h
>>> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
>>>     vDSO symbol.  */
>>>  #define HP_TIMING_NOW(var) \
>>>  ({								\
>>> -  struct timespec tv;
>>> 	\
>>> -  __clock_gettime (CLOCK_MONOTONIC, &tv);			\
>>> +  struct __timespec64 tv;
>>> 	\
>>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>>> \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);	\
>>>  })
>>>    
>>
>> Ok.
>>
>>> diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
>>> index a111864b0b..91e56d24de 100644
>>> --- a/sysdeps/generic/memusage.h
>>> +++ b/sysdeps/generic/memusage.h
>>> @@ -28,9 +28,9 @@
>>>  #ifndef GETTIME
>>>  # define GETTIME(low,high)
>>> 	   \ {
>>> 			   \
>>> -    struct timespec now;
>>> 	   \
>>> +    struct __timespec64 now;
>>> 		   \ uint64_t usecs;
>>> 			   \
>>> -    clock_gettime (CLOCK_REALTIME, &now);
>>> 	   \
>>> +    __clock_gettime64 (CLOCK_REALTIME, &now);
>>> 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
>>> (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
>>> 					   \ high = usecs >>
>>> 32;							   \  
>>
>> Is is the requirement to export __clock_gettime64 as a GLIBC_PRIVATE
>> symbol?
>>
> 
> The __clock_gettime is already exported as GLIBC_PRIVATE at
> ./time/Versions, so I'm following this pattern.
> 
> Moreover, the glibc will not build when __clock_gettime64 is not
> exported.
> 
>> In any case, I think we should try to avoid use internal symbols even
>> for distributed glibc libraries, so I think this change should go
>> once we start to export the clock_gettime64 as default symbol.
> 
> Am I correct that this is a preprocessor macro, which is in the
> exported header?

In fact __clock_gettime is used on other internal libraries and it
is required as is to avoid linknamespace pollution.  So it seems
that __clock_gettime64 should follow the same logic.

It might be misleading that for some ABI __clock_gettime and
for other __clock_gettime64 will be used internally, but it should
be ok nonetheless.
Lukasz Majewski May 4, 2020, 3:32 p.m. UTC | #8 
Hi Adhemerval,

> On 01/05/2020 08:56, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 26/03/2020 05:06, Lukasz Majewski wrote:  
> >>> diff --git a/nis/nis_call.c b/nis/nis_call.c
> >>> index 92c70e97aa..9c6f62a753 100644
> >>> --- a/nis/nis_call.c
> >>> +++ b/nis/nis_call.c
> >>> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
> >>> search_parent, nis_error status;
> >>>    directory_obj *obj;
> >>>    struct timeval now;
> >>> -  struct timespec ts;
> >>> +  struct __timespec64 ts;
> >>>    unsigned int server_used = ~0;
> >>>    unsigned int current_ep = ~0;
> >>>  
> >>> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
> >>> search_parent, if (*dir != NULL)
> >>>      return NIS_SUCCESS;
> >>>  
> >>> -  __clock_gettime (CLOCK_REALTIME, &ts);
> >>> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
> >>> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> >>> +  now = valid_timespec64_to_timeval (ts);
> >>>  
> >>>    if ((flags & NO_CACHE) == 0)
> >>>      *dir = nis_server_cache_search (name, search_parent,
> >>> &server_used,    
> >>
> >> I think it would be simpler to just remove the timeval argument on
> >> nis_server_cache_search and move the __clock_gettime64 call on the
> >> function start.   
> > 
> > Have I understood you correctly that you recommend removing the
> > "now" struct timeval argument and then call explicitly
> > __clock_gettime64 on the beginning of nis_server_cache_search
> > function?  
> 
> Yes, the nis_server_cache_search is a static function used only once
> at __nisfind_server.

Ok.

> 
> >   
> >>
> >> Also, it would require to change nis_server_cache to use a
> >> __time64_t for 'expires', otherwise this change won't help in
> >> case of a time_t overflow.
> >>  
> > 
> > Ok. I will update this. Thanks for pointing this out.
> >   
> >>  
> >>> diff --git a/sysdeps/generic/hp-timing.h
> >>> b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
> >>> --- a/sysdeps/generic/hp-timing.h
> >>> +++ b/sysdeps/generic/hp-timing.h
> >>> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
> >>>     vDSO symbol.  */
> >>>  #define HP_TIMING_NOW(var) \
> >>>  ({
> >>> 	\
> >>> -  struct timespec tv;
> >>> 	\
> >>> -  __clock_gettime (CLOCK_MONOTONIC, &tv);
> >>> \
> >>> +  struct __timespec64 tv;
> >>> 	\
> >>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
> >>> \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);
> >>> \ })
> >>>      
> >>
> >> Ok.
> >>  
> >>> diff --git a/sysdeps/generic/memusage.h
> >>> b/sysdeps/generic/memusage.h index a111864b0b..91e56d24de 100644
> >>> --- a/sysdeps/generic/memusage.h
> >>> +++ b/sysdeps/generic/memusage.h
> >>> @@ -28,9 +28,9 @@
> >>>  #ifndef GETTIME
> >>>  # define GETTIME(low,high)
> >>> 	   \ {
> >>> 			   \
> >>> -    struct timespec now;
> >>> 	   \
> >>> +    struct __timespec64 now;
> >>> 		   \ uint64_t usecs;
> >>> 			   \
> >>> -    clock_gettime (CLOCK_REALTIME, &now);
> >>> 	   \
> >>> +    __clock_gettime64 (CLOCK_REALTIME, &now);
> >>> 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
> >>> (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
> >>> 					   \ high = usecs >>
> >>> 32;							   \
> >>>  
> >>
> >> Is is the requirement to export __clock_gettime64 as a
> >> GLIBC_PRIVATE symbol?
> >>  
> > 
> > The __clock_gettime is already exported as GLIBC_PRIVATE at
> > ./time/Versions, so I'm following this pattern.
> > 
> > Moreover, the glibc will not build when __clock_gettime64 is not
> > exported.
> >   
> >> In any case, I think we should try to avoid use internal symbols
> >> even for distributed glibc libraries, so I think this change
> >> should go once we start to export the clock_gettime64 as default
> >> symbol.  
> > 
> > Am I correct that this is a preprocessor macro, which is in the
> > exported header?  
> 
> In fact __clock_gettime is used on other internal libraries and it
> is required as is to avoid linknamespace pollution.  So it seems
> that __clock_gettime64 should follow the same logic.

Ok, So then __clock_gettime64 shall be exported as well.

> 
> It might be misleading that for some ABI __clock_gettime and
> for other __clock_gettime64 will be used internally, but it should
> be ok nonetheless.
> 

All the occurrences of __clock_gettime shall be replaced by
__clock_gettime64 internally in glibc as __clock_gettime64 is Y2038
safe for e.g. ARM32 and is aliased to __clock_gettime for 64 bit archs
anyway.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Adhemerval Zanella May 4, 2020, 4:37 p.m. UTC | #9 
On 04/05/2020 12:32, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 01/05/2020 08:56, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>   
>>>> On 26/03/2020 05:06, Lukasz Majewski wrote:  
>>>>> diff --git a/nis/nis_call.c b/nis/nis_call.c
>>>>> index 92c70e97aa..9c6f62a753 100644
>>>>> --- a/nis/nis_call.c
>>>>> +++ b/nis/nis_call.c
>>>>> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
>>>>> search_parent, nis_error status;
>>>>>    directory_obj *obj;
>>>>>    struct timeval now;
>>>>> -  struct timespec ts;
>>>>> +  struct __timespec64 ts;
>>>>>    unsigned int server_used = ~0;
>>>>>    unsigned int current_ep = ~0;
>>>>>  
>>>>> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
>>>>> search_parent, if (*dir != NULL)
>>>>>      return NIS_SUCCESS;
>>>>>  
>>>>> -  __clock_gettime (CLOCK_REALTIME, &ts);
>>>>> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
>>>>> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
>>>>> +  now = valid_timespec64_to_timeval (ts);
>>>>>  
>>>>>    if ((flags & NO_CACHE) == 0)
>>>>>      *dir = nis_server_cache_search (name, search_parent,
>>>>> &server_used,    
>>>>
>>>> I think it would be simpler to just remove the timeval argument on
>>>> nis_server_cache_search and move the __clock_gettime64 call on the
>>>> function start.   
>>>
>>> Have I understood you correctly that you recommend removing the
>>> "now" struct timeval argument and then call explicitly
>>> __clock_gettime64 on the beginning of nis_server_cache_search
>>> function?  
>>
>> Yes, the nis_server_cache_search is a static function used only once
>> at __nisfind_server.
> 
> Ok.
> 
>>
>>>   
>>>>
>>>> Also, it would require to change nis_server_cache to use a
>>>> __time64_t for 'expires', otherwise this change won't help in
>>>> case of a time_t overflow.
>>>>  
>>>
>>> Ok. I will update this. Thanks for pointing this out.
>>>   
>>>>  
>>>>> diff --git a/sysdeps/generic/hp-timing.h
>>>>> b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
>>>>> --- a/sysdeps/generic/hp-timing.h
>>>>> +++ b/sysdeps/generic/hp-timing.h
>>>>> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
>>>>>     vDSO symbol.  */
>>>>>  #define HP_TIMING_NOW(var) \
>>>>>  ({
>>>>> 	\
>>>>> -  struct timespec tv;
>>>>> 	\
>>>>> -  __clock_gettime (CLOCK_MONOTONIC, &tv);
>>>>> \
>>>>> +  struct __timespec64 tv;
>>>>> 	\
>>>>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
>>>>> \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);
>>>>> \ })
>>>>>      
>>>>
>>>> Ok.
>>>>  
>>>>> diff --git a/sysdeps/generic/memusage.h
>>>>> b/sysdeps/generic/memusage.h index a111864b0b..91e56d24de 100644
>>>>> --- a/sysdeps/generic/memusage.h
>>>>> +++ b/sysdeps/generic/memusage.h
>>>>> @@ -28,9 +28,9 @@
>>>>>  #ifndef GETTIME
>>>>>  # define GETTIME(low,high)
>>>>> 	   \ {
>>>>> 			   \
>>>>> -    struct timespec now;
>>>>> 	   \
>>>>> +    struct __timespec64 now;
>>>>> 		   \ uint64_t usecs;
>>>>> 			   \
>>>>> -    clock_gettime (CLOCK_REALTIME, &now);
>>>>> 	   \
>>>>> +    __clock_gettime64 (CLOCK_REALTIME, &now);
>>>>> 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
>>>>> (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
>>>>> 					   \ high = usecs >>
>>>>> 32;							   \
>>>>>  
>>>>
>>>> Is is the requirement to export __clock_gettime64 as a
>>>> GLIBC_PRIVATE symbol?
>>>>  
>>>
>>> The __clock_gettime is already exported as GLIBC_PRIVATE at
>>> ./time/Versions, so I'm following this pattern.
>>>
>>> Moreover, the glibc will not build when __clock_gettime64 is not
>>> exported.
>>>   
>>>> In any case, I think we should try to avoid use internal symbols
>>>> even for distributed glibc libraries, so I think this change
>>>> should go once we start to export the clock_gettime64 as default
>>>> symbol.  
>>>
>>> Am I correct that this is a preprocessor macro, which is in the
>>> exported header?  
>>
>> In fact __clock_gettime is used on other internal libraries and it
>> is required as is to avoid linknamespace pollution.  So it seems
>> that __clock_gettime64 should follow the same logic.
> 
> Ok, So then __clock_gettime64 shall be exported as well.
> 
>>
>> It might be misleading that for some ABI __clock_gettime and
>> for other __clock_gettime64 will be used internally, but it should
>> be ok nonetheless.
>>
> 
> All the occurrences of __clock_gettime shall be replaced by
> __clock_gettime64 internally in glibc as __clock_gettime64 is Y2038
> safe for e.g. ARM32 and is aliased to __clock_gettime for 64 bit archs
> anyway.

I meant that some architectures does not provide __clock_gettime64 and
thus the internal symbol will be __clock_gettime. But it is not an
issue in fact.
Lukasz Majewski May 5, 2020, 6:31 p.m. UTC | #10 
Hi Adhemerval,

> On 01/05/2020 08:56, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 26/03/2020 05:06, Lukasz Majewski wrote:  
> >>> diff --git a/nis/nis_call.c b/nis/nis_call.c
> >>> index 92c70e97aa..9c6f62a753 100644
> >>> --- a/nis/nis_call.c
> >>> +++ b/nis/nis_call.c
> >>> @@ -709,7 +709,7 @@ __nisfind_server (const_nis_name name, int
> >>> search_parent, nis_error status;
> >>>    directory_obj *obj;
> >>>    struct timeval now;
> >>> -  struct timespec ts;
> >>> +  struct __timespec64 ts;
> >>>    unsigned int server_used = ~0;
> >>>    unsigned int current_ep = ~0;
> >>>  
> >>> @@ -719,8 +719,8 @@ __nisfind_server (const_nis_name name, int
> >>> search_parent, if (*dir != NULL)
> >>>      return NIS_SUCCESS;
> >>>  
> >>> -  __clock_gettime (CLOCK_REALTIME, &ts);
> >>> -  TIMESPEC_TO_TIMEVAL (&now, &ts);
> >>> +  __clock_gettime64 (CLOCK_REALTIME, &ts);
> >>> +  now = valid_timespec64_to_timeval (ts);
> >>>  
> >>>    if ((flags & NO_CACHE) == 0)
> >>>      *dir = nis_server_cache_search (name, search_parent,
> >>> &server_used,    
> >>
> >> I think it would be simpler to just remove the timeval argument on
> >> nis_server_cache_search and move the __clock_gettime64 call on the
> >> function start.   
> > 
> > Have I understood you correctly that you recommend removing the
> > "now" struct timeval argument and then call explicitly
> > __clock_gettime64 on the beginning of nis_server_cache_search
> > function?  
> 
> Yes, the nis_server_cache_search is a static function used only once
> at __nisfind_server.

I've looked into the code again and it seems like in the
__nisfind_server there are two static functions which use struct
timeval *now:

- nis_server_cache_search (...., &now)

- nis_server_cache_add (.... , &now)

Both of them are static functions. 

I would propose instead leaving the &now argument, but replace it with
struct timespec as in the above function we do compare expire with
tv_sec member of struct timeval (which can be easily changed to struct
timespec).

(I've just sent a v3 of this patch to mailing list)

> 
> >   
> >>
> >> Also, it would require to change nis_server_cache to use a
> >> __time64_t for 'expires', otherwise this change won't help in
> >> case of a time_t overflow.
> >>  
> > 
> > Ok. I will update this. Thanks for pointing this out.
> >   
> >>  
> >>> diff --git a/sysdeps/generic/hp-timing.h
> >>> b/sysdeps/generic/hp-timing.h index e2d7447212..af9d92f7f7 100644
> >>> --- a/sysdeps/generic/hp-timing.h
> >>> +++ b/sysdeps/generic/hp-timing.h
> >>> @@ -34,8 +34,8 @@ typedef uint64_t hp_timing_t;
> >>>     vDSO symbol.  */
> >>>  #define HP_TIMING_NOW(var) \
> >>>  ({
> >>> 	\
> >>> -  struct timespec tv;
> >>> 	\
> >>> -  __clock_gettime (CLOCK_MONOTONIC, &tv);
> >>> \
> >>> +  struct __timespec64 tv;
> >>> 	\
> >>> +  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
> >>> \ (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);
> >>> \ })
> >>>      
> >>
> >> Ok.
> >>  
> >>> diff --git a/sysdeps/generic/memusage.h
> >>> b/sysdeps/generic/memusage.h index a111864b0b..91e56d24de 100644
> >>> --- a/sysdeps/generic/memusage.h
> >>> +++ b/sysdeps/generic/memusage.h
> >>> @@ -28,9 +28,9 @@
> >>>  #ifndef GETTIME
> >>>  # define GETTIME(low,high)
> >>> 	   \ {
> >>> 			   \
> >>> -    struct timespec now;
> >>> 	   \
> >>> +    struct __timespec64 now;
> >>> 		   \ uint64_t usecs;
> >>> 			   \
> >>> -    clock_gettime (CLOCK_REALTIME, &now);
> >>> 	   \
> >>> +    __clock_gettime64 (CLOCK_REALTIME, &now);
> >>> 		   \ usecs = (uint64_t)now.tv_nsec / 1000 +
> >>> (uint64_t)now.tv_sec * 1000000; \ low = usecs & 0xffffffff;
> >>> 					   \ high = usecs >>
> >>> 32;							   \
> >>>  
> >>
> >> Is is the requirement to export __clock_gettime64 as a
> >> GLIBC_PRIVATE symbol?
> >>  
> > 
> > The __clock_gettime is already exported as GLIBC_PRIVATE at
> > ./time/Versions, so I'm following this pattern.
> > 
> > Moreover, the glibc will not build when __clock_gettime64 is not
> > exported.
> >   
> >> In any case, I think we should try to avoid use internal symbols
> >> even for distributed glibc libraries, so I think this change
> >> should go once we start to export the clock_gettime64 as default
> >> symbol.  
> > 
> > Am I correct that this is a preprocessor macro, which is in the
> > exported header?  
> 
> In fact __clock_gettime is used on other internal libraries and it
> is required as is to avoid linknamespace pollution.  So it seems
> that __clock_gettime64 should follow the same logic.
> 
> It might be misleading that for some ABI __clock_gettime and
> for other __clock_gettime64 will be used internally, but it should
> be ok nonetheless.
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff mbox series

Patch

diff --git a/benchtests/bench-timing.h b/benchtests/bench-timing.h
index 5b9a8384bb..a0d6f82465 100644
--- a/benchtests/bench-timing.h
+++ b/benchtests/bench-timing.h
@@ -18,7 +18,7 @@ 
 
 #undef attribute_hidden
 #define attribute_hidden
-#define __clock_gettime clock_gettime
+#define __clock_gettime __clock_gettime64
 #include <hp-timing.h>
 #include <stdint.h>
 
diff --git a/include/random-bits.h b/include/random-bits.h
index fd3fa01f9b..7561e55ca6 100644
--- a/include/random-bits.h
+++ b/include/random-bits.h
@@ -30,8 +30,8 @@ 
 static inline uint32_t
 random_bits (void)
 {
-  struct timespec tv;
-  __clock_gettime (CLOCK_MONOTONIC, &tv);
+  struct __timespec64 tv;
+  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
   /* Shuffle the lower bits to minimize the clock bias.  */
   uint32_t ret = tv.tv_nsec ^ tv.tv_sec;
   ret ^= (ret << 24) | (ret >> 8);
diff --git a/login/logout.c b/login/logout.c
index 7653fe8886..091312eb1d 100644
--- a/login/logout.c
+++ b/login/logout.c
@@ -47,8 +47,8 @@  logout (const char *line)
       memset (ut->ut_name, '\0', sizeof ut->ut_name);
       memset (ut->ut_host, '\0', sizeof ut->ut_host);
 
-      struct timespec ts;
-      __clock_gettime (CLOCK_REALTIME, &ts);
+      struct __timespec64 ts;
+      __clock_gettime64 (CLOCK_REALTIME, &ts);
       TIMESPEC_TO_TIMEVAL (&ut->ut_tv, &ts);
       ut->ut_type = DEAD_PROCESS;
 
diff --git a/login/logwtmp.c b/login/logwtmp.c
index 90406acc3d..050219c153 100644
--- a/login/logwtmp.c
+++ b/login/logwtmp.c
@@ -21,6 +21,7 @@ 
 #include <time.h>
 #include <unistd.h>
 #include <utmp.h>
+#include <struct___timespec64.h>
 
 
 void
@@ -36,8 +37,8 @@  logwtmp (const char *line, const char *name, const char *host)
   strncpy (ut.ut_name, name, sizeof ut.ut_name);
   strncpy (ut.ut_host, host, sizeof ut.ut_host);
 
-  struct timespec ts;
-  __clock_gettime (CLOCK_REALTIME, &ts);
+  struct __timespec64 ts;
+  __clock_gettime64 (CLOCK_REALTIME, &ts);
   TIMESPEC_TO_TIMEVAL (&ut.ut_tv, &ts);
 
   updwtmp (_PATH_WTMP, &ut);
diff --git a/nis/nis_call.c b/nis/nis_call.c
index 92c70e97aa..9c6f62a753 100644
--- a/nis/nis_call.c
+++ b/nis/nis_call.c
@@ -709,7 +709,7 @@  __nisfind_server (const_nis_name name, int search_parent,
   nis_error status;
   directory_obj *obj;
   struct timeval now;
-  struct timespec ts;
+  struct __timespec64 ts;
   unsigned int server_used = ~0;
   unsigned int current_ep = ~0;
 
@@ -719,8 +719,8 @@  __nisfind_server (const_nis_name name, int search_parent,
   if (*dir != NULL)
     return NIS_SUCCESS;
 
-  __clock_gettime (CLOCK_REALTIME, &ts);
-  TIMESPEC_TO_TIMEVAL (&now, &ts);
+  __clock_gettime64 (CLOCK_REALTIME, &ts);
+  now = valid_timespec64_to_timeval (ts);
 
   if ((flags & NO_CACHE) == 0)
     *dir = nis_server_cache_search (name, search_parent, &server_used,
diff --git a/sysdeps/generic/hp-timing.h b/sysdeps/generic/hp-timing.h
index e2d7447212..af9d92f7f7 100644
--- a/sysdeps/generic/hp-timing.h
+++ b/sysdeps/generic/hp-timing.h
@@ -34,8 +34,8 @@  typedef uint64_t hp_timing_t;
    vDSO symbol.  */
 #define HP_TIMING_NOW(var) \
 ({								\
-  struct timespec tv;						\
-  __clock_gettime (CLOCK_MONOTONIC, &tv);			\
+  struct __timespec64 tv;						\
+  __clock_gettime64 (CLOCK_MONOTONIC, &tv);			\
   (var) = (tv.tv_nsec + UINT64_C(1000000000) * tv.tv_sec);	\
 })
 
diff --git a/sysdeps/generic/memusage.h b/sysdeps/generic/memusage.h
index a111864b0b..91e56d24de 100644
--- a/sysdeps/generic/memusage.h
+++ b/sysdeps/generic/memusage.h
@@ -28,9 +28,9 @@ 
 #ifndef GETTIME
 # define GETTIME(low,high)						   \
   {									   \
-    struct timespec now;						   \
+    struct __timespec64 now;						   \
     uint64_t usecs;							   \
-    clock_gettime (CLOCK_REALTIME, &now);				   \
+    __clock_gettime64 (CLOCK_REALTIME, &now);				   \
     usecs = (uint64_t)now.tv_nsec / 1000 + (uint64_t)now.tv_sec * 1000000; \
     low = usecs & 0xffffffff;						   \
     high = usecs >> 32;							   \
diff --git a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
index 8cf5d303f9..5075ae0444 100644
--- a/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c
@@ -35,8 +35,8 @@  __gettimeofday_tv32 (struct __timeval32 *restrict tv32, void *restrict tz)
   if (__glibc_unlikely (tz != 0))
     memset (tz, 0, sizeof (struct timezone));
 
-  struct timespec ts;
-  __clock_gettime (CLOCK_REALTIME, &ts);
+  struct __timespec64 ts;
+  __clock_gettime64 (CLOCK_REALTIME, &ts);
 
   *tv32 = valid_timespec_to_timeval32 (ts);
   return 0;
diff --git a/sysdeps/unix/sysv/linux/clock.c b/sysdeps/unix/sysv/linux/clock.c
index 24a8df0cf5..157ae8eb3f 100644
--- a/sysdeps/unix/sysv/linux/clock.c
+++ b/sysdeps/unix/sysv/linux/clock.c
@@ -23,15 +23,12 @@ 
 clock_t
 clock (void)
 {
-  struct timespec ts;
+  struct __timespec64 ts;
 
   _Static_assert (CLOCKS_PER_SEC == 1000000,
 		  "CLOCKS_PER_SEC should be 1000000");
 
-  /* clock_gettime shouldn't fail here since CLOCK_PROCESS_CPUTIME_ID is
-     supported since 2.6.12.  Check the return value anyway in case the kernel
-     barfs on us for some reason.  */
-  if (__glibc_unlikely (__clock_gettime (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0))
+  if (__glibc_unlikely (__clock_gettime64 (CLOCK_PROCESS_CPUTIME_ID, &ts) != 0))
     return (clock_t) -1;
 
   return (ts.tv_sec * CLOCKS_PER_SEC