From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19752 invoked by alias); 3 Nov 2014 21:42:43 -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 19740 invoked by uid 89); 3 Nov 2014 21:42:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: e24smtp01.br.ibm.com Message-ID: <5457F6C7.7000105@linux.vnet.ibm.com> Date: Mon, 03 Nov 2014 21:42:00 -0000 From: Adhemerval Zanella User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Nathan Lynch CC: "GNU C. Library" Subject: Re: [PATCH v2] Add x86 32 bit vDSO time function support References: <5436D48C.2090509@linux.vnet.ibm.com> <5436F50C.9070002@codesourcery.com> <5457E5D5.6000103@linux.vnet.ibm.com> In-Reply-To: <5457E5D5.6000103@linux.vnet.ibm.com> Content-Type: multipart/mixed; boundary="------------000007070703080509020208" X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14110321-1524-0000-0000-000000EEA1A0 X-SW-Source: 2014-11/txt/msg00020.txt.bz2 This is a multi-part message in MIME format. --------------000007070703080509020208 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Content-length: 2794 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. --------------000007070703080509020208 Content-Type: text/x-csrc; name="test-vdso-i386.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="test-vdso-i386.c" Content-length: 1191 #include #include #include #include #include #ifdef __x86_64__ # define HP_TIMING_NOW(Var) \ ({ unsigned int _hi, _lo; \ asm volatile ("rdtsc" : "=a" (_lo), "=d" (_hi)); \ (Var) = ((unsigned long long int) _hi << 32) | _lo; }) #else # define HP_TIMING_NOW(Var) __asm__ __volatile__ ("rdtsc" : "=A" (Var)) #endif /* Compute the difference between START and END, storing into DIFF. */ #define HP_TIMING_DIFF(Diff, Start, End) ((Diff) = (End) - (Start)) /* Accumulate ADD into SUM. No attempt is made to be thread-safe. */ #define HP_TIMING_ACCUM_NT(Sum, Diff) ((Sum) += (Diff)) /* We use 64bit values for the times. */ typedef unsigned long long int hp_timing_t; #define NITER 10000000UL int main () { const clockid_t id = CLOCK_REALTIME; uint64_t i; hp_timing_t start, end, diff; int ret = 0; HP_TIMING_NOW (start); for (i=0; i