This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH roland/vdso_clock_gettime] x86: Clean up __vdso_clock_gettime variable.


__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  <roland@hack.frob.com>

	* 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 <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)
+  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 <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)
-
+  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 <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)
+  attribute_hidden;
 
 static inline void
 _libc_vdso_platform_setup (void)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]