From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4962 invoked by alias); 9 Oct 2014 20:50:27 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 4933 invoked by uid 89); 9 Oct 2014 20:50:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Message-ID: <5436F50C.9070002@codesourcery.com> Date: Thu, 09 Oct 2014 20:50:00 -0000 From: Nathan Lynch User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: Adhemerval Zanella CC: "GNU C. Library" Subject: Re: [PATCH v2] Add x86 32 bit vDSO time function support References: <5436D48C.2090509@linux.vnet.ibm.com> In-Reply-To: <5436D48C.2090509@linux.vnet.ibm.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-SW-Source: 2014-10/txt/msg00300.txt.bz2 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 > + . */ > + > +#ifdef SHARED > +# include > +# include > +# include > +# include > + > +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 [...] > 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 > + . */ > + > +#include > + > +#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.