[v2] Add x86 32 bit vDSO time function support

  Thanks for the review.


On 27-06-2014 18:58, Roland McGrath wrote:
>
>> 	* sysdeps/unix/sysv/linux/i386/gettimeofday.c: New file: add support
>> 	to vDSO.
> "support to use vDSO"?

Bad chose of words, I have change it to "set syscall fallback when vDSO is not presented."

>
>> --- a/sysdeps/unix/sysv/linux/i386/Makefile
>> +++ b/sysdeps/unix/sysv/linux/i386/Makefile
>> @@ -21,3 +21,7 @@ endif
>>  ifeq ($(subdir),stdlib)
>>  gen-as-const-headers += ucontext_i.sym
>>  endif
>> +
>> +ifeq ($(subdir),elf)
>> +sysdep_routines += dl-vdso
>> +endif
> Instead just move that from x86_64/Makefile to x86/Makefile so it's shared.
Fixed
>
>> +long int (*__vdso_clock_gettime) (clockid_t, struct timespec *)
>> +  __attribute__ ((nocommon));
>> +strong_alias (__vdso_clock_gettime, __GI___vdso_clock_gettime attribute_hidden)
> Use libc_hidden_data_def.
Fixed, I also changed x86_64 version.
>
>> +static inline void
>> +_libc_vdso_platform_setup (void)
> Don't use a _libc_ prefix like that on a static symbol.
> Just use a descriptive name.
Fixed, I also changed x86_64 version.
>
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/i386/time.c
>> @@ -0,0 +1,33 @@
>> +/* Copyright (C) 2014 Free Software Foundation, Inc.
> First line is a descriptive comment.
Fixed.
>
>> +# define TIME_FALLBACK  (void*)__time_syscall
> # define TIME_FALLBACK	((void *) &__time_syscall)
Fixed, I also fixed the gettimeofday.
>
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/x86/bits/libc-vdso.h
> [...]
>> +extern long int (*__vdso_clock_gettime) (clockid_t, struct timespec *);
> libc_hidden_proto here to go with libc_hidden_def.
>
> Also, this header really should not be in bits/.
> That only makes sense for installed headers.
Fixed and I moved it outside of bits.
>
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/x86/clock_gettime.c
>> @@ -0,0 +1,20 @@
>> +#include "bits/libc-vdso.h"
> Missing top comment and license header.
Fixed
>
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/x86/timespec_get.c
>> @@ -0,0 +1,10 @@
>> +#include "bits/libc-vdso.h"
> Here too.
>
I think I have addressed all the missing points, what about the following patch:

--

2014-06-29  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>
	    Stefani Seibold  <stefani@seibold.net>

	* sysdeps/unix/sysv/linux/x86_64/Makefile [sysdep_routing]: Move
	dl-vdso rule to ...
	* sysdeps/unix/sysv/linux/x86/Makefile [sysdep_routines]: ... here.
	* sysdeps/unix/sysv/linux/i386/gettimeofday.c: New file: set syscall
	fallback when vDSO is not presented.
	* sysdeps/unix/sysv/linux/x86_64/init-first.c (__vdso_clock_gettime):
	Define with libc_hidden_proto/libc_hidden_data_def definitions.
	(_libc_vdso_platform_setup): Rename to __vdso_platform_setup.
	* sysdeps/unix/sysv/linux/i386/init-first.c: New file: likewise.
	* sysdeps/unix/sysv/linux/i386/time.c: New file: likewise.
	* sysdeps/unix/sysv/linux/x86_64/bits/libc-vdso.h: Moved to ...
	* sysdeps/unix/sysv/linux/x86/libc-vdso.h: ... here.
	* sysdeps/unix/sysv/linux/x86_64/clock_gettime.c: Move to ...
	* sysdeps/unix/sysv/linux/x86/clock_gettime.c: ... here.
	* sysdeps/unix/sysv/linux/x86_64/gettimeofday.c: Move to ...
	* sysdeps/unix/sysv/linux/x86/gettimeofday.c: ... here. Also added
	fallback configurable symbol when vDSO is not available.
	* sysdeps/unix/sysv/linux/x86_64/time.c: Move to ...
	* sysdeps/unix/sysv/linux/x86/time.c: ... here. Also refactored to
	be able to redefine fallback symbol when vDSO is not available.
	* sysdeps/unix/sysv/linux/x86_64/timespec_get.c: Move to ...
	* sysdeps/unix/sysv/linux/x86/timespec_get.c: ... here.
---
  

Ping, is this patch ok now with the comments addressed?

On 29-06-2014 12:57, Adhemerval Zanella wrote:
> Thanks for the review.
>
>
> On 27-06-2014 18:58, Roland McGrath wrote:
>>> 	* sysdeps/unix/sysv/linux/i386/gettimeofday.c: New file: add support
>>> 	to vDSO.
>> "support to use vDSO"?
> Bad chose of words, I have change it to "set syscall fallback when vDSO is not presented."
>
>>> --- a/sysdeps/unix/sysv/linux/i386/Makefile
>>> +++ b/sysdeps/unix/sysv/linux/i386/Makefile
>>> @@ -21,3 +21,7 @@ endif
>>>  ifeq ($(subdir),stdlib)
>>>  gen-as-const-headers += ucontext_i.sym
>>>  endif
>>> +
>>> +ifeq ($(subdir),elf)
>>> +sysdep_routines += dl-vdso
>>> +endif
>> Instead just move that from x86_64/Makefile to x86/Makefile so it's shared.
> Fixed
>>> +long int (*__vdso_clock_gettime) (clockid_t, struct timespec *)
>>> +  __attribute__ ((nocommon));
>>> +strong_alias (__vdso_clock_gettime, __GI___vdso_clock_gettime attribute_hidden)
>> Use libc_hidden_data_def.
> Fixed, I also changed x86_64 version.
>>> +static inline void
>>> +_libc_vdso_platform_setup (void)
>> Don't use a _libc_ prefix like that on a static symbol.
>> Just use a descriptive name.
> Fixed, I also changed x86_64 version.
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/i386/time.c
>>> @@ -0,0 +1,33 @@
>>> +/* Copyright (C) 2014 Free Software Foundation, Inc.
>> First line is a descriptive comment.
> Fixed.
>>> +# define TIME_FALLBACK  (void*)__time_syscall
>> # define TIME_FALLBACK	((void *) &__time_syscall)
> Fixed, I also fixed the gettimeofday.
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/x86/bits/libc-vdso.h
>> [...]
>>> +extern long int (*__vdso_clock_gettime) (clockid_t, struct timespec *);
>> libc_hidden_proto here to go with libc_hidden_def.
>>
>> Also, this header really should not be in bits/.
>> That only makes sense for installed headers.
> Fixed and I moved it outside of bits.
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/x86/clock_gettime.c
>>> @@ -0,0 +1,20 @@
>>> +#include "bits/libc-vdso.h"
>> Missing top comment and license header.
> Fixed
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/x86/timespec_get.c
>>> @@ -0,0 +1,10 @@
>>> +#include "bits/libc-vdso.h"
>> Here too.
>>
> I think I have addressed all the missing points, what about the following patch:
>
> --
>
> 2014-06-29  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>
> 	    Stefani Seibold  <stefani@seibold.net>
>
> 	* sysdeps/unix/sysv/linux/x86_64/Makefile [sysdep_routing]: Move
> 	dl-vdso rule to ...
> 	* sysdeps/unix/sysv/linux/x86/Makefile [sysdep_routines]: ... here.
> 	* sysdeps/unix/sysv/linux/i386/gettimeofday.c: New file: set syscall
> 	fallback when vDSO is not presented.
> 	* sysdeps/unix/sysv/linux/x86_64/init-first.c (__vdso_clock_gettime):
> 	Define with libc_hidden_proto/libc_hidden_data_def definitions.
> 	(_libc_vdso_platform_setup): Rename to __vdso_platform_setup.
> 	* sysdeps/unix/sysv/linux/i386/init-first.c: New file: likewise.
> 	* sysdeps/unix/sysv/linux/i386/time.c: New file: likewise.
> 	* sysdeps/unix/sysv/linux/x86_64/bits/libc-vdso.h: Moved to ...
> 	* sysdeps/unix/sysv/linux/x86/libc-vdso.h: ... here.
> 	* sysdeps/unix/sysv/linux/x86_64/clock_gettime.c: Move to ...
> 	* sysdeps/unix/sysv/linux/x86/clock_gettime.c: ... here.
> 	* sysdeps/unix/sysv/linux/x86_64/gettimeofday.c: Move to ...
> 	* sysdeps/unix/sysv/linux/x86/gettimeofday.c: ... here. Also added
> 	fallback configurable symbol when vDSO is not available.
> 	* sysdeps/unix/sysv/linux/x86_64/time.c: Move to ...
> 	* sysdeps/unix/sysv/linux/x86/time.c: ... here. Also refactored to
> 	be able to redefine fallback symbol when vDSO is not available.
> 	* sysdeps/unix/sysv/linux/x86_64/timespec_get.c: Move to ...
> 	* sysdeps/unix/sysv/linux/x86/timespec_get.c: ... here.
> ---
>
> diff --git a/sysdeps/unix/sysv/linux/i386/gettimeofday.c b/sysdeps/unix/sysv/linux/i386/gettimeofday.c
> new file mode 100644
> index 0000000..f50aa76
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/i386/gettimeofday.c
> @@ -0,0 +1,34 @@
> +/* 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 <sys/time.h>
> +
> +#ifdef SHARED
> +
> +# include <dl-vdso.h>
> +# include <errno.h>
> +
> +/* If the vDSO is not available we fall back on the syscall.  */
> +static int
> +__gettimeofday_syscall (struct timeval *tv, struct timezone *tz)
> +{
> +  return INLINE_SYSCALL (gettimeofday, 2, tv, tz);
> +}
> +# define GETTIMEOFAY_FALLBACK (void*) &__gettimeofday_syscall
> +#endif
> +
> +#include <sysdeps/unix/sysv/linux/x86/gettimeofday.c>
> 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);
> +
> +  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/i386/time.c b/sysdeps/unix/sysv/linux/i386/time.c
> new file mode 100644
> index 0000000..8933e6b
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/i386/time.c
> @@ -0,0 +1,34 @@
> +/* time implementation call for 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 <dl-vdso.h>
> +# include <errno.h>
> +
> +/* If the vDSO is not available we fall back on the old vsyscall.  */
> +static time_t
> +__time_syscall (time_t *t)
> +{
> +  INTERNAL_SYSCALL_DECL (err);
> +  return INTERNAL_SYSCALL (time, err, 1, t);
> +}
> +# define TIME_FALLBACK  (void*) &__time_syscall
> +#endif
> +
> +#include <sysdeps/unix/sysv/linux/x86/time.c>
> diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile
> index 012125f..c827784 100644
> --- a/sysdeps/unix/sysv/linux/x86/Makefile
> +++ b/sysdeps/unix/sysv/linux/x86/Makefile
> @@ -22,3 +22,7 @@ libpthread-sysdep_routines += init-arch
>  libpthread-sysdep_routines += elision-lock elision-unlock elision-timed \
>  			      elision-trylock
>  endif
> +
> +ifeq ($(subdir),elf)
> +sysdep_routines += dl-vdso
> +endif
> 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..8635d5e
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/clock_gettime.c
> @@ -0,0 +1,38 @@
> +/* clock_gettime implementation call for Linux/x86.
> +   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; })
> +# define INTERNAL_GETTIME(id, tp) \
> +  ({ long int (*f) (clockid_t, struct timespec *) = __vdso_clock_gettime; \
> +  PTR_DEMANGLE (f);							  \
> +  f (id, tp); })
> +#endif
> +
> +#include <sysdeps/unix/sysv/linux/clock_gettime.c>
> diff --git a/sysdeps/unix/sysv/linux/x86/gettimeofday.c b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
> new file mode 100644
> index 0000000..587a7c4
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
> @@ -0,0 +1,57 @@
> +/* 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 <sys/time.h>
> +
> +#ifdef SHARED
> +
> +# include <dl-vdso.h>
> +
> +void *gettimeofday_ifunc (void) __asm__ ("__gettimeofday");
> +
> +void *
> +gettimeofday_ifunc (void)
> +{
> +  PREPARE_VERSION (linux26, "LINUX_2.6", 61765110);
> +
> +  /* If the vDSO is not available we fall back on the old vsyscall.  */
> +  return (_dl_vdso_vsym ("__vdso_gettimeofday", &linux26)
> +	  ?: GETTIMEOFAY_FALLBACK);
> +}
> +asm (".type __gettimeofday, %gnu_indirect_function");
> +
> +/* This is doing "libc_hidden_def (__gettimeofday)" but the compiler won't
> +   let us do it in C because it doesn't know we're defining __gettimeofday
> +   here in this file.  */
> +asm (".globl __GI___gettimeofday\n"
> +     "__GI___gettimeofday = __gettimeofday");
> +
> +#else
> +
> +# include <sysdep.h>
> +# include <errno.h>
> +
> +int
> +__gettimeofday (struct timeval *tv, struct timezone *tz)
> +{
> +  return INLINE_SYSCALL (gettimeofday, 2, tv, tz);
> +}
> +libc_hidden_def (__gettimeofday)
> +
> +#endif
> +weak_alias (__gettimeofday, gettimeofday)
> +libc_hidden_weak (gettimeofday)
> diff --git a/sysdeps/unix/sysv/linux/x86/libc-vdso.h b/sysdeps/unix/sysv/linux/x86/libc-vdso.h
> new file mode 100644
> index 0000000..f291924
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/libc-vdso.h
> @@ -0,0 +1,31 @@
> +/* Resolve function pointers to VDSO functions.
> +   Copyright (C) 2005-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/>.  */
> +
> +#ifndef _LIBC_VDSO_H
> +#define _LIBC_VDSO_H
> +
> +#include <time.h>
> +#include <sys/time.h>
> +
> +#ifdef SHARED
> +
> +extern long int (*__vdso_clock_gettime) (clockid_t, struct timespec *);
> +
> +#endif
> +
> +#endif /* _LIBC_VDSO_H */
> diff --git a/sysdeps/unix/sysv/linux/x86/time.c b/sysdeps/unix/sysv/linux/x86/time.c
> new file mode 100644
> index 0000000..183754b
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/time.c
> @@ -0,0 +1,52 @@
> +/* 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 <time.h>
> +
> +#ifdef SHARED
> +
> +#include <dl-vdso.h>
> +
> +void *time_ifunc (void) __asm__ ("time");
> +
> +void *
> +time_ifunc (void)
> +{
> +  PREPARE_VERSION (linux26, "LINUX_2.6", 61765110);
> +
> +  return _dl_vdso_vsym ("__vdso_time", &linux26) ?: TIME_FALLBACK;
> +}
> +asm (".type time, %gnu_indirect_function");
> +
> +/* This is doing "libc_hidden_def (time)" but the compiler won't let us do
> +   it in C because it doesn't know we're defining __gettimeofday here in
> +    this file.  */
> +asm (".globl __GI_time\n"
> +     "__GI_time = time");
> +
> +#else
> +
> +# include <sysdep.h>
> +
> +time_t
> +time (time_t *t)
> +{
> +  INTERNAL_SYSCALL_DECL (err);
> +  return INTERNAL_SYSCALL (time, err, 1, t);
> +}
> +
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/x86/timespec_get.c b/sysdeps/unix/sysv/linux/x86/timespec_get.c
> new file mode 100644
> index 0000000..ef9f655
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/timespec_get.c
> @@ -0,0 +1,28 @@
> +/* timespec_get implementation call for Linux/x86.
> +   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 INTERNAL_GETTIME(id, tp) \
> +  ({ long int (*f) (clockid_t, struct timespec *) = __vdso_clock_gettime; \
> +  PTR_DEMANGLE (f);							  \
> +  f (id, tp); })
> +#endif
> +
> +#include <./sysdeps/unix/sysv/linux/timespec_get.c>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/Makefile b/sysdeps/unix/sysv/linux/x86_64/Makefile
> index d6a9d36..9b82155 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/Makefile
> +++ b/sysdeps/unix/sysv/linux/x86_64/Makefile
> @@ -13,7 +13,3 @@ endif
>  ifeq ($(subdir),misc)
>  gen-as-const-headers += sigaltstack-offsets.sym
>  endif
> -
> -ifeq ($(subdir),elf)
> -sysdep_routines += dl-vdso
> -endif
> diff --git a/sysdeps/unix/sysv/linux/x86_64/bits/libc-vdso.h b/sysdeps/unix/sysv/linux/x86_64/bits/libc-vdso.h
> deleted file mode 100644
> index f291924..0000000
> --- a/sysdeps/unix/sysv/linux/x86_64/bits/libc-vdso.h
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -/* Resolve function pointers to VDSO functions.
> -   Copyright (C) 2005-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/>.  */
> -
> -#ifndef _LIBC_VDSO_H
> -#define _LIBC_VDSO_H
> -
> -#include <time.h>
> -#include <sys/time.h>
> -
> -#ifdef SHARED
> -
> -extern long int (*__vdso_clock_gettime) (clockid_t, struct timespec *);
> -
> -#endif
> -
> -#endif /* _LIBC_VDSO_H */
> diff --git a/sysdeps/unix/sysv/linux/x86_64/clock_gettime.c b/sysdeps/unix/sysv/linux/x86_64/clock_gettime.c
> deleted file mode 100644
> index f712110..0000000
> --- a/sysdeps/unix/sysv/linux/x86_64/clock_gettime.c
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -#include "bits/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; })
> -# define INTERNAL_GETTIME(id, tp) \
> -  ({ long int (*f) (clockid_t, struct timespec *) = __vdso_clock_gettime; \
> -  PTR_DEMANGLE (f);							  \
> -  f (id, tp); })
> -#endif
> -
> -#include "../clock_gettime.c"
> diff --git a/sysdeps/unix/sysv/linux/x86_64/gettimeofday.c b/sysdeps/unix/sysv/linux/x86_64/gettimeofday.c
> index 440ca7f..defefbc 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/x86_64/gettimeofday.c
> @@ -18,42 +18,9 @@
>  #include <sys/time.h>
>
>  #ifdef SHARED
> -
> -# include <dl-vdso.h>
> -
> +/* If the vDSO is not available we fall back on the old vsyscall.  */
>  # define VSYSCALL_ADDR_vgettimeofday	0xffffffffff600000ul
> -
> -void *gettimeofday_ifunc (void) __asm__ ("__gettimeofday");
> -
> -void *
> -gettimeofday_ifunc (void)
> -{
> -  PREPARE_VERSION (linux26, "LINUX_2.6", 61765110);
> -
> -  /* If the vDSO is not available we fall back on the old vsyscall.  */
> -  return (_dl_vdso_vsym ("__vdso_gettimeofday", &linux26)
> -	  ?: (void *) VSYSCALL_ADDR_vgettimeofday);
> -}
> -asm (".type __gettimeofday, %gnu_indirect_function");
> -
> -/* This is doing "libc_hidden_def (__gettimeofday)" but the compiler won't
> -   let us do it in C because it doesn't know we're defining __gettimeofday
> -   here in this file.  */
> -asm (".globl __GI___gettimeofday\n"
> -     "__GI___gettimeofday = __gettimeofday");
> -
> -#else
> -
> -# include <sysdep.h>
> -# include <errno.h>
> -
> -int
> -__gettimeofday (struct timeval *tv, struct timezone *tz)
> -{
> -  return INLINE_SYSCALL (gettimeofday, 2, tv, tz);
> -}
> -libc_hidden_def (__gettimeofday)
> -
> +# define GETTIMEOFAY_FALLBACK  (void*)VSYSCALL_ADDR_vgettimeofday
>  #endif
> -weak_alias (__gettimeofday, gettimeofday)
> -libc_hidden_weak (gettimeofday)
> +
> +#include <sysdeps/unix/sysv/linux/x86/gettimeofday.c>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/init-first.c b/sysdeps/unix/sysv/linux/x86_64/init-first.c
> index fb090df..827751a 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/init-first.c
> +++ b/sysdeps/unix/sysv/linux/x86_64/init-first.c
> @@ -20,20 +20,20 @@
>  # include <time.h>
>  # include <sysdep.h>
>  # include <dl-vdso.h>
> -# include <bits/libc-vdso.h>
> +# include <libc-vdso.h>
>
>  long int (*__vdso_clock_gettime) (clockid_t, struct timespec *)
>    __attribute__ ((nocommon));
> -strong_alias (__vdso_clock_gettime, __GI___vdso_clock_gettime attribute_hidden)
> +libc_hidden_proto (__vdso_clock_gettime)
> +libc_hidden_data_def (__vdso_clock_gettime)
>
>  long int (*__vdso_getcpu) (unsigned *, unsigned *, void *) attribute_hidden;
>
> -
>  extern long int __syscall_clock_gettime (clockid_t, struct timespec *);
>
>
>  static inline void
> -_libc_vdso_platform_setup (void)
> +__vdso_platform_setup (void)
>  {
>    PREPARE_VERSION (linux26, "LINUX_2.6", 61765110);
>
> @@ -41,7 +41,7 @@ _libc_vdso_platform_setup (void)
>    if (p == NULL)
>      p = __syscall_clock_gettime;
>    PTR_MANGLE (p);
> -  __GI___vdso_clock_gettime = p;
> +  __vdso_clock_gettime = p;
>
>    p = _dl_vdso_vsym ("__vdso_getcpu", &linux26);
>    /* If the vDSO is not available we fall back on the old vsyscall.  */
> @@ -52,7 +52,7 @@ _libc_vdso_platform_setup (void)
>    __vdso_getcpu = p;
>  }
>
> -# define VDSO_SETUP _libc_vdso_platform_setup
> +# define VDSO_SETUP __vdso_platform_setup
>  #endif
>
>  #include <csu/init-first.c>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/time.c b/sysdeps/unix/sysv/linux/x86_64/time.c
> index 79f1fab..ec1393b 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/time.c
> +++ b/sysdeps/unix/sysv/linux/x86_64/time.c
> @@ -16,45 +16,9 @@
>     <http://www.gnu.org/licenses/>.  */
>
>  #ifdef SHARED
> -/* Redefine time so that the compiler won't complain about the type
> -   mismatch with the IFUNC selector in strong_alias, below.  */
> -#undef time
> -#define time __redirect_time
> -#include <time.h>
> -
> -#include <dl-vdso.h>
> -
> +/* If the vDSO is not available we fall back on the old vsyscall.  */
>  #define VSYSCALL_ADDR_vtime	0xffffffffff600400
> -
> -/* Avoid DWARF definition DIE on ifunc symbol so that GDB can handle
> -   ifunc symbol properly.  */
> -extern __typeof (__redirect_time) __libc_time;
> -void *time_ifunc (void) __asm__ ("__libc_time");
> -
> -void *
> -time_ifunc (void)
> -{
> -  PREPARE_VERSION (linux26, "LINUX_2.6", 61765110);
> -
> -  /* If the vDSO is not available we fall back on the old vsyscall.  */
> -  return _dl_vdso_vsym ("__vdso_time", &linux26) ?: (void *) VSYSCALL_ADDR_vtime;
> -}
> -__asm (".type __libc_time, %gnu_indirect_function");
> -
> -#undef time
> -strong_alias (__libc_time, time)
> -libc_hidden_ver (__libc_time, time)
> -
> -#else
> -
> -# include <time.h>
> -# include <sysdep.h>
> -
> -time_t
> -time (time_t *t)
> -{
> -  INTERNAL_SYSCALL_DECL (err);
> -  return INTERNAL_SYSCALL (time, err, 1, t);
> -}
> -
> +#define TIME_FALLBACK           (void*)VSYSCALL_ADDR_vtime
>  #endif
> +
> +#include <sysdeps/unix/sysv/linux/x86/time.c>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/timespec_get.c b/sysdeps/unix/sysv/linux/x86_64/timespec_get.c
> deleted file mode 100644
> index cb26068..0000000
> --- a/sysdeps/unix/sysv/linux/x86_64/timespec_get.c
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -#include "bits/libc-vdso.h"
> -
> -#ifdef SHARED
> -# define INTERNAL_GETTIME(id, tp) \
> -  ({ long int (*f) (clockid_t, struct timespec *) = __vdso_clock_gettime; \
> -  PTR_DEMANGLE (f);							  \
> -  f (id, tp); })
> -#endif
> -
> -#include "../timespec_get.c"
>
  

