From patchwork Thu Jan 29 00:57:47 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roland McGrath X-Patchwork-Id: 4839 Received: (qmail 18900 invoked by alias); 29 Jan 2015 00:57:53 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 17676 invoked by uid 89); 29 Jan 2015 00:57:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL, BAYES_00, LOTS_OF_MONEY autolearn=no version=3.3.2 X-HELO: topped-with-meat.com MIME-Version: 1.0 From: Roland McGrath To: "GNU C. Library" Subject: [PATCH roland/vdso_clock_gettime] x86: Clean up __vdso_clock_gettime variable. Message-Id: <20150129005747.3DAE82C3A92@topped-with-meat.com> Date: Wed, 28 Jan 2015 16:57:47 -0800 (PST) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=SvUDtp+0 c=1 sm=1 tr=0 a=WkljmVdYkabdwxfqvArNOQ==:117 a=14OXPxybAAAA:8 a=kj9zAlcOel0A:10 a=hOe2yjtxAAAA:8 a=eQv4mSv9xYlrFNBTEaQA:9 a=6iF8tTL3LMT_HLbP:21 a=sd_8Z99CAQPLdulD:21 a=CjuIK1q_8ugA:10 __vdso_clock_gettime is never actually used outside libc proper. It appeared to be if one grep'd the source, but the only apparent use was in dead code in x86_64/pthread_cond_timedwait.S; that code is never used in any configuration any more, because __ASSUME_FUTEX_CLOCK_REALTIME is defined unconditionally. The part of this patch that appears substantial is just removing that dead assembly code. In fact, it does not change the code that gets assembled at all. On both x86_64-linux-gnu and i686-linux-gnu I tested that check-abi has no regressions and that there are no significant changes to generated code. The only changes at all are places that were suboptimally using a GOT slot for __vdso_clock_gettime because the declaration in scope wasn't marked as hidden. This only came about because of the highly suspicious situation that libc_hidden_proto was put next to the definition site rather than in the header file after the declaration. Hackers and reviewers should raise a red flag for any case of *hidden_proto appearing anywhere but in a header file. Assuming no objections, I'll commit this as soon as the freeze is lifted. I think it would be safe and probably even wise to do it for 2.21, if Carlos concurs. This is new code added since 2.20, and without this change it both is trivially suboptimal and bloats .dynsym with a wholly unnecessary symbol (but a GLIBC_PRIVATE one, so it's not an ABI issue). Thanks, Roland 2015-01-28 Roland McGrath * sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S: Remove all code under [!__ASSUME_FUTEX_CLOCK_REALTIME], since that is defined unconditionally nowadays. This included the only reference to __vdso_clock_gettime that appears outside libc proper. * sysdeps/unix/sysv/linux/x86_64/Versions (libc: GLIBC_PRIVATE): Remove version set (containing only __vdso_clock_gettime). * sysdeps/unix/sysv/linux/x86/libc-vdso.h (__vdso_clock_gettime): Add attribute_hidden. * sysdeps/unix/sysv/linux/i386/init-first.c (__vdso_clock_gettime): Likewise. Drop __attribute__ ((nocommon)), libc_hidden_proto, and libc_hidden_data_def. * sysdeps/unix/sysv/linux/x86_64/init-first.c: Likewise. * sysdeps/unix/sysv/linux/x86_64/x32/init-first.c: Likewise. --- a/sysdeps/unix/sysv/linux/i386/init-first.c +++ b/sysdeps/unix/sysv/linux/i386/init-first.c @@ -23,9 +23,7 @@ # 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) + attribute_hidden; static long int clock_gettime_syscall (clockid_t id, struct timespec *tp) --- a/sysdeps/unix/sysv/linux/x86/libc-vdso.h +++ b/sysdeps/unix/sysv/linux/x86/libc-vdso.h @@ -24,7 +24,8 @@ #ifdef SHARED -extern long int (*__vdso_clock_gettime) (clockid_t, struct timespec *); +extern long int (*__vdso_clock_gettime) (clockid_t, struct timespec *) + attribute_hidden; #endif --- a/sysdeps/unix/sysv/linux/x86_64/Versions +++ b/sysdeps/unix/sysv/linux/x86_64/Versions @@ -6,9 +6,6 @@ libc { modify_ldt; } - GLIBC_PRIVATE { - __vdso_clock_gettime; - } } librt { --- a/sysdeps/unix/sysv/linux/x86_64/init-first.c +++ b/sysdeps/unix/sysv/linux/x86_64/init-first.c @@ -23,10 +23,7 @@ # 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) - + attribute_hidden; long int (*__vdso_getcpu) (unsigned *, unsigned *, void *) attribute_hidden; extern long int __syscall_clock_gettime (clockid_t, struct timespec *); --- a/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S +++ b/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S @@ -59,11 +59,7 @@ __pthread_cond_timedwait: pushq %r15 cfi_adjust_cfa_offset(8) cfi_rel_offset(%r15, 0) -#ifdef __ASSUME_FUTEX_CLOCK_REALTIME -# define FRAME_SIZE (32+8) -#else -# define FRAME_SIZE (48+8) -#endif +#define FRAME_SIZE (32+8) subq $FRAME_SIZE, %rsp cfi_adjust_cfa_offset(FRAME_SIZE) cfi_remember_state @@ -105,15 +101,6 @@ __pthread_cond_timedwait: 22: xorb %r15b, %r15b -#ifndef __ASSUME_FUTEX_CLOCK_REALTIME -# ifdef PIC - cmpl $0, __have_futex_clock_realtime(%rip) -# else - cmpl $0, __have_futex_clock_realtime -# endif - je .Lreltmo -#endif - /* Get internal lock. */ movl $1, %esi xorl %eax, %eax @@ -440,204 +427,6 @@ __pthread_cond_timedwait: 47: movq (%rsp), %rax jmp 48b - -#ifndef __ASSUME_FUTEX_CLOCK_REALTIME -.Lreltmo: - /* Get internal lock. */ - movl $1, %esi - xorl %eax, %eax - LOCK -# if cond_lock == 0 - cmpxchgl %esi, (%rdi) -# else - cmpxchgl %esi, cond_lock(%rdi) -# endif - jnz 1f - - /* Unlock the mutex. */ -2: movq 16(%rsp), %rdi - xorl %esi, %esi - callq __pthread_mutex_unlock_usercnt - - testl %eax, %eax - jne 46b - - movq 8(%rsp), %rdi - incq total_seq(%rdi) - incl cond_futex(%rdi) - addl $(1 << nwaiters_shift), cond_nwaiters(%rdi) - - /* Get and store current wakeup_seq value. */ - movq 8(%rsp), %rdi - movq wakeup_seq(%rdi), %r9 - movl broadcast_seq(%rdi), %edx - movq %r9, 24(%rsp) - movl %edx, 4(%rsp) - - /* Get the current time. */ -8: -# ifdef __NR_clock_gettime - /* Get the clock number. Note that the field in the condvar - structure stores the number minus 1. */ - movq 8(%rsp), %rdi - movl cond_nwaiters(%rdi), %edi - andl $((1 << nwaiters_shift) - 1), %edi - /* Only clocks 0 and 1 are allowed so far. Both are handled in the - kernel. */ - leaq 32(%rsp), %rsi -# ifdef SHARED - mov __vdso_clock_gettime@GOTPCREL(%rip), %RAX_LP - mov (%rax), %RAX_LP - PTR_DEMANGLE (%RAX_LP) - call *%rax -# else - movl $__NR_clock_gettime, %eax - syscall -# endif - - /* Compute relative timeout. */ - movq (%r13), %rcx - movq 8(%r13), %rdx - subq 32(%rsp), %rcx - subq 40(%rsp), %rdx -# else - leaq 24(%rsp), %rdi - xorl %esi, %esi - /* This call works because we directly jump to a system call entry - which preserves all the registers. */ - call JUMPTARGET(__gettimeofday) - - /* Compute relative timeout. */ - movq 40(%rsp), %rax - movl $1000, %edx - mul %rdx /* Milli seconds to nano seconds. */ - movq (%r13), %rcx - movq 8(%r13), %rdx - subq 32(%rsp), %rcx - subq %rax, %rdx -# endif - jns 12f - addq $1000000000, %rdx - decq %rcx -12: testq %rcx, %rcx - movq 8(%rsp), %rdi - movq $-ETIMEDOUT, %r14 - js 6f - - /* Store relative timeout. */ -21: movq %rcx, 32(%rsp) - movq %rdx, 40(%rsp) - - movl cond_futex(%rdi), %r12d - - /* Unlock. */ - LOCK -# if cond_lock == 0 - decl (%rdi) -# else - decl cond_lock(%rdi) -# endif - jne 3f - -.LcleanupSTART2: -4: callq __pthread_enable_asynccancel - movl %eax, (%rsp) - - leaq 32(%rsp), %r10 - LP_OP(cmp) $-1, dep_mutex(%rdi) - movq %r12, %rdx -# ifdef __ASSUME_PRIVATE_FUTEX - movl $FUTEX_WAIT, %eax - movl $(FUTEX_WAIT|FUTEX_PRIVATE_FLAG), %esi - cmove %eax, %esi -# else - movl $0, %eax - movl %fs:PRIVATE_FUTEX, %esi - cmove %eax, %esi -# if FUTEX_WAIT != 0 - orl $FUTEX_WAIT, %esi -# endif -# endif - addq $cond_futex, %rdi - movl $SYS_futex, %eax - syscall - movq %rax, %r14 - - movl (%rsp), %edi - callq __pthread_disable_asynccancel -.LcleanupEND2: - - /* Lock. */ - movq 8(%rsp), %rdi - movl $1, %esi - xorl %eax, %eax - LOCK -# if cond_lock == 0 - cmpxchgl %esi, (%rdi) -# else - cmpxchgl %esi, cond_lock(%rdi) -# endif - jne 5f - -6: movl broadcast_seq(%rdi), %edx - - movq woken_seq(%rdi), %rax - - movq wakeup_seq(%rdi), %r9 - - cmpl 4(%rsp), %edx - jne 53b - - cmpq 24(%rsp), %r9 - jbe 15f - - cmpq %rax, %r9 - ja 39b - -15: cmpq $-ETIMEDOUT, %r14 - jne 8b - - jmp 99b - - /* Initial locking failed. */ -1: -# if cond_lock != 0 - addq $cond_lock, %rdi -# endif - LP_OP(cmp) $-1, dep_mutex-cond_lock(%rdi) - movl $LLL_PRIVATE, %eax - movl $LLL_SHARED, %esi - cmovne %eax, %esi - callq __lll_lock_wait - jmp 2b - - /* Unlock in loop requires wakeup. */ -3: -# if cond_lock != 0 - addq $cond_lock, %rdi -# endif - LP_OP(cmp) $-1, dep_mutex-cond_lock(%rdi) - movl $LLL_PRIVATE, %eax - movl $LLL_SHARED, %esi - cmovne %eax, %esi - callq __lll_unlock_wake - jmp 4b - - /* Locking in loop failed. */ -5: -# if cond_lock != 0 - addq $cond_lock, %rdi -# endif - LP_OP(cmp) $-1, dep_mutex-cond_lock(%rdi) - movl $LLL_PRIVATE, %eax - movl $LLL_SHARED, %esi - cmovne %eax, %esi - callq __lll_lock_wait -# if cond_lock != 0 - subq $cond_lock, %rdi -# endif - jmp 6b -#endif .size __pthread_cond_timedwait, .-__pthread_cond_timedwait versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait, GLIBC_2_3_2) @@ -815,12 +604,6 @@ __condvar_cleanup2: .uleb128 .LcleanupEND1-.LcleanupSTART1 .uleb128 __condvar_cleanup2-.LSTARTCODE .uleb128 0 -#ifndef __ASSUME_FUTEX_CLOCK_REALTIME - .uleb128 .LcleanupSTART2-.LSTARTCODE - .uleb128 .LcleanupEND2-.LcleanupSTART2 - .uleb128 __condvar_cleanup2-.LSTARTCODE - .uleb128 0 -#endif .uleb128 .LcallUR-.LSTARTCODE .uleb128 .LENDCODE-.LcallUR .uleb128 0 --- a/sysdeps/unix/sysv/linux/x86_64/x32/init-first.c +++ b/sysdeps/unix/sysv/linux/x86_64/x32/init-first.c @@ -21,9 +21,7 @@ # 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) + attribute_hidden; static inline void _libc_vdso_platform_setup (void)