This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 06/11] Change most internal uses of time to __clock_gettime.


Hi Adhemerval,

> As for gettimeofday, time will be implemented based on clock_gettime
> on all platforms and internal code should use clock_gettime
> directly.  In addition to removing a layer of indirection, this will
> allow us to remove the PLT-bypass gunk for gettimeofday.
> 
> The changed code always assumes __clock_gettime (CLOCK_REALTIME)
> or __clock_gettime (CLOCK_REALTIME_COURSE) (for Linux case) cannot
				     ^^^^^^ - coarse ?

> fail, using the same rationale for gettimeofday change.  And internal
> helper was added (time_now).
> 
> Checked on x86_64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
> powerpc64-linux-gnu, and powerpc-linux-gnu.
> ---
>  include/time.h                         | 11 +++-
>  misc/syslog.c                          |  2 +-
>  nscd/nscd_gethst_r.c                   |  2 +-
>  nscd/nscd_helper.c                     |  4 +-
>  string/strfry.c                        |  4 +-
>  sysdeps/unix/sysv/linux/check_native.c |  2 +-
>  sysdeps/unix/sysv/linux/check_pf.c     |  2 +-
>  sysdeps/unix/sysv/linux/getsysstats.c  |  2 +-
>  sysdeps/unix/sysv/linux/i386/time.c    | 34 ------------
>  sysdeps/unix/sysv/linux/ifaddrs.c      |  2 +-
>  sysdeps/unix/sysv/linux/powerpc/time.c | 71
> ++++++++------------------ sysdeps/unix/sysv/linux/x86/time.c     |
> 34 ++++-------- time/getdate.c                         |  2 +-
>  time/time.c                            |  1 -
>  14 files changed, 52 insertions(+), 121 deletions(-)
>  delete mode 100644 sysdeps/unix/sysv/linux/i386/time.c
> 
> diff --git a/include/time.h b/include/time.h
> index d93b16a781..3eac3dcb8d 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -7,12 +7,12 @@
>  # include <stdbool.h>
>  # include <time/mktime-internal.h>
>  # include <endian.h>
> +# include <time-internal.h>
>  
>  extern __typeof (strftime_l) __strftime_l;
>  libc_hidden_proto (__strftime_l)
>  extern __typeof (strptime_l) __strptime_l;
>  
> -libc_hidden_proto (time)
>  libc_hidden_proto (asctime)
>  libc_hidden_proto (mktime)
>  libc_hidden_proto (timelocal)
> @@ -236,5 +236,14 @@ valid_timespec64_to_timeval (const struct
> __timespec64 ts64) 
>    return tv;
>  }
> +
> +/* Helper function to get time in seconds, similar to time.  */
> +static inline time_t
> +time_now (void)
> +{
> +  struct timespec ts;
> +  __clock_gettime (TIME_CLOCK_GETTIME_CLOCKID, &ts);
> +  return ts.tv_sec;
> +}
>  #endif
>  #endif
> diff --git a/misc/syslog.c b/misc/syslog.c
> index cf2deef533..fd6537edf6 100644
> --- a/misc/syslog.c
> +++ b/misc/syslog.c
> @@ -205,7 +205,7 @@ __vsyslog_internal(int pri, const char *fmt,
> va_list ap, {
>  	    __fsetlocking (f, FSETLOCKING_BYCALLER);
>  	    fprintf (f, "<%d>", pri);
> -	    now = time (NULL);
> +	    now = time_now ();
>  	    f->_IO_write_ptr += __strftime_l (f->_IO_write_ptr,
>  					      f->_IO_write_end
>  					      - f->_IO_write_ptr,
> diff --git a/nscd/nscd_gethst_r.c b/nscd/nscd_gethst_r.c
> index 18c6be0d48..8eca90fd52 100644
> --- a/nscd/nscd_gethst_r.c
> +++ b/nscd/nscd_gethst_r.c
> @@ -113,7 +113,7 @@ __nscd_get_nl_timestamp (void)
>    if (map == NULL
>        || (map != NO_MAPPING
>  	  && map->head->nscd_certainly_running == 0
> -	  && map->head->timestamp + MAPPING_TIMEOUT < time (NULL)))
> +	  && map->head->timestamp + MAPPING_TIMEOUT < time_now ()))
>      map = __nscd_get_mapping (GETFDHST, "hosts",
> &__hst_map_handle.mapped); 
>    if (map == NO_MAPPING)
> diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
> index 7361fe2e0a..d50615e13e 100644
> --- a/nscd/nscd_helper.c
> +++ b/nscd/nscd_helper.c
> @@ -348,7 +348,7 @@ __nscd_get_mapping (request_type type, const char
> *key, thread got stuck.  */
>  	  || __builtin_expect (! head->nscd_certainly_running
>  			       && (head->timestamp + MAPPING_TIMEOUT
> -				   < time (NULL)), 0))
> +				   < time_now ()), 0))
>  	{
>  	out_unmap:
>  	  __munmap (mapping, mapsize);
> @@ -414,7 +414,7 @@ __nscd_get_map_ref (request_type type, const char
> *name, /* If not mapped or timestamp not updated, request new map.  */
>        if (cur == NULL
>  	  || (cur->head->nscd_certainly_running == 0
> -	      && cur->head->timestamp + MAPPING_TIMEOUT < time
> (NULL))
> +	      && cur->head->timestamp + MAPPING_TIMEOUT < time_now
> ()) || cur->head->data_size > cur->datasize)
>  	cur = __nscd_get_mapping (type, name,
>  				  (struct mapped_database **)
> &mapptr->mapped); diff --git a/string/strfry.c b/string/strfry.c
> index 6306f06ae4..8b293af185 100644
> --- a/string/strfry.c
> +++ b/string/strfry.c
> @@ -17,7 +17,7 @@
>  
>  #include <string.h>
>  #include <stdlib.h>
> -#include <time.h>
> +#include <random-bits.h>
>  #include <unistd.h>
>  
>  char *
> @@ -30,7 +30,7 @@ strfry (char *string)
>      {
>        static char state[32];
>        rdata.state = NULL;
> -      __initstate_r (time (NULL) ^ getpid (),
> +      __initstate_r (random_bits (),
>  		     state, sizeof (state), &rdata);
>        init = 1;
>      }
> diff --git a/sysdeps/unix/sysv/linux/check_native.c
> b/sysdeps/unix/sysv/linux/check_native.c index 3e57629d8c..82e2a0d83f
> 100644 --- a/sysdeps/unix/sysv/linux/check_native.c
> +++ b/sysdeps/unix/sysv/linux/check_native.c
> @@ -69,7 +69,7 @@ __check_native (uint32_t a1_index, int *a1_native,
>    req.nlh.nlmsg_type = RTM_GETLINK;
>    req.nlh.nlmsg_flags = NLM_F_ROOT | NLM_F_MATCH | NLM_F_REQUEST;
>    req.nlh.nlmsg_pid = 0;
> -  req.nlh.nlmsg_seq = time (NULL);
> +  req.nlh.nlmsg_seq = time_now ();
>    req.g.rtgen_family = AF_UNSPEC;
>  
>    assert (sizeof (req) - offsetof (struct req, pad) == 3);
> diff --git a/sysdeps/unix/sysv/linux/check_pf.c
> b/sysdeps/unix/sysv/linux/check_pf.c index 97a30f63fc..bcb9c602aa
> 100644 --- a/sysdeps/unix/sysv/linux/check_pf.c
> +++ b/sysdeps/unix/sysv/linux/check_pf.c
> @@ -126,7 +126,7 @@ make_request (int fd, pid_t pid)
>    req.nlh.nlmsg_type = RTM_GETADDR;
>    req.nlh.nlmsg_flags = NLM_F_ROOT | NLM_F_MATCH | NLM_F_REQUEST;
>    req.nlh.nlmsg_pid = 0;
> -  req.nlh.nlmsg_seq = time (NULL);
> +  req.nlh.nlmsg_seq = time_now ();
>    req.g.rtgen_family = AF_UNSPEC;
>  
>    assert (sizeof (req) - offsetof (struct req, pad) == 3);
> diff --git a/sysdeps/unix/sysv/linux/getsysstats.c
> b/sysdeps/unix/sysv/linux/getsysstats.c index 41ceb9a320..6457193227
> 100644 --- a/sysdeps/unix/sysv/linux/getsysstats.c
> +++ b/sysdeps/unix/sysv/linux/getsysstats.c
> @@ -128,7 +128,7 @@ __get_nprocs (void)
>    static int cached_result = -1;
>    static time_t timestamp;
>  
> -  time_t now = time (NULL);
> +  time_t now = time_now ();
>    time_t prev = timestamp;
>    atomic_read_barrier ();
>    if (now == prev && cached_result > -1)
> diff --git a/sysdeps/unix/sysv/linux/i386/time.c
> b/sysdeps/unix/sysv/linux/i386/time.c deleted file mode 100644
> index 1bbe079f65..0000000000
> --- a/sysdeps/unix/sysv/linux/i386/time.c
> +++ /dev/null
> @@ -1,34 +0,0 @@
> -/* time -- Get number of seconds since Epoch.  Linux/i386 version.
> -   Copyright (C) 2015-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/>.  */
> -
> -#ifdef SHARED
> -# define time __redirect_time
> -#endif
> -
> -#include <time.h>
> -
> -#ifdef SHARED
> -# undef time
> -# define time_type __redirect_time
> -
> -# undef libc_hidden_def
> -# define libc_hidden_def(name)  \
> -  __hidden_ver1 (__time_syscall, __GI_time, __time_syscall);
> -#endif
> -
> -#include <sysdeps/unix/sysv/linux/x86/time.c>
> diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c
> b/sysdeps/unix/sysv/linux/ifaddrs.c index 0c36001660..2b89c7a3af
> 100644 --- a/sysdeps/unix/sysv/linux/ifaddrs.c
> +++ b/sysdeps/unix/sysv/linux/ifaddrs.c
> @@ -102,7 +102,7 @@ __netlink_sendreq (struct netlink_handle *h, int
> type) struct sockaddr_nl nladdr;
>  
>    if (h->seq == 0)
> -    h->seq = time (NULL);
> +    h->seq = time_now ();
>  
>    req.nlh.nlmsg_len = sizeof (req);
>    req.nlh.nlmsg_type = type;
> diff --git a/sysdeps/unix/sysv/linux/powerpc/time.c
> b/sysdeps/unix/sysv/linux/powerpc/time.c index e957b81751..80a4c73416
> 100644 --- a/sysdeps/unix/sysv/linux/powerpc/time.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/time.c
> @@ -16,68 +16,37 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#ifdef SHARED
> -# ifndef __powerpc64__
> -#  define time __redirect_time
> -# else
> -#  define __redirect_time time
> -# endif
> -
> -# include <time.h>
> -# include <sysdep.h>
> -# include <dl-vdso.h>
> -# include <libc-vdso.h>
> -# include <dl-machine.h>
> +#include <time.h>
> +#include <sysdep.h>
>  
> -# ifndef __powerpc64__
> -#  undef time
> +#ifdef HAVE_TIME_VSYSCALL
> +# define HAVE_VSYSCALL
> +#endif
> +#include <sysdep-vdso.h>
>  
> -time_t
> -__time_vsyscall (time_t *t)
> +static time_t
> +time_vsyscall (time_t *t)
>  {
>    return INLINE_VSYSCALL (time, 1, t);
>  }
>  
> -/* __GI_time is defined as hidden and for ppc32 it enables the
> -   compiler make a local call (symbol@local) for internal GLIBC
> usage. It
> -   means the PLT won't be used and the ifunc resolver will be called
> directly.
> -   For ppc64 a call to a function in another translation unit might
> use a
> -   different toc pointer thus disallowing direct branchess and
> making internal
> -   ifuncs calls safe.  */
> -#  undef libc_hidden_def
> -#  define libc_hidden_def(name)
> 	\
> -  __hidden_ver1 (__time_vsyscall, __GI_time, __time_vsyscall);
> -
> -# endif /* !__powerpc64__  */
> -
> -static time_t
> -time_syscall (time_t *t)
> -{
> -  struct timeval tv;
> -  time_t result;
> -
> -  if (INLINE_VSYSCALL (gettimeofday, 2, &tv, NULL) < 0)
> -    result = (time_t) -1;
> -  else
> -    result = (time_t) tv.tv_sec;
> -
> -  if (t != NULL)
> -    *t = result;
> -  return result;
> -}
> +#ifdef SHARED
> +# include <dl-vdso.h>
> +# include <libc-vdso.h>
>  
>  # define INIT_ARCH() \
>    void *vdso_time = get_vdso_symbol (HAVE_TIME_VSYSCALL);
>  
>  /* If the vDSO is not available we fall back to the syscall.  */
> -libc_ifunc_hidden (__redirect_time, time,
> -		   vdso_time
> -		   ? VDSO_IFUNC_RET (vdso_time)
> -		   : (void *) time_syscall);
> -libc_hidden_def (time)
> +libc_ifunc (time,
> +	    vdso_time
> +	    ? VDSO_IFUNC_RET (vdso_time)
> +	    : (void *) time_vsyscall);
>  
>  #else
> -
> -#include <time/time.c>
> -
> +time_t
> +time (time_t *t)
> +{
> +  return time_vsyscall (t);
> +}
>  #endif /* !SHARED */
> diff --git a/sysdeps/unix/sysv/linux/x86/time.c
> b/sysdeps/unix/sysv/linux/x86/time.c index 2e47661be3..4a03c46d21
> 100644 --- a/sysdeps/unix/sysv/linux/x86/time.c
> +++ b/sysdeps/unix/sysv/linux/x86/time.c
> @@ -17,43 +17,31 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <time.h>
> +#include <sysdep.h>
>  
> -#ifdef SHARED
> -
> -#include <dl-vdso.h>
> -#include <errno.h>
> +#ifdef HAVE_TIME_VSYSCALL
> +# define HAVE_VSYSCALL
> +#endif
>  #include <sysdep-vdso.h>
>  
>  static time_t
> -__time_syscall (time_t *t)
> +time_vsyscall (time_t *t)
>  {
> -  INTERNAL_SYSCALL_DECL (err);
> -  return INTERNAL_SYSCALL (time, err, 1, t);
> +  return INLINE_VSYSCALL (time, 1, t);
>  }
>  
> -# ifndef time_type
> -/* The i386 time.c includes this file with a defined time_type macro.
> -   For x86_64 we have to define it to time as the internal symbol is
> the
> -   ifunc'ed one.  */
> -#  define time_type time
> -# endif
> +#ifdef SHARED
> +# include <dl-vdso.h>
> +# include <libc-vdso.h>
>  
>  #undef INIT_ARCH
>  #define INIT_ARCH()
>  /* If the vDSO is not available we fall back on the syscall.  */
> -libc_ifunc_hidden (time_type, time,
> -		   (get_vdso_symbol ("__vdso_time") ?:
> __time_syscall)) -libc_hidden_def (time)
> -
> +libc_ifunc (time, (get_vdso_symbol ("__vdso_time") ?: time_vsyscall))
>  #else
> -
> -# include <sysdep.h>
> -
>  time_t
>  time (time_t *t)
>  {
> -  INTERNAL_SYSCALL_DECL (err);
> -  return INTERNAL_SYSCALL (time, err, 1, t);
> +  return time_vsyscall (t);
>  }
> -
>  #endif
> diff --git a/time/getdate.c b/time/getdate.c
> index 305e7d9dc4..8e15af9f15 100644
> --- a/time/getdate.c
> +++ b/time/getdate.c
> @@ -219,7 +219,7 @@ __getdate_r (const char *string, struct tm *tp)
>      return 7;
>  
>    /* Get current time.  */
> -  timer = time (NULL);
> +  timer = time_now ();
>    __localtime_r (&timer, &tm);
>  
>    /* If only the weekday is given, today is assumed if the given day
> diff --git a/time/time.c b/time/time.c
> index 09a2b4fc88..cfa92cbbb8 100644
> --- a/time/time.c
> +++ b/time/time.c
> @@ -29,4 +29,3 @@ time (time_t *timer)
>      *timer = ts.tv_sec;
>    return ts.tv_sec;
>  }
> -libc_hidden_def (time)


Reviewed-by: Lukasz Majewski <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

Attachment: pgpEpMGN0yaMK.pgp
Description: OpenPGP digital signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]