> 	* sysdeps/unix/sysv/linux/x86_64/Makefile [sysdep_routing]: Move
> 	dl-vdso rule to ...
> 	* sysdeps/unix/sysv/linux/x86/Makefile [sysdep_routines]: ... here.

Typo: s/routing/routines/.  Also, [] is for identifying conditional
sections (if* in makefiles, #if* in C).  Use () for identifying the entity
being changed.  Also, there is no rule here.  It's just appending it to the
list (or not).  I would have written:

	* sysdeps/unix/sysv/linux/x86/Makefile [$(subdir) = elf]
	(sysdep_routines): Add dl-vdso here, ...
	* sysdeps/unix/sysv/linux/x86_64/Makefile [$(subdir) = elf]
	(sysdep_routines): ... not here.

> 	* sysdeps/unix/sysv/linux/x86/gettimeofday.c: ... here. Also added

Two spaces between sentences.

> 	* sysdeps/unix/sysv/linux/x86/time.c: ... here. Also refactored to

And here.

> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/i386/gettimeofday.c
> @@ -0,0 +1,34 @@
> +/* Copyright (C) 2014 Free Software Foundation, Inc.

Still needs a top-line descriptive comment.

> +# define GETTIMEOFAY_FALLBACK (void*) &__gettimeofday_syscall

Put parens around the rhs so it's a single syntactic unit.

> +++ b/sysdeps/unix/sysv/linux/i386/time.c
> @@ -0,0 +1,34 @@
> +/* time implementation call for Linux/i386.

/* time -- Get number of seconds since Epoch.  Linux/i386 version.

> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/clock_gettime.c
> @@ -0,0 +1,38 @@
> +/* clock_gettime implementation call for Linux/x86.

/* Get the current value of a clock.  Linux/x86 version.

(Here I copied the description from the stub file rt/clock_gettime.c.)

> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
> @@ -0,0 +1,57 @@
> +/* Copyright (C) 2014 Free Software Foundation, Inc.

Still needs a top-line descriptive comment.

> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/time.c
> @@ -0,0 +1,52 @@
> +/* Copyright (C) 2014 Free Software Foundation, Inc.

Still needs a top-line descriptive comment.

> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/timespec_get.c
> @@ -0,0 +1,28 @@
> +/* timespec_get implementation call for Linux/x86.

This is not a sensical English sentence fragment.
I think you meant "timespec_get call implementation".
But that's not actually descriptive.

> +#ifdef SHARED
> +# define INTERNAL_GETTIME(id, tp) \
> +  ({ long int (*f) (clockid_t, struct timespec *) = __vdso_clock_gettime; \
> +  PTR_DEMANGLE (f);							  \
> +  f (id, tp); })

Why isn't this just an inline function?  If it had to be a macro, it should
have line breaks around ({ and }) to be more readable.  Either way, it
should use (*f) (...) to call via the function pointer.


You didn't mention what testing you did.  For this sort of change, it is
important to test (and report about it) for more than one kernel version,
including at least one and one without the vDSO support that this code
should use but not rely on.

I tend to think this is getting a bit close to freeze time for a
substantial semantic change like this one.  But I'll defer that paranoia to
others, and if the folks here who are distro package maintainers are not
worried about it then I won't object.


Thanks,
Roland