This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] Add x86 32 bit vDSO time function support
- From: Nathan Lynch <Nathan_Lynch at codesourcery dot com>
- To: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- Cc: "GNU C. Library" <libc-alpha at sourceware dot org>
- Date: Thu, 9 Oct 2014 15:50:20 -0500
- Subject: Re: [PATCH v2] Add x86 32 bit vDSO time function support
- Authentication-results: sourceware.org; auth=none
- References: <5436D48C dot 2090509 at linux dot vnet dot ibm dot com>
On 10/09/2014 01:31 PM, Adhemerval Zanella wrote:
> diff --git a/sysdeps/unix/sysv/linux/i386/init-first.c b/sysdeps/unix/sysv/linux/i386/init-first.c
> new file mode 100644
> index 0000000..dc3b1ba
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/i386/init-first.c
> @@ -0,0 +1,52 @@
> +/* Initialization code run first thing by the ELF startup code. Linux/i386.
> + Copyright (C) 2014 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifdef SHARED
> +# include <time.h>
> +# include <sysdep.h>
> +# include <dl-vdso.h>
> +# include <libc-vdso.h>
> +
> +long int (*__vdso_clock_gettime) (clockid_t, struct timespec *)
> + __attribute__ ((nocommon));
> +libc_hidden_proto (__vdso_clock_gettime)
> +libc_hidden_data_def (__vdso_clock_gettime)
> +
> +static long int
> +clock_gettime_syscall (clockid_t id, struct timespec *tp)
> +{
> + INTERNAL_SYSCALL_DECL (err);
> + return INTERNAL_SYSCALL (clock_gettime, err, 2, id, tp);
> +}
> +
> +static inline void
> +__vdso_platform_setup (void)
> +{
> + PREPARE_VERSION (linux26, "LINUX_2.6", 61765110);
Perhaps:
PREPARE_VERSION_KNOWN (linux26, LINUX_2_6);
(here and several other places)
> +
> + void *p = _dl_vdso_vsym ("__vdso_clock_gettime", &linux26);
> + if (p == NULL)
> + p = clock_gettime_syscall;
> + PTR_MANGLE (p);
> + __vdso_clock_gettime = p;
> +}
> +
> +# define VDSO_SETUP __vdso_platform_setup
> +#endif
> +
> +#include <csu/init-first.c>
[...]
> diff --git a/sysdeps/unix/sysv/linux/x86/clock_gettime.c b/sysdeps/unix/sysv/linux/x86/clock_gettime.c
> new file mode 100644
> index 0000000..2547a8c
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/clock_gettime.c
> @@ -0,0 +1,38 @@
> +/* Get the current value of a clock. Linux/x86 version.
> + Copyright (C) 2014 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <libc-vdso.h>
> +
> +#ifdef SHARED
> +# define SYSCALL_GETTIME(id, tp) \
> + ({ long int (*f) (clockid_t, struct timespec *) = __vdso_clock_gettime; \
> + long int v_ret; \
> + PTR_DEMANGLE (f); \
> + v_ret = (*f) (id, tp); \
> + if (INTERNAL_SYSCALL_ERROR_P (v_ret, )) { \
> + __set_errno (INTERNAL_SYSCALL_ERRNO (v_ret, )); \
> + v_ret = -1; \
> + } \
> + v_ret; })
Does introducing the dispatch through function pointer here cause a
measurable performance regression on i386 kernels which lack the VDSO?
If so, is that a concern?
When I've tried this approach on ARM, it appears to do so (around 5%
slowdown).
> +# define INTERNAL_GETTIME(id, tp) \
> + ({ long int (*f) (clockid_t, struct timespec *) = __vdso_clock_gettime; \
> + PTR_DEMANGLE (f); \
> + (*f) (id, tp); })
> +#endif
I'm probably missing something, but I am failing to see the need for an
INTERNAL_GETTIME definition in
sysdeps/unix/sysv/linux/x86/clock_gettime.c. I know this patch is
merely moving existing code, but sysdeps/unix/sysv/linux/clock_gettime.c
does not use INTERNAL_GETTIME, and neither does
sysdeps/unix/clock_gettime.c.
INTERNAL_GETTIME is needed for timespec_get, but I am not seeing the
need to duplicate it for clock_gettime.