[PATCH v2] Add x86 32 bit vDSO time function support

[Adhemerval Zanella]

On 03-11-2014 18:30, Adhemerval Zanella wrote:
> On 09-10-2014 17:50, Nathan Lynch wrote:
>> On 10/09/2014 01:31 PM, Adhemerval Zanella wrote:
>>> +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)
> Thanks, I fixed it on all the places nows.
>
>>> +#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).
> Using a simple benchmark (in attachments) the difference in such scenarios is not
> as drastic as ARM it seems:
>
> kernel: Linux birita 3.13.0-39
> CPU:    Intel(R) Core(TM)2 CPU          6600  @ 2.40GHz
>
> EGLIBC 2.19-0ubuntu6.3: 1415.12 cycles
> GLIBC 2.20 master:      1421.66 cycles
>
>>
>>> +# 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.
> i386 does not define HAVE_CLOCK_GETTIME_VSYSCALL and thus:
>
> sysdeps/unix/sysv/linux/clock_gettime.c:
>
>  26: # define INTERNAL_VSYSCALL INTERNAL_SYSCALL
>
> and then if INTERNAL_GETTIME is not defined, it will as:
>
>  37 #ifndef INTERNAL_GETTIME
>  38 # define INTERNAL_GETTIME(id, tp) \
>  39   INTERNAL_VSYSCALL (clock_gettime, err, 2, id, tp)
>  40 #endif
>
> And without proper set the PTR_DEMANGLE is not called either.
>
> With PREPARE_VERSION_KNOWN fixes, is it ok to commit?
>
Send the missing simple benchmark.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test-vdso-i386.c
Type: text/x-csrc
Size: 1191 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20141103/4dd0d40d/attachment.bin>