public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
@ 2011-01-12 17:54 Rayson Ho
  2011-01-12 18:18 ` Roland McGrath
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-01-12 17:54 UTC (permalink / raw)
  To: libc-alpha; +Cc: systemtap

What is it?
This is a patch against the "remotes/origin/roland/systemtap" branch. It
adds SystemTap probe points to the Pthread library to allow developers
and system administrators to trace and debug pthread programs.

ChangeLog:

2011-01-12  Rayson Ho <rho@redhat.com>
        
	* DESIGN-systemtap-probes.txt: New file. (documentation)
	* pthread_create.c: Added SystemTap Pthreads probes.
	* pthread_join.c: Likewise.
	* pthread_mutex_destroy.c: Likewise.
	* pthread_mutex_init.c: Likewise.
	* pthread_mutex_lock.c: Likewise.
	* pthread_mutex_unlock.c: Likewise.
	* pthread_rwlock_destroy.c: Likewise.
	* pthread_rwlock_rdlock.c: Likewise.
	* pthread_rwlock_unlock.c: Likewise.
	* pthread_rwlock_wrlock.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.

Testing:
- no regressions with or without systemtap probes compiled into pthread
- compilation works with or without systemtap enabled


diff --git a/nptl/DESIGN-systemtap-probes.txt
b/nptl/DESIGN-systemtap-probes.txt
index e69de29..17ec433 100644
--- a/nptl/DESIGN-systemtap-probes.txt
+++ b/nptl/DESIGN-systemtap-probes.txt
@@ -0,0 +1,33 @@
+Systemtap is a dynamic tracing/instrumenting tool available on Linux.
Probes that are not fired have close to zero runtime overhead.
+
+The following probes are available for NPTL:
+
+Thread creation & Join Probes
+=============================
+create   - probe for pthread_create(3) - arg1 = thread ID, arg2 =
start_routine, arg3 = arguments
+start    - probe for actual thread creation, arg1 = struct pthread
(members include thread ID, process ID)
+join     - probe for pthread_join(3)   - arg1 = thread ID
+join_ret - probe for pthread_join(3) return - arg1 = thread ID, arg2 =
return value
+
+Lock-related Probes
+===================
+mutex_init    - probe for pthread_mutex_init(3) - arg1 = address of
mutex lock
+mutex_acquired- probe for pthread_mutex_lock(3) - arg1 = address of
mutex lock
+mutex_block   - probe for resume from _possible_ mutex block event -
arg1 = address of mutex lock
+mutex_entry   - probe for entry to the pthread_mutex_lock(3) function,
- arg1 = address of mutex lock
+mutex_release - probe for pthread_mutex_unlock(3) after the successful
release of a mutex lock - arg1 = address of mutex lock
+mutex_destroy - probe for pthread_mutex_destroy(3) - arg1 = address of
mutex lock
+rwlock_destroy- probe for pthread_rwlock_destroy(3) - arg1 = address of
rw lock
+rwlock_acquire_write-probe for pthread_rwlock_wrlock(3) - arg1 =
address of rw lock
+rwlock_unlock - probe for pthread_rwlock_unlock(3) - arg1 = address of
rw lock
+
+lock_wait         - probe in low-level lock code, only fired when futex
is called (i.e. when trying to acquire a contented lock)
+lock_wait_private - probe in low-level lock code, only fired when futex
is called (i.e. when trying to acquire a contented lock)
+
+Condition variable Probes
+=========================
+cond_init - probe for pthread_cond_init(3) - arg1 = condition, arg2 =
attr
+cond_destroy- probe for pthread_condattr_destroy(3) - arg1 = attr
+cond_wait - probe for pthread_cond_wait(3) - arg1 = condition, arg2 =
mutex lock
+cond_signal - probe for pthread_cond_signal(3) - arg1 = condition
+cond_broadcast - probe for pthread_cond_broadcast(3) - arg1 = condition
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 4075dd9..5fdf2c9 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr,
start_routine, arg)
   /* Pass the descriptor to the caller.  */
   *newthread = (pthread_t) pd;
 
+  LIBC_PROBE (newthread, 2, start_routine, arg);
+
   /* Start the thread.  */
   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
 }
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 6a87a8b..447ee31 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -23,6 +23,8 @@
 #include <atomic.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 static void
 cleanup (void *arg)
@@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
   struct pthread *self = THREAD_SELF;
   int result = 0;
 
+  LIBC_PROBE(join,1, threadid);
+
   /* During the wait we change to asynchronous cancellation.  If we
      are canceled the thread we are waiting for must be marked as
      un-wait-ed for again.  */
@@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
       __free_tcb (pd);
     }
 
+  LIBC_PROBE(join_ret, 3, threadid, result, pd->result);
+
   return result;
 }
diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
index e2c9f8a..181c4aa 100644
--- a/nptl/pthread_mutex_destroy.c
+++ b/nptl/pthread_mutex_destroy.c
@@ -20,11 +20,15 @@
 #include <errno.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 int
 __pthread_mutex_destroy (mutex)
      pthread_mutex_t *mutex;
 {
+  LIBC_PROBE(cond_destroy, 1, mutex);
+
   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
       && mutex->__data.__nusers != 0)
     return EBUSY;
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index d9b1ef0..d22abea 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -24,6 +24,8 @@
 #include <kernel-features.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 static const struct pthread_mutexattr default_attr =
   {
     /* Default is a normal mutex, not shared between processes.  */
@@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
   // mutex->__spins = 0;	already done by memset
   // mutex->__next = NULL;	already done by memset
 
+  LIBC_PROBE(mutex_init, 1, mutex);
+
   return 0;
 }
 strong_alias (__pthread_mutex_init, pthread_mutex_init)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 50dc188..c526da8 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -24,6 +24,7 @@
 #include <not-cancel.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 
 #ifndef LLL_MUTEX_LOCK
@@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
 
   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
+
+  LIBC_PROBE(mutex_entry, 1, mutex);
+
   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
     return __pthread_mutex_lock_full (mutex);
 
@@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
       /* Normal mutex.  */
       LLL_MUTEX_LOCK (mutex);
       assert (mutex->__data.__owner == 0);
+
+      LIBC_PROBE(mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
     {
@@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
 
       assert (mutex->__data.__owner == 0);
       mutex->__data.__count = 1;
+
+      LIBC_PROBE(mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
     {
@@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
 	    }
 	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 
+          LIBC_PROBE(mutex_block, 1, mutex);
+
 	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
 	}
       assert (mutex->__data.__owner == 0);
@@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE(mutex_acquire, 1, mutex);
+
   return 0;
 }
 
@@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  }
 	while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
 
+        LIBC_PROBE(mutex_block, 1, mutex);
+
 	assert (mutex->__data.__owner == 0);
 	mutex->__data.__count = 1;
       }
@@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE(mutex_acquire, 1, mutex);
+
   return 0;
 }
 #ifndef __pthread_mutex_lock
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index f9fe10b..479e500 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 static int
 internal_function
@@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       /* Unlock.  */
       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return 0;
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
@@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
int decr)
 			PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return __pthread_tpp_change_priority (oldprio, -1);
 
     default:
@@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
int decr)
       return EINVAL;
     }
 
+  LIBC_PROBE (mutex_release, 1, mutex);
   return 0;
 }
 
diff --git a/nptl/pthread_rwlock_destroy.c
b/nptl/pthread_rwlock_destroy.c
index 28fd24b..84aa693 100644
--- a/nptl/pthread_rwlock_destroy.c
+++ b/nptl/pthread_rwlock_destroy.c
@@ -18,12 +18,15 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
 __pthread_rwlock_destroy (rwlock)
      pthread_rwlock_t *rwlock;
 {
+  LIBC_PROBE (rwlock_destroy, 1, rwlock);
+
   /* Nothing to be done.  For now.  */
   return 0;
 }
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 31eb508..e8637b1 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire read lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_rdlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (rlock_entry, 1, rwlock);
+
   /* Make sure we are along.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -49,6 +52,12 @@ __pthread_rwlock_rdlock (rwlock)
 	      --rwlock->__data.__nr_readers;
 	      result = EAGAIN;
 	    }
+          else
+            {
+	      /* systemtap pthread probe - this is the only place where
+	         we get this read-write lock */
+              LIBC_PROBE (rwlock_acquire_read, 1, rwlock);
+            }
 
 	  break;
 	}
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
index a7ef71a..a6e8d87 100644
--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -22,11 +22,14 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 /* Unlock RWLOCK.  */
 int
 __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
 {
+  LIBC_PROBE (rwlock_unlock, 1, rwlock);
+
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
   if (rwlock->__data.__writer)
     rwlock->__data.__writer = 0;
diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
index 64fe970..2f5fcb2 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire write lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (wlock_entry, 1, rwlock);
+
   /* Make sure we are along.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -41,6 +44,11 @@ __pthread_rwlock_wrlock (rwlock)
 	{
 	  /* Mark self as writer.  */
 	  rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
+
+	  /* systemtap pthread probe - this is the only place where we can
+	     get this read-write lock. */
+          LIBC_PROBE (rwlock_acquire_write, 1, rwlock);
+
 	  break;
 	}
 
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
index 3195db2..b789f64 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
@@ -22,6 +22,15 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#if defined USE_STAP_PROBE && (defined IN_LIB || !defined NOT_IN_libc)
+ #include <stap-probe.h>
+ #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
LIBC_PROBE_ASM(lll_lock_wait_private, arg1)
+ #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
LIBC_PROBE_ASM(lll_lock_wait, arg1)
+#else
+ #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
+ #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
+#endif
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +100,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(%rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
@@ -130,7 +140,8 @@ __lll_lock_wait:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	PTHREAD_PROBE_LL_LOCKWAIT(%rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-12 17:54 [PATCH] Adding systemtap probe points in pthread library (slightly revised again) Rayson Ho
@ 2011-01-12 18:18 ` Roland McGrath
  2011-01-12 18:50   ` Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-01-12 18:18 UTC (permalink / raw)
  To: Rayson Ho; +Cc: libc-alpha, systemtap

> +  LIBC_PROBE(join,1, threadid);

Missing space before paren, missing space after comma.
Half the additions have similar whitespace errors.
Please just read over ALL YOUR CODE before you submit.

> +          else
> +            {
> +	      /* systemtap pthread probe - this is the only place where
> +	         we get this read-write lock */
> +              LIBC_PROBE (rwlock_acquire_read, 1, rwlock);
> +            }

Don't add braces around a single statement.  This comment is not formatted
properly.  Look at the existing code and see how we write comments, please.
This comment itself doesn't seem to say anything useful anyway.

> +1:	PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(%rdi)

Again, missing space before paren.

This stuff is not rocket science, and this is the nth time in a review of
the same code I've told you about the same errors.  Just look at the
existing code and match the formatting conventions you see there.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-12 18:18 ` Roland McGrath
@ 2011-01-12 18:50   ` Rayson Ho
  2011-01-12 20:07     ` Bert Wesarg
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-01-12 18:50 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-alpha, systemtap

Thanks a lot for the comment.

- fixed missing space before paren for the MARCOs
- removed comments (most of the comments were originally written to
explain why the probes were placed in the specific locations...)


What is it?
This is a patch against the "remotes/origin/roland/systemtap" branch. It
adds SystemTap probe points to the Pthread library to allow developers
and system administrators to trace and debug pthread programs.

ChangeLog:

2011-01-12  Rayson Ho <rho@redhat.com>
        
	* DESIGN-systemtap-probes.txt: New file. (documentation)
	* pthread_create.c: Added SystemTap Pthreads probes.
	* pthread_join.c: Likewise.
	* pthread_mutex_destroy.c: Likewise.
	* pthread_mutex_init.c: Likewise.
	* pthread_mutex_lock.c: Likewise.
	* pthread_mutex_unlock.c: Likewise.
	* pthread_rwlock_destroy.c: Likewise.
	* pthread_rwlock_rdlock.c: Likewise.
	* pthread_rwlock_unlock.c: Likewise.
	* pthread_rwlock_wrlock.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.

Testing:
- no regressions with or without systemtap probes compiled into pthread
- compilation works with or without systemtap enabled

diff --git a/nptl/DESIGN-systemtap-probes.txt
b/nptl/DESIGN-systemtap-probes.txt
index e69de29..17ec433 100644
--- a/nptl/DESIGN-systemtap-probes.txt
+++ b/nptl/DESIGN-systemtap-probes.txt
@@ -0,0 +1,33 @@
+Systemtap is a dynamic tracing/instrumenting tool available on Linux.
Probes that are not fired have close to zero runtime overhead.
+
+The following probes are available for NPTL:
+
+Thread creation & Join Probes
+=============================
+create   - probe for pthread_create(3) - arg1 = thread ID, arg2 =
start_routine, arg3 = arguments
+start    - probe for actual thread creation, arg1 = struct pthread
(members include thread ID, process ID)
+join     - probe for pthread_join(3)   - arg1 = thread ID
+join_ret - probe for pthread_join(3) return - arg1 = thread ID, arg2 =
return value
+
+Lock-related Probes
+===================
+mutex_init    - probe for pthread_mutex_init(3) - arg1 = address of
mutex lock
+mutex_acquired- probe for pthread_mutex_lock(3) - arg1 = address of
mutex lock
+mutex_block   - probe for resume from _possible_ mutex block event -
arg1 = address of mutex lock
+mutex_entry   - probe for entry to the pthread_mutex_lock(3) function,
- arg1 = address of mutex lock
+mutex_release - probe for pthread_mutex_unlock(3) after the successful
release of a mutex lock - arg1 = address of mutex lock
+mutex_destroy - probe for pthread_mutex_destroy(3) - arg1 = address of
mutex lock
+rwlock_destroy- probe for pthread_rwlock_destroy(3) - arg1 = address of
rw lock
+rwlock_acquire_write-probe for pthread_rwlock_wrlock(3) - arg1 =
address of rw lock
+rwlock_unlock - probe for pthread_rwlock_unlock(3) - arg1 = address of
rw lock
+
+lock_wait         - probe in low-level lock code, only fired when futex
is called (i.e. when trying to acquire a contented lock)
+lock_wait_private - probe in low-level lock code, only fired when futex
is called (i.e. when trying to acquire a contented lock)
+
+Condition variable Probes
+=========================
+cond_init - probe for pthread_cond_init(3) - arg1 = condition, arg2 =
attr
+cond_destroy- probe for pthread_condattr_destroy(3) - arg1 = attr
+cond_wait - probe for pthread_cond_wait(3) - arg1 = condition, arg2 =
mutex lock
+cond_signal - probe for pthread_cond_signal(3) - arg1 = condition
+cond_broadcast - probe for pthread_cond_broadcast(3) - arg1 = condition
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 4075dd9..5fdf2c9 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr,
start_routine, arg)
   /* Pass the descriptor to the caller.  */
   *newthread = (pthread_t) pd;
 
+  LIBC_PROBE (newthread, 2, start_routine, arg);
+
   /* Start the thread.  */
   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
 }
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 6a87a8b..7cbeb1d 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -23,6 +23,8 @@
 #include <atomic.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 static void
 cleanup (void *arg)
@@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
   struct pthread *self = THREAD_SELF;
   int result = 0;
 
+  LIBC_PROBE (join,1, threadid);
+
   /* During the wait we change to asynchronous cancellation.  If we
      are canceled the thread we are waiting for must be marked as
      un-wait-ed for again.  */
@@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
       __free_tcb (pd);
     }
 
+  LIBC_PROBE (join_ret, 3, threadid, result, pd->result);
+
   return result;
 }
diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
index e2c9f8a..00037c5 100644
--- a/nptl/pthread_mutex_destroy.c
+++ b/nptl/pthread_mutex_destroy.c
@@ -20,11 +20,15 @@
 #include <errno.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 int
 __pthread_mutex_destroy (mutex)
      pthread_mutex_t *mutex;
 {
+  LIBC_PROBE (cond_destroy, 1, mutex);
+
   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
       && mutex->__data.__nusers != 0)
     return EBUSY;
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index d9b1ef0..a025a38 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -24,6 +24,8 @@
 #include <kernel-features.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 static const struct pthread_mutexattr default_attr =
   {
     /* Default is a normal mutex, not shared between processes.  */
@@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
   // mutex->__spins = 0;	already done by memset
   // mutex->__next = NULL;	already done by memset
 
+  LIBC_PROBE (mutex_init, 1, mutex);
+
   return 0;
 }
 strong_alias (__pthread_mutex_init, pthread_mutex_init)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 50dc188..fdec784 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -24,6 +24,7 @@
 #include <not-cancel.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 
 #ifndef LLL_MUTEX_LOCK
@@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
 
   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
+
+  LIBC_PROBE (mutex_entry, 1, mutex);
+
   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
     return __pthread_mutex_lock_full (mutex);
 
@@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
       /* Normal mutex.  */
       LLL_MUTEX_LOCK (mutex);
       assert (mutex->__data.__owner == 0);
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
     {
@@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
 
       assert (mutex->__data.__owner == 0);
       mutex->__data.__count = 1;
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
     {
@@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
 	    }
 	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 
+          LIBC_PROBE (mutex_block, 1, mutex);
+
 	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
 	}
       assert (mutex->__data.__owner == 0);
@@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquire, 1, mutex);
+
   return 0;
 }
 
@@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  }
 	while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
 
+        LIBC_PROBE (mutex_block, 1, mutex);
+
 	assert (mutex->__data.__owner == 0);
 	mutex->__data.__count = 1;
       }
@@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquire, 1, mutex);
+
   return 0;
 }
 #ifndef __pthread_mutex_lock
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index f9fe10b..479e500 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 static int
 internal_function
@@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       /* Unlock.  */
       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return 0;
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
@@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
int decr)
 			PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return __pthread_tpp_change_priority (oldprio, -1);
 
     default:
@@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
int decr)
       return EINVAL;
     }
 
+  LIBC_PROBE (mutex_release, 1, mutex);
   return 0;
 }
 
diff --git a/nptl/pthread_rwlock_destroy.c
b/nptl/pthread_rwlock_destroy.c
index 28fd24b..84aa693 100644
--- a/nptl/pthread_rwlock_destroy.c
+++ b/nptl/pthread_rwlock_destroy.c
@@ -18,12 +18,15 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
 __pthread_rwlock_destroy (rwlock)
      pthread_rwlock_t *rwlock;
 {
+  LIBC_PROBE (rwlock_destroy, 1, rwlock);
+
   /* Nothing to be done.  For now.  */
   return 0;
 }
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 31eb508..610f223 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire read lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_rdlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (rlock_entry, 1, rwlock);
+
   /* Make sure we are along.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -49,6 +52,8 @@ __pthread_rwlock_rdlock (rwlock)
 	      --rwlock->__data.__nr_readers;
 	      result = EAGAIN;
 	    }
+          else
+            LIBC_PROBE (rwlock_acquire_read, 1, rwlock);
 
 	  break;
 	}
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
index a7ef71a..a6e8d87 100644
--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -22,11 +22,14 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 /* Unlock RWLOCK.  */
 int
 __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
 {
+  LIBC_PROBE (rwlock_unlock, 1, rwlock);
+
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
   if (rwlock->__data.__writer)
     rwlock->__data.__writer = 0;
diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
index 64fe970..748062e 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire write lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (wlock_entry, 1, rwlock);
+
   /* Make sure we are along.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -41,6 +44,9 @@ __pthread_rwlock_wrlock (rwlock)
 	{
 	  /* Mark self as writer.  */
 	  rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
+
+          LIBC_PROBE (rwlock_acquire_write, 1, rwlock);
+
 	  break;
 	}
 
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
index 3195db2..f0664f9 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
@@ -22,6 +22,15 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#if defined USE_STAP_PROBE && (defined IN_LIB || !defined NOT_IN_libc)
+ #include <stap-probe.h>
+ #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
LIBC_PROBE_ASM(lll_lock_wait_private, arg1)
+ #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
LIBC_PROBE_ASM(lll_lock_wait, arg1)
+#else
+ #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
+ #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
+#endif
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +100,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE (%rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
@@ -130,7 +140,8 @@ __lll_lock_wait:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	PTHREAD_PROBE_LL_LOCKWAIT (%rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax

Rayson





On Wed, 2011-01-12 at 10:18 -0800, Roland McGrath wrote:
> > +  LIBC_PROBE(join,1, threadid);
> 
> Missing space before paren, missing space after comma.
> Half the additions have similar whitespace errors.
> Please just read over ALL YOUR CODE before you submit.
> 
> > +          else
> > +            {
> > +	      /* systemtap pthread probe - this is the only place where
> > +	         we get this read-write lock */
> > +              LIBC_PROBE (rwlock_acquire_read, 1, rwlock);
> > +            }
> 
> Don't add braces around a single statement.  This comment is not formatted
> properly.  Look at the existing code and see how we write comments, please.
> This comment itself doesn't seem to say anything useful anyway.
> 
> > +1:	PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(%rdi)
> 
> Again, missing space before paren.
> 
> This stuff is not rocket science, and this is the nth time in a review of
> the same code I've told you about the same errors.  Just look at the
> existing code and match the formatting conventions you see there.
> 
> 
> Thanks,
> Roland


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-12 18:50   ` Rayson Ho
@ 2011-01-12 20:07     ` Bert Wesarg
  2011-01-19 18:03       ` Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Bert Wesarg @ 2011-01-12 20:07 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Roland McGrath, libc-alpha, systemtap

On Wed, Jan 12, 2011 at 19:50, Rayson Ho <rho@redhat.com> wrote:
> Thanks a lot for the comment.
>
> - fixed missing space before paren for the MARCOs
> - removed comments (most of the comments were originally written to
> explain why the probes were placed in the specific locations...)
>
>
> What is it?
> This is a patch against the "remotes/origin/roland/systemtap" branch. It
> adds SystemTap probe points to the Pthread library to allow developers
> and system administrators to trace and debug pthread programs.
>
> ChangeLog:
>
> 2011-01-12  Rayson Ho <rho@redhat.com>
>
>        * DESIGN-systemtap-probes.txt: New file. (documentation)
>        * pthread_create.c: Added SystemTap Pthreads probes.
>        * pthread_join.c: Likewise.
>        * pthread_mutex_destroy.c: Likewise.
>        * pthread_mutex_init.c: Likewise.
>        * pthread_mutex_lock.c: Likewise.
>        * pthread_mutex_unlock.c: Likewise.
>        * pthread_rwlock_destroy.c: Likewise.
>        * pthread_rwlock_rdlock.c: Likewise.
>        * pthread_rwlock_unlock.c: Likewise.
>        * pthread_rwlock_wrlock.c: Likewise.
>        * sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.
>
> Testing:
> - no regressions with or without systemtap probes compiled into pthread
> - compilation works with or without systemtap enabled
>
> diff --git a/nptl/DESIGN-systemtap-probes.txt
> b/nptl/DESIGN-systemtap-probes.txt
> index e69de29..17ec433 100644
> --- a/nptl/DESIGN-systemtap-probes.txt
> +++ b/nptl/DESIGN-systemtap-probes.txt
> @@ -0,0 +1,33 @@
> +Systemtap is a dynamic tracing/instrumenting tool available on Linux.
> Probes that are not fired have close to zero runtime overhead.
> +
> +The following probes are available for NPTL:
> +
> +Thread creation & Join Probes
> +=============================
> +create   - probe for pthread_create(3) - arg1 = thread ID, arg2 =
> start_routine, arg3 = arguments
> +start    - probe for actual thread creation, arg1 = struct pthread
> (members include thread ID, process ID)
> +join     - probe for pthread_join(3)   - arg1 = thread ID
> +join_ret - probe for pthread_join(3) return - arg1 = thread ID, arg2 =
> return value
> +
> +Lock-related Probes
> +===================
> +mutex_init    - probe for pthread_mutex_init(3) - arg1 = address of
> mutex lock
> +mutex_acquired- probe for pthread_mutex_lock(3) - arg1 = address of
> mutex lock
> +mutex_block   - probe for resume from _possible_ mutex block event -
> arg1 = address of mutex lock
> +mutex_entry   - probe for entry to the pthread_mutex_lock(3) function,
> - arg1 = address of mutex lock
> +mutex_release - probe for pthread_mutex_unlock(3) after the successful
> release of a mutex lock - arg1 = address of mutex lock
> +mutex_destroy - probe for pthread_mutex_destroy(3) - arg1 = address of
> mutex lock
> +rwlock_destroy- probe for pthread_rwlock_destroy(3) - arg1 = address of
> rw lock
> +rwlock_acquire_write-probe for pthread_rwlock_wrlock(3) - arg1 =
> address of rw lock
> +rwlock_unlock - probe for pthread_rwlock_unlock(3) - arg1 = address of
> rw lock
> +
> +lock_wait         - probe in low-level lock code, only fired when futex
> is called (i.e. when trying to acquire a contented lock)
> +lock_wait_private - probe in low-level lock code, only fired when futex
> is called (i.e. when trying to acquire a contented lock)
> +
> +Condition variable Probes
> +=========================
> +cond_init - probe for pthread_cond_init(3) - arg1 = condition, arg2 =
> attr
> +cond_destroy- probe for pthread_condattr_destroy(3) - arg1 = attr
> +cond_wait - probe for pthread_cond_wait(3) - arg1 = condition, arg2 =
> mutex lock
> +cond_signal - probe for pthread_cond_signal(3) - arg1 = condition
> +cond_broadcast - probe for pthread_cond_broadcast(3) - arg1 = condition
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 4075dd9..5fdf2c9 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr,
> start_routine, arg)
>   /* Pass the descriptor to the caller.  */
>   *newthread = (pthread_t) pd;
>
> +  LIBC_PROBE (newthread, 2, start_routine, arg);

I'm not familiar with this macro, but I suspect that the first
argument will be the name of the probe. In this case 'newthread'. Is
this intended? There is no 'newthread' probe documented in
nptl/DESIGN-systemtap-probes.txt but a 'create' probe. Also, there is
a parameter named 'newthread'. I must also missed the 'start' probe.

> +
>   /* Start the thread.  */
>   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
>  }
> diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
> index 6a87a8b..7cbeb1d 100644
> --- a/nptl/pthread_join.c
> +++ b/nptl/pthread_join.c
> @@ -23,6 +23,8 @@
>  #include <atomic.h>
>  #include "pthreadP.h"
>
> +#include <stap-probe.h>
> +
>
>  static void
>  cleanup (void *arg)
> @@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
>   struct pthread *self = THREAD_SELF;
>   int result = 0;
>
> +  LIBC_PROBE (join,1, threadid);
> +
>   /* During the wait we change to asynchronous cancellation.  If we
>      are canceled the thread we are waiting for must be marked as
>      un-wait-ed for again.  */
> @@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
>       __free_tcb (pd);
>     }
>
> +  LIBC_PROBE (join_ret, 3, threadid, result, pd->result);
> +
>   return result;
>  }
> diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
> index e2c9f8a..00037c5 100644
> --- a/nptl/pthread_mutex_destroy.c
> +++ b/nptl/pthread_mutex_destroy.c
> @@ -20,11 +20,15 @@
>  #include <errno.h>
>  #include "pthreadP.h"
>
> +#include <stap-probe.h>
> +
>
>  int
>  __pthread_mutex_destroy (mutex)
>      pthread_mutex_t *mutex;
>  {
> +  LIBC_PROBE (cond_destroy, 1, mutex);
> +
>   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
>       && mutex->__data.__nusers != 0)
>     return EBUSY;
> diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
> index d9b1ef0..a025a38 100644
> --- a/nptl/pthread_mutex_init.c
> +++ b/nptl/pthread_mutex_init.c
> @@ -24,6 +24,8 @@
>  #include <kernel-features.h>
>  #include "pthreadP.h"
>
> +#include <stap-probe.h>
> +
>  static const struct pthread_mutexattr default_attr =
>   {
>     /* Default is a normal mutex, not shared between processes.  */
> @@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
>   // mutex->__spins = 0;       already done by memset
>   // mutex->__next = NULL;     already done by memset
>
> +  LIBC_PROBE (mutex_init, 1, mutex);
> +
>   return 0;
>  }
>  strong_alias (__pthread_mutex_init, pthread_mutex_init)
> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
> index 50dc188..fdec784 100644
> --- a/nptl/pthread_mutex_lock.c
> +++ b/nptl/pthread_mutex_lock.c
> @@ -24,6 +24,7 @@
>  #include <not-cancel.h>
>  #include "pthreadP.h"
>  #include <lowlevellock.h>
> +#include <stap-probe.h>
>
>
>  #ifndef LLL_MUTEX_LOCK
> @@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
>   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
>
>   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
> +
> +  LIBC_PROBE (mutex_entry, 1, mutex);
> +
>   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
>     return __pthread_mutex_lock_full (mutex);
>
> @@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
>       /* Normal mutex.  */
>       LLL_MUTEX_LOCK (mutex);
>       assert (mutex->__data.__owner == 0);
> +
> +      LIBC_PROBE (mutex_block, 1, mutex);
>     }
>   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
>     {
> @@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
>
>       assert (mutex->__data.__owner == 0);
>       mutex->__data.__count = 1;
> +
> +      LIBC_PROBE (mutex_block, 1, mutex);
>     }
>   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
>     {
> @@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
>            }
>          while (LLL_MUTEX_TRYLOCK (mutex) != 0);
>
> +          LIBC_PROBE (mutex_block, 1, mutex);
> +
>          mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
>        }
>       assert (mutex->__data.__owner == 0);
> @@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
>   ++mutex->__data.__nusers;
>  #endif
>
> +  LIBC_PROBE (mutex_acquire, 1, mutex);
> +
>   return 0;
>  }
>
> @@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
>          }
>        while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
>
> +        LIBC_PROBE (mutex_block, 1, mutex);
> +
>        assert (mutex->__data.__owner == 0);
>        mutex->__data.__count = 1;
>       }
> @@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
>   ++mutex->__data.__nusers;
>  #endif
>
> +  LIBC_PROBE (mutex_acquire, 1, mutex);
> +
>   return 0;
>  }
>  #ifndef __pthread_mutex_lock
> diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
> index f9fe10b..479e500 100644
> --- a/nptl/pthread_mutex_unlock.c
> +++ b/nptl/pthread_mutex_unlock.c
> @@ -22,6 +22,7 @@
>  #include <stdlib.h>
>  #include "pthreadP.h"
>  #include <lowlevellock.h>
> +#include <stap-probe.h>
>
>  static int
>  internal_function
> @@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
>
>       /* Unlock.  */
>       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
> +
> +      LIBC_PROBE (mutex_release, 1, mutex);
> +
>       return 0;
>     }
>   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
> @@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
> int decr)
>                        PTHREAD_MUTEX_PSHARED (mutex));
>
>       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
> +
> +      LIBC_PROBE (mutex_release, 1, mutex);
> +
>       return __pthread_tpp_change_priority (oldprio, -1);
>
>     default:
> @@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
> int decr)
>       return EINVAL;
>     }
>
> +  LIBC_PROBE (mutex_release, 1, mutex);
>   return 0;
>  }
>
> diff --git a/nptl/pthread_rwlock_destroy.c
> b/nptl/pthread_rwlock_destroy.c
> index 28fd24b..84aa693 100644
> --- a/nptl/pthread_rwlock_destroy.c
> +++ b/nptl/pthread_rwlock_destroy.c
> @@ -18,12 +18,15 @@
>    02111-1307 USA.  */
>
>  #include "pthreadP.h"
> +#include <stap-probe.h>
>
>
>  int
>  __pthread_rwlock_destroy (rwlock)
>      pthread_rwlock_t *rwlock;
>  {
> +  LIBC_PROBE (rwlock_destroy, 1, rwlock);
> +
>   /* Nothing to be done.  For now.  */
>   return 0;
>  }
> diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
> index 31eb508..610f223 100644
> --- a/nptl/pthread_rwlock_rdlock.c
> +++ b/nptl/pthread_rwlock_rdlock.c
> @@ -22,6 +22,7 @@
>  #include <lowlevellock.h>
>  #include <pthread.h>
>  #include <pthreadP.h>
> +#include <stap-probe.h>
>
>
>  /* Acquire read lock for RWLOCK.  */
> @@ -31,6 +32,8 @@ __pthread_rwlock_rdlock (rwlock)
>  {
>   int result = 0;
>
> +  LIBC_PROBE (rlock_entry, 1, rwlock);
> +

Again, there is no 'rlock_entry' probe documented, and should it
probably called 'rwlock_entry'?

>   /* Make sure we are along.  */
>   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
>
> @@ -49,6 +52,8 @@ __pthread_rwlock_rdlock (rwlock)
>              --rwlock->__data.__nr_readers;
>              result = EAGAIN;
>            }
> +          else
> +            LIBC_PROBE (rwlock_acquire_read, 1, rwlock);

This one is also not documented, isn't it?

>
>          break;
>        }
> diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
> index a7ef71a..a6e8d87 100644
> --- a/nptl/pthread_rwlock_unlock.c
> +++ b/nptl/pthread_rwlock_unlock.c
> @@ -22,11 +22,14 @@
>  #include <lowlevellock.h>
>  #include <pthread.h>
>  #include <pthreadP.h>
> +#include <stap-probe.h>
>
>  /* Unlock RWLOCK.  */
>  int
>  __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
>  {
> +  LIBC_PROBE (rwlock_unlock, 1, rwlock);
> +
>   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
>   if (rwlock->__data.__writer)
>     rwlock->__data.__writer = 0;
> diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
> index 64fe970..748062e 100644
> --- a/nptl/pthread_rwlock_wrlock.c
> +++ b/nptl/pthread_rwlock_wrlock.c
> @@ -22,6 +22,7 @@
>  #include <lowlevellock.h>
>  #include <pthread.h>
>  #include <pthreadP.h>
> +#include <stap-probe.h>
>
>
>  /* Acquire write lock for RWLOCK.  */
> @@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
>  {
>   int result = 0;
>
> +  LIBC_PROBE (wlock_entry, 1, rwlock);
> +

Also not documented, and name should be 'rwlock_entry_write' or something.

>   /* Make sure we are along.  */
>   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
>
> @@ -41,6 +44,9 @@ __pthread_rwlock_wrlock (rwlock)
>        {
>          /* Mark self as writer.  */
>          rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
> +
> +          LIBC_PROBE (rwlock_acquire_write, 1, rwlock);
> +
>          break;
>        }
>
> diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
> b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
> index 3195db2..f0664f9 100644
> --- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
> +++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
> @@ -22,6 +22,15 @@
>  #include <kernel-features.h>
>  #include <lowlevellock.h>
>
> +#if defined USE_STAP_PROBE && (defined IN_LIB || !defined NOT_IN_libc)
> + #include <stap-probe.h>
> + #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
> LIBC_PROBE_ASM(lll_lock_wait_private, arg1)

If 'lll_lock_wait_private' will be the name of the probe, than the
documentation should be updated.

> + #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
> LIBC_PROBE_ASM(lll_lock_wait, arg1)

Likewise.

> +#else
> + #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
> + #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
> +#endif
> +
>        .text
>
>  #ifdef __ASSUME_PRIVATE_FUTEX
> @@ -91,7 +100,8 @@ __lll_lock_wait_private:
>        cmpl    %edx, %eax      /* NB:   %edx == 2 */
>        jne     2f
>
> -1:     movl    $SYS_futex, %eax
> +1:     PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE (%rdi)
> +       movl    $SYS_futex, %eax
>        syscall
>
>  2:     movl    %edx, %eax
> @@ -130,7 +140,8 @@ __lll_lock_wait:
>        cmpl    %edx, %eax      /* NB:   %edx == 2 */
>        jne     2f
>
> -1:     movl    $SYS_futex, %eax
> +1:     PTHREAD_PROBE_LL_LOCKWAIT (%rdi)
> +       movl    $SYS_futex, %eax
>        syscall
>
>  2:     movl    %edx, %eax

Bert

>
> Rayson

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-12 20:07     ` Bert Wesarg
@ 2011-01-19 18:03       ` Rayson Ho
  2011-01-19 18:36         ` Roland McGrath
  2011-01-19 18:38         ` [PATCH] Adding systemtap probe points in pthread library (slightly revised again) Bert Wesarg
  0 siblings, 2 replies; 32+ messages in thread
From: Rayson Ho @ 2011-01-19 18:03 UTC (permalink / raw)
  To: Bert Wesarg, Roland McGrath; +Cc: libc-alpha, systemtap

On Wed, 2011-01-12 at 21:07 +0100, Bert Wesarg wrote:
> I'm not familiar with this macro, but I suspect that the first
> argument will be the name of the probe. In this case 'newthread'. Is
> this intended? There is no 'newthread' probe documented in
> nptl/DESIGN-systemtap-probes.txt but a 'create' probe. Also, there is
> a parameter named 'newthread'. I must also missed the 'start' probe.

Thanks Bert and Roland for the comments. I've fixed the documentation,
and I've added new code that uses the ASM probe. Please review my patch
again:
 
What is it?
This is a patch against the "remotes/origin/roland/systemtap" branch. It
adds SystemTap probe points to the Pthread library to allow developers
and system administrators to trace and debug pthread programs.

ChangeLog:

	* nptl/DESIGN-systemtap-probes.txt: New file.
	* nptl/pthread_create.c: SystemTap probes.
	* nptl/pthread_join.c: Likewise.
	* nptl/pthread_mutex_destroy.c: Likewise.
	* nptl/pthread_mutex_init.c: Likewise.
	* nptl/pthread_mutex_lock.c: Likewise.
	* nptl/pthread_mutex_unlock.c: Likewise.
	* nptl/pthread_rwlock_destroy.c: Likewise.
	* nptl/pthread_rwlock_rdlock.c: Likewise.
	* nptl/pthread_rwlock_unlock.c: Likewise.
	* nptl/pthread_rwlock_wrlock.c: Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S:
Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S: Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S:
Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S: Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S:
Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S:
Likewise.


Testing:
- no regressions with or without systemtap probes compiled into pthread
- compilation works with or without systemtap enabled


diff --git a/nptl/DESIGN-systemtap-probes.txt
b/nptl/DESIGN-systemtap-probes.txt
index e69de29..0425824 100644
--- a/nptl/DESIGN-systemtap-probes.txt
+++ b/nptl/DESIGN-systemtap-probes.txt
@@ -0,0 +1,40 @@

+Systemtap is a dynamic tracing/instrumenting tool available on Linux.
Probes that are not fired at run time have close to zero performance
overhead.
+
+The following probes are available for NPTL:
+
+Thread creation & Join Probes
+=============================
+pthread_create - probe for pthread_create(3) - arg1 = start_routine,
arg2 = arguments
+pthread_start - probe for actual thread creation, arg1 = struct pthread
(members include thread ID, process ID)
+pthread_join - probe for pthread_join(3) - arg1 = thread ID
+pthread_join_ret - probe for pthread_join(3) return - arg1 = thread ID,
arg2 = return value
+
+Lock-related Probes
+===================
+mutex_init    - probe for pthread_mutex_init(3) - arg1 = address of
mutex lock
+mutex_acquired - probe for pthread_mutex_lock(3) - arg1 = address of
mutex lock
+mutex_block   - probe for resume from _possible_ mutex block event -
arg1 = address of mutex lock
+mutex_entry   - probe for entry to the pthread_mutex_lock(3) function -
arg1 = address of mutex lock
+mutex_release - probe for pthread_mutex_unlock(3) after the successful
release of a mutex lock - arg1 = address of mutex lock
+mutex_destroy - probe for pthread_mutex_destroy(3) - arg1 = address of
mutex lock
+
+wrlock_entry - probe for entry to the pthread_rwlock_wrlock(3) function
- arg1 = address of rw lock
+rdlock_entry - probe for entry to the pthread_rwlock_rdlock(3) function
- arg1 = address of rw lock
+
+rwlock_destroy - probe for pthread_rwlock_destroy(3) - arg1 = address
of rw lock
+rwlock_acquire_write - probe for pthread_rwlock_wrlock(3) - arg1 =
address of rw lock
+rdlock_acquire_read - probe for pthread_rwlock_rdlock(3) after
successfully getting the lock - - arg1 = address of rw lock
+rwlock_unlock - probe for pthread_rwlock_unlock(3) - arg1 = address of
rw lock
+
+lll_futex_wait         - probe in low-level (assembly language) locking
code, only fired when futex(2) is called (i.e. when trying to acquire a
contented lock)
+lll_futex_wait_private - probe in low-level (assembly language) locking
code, only fired when futex(2) is called (i.e. when trying to acquire a
contented lock)
+lll_futex_wake         - probe in low-level (assembly language) locking
code, only fired when futex(2) is called (FUTEX_WAIT) 
+
+Condition variable Probes
+=========================
+cond_init - probe for pthread_cond_init(3) - arg1 = condition, arg2 =
attr
+cond_destroy- probe for pthread_condattr_destroy(3) - arg1 = attr
+cond_wait - probe for pthread_cond_wait(3) - arg1 = condition, arg2 =
mutex lock
+cond_timedwait - probe for pthread_cond_timedwait(3) - arg1 =
condition, arg2 = mutex lock, arg3 = timespec
+cond_signal - probe for pthread_cond_signal(3) - arg1 = condition
+cond_broadcast - probe for pthread_cond_broadcast(3) - arg1 = condition
diff --git a/nptl/pthread_cond_broadcast.c
b/nptl/pthread_cond_broadcast.c
index 22523c2..713fc42 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
@@ -32,8 +33,11 @@ int
 __pthread_cond_broadcast (cond)
      pthread_cond_t *cond;
 {
+  LIBC_PROBE (cond_broadcast, 1, cond);
+
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
   /* Make sure we are alone.  */
 
diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
index 65c01b1..caec6ca 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -20,6 +20,7 @@
 
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -42,6 +43,8 @@ __pthread_cond_init (cond, cond_attr)
 			  ? NULL : (void *) ~0l);
   cond->__data.__broadcast_seq = 0;
 
+  LIBC_PROBE (cond_init, 2, cond, cond_attr);
+
   return 0;
 }
 versioned_symbol (libpthread, __pthread_cond_init,
diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
index 023bbb5..6615f21 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -35,6 +35,8 @@ __pthread_cond_signal (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_signal, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_timedwait.c
b/nptl/pthread_cond_timedwait.c
index 7278ec4..4e4e33d 100644
--- a/nptl/pthread_cond_timedwait.c
+++ b/nptl/pthread_cond_timedwait.c
@@ -65,7 +65,7 @@ __pthread_cond_timedwait (cond, mutex, abstime)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
 
-  /* Make sure we are along.  */
+  /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
   /* Now we can release the mutex.  */
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 670fba5..aee6a99 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 
@@ -101,7 +102,9 @@ __pthread_cond_wait (cond, mutex)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
   		? LLL_SHARED : LLL_PRIVATE;
 
-  /* Make sure we are along.  */
+  LIBC_PROBE (cond_wait, 2, cond, mutex);
+
+  /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
   /* Now we can release the mutex.  */
diff --git a/nptl/pthread_condattr_destroy.c
b/nptl/pthread_condattr_destroy.c
index e6d069e..9c84278 100644
--- a/nptl/pthread_condattr_destroy.c
+++ b/nptl/pthread_condattr_destroy.c
@@ -25,6 +25,8 @@ __pthread_condattr_destroy (attr)
      pthread_condattr_t *attr;
 {
   /* Nothing to be done.  */
+
+  LIBC_PROBE (cond_destroy, 1, attr);
   return 0;
 }
 strong_alias (__pthread_condattr_destroy, pthread_condattr_destroy)
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 4075dd9..d574cc5 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr,
start_routine, arg)
   /* Pass the descriptor to the caller.  */
   *newthread = (pthread_t) pd;
 
+  LIBC_PROBE (pthread_create, 2, start_routine, arg);
+
   /* Start the thread.  */
   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
 }
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 6a87a8b..609e2cf 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -23,6 +23,8 @@
 #include <atomic.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 static void
 cleanup (void *arg)
@@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
   struct pthread *self = THREAD_SELF;
   int result = 0;
 
+  LIBC_PROBE (pthread_join, 1, threadid);
+
   /* During the wait we change to asynchronous cancellation.  If we
      are canceled the thread we are waiting for must be marked as
      un-wait-ed for again.  */
@@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
       __free_tcb (pd);
     }
 
+  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
+
   return result;
 }
diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
index e2c9f8a..45c80b8 100644
--- a/nptl/pthread_mutex_destroy.c
+++ b/nptl/pthread_mutex_destroy.c
@@ -20,11 +20,15 @@
 #include <errno.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 int
 __pthread_mutex_destroy (mutex)
      pthread_mutex_t *mutex;
 {
+  LIBC_PROBE (mutex_destroy, 1, mutex);
+
   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
       && mutex->__data.__nusers != 0)
     return EBUSY;
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index d9b1ef0..a025a38 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -24,6 +24,8 @@
 #include <kernel-features.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 static const struct pthread_mutexattr default_attr =
   {
     /* Default is a normal mutex, not shared between processes.  */
@@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
   // mutex->__spins = 0;	already done by memset
   // mutex->__next = NULL;	already done by memset
 
+  LIBC_PROBE (mutex_init, 1, mutex);
+
   return 0;
 }
 strong_alias (__pthread_mutex_init, pthread_mutex_init)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 50dc188..010d43e 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -24,6 +24,7 @@
 #include <not-cancel.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 
 #ifndef LLL_MUTEX_LOCK
@@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
 
   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
+
+  LIBC_PROBE (mutex_entry, 1, mutex);
+
   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
     return __pthread_mutex_lock_full (mutex);
 
@@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
       /* Normal mutex.  */
       LLL_MUTEX_LOCK (mutex);
       assert (mutex->__data.__owner == 0);
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
     {
@@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
 
       assert (mutex->__data.__owner == 0);
       mutex->__data.__count = 1;
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
     {
@@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
 	    }
 	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 
+          LIBC_PROBE (mutex_block, 1, mutex);
+
 	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
 	}
       assert (mutex->__data.__owner == 0);
@@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 
@@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  }
 	while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
 
+        LIBC_PROBE (mutex_block, 1, mutex);
+
 	assert (mutex->__data.__owner == 0);
 	mutex->__data.__count = 1;
       }
@@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 #ifndef __pthread_mutex_lock
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index f9fe10b..479e500 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 static int
 internal_function
@@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       /* Unlock.  */
       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return 0;
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
@@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
int decr)
 			PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return __pthread_tpp_change_priority (oldprio, -1);
 
     default:
@@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
int decr)
       return EINVAL;
     }
 
+  LIBC_PROBE (mutex_release, 1, mutex);
   return 0;
 }
 
diff --git a/nptl/pthread_rwlock_destroy.c
b/nptl/pthread_rwlock_destroy.c
index 28fd24b..84aa693 100644
--- a/nptl/pthread_rwlock_destroy.c
+++ b/nptl/pthread_rwlock_destroy.c
@@ -18,12 +18,15 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
 __pthread_rwlock_destroy (rwlock)
      pthread_rwlock_t *rwlock;
 {
+  LIBC_PROBE (rwlock_destroy, 1, rwlock);
+
   /* Nothing to be done.  For now.  */
   return 0;
 }
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 31eb508..aee4011 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire read lock for RWLOCK.  */
@@ -31,7 +32,9 @@ __pthread_rwlock_rdlock (rwlock)
 {
   int result = 0;
 
-  /* Make sure we are along.  */
+  LIBC_PROBE (rdlock_entry, 1, rwlock);
+
+  /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
   while (1)
@@ -49,6 +52,8 @@ __pthread_rwlock_rdlock (rwlock)
 	      --rwlock->__data.__nr_readers;
 	      result = EAGAIN;
 	    }
+          else
+            LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
 
 	  break;
 	}
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
index a7ef71a..a6e8d87 100644
--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -22,11 +22,14 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 /* Unlock RWLOCK.  */
 int
 __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
 {
+  LIBC_PROBE (rwlock_unlock, 1, rwlock);
+
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
   if (rwlock->__data.__writer)
     rwlock->__data.__writer = 0;
diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
index 64fe970..41b13a9 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire write lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (wrlock_entry, 1, rwlock);
+
   /* Make sure we are along.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -41,6 +44,9 @@ __pthread_rwlock_wrlock (rwlock)
 	{
 	  /* Mark self as writer.  */
 	  rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
+
+          LIBC_PROBE (rwlock_acquire_write, 1, rwlock);
+
 	  break;
 	}
 
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
index 3195db2..0ffe43e 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
@@ -22,6 +22,15 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#if defined USE_STAP_PROBE && (defined IN_LIB || !defined NOT_IN_libc)
+ #include <stap-probe.h>
+ #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
LIBC_PROBE_ASM(lll_futex_wait_private, arg1)
+ #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
LIBC_PROBE_ASM(lll_futex_wait, arg1)
+#else
+ #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
+ #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
+#endif
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +100,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE (%rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
@@ -130,7 +140,8 @@ __lll_lock_wait:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	PTHREAD_PROBE_LL_LOCKWAIT (%rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index 9b15bfb..5f1e264 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -20,6 +20,12 @@
 #ifndef _LOWLEVELLOCK_H
 #define _LOWLEVELLOCK_H	1
 
+#if (defined USE_STAP_PROBE) && (defined IS_IN_libpthread)
+ #include <stap-probe.h>
+#else
+ #define LIBC_PROBE(arg1, arg2, arg3, arg4)
+#endif
+
 #ifndef __ASSEMBLER__
 # include <time.h>
 # include <sys/param.h>
@@ -227,6 +233,7 @@ LLL_STUB_UNWIND_INFO_END
   do {									      \
     int __ignore;							      \
     register __typeof (nr) _nr __asm ("edx") = (nr);			      \
+    LIBC_PROBE (lll_futex_wake, 2, futex, nr);
\
     __asm __volatile ("syscall"						      \
 		      : "=a" (__ignore)					      \
 		      : "0" (SYS_futex), "D" (futex),			      \
diff --git
a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
index 224a560..c3e80dc 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
@@ -25,7 +25,7 @@
 #include <kernel-features.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -35,6 +35,10 @@
 	.align	16
 __pthread_cond_broadcast:
 
+#if (defined USE_STAP_PROBE) && (defined IS_IN_libpthread)
+        LIBC_PROBE_ASM (cond_broadcast, %rdi)
+#endif
+
 	/* Get internal lock.  */
 	movl	$1, %esi
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
index d1d83a8..cabb396 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
@@ -24,6 +24,7 @@
 #include <pthread-pi-defines.h>
 #include <kernel-features.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 
 	.text
@@ -34,6 +35,10 @@
 	.align	16
 __pthread_cond_signal:
 
+#if (defined USE_STAP_PROBE) && (defined IS_IN_libpthread)
+        LIBC_PROBE_ASM (cond_signal, %rdi)
+#endif
+
 	/* Get internal lock.  */
 	movq	%rdi, %r8
 	movl	$1, %esi
diff --git
a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
index e6535fb..5bab689 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -41,6 +42,11 @@
 __pthread_cond_timedwait:
 .LSTARTCODE:
 	cfi_startproc
+
+#if (defined USE_STAP_PROBE) && (defined IS_IN_libpthread)
+        LIBC_PROBE_ASM (cond_timedwait, %rdi)
+#endif
+
 #ifdef SHARED
 	cfi_personality(DW_EH_PE_pcrel | DW_EH_PE_sdata4 | DW_EH_PE_indirect,
 			DW.ref.__gcc_personality_v0)
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
index f5b929e..da6bf9b 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <tcb-offsets.h>
 #include <pthread-pi-defines.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -34,6 +35,11 @@
 	.type	__pthread_cond_wait, @function
 	.align	16
 __pthread_cond_wait:
+
+#if (defined USE_STAP_PROBE) && (defined IS_IN_libpthread)
+        LIBC_PROBE_ASM (cond_wait, %rdi)
+#endif
+
 .LSTARTCODE:
 	cfi_startproc
 #ifdef SHARED
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
index 35eb09c..efc3c20 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,11 @@
 	.align	16
 __pthread_rwlock_rdlock:
 	cfi_startproc
+
+#if (defined USE_STAP_PROBE) && (defined IS_IN_libpthread)
+        LIBC_PROBE_ASM (rdlock_entry, %rdi)
+#endif
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
index be6b8d8..4254cfc 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,11 @@
 	.align	16
 __pthread_rwlock_wrlock:
 	cfi_startproc
+
+#if (defined USE_STAP_PROBE) && (defined IS_IN_libpthread)
+        LIBC_PROBE_ASM (wrlock_entry, %rdi)
+#endif
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-19 18:03       ` Rayson Ho
@ 2011-01-19 18:36         ` Roland McGrath
  2011-01-19 18:46           ` Rayson Ho
  2011-01-19 18:38         ` [PATCH] Adding systemtap probe points in pthread library (slightly revised again) Bert Wesarg
  1 sibling, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-01-19 18:36 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Bert Wesarg, libc-alpha, systemtap

Unrelated changes, even trivial ones like comment typo fixes, should be in
a separate patch/commit.  I committed those typo fixes myself separately,
and rebased the roland/systemtap branch after that.

Please include the ChangeLog header line in your post.  nptl/ChangeLog is
separate, so log entries go in that file and those don't get the nptl/
prefix on the file names in the log.  It is correct to keep lines under 80
characters, but when you wrap them you need to indent the wrapped lines.

It appears there are some long lines in DESIGN-systemtap-probes.txt and
there are two problems there.  First is that you should keep the lines in
the file itself under 80 characters.  Much worse is that you allowed your
patch to be word-wrapped, so it is no longer a valid patch at all.

Don't use man-page reference style.  The name of the function is
just pthread_create, not pthread_create(3) or pthread_create().

To make the file more readable, write the probe descriptions something like:

pthread_create - probe for pthread_create
	       arg1 = start_routine
	       arg2 = arguments

In that case, "arguments" is a confusing and wrong thing to write, when
what it is in fact is the "argument to start_routine".

Cases like this:

+#if (defined USE_STAP_PROBE) && (defined IS_IN_libpthread)

have gratuitous parentheses that are of no use at all.
Remove them.  In some quarters, the thing to write is "defined (FOO)".
But in glibc sources, we just write "defined FOO".

There is no need to test USE_STAP_PROBE at all.  
That's the whole point of the LIBC_PROBE* macros.
They expand to nothing if USE_STAP_PROBE is disabled.
Just use them directly.  Use plain #ifdef IS_IN_libpthread where appropriate.

It's not at all clear to me that conditionalizing probes on
IS_IN_libpthread is actually appropriate at all.  Please explain your
rationale.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-19 18:03       ` Rayson Ho
  2011-01-19 18:36         ` Roland McGrath
@ 2011-01-19 18:38         ` Bert Wesarg
  2011-01-19 18:54           ` Roland McGrath
  1 sibling, 1 reply; 32+ messages in thread
From: Bert Wesarg @ 2011-01-19 18:38 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Roland McGrath, libc-alpha, systemtap

On Wed, Jan 19, 2011 at 18:53, Rayson Ho <rho@redhat.com> wrote:
> On Wed, 2011-01-12 at 21:07 +0100, Bert Wesarg wrote:
>> I'm not familiar with this macro, but I suspect that the first
>> argument will be the name of the probe. In this case 'newthread'. Is
>> this intended? There is no 'newthread' probe documented in
>> nptl/DESIGN-systemtap-probes.txt but a 'create' probe. Also, there is
>> a parameter named 'newthread'. I must also missed the 'start' probe.

And I still miss the 'pthread_start' probe. Do I missed it?

>
> Thanks Bert and Roland for the comments. I've fixed the documentation,
> and I've added new code that uses the ASM probe. Please review my patch
> again:
>

> diff --git a/nptl/DESIGN-systemtap-probes.txt
> b/nptl/DESIGN-systemtap-probes.txt
> index e69de29..0425824 100644
> --- a/nptl/DESIGN-systemtap-probes.txt
> +++ b/nptl/DESIGN-systemtap-probes.txt
> @@ -0,0 +1,40 @@
>
> +Systemtap is a dynamic tracing/instrumenting tool available on Linux.
> Probes that are not fired at run time have close to zero performance
> overhead.
> +
> +The following probes are available for NPTL:
> +
> +Thread creation & Join Probes
> +=============================
> +pthread_create - probe for pthread_create(3) - arg1 = start_routine,
> arg2 = arguments
> +pthread_start - probe for actual thread creation, arg1 = struct pthread
> (members include thread ID, process ID)
> +pthread_join - probe for pthread_join(3) - arg1 = thread ID
> +pthread_join_ret - probe for pthread_join(3) return - arg1 = thread ID,
> arg2 = return value

Why does these probes have the 'pthread_' prefix, but all pthread
related probes below not? Also, 'join_ret' is the only one with an
abbreviated verb. Maybe 'join_return' or simply 'joined'?

> +
> +Lock-related Probes
> +===================
> +mutex_init    - probe for pthread_mutex_init(3) - arg1 = address of
> mutex lock
> +mutex_acquired - probe for pthread_mutex_lock(3) - arg1 = address of
> mutex lock
> +mutex_block   - probe for resume from _possible_ mutex block event -
> arg1 = address of mutex lock
> +mutex_entry   - probe for entry to the pthread_mutex_lock(3) function -
> arg1 = address of mutex lock
> +mutex_release - probe for pthread_mutex_unlock(3) after the successful
> release of a mutex lock - arg1 = address of mutex lock
> +mutex_destroy - probe for pthread_mutex_destroy(3) - arg1 = address of
> mutex lock

Could we keep the naming consistent, its 'acquired' but not
'released'. Its' 'entry' (a noun) but all the others aren't nouns. i
would also suggest to keep the name close to the function name.
Something like:

mutex_init
mutex_lock_locked
mutex_lock_blocking
mutex_lock_enter
mutex_unlock

> +
> +wrlock_entry - probe for entry to the pthread_rwlock_wrlock(3) function
> - arg1 = address of rw lock
> +rdlock_entry - probe for entry to the pthread_rwlock_rdlock(3) function
> - arg1 = address of rw lock
> +
> +rwlock_destroy - probe for pthread_rwlock_destroy(3) - arg1 = address
> of rw lock
> +rwlock_acquire_write - probe for pthread_rwlock_wrlock(3) - arg1 =
> address of rw lock
> +rdlock_acquire_read - probe for pthread_rwlock_rdlock(3) after
> successfully getting the lock - - arg1 = address of rw lock
> +rwlock_unlock - probe for pthread_rwlock_unlock(3) - arg1 = address of
> rw lock
> +
> +lll_futex_wait         - probe in low-level (assembly language) locking
> code, only fired when futex(2) is called (i.e. when trying to acquire a
> contented lock)
> +lll_futex_wait_private - probe in low-level (assembly language) locking
> code, only fired when futex(2) is called (i.e. when trying to acquire a
> contented lock)
> +lll_futex_wake         - probe in low-level (assembly language) locking
> code, only fired when futex(2) is called (FUTEX_WAIT)
> +
> +Condition variable Probes
> +=========================
> +cond_init - probe for pthread_cond_init(3) - arg1 = condition, arg2 =
> attr
> +cond_destroy- probe for pthread_condattr_destroy(3) - arg1 = attr
> +cond_wait - probe for pthread_cond_wait(3) - arg1 = condition, arg2 =
> mutex lock
> +cond_timedwait - probe for pthread_cond_timedwait(3) - arg1 =
> condition, arg2 = mutex lock, arg3 = timespec
> +cond_signal - probe for pthread_cond_signal(3) - arg1 = condition
> +cond_broadcast - probe for pthread_cond_broadcast(3) - arg1 = condition

> diff --git a/nptl/pthread_cond_timedwait.c
> b/nptl/pthread_cond_timedwait.c
> index 7278ec4..4e4e33d 100644
> --- a/nptl/pthread_cond_timedwait.c
> +++ b/nptl/pthread_cond_timedwait.c
> @@ -65,7 +65,7 @@ __pthread_cond_timedwait (cond, mutex, abstime)
>   int pshared = (cond->__data.__mutex == (void *) ~0l)
>                ? LLL_SHARED : LLL_PRIVATE;
>
> -  /* Make sure we are along.  */
> +  /* Make sure we are alone.  */
>   lll_lock (cond->__data.__lock, pshared);
>
>   /* Now we can release the mutex.  */

I would have expected the 'cond_timedwait' probe here.

> diff --git a/nptl/pthread_condattr_destroy.c
> b/nptl/pthread_condattr_destroy.c
> index e6d069e..9c84278 100644
> --- a/nptl/pthread_condattr_destroy.c
> +++ b/nptl/pthread_condattr_destroy.c
> @@ -25,6 +25,8 @@ __pthread_condattr_destroy (attr)
>      pthread_condattr_t *attr;
>  {
>   /* Nothing to be done.  */
> +
> +  LIBC_PROBE (cond_destroy, 1, attr);

Judging from the file and the function name in the hunk header, I
think this probe does not belong here.

>   return 0;
>  }
>  strong_alias (__pthread_condattr_destroy, pthread_condattr_destroy)

> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 4075dd9..d574cc5 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr,
> start_routine, arg)
>   /* Pass the descriptor to the caller.  */
>   *newthread = (pthread_t) pd;
>
> +  LIBC_PROBE (pthread_create, 2, start_routine, arg);
> +
>   /* Start the thread.  */
>   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
>  }

Bert

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-19 18:36         ` Roland McGrath
@ 2011-01-19 18:46           ` Rayson Ho
  2011-01-19 18:56             ` Roland McGrath
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-01-19 18:46 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Bert Wesarg, libc-alpha, systemtap

On Wed, 2011-01-19 at 10:35 -0800, Roland McGrath wrote:
> It's not at all clear to me that conditionalizing probes on
> IS_IN_libpthread is actually appropriate at all.  Please explain your
> rationale.

Hi Roland,

Some of the headers are used elsewhere, and thus I needed to make sure
other locations don't get the probes by just including/using the
functions (like lock functions, for example).

Rayson


> 
> 
> Thanks,
> Roland


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-19 18:38         ` [PATCH] Adding systemtap probe points in pthread library (slightly revised again) Bert Wesarg
@ 2011-01-19 18:54           ` Roland McGrath
  2011-01-19 19:04             ` Bert Wesarg
  0 siblings, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-01-19 18:54 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Rayson Ho, libc-alpha, systemtap

> And I still miss the 'pthread_start' probe. Do I missed it?

It's already in the roland/systemtap branch.  He just documented it.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-19 18:46           ` Rayson Ho
@ 2011-01-19 18:56             ` Roland McGrath
  2011-01-26 18:24               ` Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-01-19 18:56 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Bert Wesarg, libc-alpha, systemtap

> Some of the headers are used elsewhere, and thus I needed to make sure
> other locations don't get the probes by just including/using the
> functions (like lock functions, for example).

In all places that code is used, the same stuff is happening.  You want
probes there too.  Please look at all the places where these probes get
defined if you make them unconditional, and then give specific rationale
why each one should be omitted.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-19 18:54           ` Roland McGrath
@ 2011-01-19 19:04             ` Bert Wesarg
  0 siblings, 0 replies; 32+ messages in thread
From: Bert Wesarg @ 2011-01-19 19:04 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Rayson Ho, libc-alpha, systemtap

On Wed, Jan 19, 2011 at 19:54, Roland McGrath <roland@redhat.com> wrote:
>> And I still miss the 'pthread_start' probe. Do I missed it?
>
> It's already in the roland/systemtap branch.  He just documented it.

Thanks for the info.

Bert

>
>
> Thanks,
> Roland
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-19 18:56             ` Roland McGrath
@ 2011-01-26 18:24               ` Rayson Ho
  2011-01-26 18:51                 ` Bert Wesarg
  2011-01-26 18:54                 ` Roland McGrath
  0 siblings, 2 replies; 32+ messages in thread
From: Rayson Ho @ 2011-01-26 18:24 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-alpha, systemtap

Hi Roland,

1) I googled and found the glibc "Contribution checklist", and my patch below is now formatted according to the standard set by it (hopefully, the line wrap-around issues and the spacing issues are all addressed).

2) And I think I've fixed an issue related to why I needed to check #if (defined USE_STAP_PROBE) -- in stap-probe.h, when USE_STAP_PROBE is not enabled, it is still defined to STAP_PROBE_ASM (IN_LIB, name, template), and I believe it should be defined to something that nullifies it or to dummy. 

diff --git a/include/stap-probe.h b/include/stap-probe.h
index 7d8daad..58ffb72 100644
--- a/include/stap-probe.h
+++ b/include/stap-probe.h
@@ -56,8 +56,7 @@
 
 # define LIBC_PROBE(name, n, ...)      DUMMY_PROBE##n (__VA_ARGS__)
 
-# define LIBC_PROBE_ASM(name, template) \
-  STAP_PROBE_ASM (IN_LIB, name, template)
+# define LIBC_PROBE_ASM(name, template) 
 
 # define LIBC_PROBE_ASM_OPERANDS(n, ...) \
   [__probe_dummy__] "g" (({ DUMMY_PROBE##n (__VA_ARGS__); 0; }))

(It just finished compiling with and without systemtap enabled, so I think it is safe to enable this code. However, it is not part of my patch because it is not related to the pthread probes. I think just nullifying it might not be the best way to fix it, but I will leave it to you as you are the inventor of this new sdt.h and stap-probe.h in glibc :D )

3) I still need to check for "IS_IN_libpthread" because I don't want to have probes fired because of non pthread related activities. And example of this is __libc_fork(), which does not have interactions with the "normal" pthread APIs, but it calls lll_futex_wake() when forking a child. I've read the code in all places that use those synchronization mechanisms, and found that it can be useful to trace synchronizations inside glibc, but it is just that they are not totally related to pthread calls. However, if you think it makes sense to have the probes there as they are useful to the programmers/users in general and not those only interested in pthread, I will just take the #defines out and clean up all places that have #define checks for avoiding the probes polluting the tracing space.

Addition to Changelog (against nptl/ChangeLog):

nptl/ChangeLog
2011-01-26  Rayson Ho  <rho@redhat.com>

	* DESIGN-systemtap-probes.txt: New file.
	* pthread_create.c: SystemTap probes.
	* pthread_join.c: Likewise.
	* pthread_mutex_destroy.c: Likewise.
	* pthread_mutex_init.c: Likewise.
	* pthread_mutex_lock.c: Likewise.
	* pthread_mutex_unlock.c: Likewise.
	* pthread_rwlock_destroy.c: Likewise.
	* pthread_rwlock_rdlock.c: Likewise.
	* pthread_rwlock_unlock.c: Likewise.
	* pthread_rwlock_wrlock.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S: Likewise.

diff --git a/nptl/DESIGN-systemtap-probes.txt b/nptl/DESIGN-systemtap-probes.txt
index e69de29..5fd7e45 100644
--- a/nptl/DESIGN-systemtap-probes.txt
+++ b/nptl/DESIGN-systemtap-probes.txt
@@ -0,0 +1,78 @@
+Systemtap is a dynamic tracing/instrumenting tool available on Linux. Probes
+that are not fired at run time have close to zero overhead.
+
+The following probes are available for NPTL:
+
+Thread creation & Join Probes
+=============================
+pthread_create - probe for pthread_create
+               arg1 = start_routine
+               arg2 = arguments to start_routine
+pthread_start - probe for actual thread creation
+              arg1 = struct pthread (members include thread ID, process ID)
+              arg2 = address of start_routine
+              arg3 = pointer to the list of arguments
+pthread_join - probe for pthread_join
+             arg1 = thread ID
+pthread_join_ret - probe for pthread_join return
+                 arg1 = thread ID
+                 arg2 = return value
+
+Lock-related Probes
+===================
+mutex_init    - probe for pthread_mutex_init
+              arg1 = address of mutex lock
+mutex_acquired - probe for pthread_mutex_lock
+               arg1 = address of mutex lock
+mutex_block   - probe for resume from _possible_ mutex block event
+              arg1 = address of mutex lock
+mutex_entry   - probe for entry to the pthread_mutex_lock function
+              arg1 = address of mutex lock
+mutex_release - probe for pthread_mutex_unlock after the successful release of a
+                mutex lock
+              arg1 = address of mutex lock
+mutex_destroy - probe for pthread_mutex_destroy
+              arg1 = address of mutex lock
+
+wrlock_entry - probe for entry to the pthread_rwlock_wrlock function
+             arg1 = address of rw lock
+rdlock_entry - probe for entry to the pthread_rwlock_rdlock function
+             arg1 = address of rw lock
+
+rwlock_destroy - probe for pthread_rwlock_destroy
+               arg1 = address of rw lock
+rwlock_acquire_write - probe for pthread_rwlock_wrlock
+                     arg1 = address of rw lock
+rdlock_acquire_read - probe for pthread_rwlock_rdlock after successfully getting
+                      the lock
+                    arg1 = address of rw lock
+rwlock_unlock - probe for pthread_rwlock_unlock
+              arg1 = address of rw lock
+
+lll_futex_wait - probe in low-level (assembly language) locking code, only fired
+                 when futex is called (i.e. when trying to acquire a contented
+                 lock)
+lll_futex_wait_private - probe in low-level (assembly language) locking code,
+                         only fired when futex is called (i.e. when trying to
+                         acquire a contented lock)
+lll_futex_wake - probe in low-level (assembly language) locking code, only fired
+                 when futex is called (FUTEX_WAIT), arg1 = 
+
+Condition variable Probes
+=========================
+cond_init - probe for pthread_cond_init
+          arg1 = condition
+          arg2 = attr
+cond_destroy - probe for pthread_condattr_destroy
+             arg1 = attr
+cond_wait - probe for pthread_cond_wait
+          arg1 = condition
+          arg2 = mutex lock
+cond_timedwait - probe for pthread_cond_timedwait
+               arg1 = condition
+               arg2 = mutex lock
+               arg3 = timespec
+cond_signal - probe for pthread_cond_signal
+            arg1 = condition
+cond_broadcast - probe for pthread_cond_broadcast
+               arg1 = condition
diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
index 22523c2..a2e462f 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
@@ -32,6 +33,8 @@ int
 __pthread_cond_broadcast (cond)
      pthread_cond_t *cond;
 {
+  LIBC_PROBE (cond_broadcast, 1, cond);
+
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
   /* Make sure we are alone.  */
diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
index 65c01b1..caec6ca 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -20,6 +20,7 @@
 
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -42,6 +43,8 @@ __pthread_cond_init (cond, cond_attr)
 			  ? NULL : (void *) ~0l);
   cond->__data.__broadcast_seq = 0;
 
+  LIBC_PROBE (cond_init, 2, cond, cond_attr);
+
   return 0;
 }
 versioned_symbol (libpthread, __pthread_cond_init,
diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
index 023bbb5..6615f21 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -35,6 +35,8 @@ __pthread_cond_signal (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
                ? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_signal, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 467a03a..bac1bef 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 
@@ -101,6 +102,8 @@ __pthread_cond_wait (cond, mutex)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
                ? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_wait, 2, cond, mutex);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_condattr_destroy.c b/nptl/pthread_condattr_destroy.c
index e6d069e..1d5bc88 100644
--- a/nptl/pthread_condattr_destroy.c
+++ b/nptl/pthread_condattr_destroy.c
@@ -18,6 +18,7 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -25,6 +26,8 @@ __pthread_condattr_destroy (attr)
      pthread_condattr_t *attr;
 {
   /* Nothing to be done.  */
+
+  LIBC_PROBE (cond_destroy, 1, attr);
   return 0;
 }
 strong_alias (__pthread_condattr_destroy, pthread_condattr_destroy)
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 4075dd9..d574cc5 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
   /* Pass the descriptor to the caller.  */
   *newthread = (pthread_t) pd;
 
+  LIBC_PROBE (pthread_create, 2, start_routine, arg);
+
   /* Start the thread.  */
   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
 }
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 6a87a8b..609e2cf 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -23,6 +23,8 @@
 #include <atomic.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 static void
 cleanup (void *arg)
@@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
   struct pthread *self = THREAD_SELF;
   int result = 0;
 
+  LIBC_PROBE (pthread_join, 1, threadid);
+
   /* During the wait we change to asynchronous cancellation.  If we
      are canceled the thread we are waiting for must be marked as
      un-wait-ed for again.  */
@@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
       __free_tcb (pd);
     }
 
+  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
+
   return result;
 }
diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
index e2c9f8a..45c80b8 100644
--- a/nptl/pthread_mutex_destroy.c
+++ b/nptl/pthread_mutex_destroy.c
@@ -20,11 +20,15 @@
 #include <errno.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 int
 __pthread_mutex_destroy (mutex)
      pthread_mutex_t *mutex;
 {
+  LIBC_PROBE (mutex_destroy, 1, mutex);
+
   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
       && mutex->__data.__nusers != 0)
     return EBUSY;
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index d9b1ef0..a025a38 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -24,6 +24,8 @@
 #include <kernel-features.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 static const struct pthread_mutexattr default_attr =
   {
     /* Default is a normal mutex, not shared between processes.  */
@@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
   // mutex->__spins = 0;	already done by memset
   // mutex->__next = NULL;	already done by memset
 
+  LIBC_PROBE (mutex_init, 1, mutex);
+
   return 0;
 }
 strong_alias (__pthread_mutex_init, pthread_mutex_init)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 50dc188..010d43e 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -24,6 +24,7 @@
 #include <not-cancel.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 
 #ifndef LLL_MUTEX_LOCK
@@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
 
   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
+
+  LIBC_PROBE (mutex_entry, 1, mutex);
+
   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
     return __pthread_mutex_lock_full (mutex);
 
@@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
       /* Normal mutex.  */
       LLL_MUTEX_LOCK (mutex);
       assert (mutex->__data.__owner == 0);
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
     {
@@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
 
       assert (mutex->__data.__owner == 0);
       mutex->__data.__count = 1;
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
     {
@@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
 	    }
 	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 
+          LIBC_PROBE (mutex_block, 1, mutex);
+
 	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
 	}
       assert (mutex->__data.__owner == 0);
@@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 
@@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  }
 	while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
 
+        LIBC_PROBE (mutex_block, 1, mutex);
+
 	assert (mutex->__data.__owner == 0);
 	mutex->__data.__count = 1;
       }
@@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 #ifndef __pthread_mutex_lock
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index f9fe10b..479e500 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 static int
 internal_function
@@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       /* Unlock.  */
       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return 0;
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
@@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
 			PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return __pthread_tpp_change_priority (oldprio, -1);
 
     default:
@@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
       return EINVAL;
     }
 
+  LIBC_PROBE (mutex_release, 1, mutex);
   return 0;
 }
 
diff --git a/nptl/pthread_rwlock_destroy.c b/nptl/pthread_rwlock_destroy.c
index 28fd24b..84aa693 100644
--- a/nptl/pthread_rwlock_destroy.c
+++ b/nptl/pthread_rwlock_destroy.c
@@ -18,12 +18,15 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
 __pthread_rwlock_destroy (rwlock)
      pthread_rwlock_t *rwlock;
 {
+  LIBC_PROBE (rwlock_destroy, 1, rwlock);
+
   /* Nothing to be done.  For now.  */
   return 0;
 }
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 2feac57..09bad14 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire read lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_rdlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (rdlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -49,6 +52,8 @@ __pthread_rwlock_rdlock (rwlock)
              --rwlock->__data.__nr_readers;
              result = EAGAIN;
            }
+          else
+            LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
 
          break;
        }
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
index a7ef71a..a6e8d87 100644
--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -22,11 +22,14 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 /* Unlock RWLOCK.  */
 int
 __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
 {
+  LIBC_PROBE (rwlock_unlock, 1, rwlock);
+
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
   if (rwlock->__data.__writer)
     rwlock->__data.__writer = 0;
diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
index 9d5f135..5b93b3c 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire write lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (wrlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -41,6 +44,9 @@ __pthread_rwlock_wrlock (rwlock)
        {
          /* Mark self as writer.  */
          rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
+
+          LIBC_PROBE (rwlock_acquire_write, 1, rwlock);
+
          break;
        }
 
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
index 3195db2..0ffe43e 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
@@ -22,6 +22,15 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#if defined USE_STAP_PROBE && (defined IN_LIB || !defined NOT_IN_libc)
+ #include <stap-probe.h>
+ #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1) LIBC_PROBE_ASM(lll_futex_wait_private, arg1)
+ #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)         LIBC_PROBE_ASM(lll_futex_wait, arg1)
+#else
+ #define PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE(arg1)
+ #define PTHREAD_PROBE_LL_LOCKWAIT(arg1)
+#endif
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +100,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	PTHREAD_PROBE_LL_LOCKWAIT_PRIVATE (%rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
@@ -130,7 +140,8 @@ __lll_lock_wait:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	PTHREAD_PROBE_LL_LOCKWAIT (%rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index 9b15bfb..5f1e264 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -20,6 +20,12 @@
 #ifndef _LOWLEVELLOCK_H
 #define _LOWLEVELLOCK_H	1
 
+#if defined USE_STAP_PROBE && defined IS_IN_libpthread
+ #include <stap-probe.h>
+#else
+ #define LIBC_PROBE(arg1, arg2, arg3, arg4)
+#endif
+
 #ifndef __ASSEMBLER__
 # include <time.h>
 # include <sys/param.h>
@@ -227,6 +233,7 @@ LLL_STUB_UNWIND_INFO_END
   do {									      \
     int __ignore;							      \
     register __typeof (nr) _nr __asm ("edx") = (nr);			      \
+    LIBC_PROBE (lll_futex_wake, 2, futex, nr);                                \
     __asm __volatile ("syscall"						      \
 		      : "=a" (__ignore)					      \
 		      : "0" (SYS_futex), "D" (futex),			      \
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
index 224a560..3ab3340 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
@@ -25,7 +25,7 @@
 #include <kernel-features.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
-
+#include <stap-probe.h>
 
        .text
 
@@ -35,6 +35,10 @@
        .align  16
 __pthread_cond_broadcast:
 
+#if defined IS_IN_libpthread
+        LIBC_PROBE_ASM (cond_broadcast, %rdi)
+#endif
+
        /* Get internal lock.  */
        movl    $1, %esi
        xorl    %eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
index d1d83a8..a92f311 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
@@ -24,6 +24,7 @@
 #include <pthread-pi-defines.h>
 #include <kernel-features.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 
        .text
@@ -34,6 +35,10 @@
        .align  16
 __pthread_cond_signal:
 
+#if defined IS_IN_libpthread
+        LIBC_PROBE_ASM (cond_signal, %rdi)
+#endif
+
        /* Get internal lock.  */
        movq    %rdi, %r8
        movl    $1, %esi
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
index e6535fb..374ecf0 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -41,6 +42,11 @@
 __pthread_cond_timedwait:
 .LSTARTCODE:
        cfi_startproc
+
+#if defined IS_IN_libpthread
+        LIBC_PROBE_ASM (cond_timedwait, %rdi)
+#endif
+
 #ifdef SHARED
        cfi_personality(DW_EH_PE_pcrel | DW_EH_PE_sdata4 | DW_EH_PE_indirect,
                        DW.ref.__gcc_personality_v0)
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
index f5b929e..5d8f4cf 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <tcb-offsets.h>
 #include <pthread-pi-defines.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -34,6 +35,11 @@
        .type   __pthread_cond_wait, @function
        .align  16
 __pthread_cond_wait:
+
+#if defined IS_IN_libpthread
+        LIBC_PROBE_ASM (cond_wait, %rdi)
+#endif
+
 .LSTARTCODE:
        cfi_startproc
 #ifdef SHARED
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
index 35eb09c..ef4a79e 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
        .text
 
@@ -31,6 +31,11 @@
        .align  16
 __pthread_rwlock_rdlock:
        cfi_startproc
+
+#if defined IS_IN_libpthread
+        LIBC_PROBE_ASM (rdlock_entry, %rdi)
+#endif
+
        xorq    %r10, %r10
 
        /* Get the lock.  */
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
index be6b8d8..04abe2b 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
        .text
 
@@ -31,6 +31,11 @@
        .align  16
 __pthread_rwlock_wrlock:
        cfi_startproc
+
+#if defined IS_IN_libpthread
+        LIBC_PROBE_ASM (wrlock_entry, %rdi)
+#endif
+
        xorq    %r10, %r10
 
        /* Get the lock.  */


----- Original Message -----
From: "Roland McGrath" <roland@redhat.com>
To: "Rayson Ho" <rho@redhat.com>
Cc: "Bert Wesarg" <bert.wesarg@googlemail.com>, libc-alpha@sourceware.org, systemtap@sources.redhat.com
Sent: Wednesday, January 19, 2011 1:56:08 PM
Subject: Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)

> Some of the headers are used elsewhere, and thus I needed to make sure
> other locations don't get the probes by just including/using the
> functions (like lock functions, for example).

In all places that code is used, the same stuff is happening.  You want
probes there too.  Please look at all the places where these probes get
defined if you make them unconditional, and then give specific rationale
why each one should be omitted.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-26 18:24               ` Rayson Ho
@ 2011-01-26 18:51                 ` Bert Wesarg
  2011-01-26 18:54                 ` Roland McGrath
  1 sibling, 0 replies; 32+ messages in thread
From: Bert Wesarg @ 2011-01-26 18:51 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Roland McGrath, libc-alpha, systemtap

Hi,

On Wed, Jan 26, 2011 at 19:24, Rayson Ho <rho@redhat.com> wrote:
> +cond_destroy - probe for pthread_condattr_destroy

Why is this probe for pthread_condattr_destroy()?

Bert

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-26 18:24               ` Rayson Ho
  2011-01-26 18:51                 ` Bert Wesarg
@ 2011-01-26 18:54                 ` Roland McGrath
  2011-01-27  9:11                   ` Rayson Ho
  1 sibling, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-01-26 18:54 UTC (permalink / raw)
  To: Rayson Ho; +Cc: libc-alpha, systemtap

> 1) I googled and found the glibc "Contribution checklist", and my patch below is now formatted according to the standard set by it (hopefully, the line wrap-around issues and the spacing issues are all addressed).

Now you just need to learn to line-wrap your email text too. ;-)

> 2) And I think I've fixed an issue related to why I needed to check #if
>    (defined USE_STAP_PROBE) -- in stap-probe.h, when USE_STAP_PROBE is
>    not enabled, it is still defined to STAP_PROBE_ASM (IN_LIB, name,
>    template), and I believe it should be defined to something that
>    nullifies it or to dummy.

Oh, indeed.  I've fixed that in the branch now (rebased, not mergeable).

> 3) I still need to check for "IS_IN_libpthread" because I don't want to
>    have probes fired because of non pthread related activities. And
>    example of this is __libc_fork(), which does not have interactions
>    with the "normal" pthread APIs, but it calls lll_futex_wake() when
>    forking a child. I've read the code in all places that use those
>    synchronization mechanisms, and found that it can be useful to trace
>    synchronizations inside glibc, but it is just that they are not
>    totally related to pthread calls. However, if you think it makes sense
>    to have the probes there as they are useful to the programmers/users
>    in general and not those only interested in pthread, I will just take
>    the #defines out and clean up all places that have #define checks for
>    avoiding the probes polluting the tracing space.

If you have probes in the lowlevellock code at all, then I think it makes
sense to have them in all its incarnations.  Those probes are called
"lll_*" so they are about the low-level internals rather than API-level
pthread calls anyway.  With the macros as they are, they will wind up with
provider "libc" instead of "libpthread", so tracing scripts can distinguish
them that way.

You didn't say anything about the conditionalization of cases other than
lll_* code.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-26 18:54                 ` Roland McGrath
@ 2011-01-27  9:11                   ` Rayson Ho
  2011-01-28  7:20                     ` Roland McGrath
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-01-27  9:11 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-alpha, systemtap

On Wed, 2011-01-26 at 10:54 -0800, Roland McGrath wrote:
> If you have probes in the lowlevellock code at all, then I think it makes
> sense to have them in all its incarnations.  Those probes are called
> "lll_*" so they are about the low-level internals rather than API-level
> pthread calls anyway.  With the macros as they are, they will wind up with
> provider "libc" instead of "libpthread", so tracing scripts can distinguish
> them that way.
> 
> You didn't say anything about the conditionalization of cases other than
> lll_* code.

The problem I had at the beginning was related to some modules not
defining IN_LIB, so the following code was complaining:

# ifndef NOT_IN_libc
#  define IN_LIB        libc
# elif !defined IN_LIB
#  error "missing -DIN_LIB=... -- not extra-lib.mk?"
# endif

Some code (files) really are not libraries, examples of those are utils
in iconv. And some do not include extra-lib.mk, as they do their own
"-DNOT_IN_libc=1" in their Makefiles.

The simplest hack would be to not "#error" the  define, but to quietly
define IN_LIB to something else.

A better fix but involves touch a lot of Makefiles this one -- note that
I defined IN_LIB for non libraries code to "nonlib":

===============================================================================

diff --git a/Makerules b/Makerules
index 9bfe550..4ff8211 100644
--- a/Makerules
+++ b/Makerules
@@ -1202,7 +1202,7 @@ include $(patsubst %,$(..)cppflags-iterator.mk,$(cpp-srcs-left))
 endif

 # The include magic above causes those files to use this variable for flags.
-CPPFLAGS-nonlib = -DNOT_IN_libc=1
+CPPFLAGS-nonlib = -DNOT_IN_libc=1 -DIN_LIB=nonlib


 ifeq ($(versioning),yes)
diff --git a/elf/Makefile b/elf/Makefile
index b0420f5..ba03077 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -455,7 +455,7 @@ CFLAGS-ldconfig.c = $(SYSCONF-FLAGS) -D'LIBDIR="$(libdir)"' \
 CFLAGS-dl-cache.c = $(SYSCONF-FLAGS)
 CFLAGS-cache.c = $(SYSCONF-FLAGS)

-CPPFLAGS-.os += $(if $(filter $(@F),$(patsubst %,%.os,$(all-rtld-routines))),-DNOT_IN_libc=1 -DIS_IN_rtld=1)
+CPPFLAGS-.os += $(if $(filter $(@F),$(patsubst %,%.os,$(all-rtld-routines))),-DNOT_IN_libc=1 -DIS_IN_rtld=1 -DIN_LIB=rtld)

 test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(strip $(modules-names))))
 generated += $(addsuffix .so,$(strip $(modules-names)))
diff --git a/elf/rtld-Rules b/elf/rtld-Rules
index 9f31a56..e99c81b 100644
--- a/elf/rtld-Rules
+++ b/elf/rtld-Rules
@@ -122,6 +122,6 @@ ifdef rtld-depfiles
 endif

 # This here is the whole point of all the shenanigans.
-rtld-CPPFLAGS := -DNOT_IN_libc=1 -DIS_IN_rtld=1
+rtld-CPPFLAGS := -DNOT_IN_libc=1 -DIS_IN_rtld=1 -DIN_LIB=rtld

 endif
diff --git a/iconv/Makefile b/iconv/Makefile
index 77a9ad7..6abf27b 100644
--- a/iconv/Makefile
+++ b/iconv/Makefile
@@ -63,13 +63,13 @@ CFLAGS-gconv_cache.c += -DGCONV_DIR='"$(gconvdir)"'
 CFLAGS-gconv_conf.c = -DGCONV_PATH='"$(gconvdir)"'
 CFLAGS-iconvconfig.c = -DGCONV_PATH='"$(gconvdir)"' -DGCONV_DIR='"$(gconvdir)"'

-CPPFLAGS-iconv_prog = -DNOT_IN_libc
-CPPFLAGS-iconv_charmap = -DNOT_IN_libc
-CPPFLAGS-iconvconfig = -DNOT_IN_libc
-CPPFLAGS-linereader = -DNOT_IN_libc
-CPPFLAGS-strtab = -DNOT_IN_libc
-CPPFLAGS-charmap = -DNOT_IN_libc
-CPPFLAGS-charmap-dir = -DNOT_IN_libc
+CPPFLAGS-iconv_prog = -DNOT_IN_libc -DIN_LIB=nonlib
+CPPFLAGS-iconv_charmap = -DNOT_IN_libc -DIN_LIB=nonlib
+CPPFLAGS-iconvconfig = -DNOT_IN_libc -DIN_LIB=nonlib
+CPPFLAGS-linereader = -DNOT_IN_libc -DIN_LIB=nonlib
+CPPFLAGS-strtab = -DNOT_IN_libc -DIN_LIB=nonlib
+CPPFLAGS-charmap = -DNOT_IN_libc -DIN_LIB=nonlib
+CPPFLAGS-charmap-dir = -DNOT_IN_libc -DIN_LIB=nonlib

 include ../Rules

diff --git a/iconvdata/Makefile b/iconvdata/Makefile
index d8fb282..bd25641 100644
--- a/iconvdata/Makefile
+++ b/iconvdata/Makefile
@@ -79,7 +79,7 @@ tst-iconv7-ENV = LOCPATH=$(common-objpfx)localedata
 endif

 # No code here is in libc.so.
-CPPFLAGS += -DNOT_IN_libc
+CPPFLAGS += -DNOT_IN_libc -DIN_LIB=localedata

 libJIS-routines := jis0201 jis0208 jis0212
 libKSC-routines := ksc5601
diff --git a/malloc/Makefile b/malloc/Makefile
index e7ec1ab..412e7ba 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -57,7 +57,7 @@ aux := set-freeres thread-freeres

 include ../Makeconfig

-CPPFLAGS-memusagestat = -DNOT_IN_libc
+CPPFLAGS-memusagestat = -DNOT_IN_libc -DIN_LIB=libmalloc

 # The Perl script to analyze the output of the mtrace functions.
 ifneq ($(PERL),no)

===============================================================================

And with the changes in the Makefiles, the ugly #define code can be
cleaned up.

Addition to Changelog (against nptl/ChangeLog):

2011-01-26  Rayson Ho  <rho@redhat.com>

	* DESIGN-systemtap-probes.txt: New file.
	* pthread_cond_broadcast.c: SystemTap probes.
	* pthread_cond_init.c: Likewise.
	* pthread_cond_signal.c: Likewise.
	* pthread_cond_wait.c: Likewise.
	* pthread_condattr_destroy.c: Likewise.
	* pthread_cond_destroy.c: Likewise.
	* pthread_create.c: Likewise.
	* pthread_join.c: Likewise.
	* pthread_mutex_destroy.c: Likewise.
	* pthread_mutex_init.c: Likewise.
	* pthread_mutex_lock.c: Likewise.
	* pthread_mutex_unlock.c: Likewise.
	* pthread_rwlock_destroy.c: Likewise.
	* pthread_rwlock_rdlock.c: Likewise.
	* pthread_rwlock_unlock.c: Likewise.
	* pthread_rwlock_wrlock.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S: Likewise.


And the patch:

===============================================================================

diff --git a/nptl/DESIGN-systemtap-probes.txt b/nptl/DESIGN-systemtap-probes.txt
index e69de29..dff4dc1 100644
--- a/nptl/DESIGN-systemtap-probes.txt
+++ b/nptl/DESIGN-systemtap-probes.txt
@@ -0,0 +1,80 @@
+Systemtap is a dynamic tracing/instrumenting tool available on Linux. Probes
+that are not fired at run time have close to zero overhead.
+
+The following probes are available for NPTL:
+
+Thread creation & Join Probes
+=============================
+pthread_create - probe for pthread_create
+               arg1 = start_routine
+               arg2 = arguments to start_routine
+pthread_start - probe for actual thread creation
+              arg1 = struct pthread (members include thread ID, process ID)
+              arg2 = address of start_routine
+              arg3 = pointer to the list of arguments
+pthread_join - probe for pthread_join
+             arg1 = thread ID
+pthread_join_ret - probe for pthread_join return
+                 arg1 = thread ID
+                 arg2 = return value
+
+Lock-related Probes
+===================
+mutex_init    - probe for pthread_mutex_init
+              arg1 = address of mutex lock
+mutex_acquired - probe for pthread_mutex_lock
+               arg1 = address of mutex lock
+mutex_block   - probe for resume from _possible_ mutex block event
+              arg1 = address of mutex lock
+mutex_entry   - probe for entry to the pthread_mutex_lock function
+              arg1 = address of mutex lock
+mutex_release - probe for pthread_mutex_unlock after the successful release of a
+                mutex lock
+              arg1 = address of mutex lock
+mutex_destroy - probe for pthread_mutex_destroy
+              arg1 = address of mutex lock
+
+wrlock_entry - probe for entry to the pthread_rwlock_wrlock function
+             arg1 = address of rw lock
+rdlock_entry - probe for entry to the pthread_rwlock_rdlock function
+             arg1 = address of rw lock
+
+rwlock_destroy - probe for pthread_rwlock_destroy
+               arg1 = address of rw lock
+rwlock_acquire_write - probe for pthread_rwlock_wrlock
+                     arg1 = address of rw lock
+rdlock_acquire_read - probe for pthread_rwlock_rdlock after successfully getting
+                      the lock
+                    arg1 = address of rw lock
+rwlock_unlock - probe for pthread_rwlock_unlock
+              arg1 = address of rw lock
+
+lll_futex_wait - probe in low-level (assembly language) locking code, only fired
+                 when futex is called (i.e. when trying to acquire a contented
+                 lock)
+lll_futex_wait_private - probe in low-level (assembly language) locking code,
+                         only fired when futex is called (i.e. when trying to
+                         acquire a contented lock)
+lll_futex_wake - probe in low-level (assembly language) locking code, only fired
+                 when futex is called (FUTEX_WAIT), arg1 = 
+
+Condition variable Probes
+=========================
+cond_init - probe for pthread_cond_init
+          arg1 = condition
+          arg2 = attr
+condattr_destroy - probe for pthread_condattr_destroy
+             arg1 = attr
+cond_destroy - probe for pthread_cond_destroy
+             arg1 = cond
+cond_wait - probe for pthread_cond_wait
+          arg1 = condition
+          arg2 = mutex lock
+cond_timedwait - probe for pthread_cond_timedwait
+               arg1 = condition
+               arg2 = mutex lock
+               arg3 = timespec
+cond_signal - probe for pthread_cond_signal
+            arg1 = condition
+cond_broadcast - probe for pthread_cond_broadcast
+               arg1 = condition
diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
index 22523c2..a2e462f 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
@@ -32,6 +33,8 @@ int
 __pthread_cond_broadcast (cond)
      pthread_cond_t *cond;
 {
+  LIBC_PROBE (cond_broadcast, 1, cond);
+
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
   /* Make sure we are alone.  */
diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
index 65c01b1..caec6ca 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -20,6 +20,7 @@
 
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -42,6 +43,8 @@ __pthread_cond_init (cond, cond_attr)
 			  ? NULL : (void *) ~0l);
   cond->__data.__broadcast_seq = 0;
 
+  LIBC_PROBE (cond_init, 2, cond, cond_attr);
+
   return 0;
 }
 versioned_symbol (libpthread, __pthread_cond_init,
diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
index 023bbb5..6615f21 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -35,6 +35,8 @@ __pthread_cond_signal (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
                ? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_signal, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 467a03a..bac1bef 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 
@@ -101,6 +102,8 @@ __pthread_cond_wait (cond, mutex)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
                ? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_wait, 2, cond, mutex);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_condattr_destroy.c b/nptl/pthread_condattr_destroy.c
index e6d069e..5a604bc 100644
--- a/nptl/pthread_condattr_destroy.c
+++ b/nptl/pthread_condattr_destroy.c
@@ -18,6 +18,7 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -25,6 +26,8 @@ __pthread_condattr_destroy (attr)
      pthread_condattr_t *attr;
 {
   /* Nothing to be done.  */
+
+  LIBC_PROBE (condattr_destroy, 1, attr);
   return 0;
 }
 strong_alias (__pthread_condattr_destroy, pthread_condattr_destroy)

diff --git a/nptl/pthread_cond_destroy.c b/nptl/pthread_cond_destroy.c
index 35135a6..a28d061 100644
--- a/nptl/pthread_cond_destroy.c
+++ b/nptl/pthread_cond_destroy.c
@@ -20,6 +20,7 @@
 #include <errno.h>
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -29,6 +30,8 @@ __pthread_cond_destroy (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
                ? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_destroy, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);

diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 4075dd9..d574cc5 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
   /* Pass the descriptor to the caller.  */
   *newthread = (pthread_t) pd;
 
+  LIBC_PROBE (pthread_create, 2, start_routine, arg);
+
   /* Start the thread.  */
   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
 }
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 6a87a8b..609e2cf 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -23,6 +23,8 @@
 #include <atomic.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 static void
 cleanup (void *arg)
@@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
   struct pthread *self = THREAD_SELF;
   int result = 0;
 
+  LIBC_PROBE (pthread_join, 1, threadid);
+
   /* During the wait we change to asynchronous cancellation.  If we
      are canceled the thread we are waiting for must be marked as
      un-wait-ed for again.  */
@@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
       __free_tcb (pd);
     }
 
+  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
+
   return result;
 }
diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
index e2c9f8a..45c80b8 100644
--- a/nptl/pthread_mutex_destroy.c
+++ b/nptl/pthread_mutex_destroy.c
@@ -20,11 +20,15 @@
 #include <errno.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 int
 __pthread_mutex_destroy (mutex)
      pthread_mutex_t *mutex;
 {
+  LIBC_PROBE (mutex_destroy, 1, mutex);
+
   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
       && mutex->__data.__nusers != 0)
     return EBUSY;
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index d9b1ef0..a025a38 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -24,6 +24,8 @@
 #include <kernel-features.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 static const struct pthread_mutexattr default_attr =
   {
     /* Default is a normal mutex, not shared between processes.  */
@@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
   // mutex->__spins = 0;	already done by memset
   // mutex->__next = NULL;	already done by memset
 
+  LIBC_PROBE (mutex_init, 1, mutex);
+
   return 0;
 }
 strong_alias (__pthread_mutex_init, pthread_mutex_init)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 50dc188..010d43e 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -24,6 +24,7 @@
 #include <not-cancel.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 
 #ifndef LLL_MUTEX_LOCK
@@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
 
   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
+
+  LIBC_PROBE (mutex_entry, 1, mutex);
+
   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
     return __pthread_mutex_lock_full (mutex);
 
@@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
       /* Normal mutex.  */
       LLL_MUTEX_LOCK (mutex);
       assert (mutex->__data.__owner == 0);
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
     {
@@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
 
       assert (mutex->__data.__owner == 0);
       mutex->__data.__count = 1;
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
     {
@@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
 	    }
 	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 
+          LIBC_PROBE (mutex_block, 1, mutex);
+
 	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
 	}
       assert (mutex->__data.__owner == 0);
@@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 
@@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  }
 	while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
 
+        LIBC_PROBE (mutex_block, 1, mutex);
+
 	assert (mutex->__data.__owner == 0);
 	mutex->__data.__count = 1;
       }
@@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 #ifndef __pthread_mutex_lock
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index f9fe10b..479e500 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 static int
 internal_function
@@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       /* Unlock.  */
       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return 0;
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
@@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
 			PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return __pthread_tpp_change_priority (oldprio, -1);
 
     default:
@@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
       return EINVAL;
     }
 
+  LIBC_PROBE (mutex_release, 1, mutex);
   return 0;
 }
 
diff --git a/nptl/pthread_rwlock_destroy.c b/nptl/pthread_rwlock_destroy.c
index 28fd24b..84aa693 100644
--- a/nptl/pthread_rwlock_destroy.c
+++ b/nptl/pthread_rwlock_destroy.c
@@ -18,12 +18,15 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
 __pthread_rwlock_destroy (rwlock)
      pthread_rwlock_t *rwlock;
 {
+  LIBC_PROBE (rwlock_destroy, 1, rwlock);
+
   /* Nothing to be done.  For now.  */
   return 0;
 }
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 2feac57..09bad14 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire read lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_rdlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (rdlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -49,6 +52,8 @@ __pthread_rwlock_rdlock (rwlock)
              --rwlock->__data.__nr_readers;
              result = EAGAIN;
            }
+          else
+            LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
 
          break;
        }
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
index a7ef71a..a6e8d87 100644
--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -22,11 +22,14 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 /* Unlock RWLOCK.  */
 int
 __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
 {
+  LIBC_PROBE (rwlock_unlock, 1, rwlock);
+
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
   if (rwlock->__data.__writer)
     rwlock->__data.__writer = 0;
diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
index 9d5f135..5b93b3c 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire write lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (wrlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -41,6 +44,9 @@ __pthread_rwlock_wrlock (rwlock)
        {
          /* Mark self as writer.  */
          rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
+
+          LIBC_PROBE (rwlock_acquire_write, 1, rwlock);
+
          break;
        }

 
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
index 3195db2..568643c 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
@@ -22,6 +22,8 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#include <stap-probe.h>
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +93,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE_ASM (lll_futex_wait_private, %rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
@@ -130,7 +133,8 @@ __lll_lock_wait:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE_ASM (lll_futex_wait, %rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index 9b15bfb..4871b5b 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -20,6 +20,8 @@
 #ifndef _LOWLEVELLOCK_H
 #define _LOWLEVELLOCK_H	1
 
+#include <stap-probe.h>
+
 #ifndef __ASSEMBLER__
 # include <time.h>
 # include <sys/param.h>
@@ -227,6 +229,7 @@ LLL_STUB_UNWIND_INFO_END
   do {									      \
     int __ignore;							      \
     register __typeof (nr) _nr __asm ("edx") = (nr);			      \
+    LIBC_PROBE (lll_futex_wake, 2, futex, nr);                                \
     __asm __volatile ("syscall"						      \
 		      : "=a" (__ignore)					      \
 		      : "0" (SYS_futex), "D" (futex),			      \
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
index 224a560..c56deae 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
@@ -25,7 +25,7 @@
 #include <kernel-features.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -35,6 +35,8 @@
 	.align	16
 __pthread_cond_broadcast:
 
+        LIBC_PROBE_ASM (cond_broadcast, %rdi)
+
 	/* Get internal lock.  */
 	movl	$1, %esi
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
index d1d83a8..739fedd 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
@@ -24,6 +24,7 @@
 #include <pthread-pi-defines.h>
 #include <kernel-features.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 
 	.text
@@ -34,6 +35,8 @@
 	.align	16
 __pthread_cond_signal:
 
+        LIBC_PROBE_ASM (cond_signal, %rdi)
+
 	/* Get internal lock.  */
 	movq	%rdi, %r8
 	movl	$1, %esi
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
index e6535fb..06fbdae 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -41,6 +42,9 @@
 __pthread_cond_timedwait:
 .LSTARTCODE:
 	cfi_startproc
+
+        LIBC_PROBE_ASM (cond_timedwait, %rdi)
+
 #ifdef SHARED
 	cfi_personality(DW_EH_PE_pcrel | DW_EH_PE_sdata4 | DW_EH_PE_indirect,
 			DW.ref.__gcc_personality_v0)
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
index f5b929e..cb0f5bd 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <tcb-offsets.h>
 #include <pthread-pi-defines.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -34,6 +35,9 @@
 	.type	__pthread_cond_wait, @function
 	.align	16
 __pthread_cond_wait:
+
+        LIBC_PROBE_ASM (cond_wait, %rdi)
+
 .LSTARTCODE:
 	cfi_startproc
 #ifdef SHARED
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
index 35eb09c..09c1db1 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,9 @@
 	.align	16
 __pthread_rwlock_rdlock:
 	cfi_startproc
+
+        LIBC_PROBE_ASM (rdlock_entry, %rdi)
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
index be6b8d8..961b50b 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,9 @@
 	.align	16
 __pthread_rwlock_wrlock:
 	cfi_startproc
+
+        LIBC_PROBE_ASM (wrlock_entry, %rdi)
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */

===============================================================================

Thanks,
Rayson




> 
> 
> Thanks,
> Roland


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-27  9:11                   ` Rayson Ho
@ 2011-01-28  7:20                     ` Roland McGrath
  2011-02-02  8:12                       ` Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-01-28  7:20 UTC (permalink / raw)
  To: Rayson Ho; +Cc: libc-alpha, systemtap

> The problem I had at the beginning was related to some modules not
> defining IN_LIB, so the following code was complaining:

I see.  No such code should be actually using these macros.  But it may
wind up indirectly including lowlevellock.h for some reason.  Is that
what you saw?  Please cite the particular compilation errors you saw.

> Some code (files) really are not libraries, examples of those are utils
> in iconv. And some do not include extra-lib.mk, as they do their own
> "-DNOT_IN_libc=1" in their Makefiles.

Sure, but I really don't think any of that code ought to be using the
macros where you have probes used.

> The simplest hack would be to not "#error" the  define, but to quietly
> define IN_LIB to something else.

If the header is used but not the macros that use the probe macros, then
the safe thing is to define IN_LIB to something that ensures a syntax
error.  Then there won't be the #error failures just from having the header
file, but if any uses of those ever macros leaks into non-library code,
then compilation will fail loudly so we can see what is going on.  We don't
want to quietly insert probes with stupid names into places where we don't
intend to have probes.

> A better fix but involves touch a lot of Makefiles this one -- note that
> I defined IN_LIB for non libraries code to "nonlib":

No, this is a bad change to make.

> +condattr_destroy - probe for pthread_condattr_destroy

Who cares about that?  It makes no sense to have this probe an no other
probes about pthread_*attr_* calls.  I see no reason anyone would ever
wants probes on these calls.  They are ancillary machinery to making
pthread_*_init calls, nothing interesting themselves.

> @@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
>    /* Pass the descriptor to the caller.  */
>    *newthread = (pthread_t) pd;
>  
> +  LIBC_PROBE (pthread_create, 2, start_routine, arg);

Why doesn't this get all four arguments?

>  __pthread_cond_timedwait:
>  .LSTARTCODE:
>  	cfi_startproc
> +
> +        LIBC_PROBE_ASM (cond_timedwait, %rdi)

Why doesn't this get all the arguments?

>  __pthread_cond_wait:
> +
> +        LIBC_PROBE_ASM (cond_wait, %rdi)

Why doesn't this get all the arguments?


Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-01-28  7:20                     ` Roland McGrath
@ 2011-02-02  8:12                       ` Rayson Ho
  2011-02-04 19:04                         ` Roland McGrath
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-02-02  8:12 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-alpha, systemtap

On Thu, 2011-01-27 at 23:20 -0800, Roland McGrath wrote:
> > The problem I had at the beginning was related to some modules not
> > defining IN_LIB, so the following code was complaining:
> 
> I see.  No such code should be actually using these macros.  But it may
> wind up indirectly including lowlevellock.h for some reason.  Is that
> what you saw?  Please cite the particular compilation errors you saw.

That's exactly the issue in the first place - lowlevellock.h is
indirectly included in many places. Without the Makefile changes, IN_LIB
is not defined for the non-library modules.

The compilation error message is:

gcc iconv_charmap.c -c -std=gnu99 -fgnu89-inline -O -Wall -Winline
-Wwrite-strings -fmerge-all-constants -g -Wstrict-prototypes
-I../locale/programs   -I/home/rayson/tmp922st/systemtap/includes
-I../include -I/home/rayson/tmp929glibc/glibc/bld/iconv
-I/home/rayson/tmp929glibc/glibc/bld -I../sysdeps/x86_64/elf
-I../nptl/sysdeps/unix/sysv/linux/x86_64
-I../sysdeps/unix/sysv/linux/x86_64
-I../sysdeps/unix/sysv/linux/wordsize-64
-I../nptl/sysdeps/unix/sysv/linux -I../nptl/sysdeps/pthread
-I../sysdeps/pthread -I../sysdeps/unix/sysv/linux -I../sysdeps/gnu
-I../sysdeps/unix/common -I../sysdeps/unix/mman -I../sysdeps/unix/inet
-I../nptl/sysdeps/unix/sysv -I../sysdeps/unix/sysv
-I../sysdeps/unix/x86_64 -I../nptl/sysdeps/unix -I../sysdeps/unix
-I../sysdeps/posix -I../sysdeps/x86_64/fpu -I../sysdeps/x86_64/multiarch
-I../nptl/sysdeps/x86_64 -I../sysdeps/x86_64 -I../sysdeps/wordsize-64
-I../sysdeps/ieee754/ldbl-96 -I../sysdeps/ieee754/dbl-64/wordsize-64
-I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32
-I../sysdeps/ieee754 -I../sysdeps/generic/elf -I../sysdeps/generic
-I../nptl  -I.. -I../libio -I.  -D_LIBC_REENTRANT
-include ../include/libc-symbols.h      -DNOT_IN_libc
-o /home/rayson/tmp929glibc/glibc/bld/iconv/iconv_charmap.o -MD -MP
-MF /home/rayson/tmp929glibc/glibc/bld/iconv/iconv_charmap.o.dt
-MT /home/rayson/tmp929glibc/glibc/bld/iconv/iconv_charmap.o
In file included
from ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h:23,
                 from ../nptl/descr.h:30,
                 from ../nptl/sysdeps/x86_64/tls.h:98,
                 from ../include/tls.h:6,
                 from ../include/errno.h:22,
                 from iconv_charmap.c:21:
../include/stap-probe.h:39:4: error: #error "missing -DIN_LIB=... -- not
extra-lib.mk?"
make[2]: *** [/home/rayson/tmp929glibc/glibc/bld/iconv/iconv_charmap.o]
Error 1
make[2]: Leaving directory `/home/rayson/tmp929glibc/glibc/iconv'
make[1]: *** [iconv/others] Error 2
make[1]: Leaving directory `/home/rayson/tmp929glibc/glibc'
make: *** [all] Error 2

I tried to find a better solution but I think it is deep in the include
chain, so I think we will need to either fix it/workaround it from the
root stap-probe.h, or define IN_LIB to satify the #define requirement.


> Sure, but I really don't think any of that code ought to be using the
> macros where you have probes used.

Except those that indirectly include lowlevellock.h .


> 
> If the header is used but not the macros that use the probe macros, then
> the safe thing is to define IN_LIB to something that ensures a syntax
> error.  Then there won't be the #error failures just from having the header
> file, but if any uses of those ever macros leaks into non-library code,
> then compilation will fail loudly so we can see what is going on.  We don't
> want to quietly insert probes with stupid names into places where we don't
> intend to have probes.
> 
> > A better fix but involves touch a lot of Makefiles this one -- note that
> > I defined IN_LIB for non libraries code to "nonlib":
> 
> No, this is a bad change to make.

It's actually 6 Makefile changes, so may not sound as what I described
in my previous email.


> 
> > +condattr_destroy - probe for pthread_condattr_destroy
> 
> Who cares about that?  It makes no sense to have this probe an no other
> probes about pthread_*attr_* calls.  I see no reason anyone would ever
> wants probes on these calls.  They are ancillary machinery to making
> pthread_*_init calls, nothing interesting themselves.
> 
> > @@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
> >    /* Pass the descriptor to the caller.  */
> >    *newthread = (pthread_t) pd;
> >  
> > +  LIBC_PROBE (pthread_create, 2, start_routine, arg);
> 
> Why doesn't this get all four arguments?
> 
> >  __pthread_cond_timedwait:
> >  .LSTARTCODE:
> >  	cfi_startproc
> > +
> > +        LIBC_PROBE_ASM (cond_timedwait, %rdi)
> 
> Why doesn't this get all the arguments?
> 
> >  __pthread_cond_wait:
> > +
> > +        LIBC_PROBE_ASM (cond_wait, %rdi)
> 
> Why doesn't this get all the arguments?

All those are fixed -- I tried to use STAP_PROBE_ASM &
STAP_PROBE_ASM_TEMPLATE in those places and having some issues, and I
just changed it to much simpler LIBC_PROBE().

Rayson


> 
> 
> Thanks,
> Roland


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-02  8:12                       ` Rayson Ho
@ 2011-02-04 19:04                         ` Roland McGrath
  2011-02-08 22:18                           ` Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-02-04 19:04 UTC (permalink / raw)
  To: Rayson Ho; +Cc: libc-alpha, systemtap

> > I see.  No such code should be actually using these macros.  But it may
> > wind up indirectly including lowlevellock.h for some reason.  Is that
> > what you saw?  Please cite the particular compilation errors you saw.
> 
> That's exactly the issue in the first place - lowlevellock.h is
> indirectly included in many places. Without the Makefile changes, IN_LIB
> is not defined for the non-library modules.

I've fixed that problem differently in the branch.  It should now bomb
out--as we want it to--if any macros containing probes are actually used in
files that don't define IN_LIB, but won't just because of #include chains.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-04 19:04                         ` Roland McGrath
@ 2011-02-08 22:18                           ` Rayson Ho
  2011-02-08 22:48                             ` Roland McGrath
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-02-08 22:18 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-alpha, systemtap

Thanks Roland. I've tried your fix and it works for most cases.

However, I just clone a clean branch and merged my changes on top, and
found that files in the rtld module still needs to have IN_LIB defined.
If I don't do that, then it would not compile because for example
dl-lookup.c calls lll_futex_wake() (via. THREAD_GSCOPE_RESET_FLAG). So
there really are files that eventually use the LIBC_PROBE macro outside
of the pthread module & other modules that have IN_LIB properly defined.

I looked at your changes and found that it adds ",,," to the definition
to IN_LIB, so that when it is used it would bomb out. However, as those
places really use the LIBC_PROBE() macro, I don't think there is a way
to work around it than to define IN_LIB. With your fix, things are a lot
better but there are still 2 Makefile changes required:

1) elf/Makefile

- add a parallel case (-DIN_LIB=rtld) when "-DNOT_IN_libc=1" is defined.

2) elf/rtld-Rules

- similarly, add a parallel case (-DIN_LIB=rtld) when "-DNOT_IN_libc=1"
is defined.

And I believe they are correct fixes, as the existing code already has
the macro "-DIS_IN_rtld=1" in those Makefiles.

Secondly, testing against the systemtap disabled case, I found that if a
probe is used in an assembly file (which was what I changed last week
instead of using LIBC_PROBE_ASM), then the definitions in stap-probe.h
would cause it to blow up because we are inserting C code do {}
while(0).

So my fix is to define the DUMMY_PROBEs to "nothing" when it 
USE_STAP_PROBE is not defined & we are in ASSEMBLER mode:

===================================================================

diff --git a/include/stap-probe.h b/include/stap-probe.h
index 9ed6206..1915ce6 100644
--- a/include/stap-probe.h
+++ b/include/stap-probe.h
@@ -68,31 +68,39 @@
 /* This silliness lets us evaluate all the arguments for each arity
    of probe.  My kingdom for a real macro system.  */

-# define DUMMY_PROBE0()                        do {} while (0)
-# define DUMMY_PROBE1(a1)              do {} while ((void) (a1), 0)
-# define DUMMY_PROBE2(a1, a2)          do {} while ((void) (a1), \
+# ifdef __ASSEMBLER__
+#  define DUMMY_PROBE0()                 /* Nothing.  */
+#  define DUMMY_PROBE1(a1)               /* Nothing.  */
+#  define DUMMY_PROBE2(a1, a2)           /* Nothing.  */
+#  define DUMMY_PROBE3(a1, a2, a3)       /* Nothing.  */
+
+# else
+
+#  define DUMMY_PROBE0()                       do {} while (0)
+#  define DUMMY_PROBE1(a1)             do {} while ((void) (a1), 0)
+#  define DUMMY_PROBE2(a1, a2)         do {} while ((void) (a1), \
                                                     (void) (a2), 0)
-# define DUMMY_PROBE3(a1, a2, a3)      do {} while ((void) (a1), \
+#  define DUMMY_PROBE3(a1, a2, a3)     do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), 0)
-# define DUMMY_PROBE4(a1, a2, a3, a4)  do {} while ((void) (a1), \
+#  define DUMMY_PROBE4(a1, a2, a3, a4) do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
                                                     (void) (a4), 0)
-# define DUMMY_PROBE5(a1, a2, a3, a4, a5)                        \
+#  define DUMMY_PROBE5(a1, a2, a3, a4, a5)                       \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
                                                     (void) (a4), \
                                                     (void) (a5), 0)
-# define DUMMY_PROBE6(a1, a2, a3, a4, a5, a6)                    \
+#  define DUMMY_PROBE6(a1, a2, a3, a4, a5, a6)                   \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
                                                     (void) (a4), \
                                                     (void) (a5), \
                                                     (void) (a6), 0)
-# define DUMMY_PROBE7(a1, a2, a3, a4, a5, a6, a7)                \
+#  define DUMMY_PROBE7(a1, a2, a3, a4, a5, a6, a7)               \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
@@ -100,7 +108,7 @@
                                                     (void) (a5), \
                                                     (void) (a6), \
                                                     (void) (a7), 0)
-# define DUMMY_PROBE8(a1, a2, a3, a4, a5, a6, a7, a8)            \
+#  define DUMMY_PROBE8(a1, a2, a3, a4, a5, a6, a7, a8)           \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
@@ -109,7 +117,7 @@
                                                     (void) (a6), \
                                                     (void) (a7), \
                                                     (void) (a8), 0)
-# define DUMMY_PROBE9(a1, a2, a3, a4, a5, a6, a7, a8, a9)        \
+#  define DUMMY_PROBE9(a1, a2, a3, a4, a5, a6, a7, a8, a9)       \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
@@ -119,7 +127,7 @@
                                                     (void) (a7), \
                                                     (void) (a8), \
                                                     (void) (a9), 0)
-# define DUMMY_PROBE10(a1, a2, a3, a4, a5, a6, a7, a8, a9, a10)
\
+#  define DUMMY_PROBE10(a1, a2, a3, a4, a5, a6, a7, a8, a9, a10)
\
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
@@ -130,6 +138,7 @@
                                                     (void) (a8), \
                                                     (void) (a9), \
                                                     (void) (a10), 0)
+#endif /* __ASSEMBLER__ */

 #endif /* USE_STAP_PROBE.  */

===================================================================

Please let me know if you are OK with those changes, and if so I will
send the pthread patches to the list tonight.

Rayson




On Fri, 2011-02-04 at 11:03 -0800, Roland McGrath wrote:
> > > I see.  No such code should be actually using these macros.  But it may
> > > wind up indirectly including lowlevellock.h for some reason.  Is that
> > > what you saw?  Please cite the particular compilation errors you saw.
> > 
> > That's exactly the issue in the first place - lowlevellock.h is
> > indirectly included in many places. Without the Makefile changes, IN_LIB
> > is not defined for the non-library modules.
> 
> I've fixed that problem differently in the branch.  It should now bomb
> out--as we want it to--if any macros containing probes are actually used in
> files that don't define IN_LIB, but won't just because of #include chains.
> 
> 
> Thanks,
> Roland


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-08 22:18                           ` Rayson Ho
@ 2011-02-08 22:48                             ` Roland McGrath
  2011-02-09 17:09                               ` Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-02-08 22:48 UTC (permalink / raw)
  To: Rayson Ho; +Cc: libc-alpha, systemtap

I fixed the assembly case in a cleaner way.  Good catch there.
Defining IN_LIB for the rtld cases is indeed correct and I added those.

Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-08 22:48                             ` Roland McGrath
@ 2011-02-09 17:09                               ` Rayson Ho
  2011-02-09 18:00                                 ` Bert Wesarg
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-02-09 17:09 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-alpha, systemtap

Thanks again, Roland! I've applied my changes on top of your latest
branch refresh, and it works with & without those probes enabled.

So here's my pthread probing patch again:

==========================================================


Addition to Changelog (against nptl/ChangeLog):

2011-02-09  Rayson Ho  <rho@redhat.com>

        * DESIGN-systemtap-probes.txt: New file.
        * pthread_cond_broadcast.c: SystemTap probes.
        * pthread_cond_init.c: Likewise.
        * pthread_cond_signal.c: Likewise.
        * pthread_cond_wait.c: Likewise.
        * pthread_cond_destroy.c: Likewise.
        * pthread_create.c: Likewise.
        * pthread_join.c: Likewise.
        * pthread_mutex_destroy.c: Likewise.
        * pthread_mutex_init.c: Likewise.
        * pthread_mutex_lock.c: Likewise.
        * pthread_mutex_unlock.c: Likewise.
        * pthread_rwlock_destroy.c: Likewise.
        * pthread_rwlock_rdlock.c: Likewise.
        * pthread_rwlock_unlock.c: Likewise.
        * pthread_rwlock_wrlock.c: Likewise.
        * sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.
        * sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Likewise.
        * sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S: Likewise.
        * sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S: Likewise.
        * sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S: Likewise.
        * sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S: Likewise.
        * sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S: Likewise.
        * sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S: Likewise.

diff --git a/nptl/DESIGN-systemtap-probes.txt b/nptl/DESIGN-systemtap-probes.txt
index e69de29..5485ce4 100644
--- a/nptl/DESIGN-systemtap-probes.txt
+++ b/nptl/DESIGN-systemtap-probes.txt
@@ -0,0 +1,88 @@
+Systemtap is a dynamic tracing/instrumenting tool available on Linux. Probes
+that are not fired at run time have close to zero overhead.
+
+The following probes are available for NPTL:
+
+Thread creation & Join Probes
+=============================
+pthread_create - probe for pthread_create
+               arg1 = pointer (pthread_t*) to thread
+               arg2 = pointer (pthread_attr_t*) to attr
+               arg3 = pointer (void *) to start_routine
+               arg4 = arguments to start_routine
+pthread_start - probe for actual thread creation
+              arg1 = struct pthread (members include thread ID, process ID)
+              arg2 = address of start_routine
+              arg3 = pointer to the list of arguments
+pthread_join - probe for pthread_join
+             arg1 = thread ID
+pthread_join_ret - probe for pthread_join return
+                 arg1 = thread ID
+                 arg2 = return value
+
+Lock-related Probes
+===================
+mutex_init    - probe for pthread_mutex_init
+              arg1 = address of mutex lock
+mutex_acquired - probe for pthread_mutex_lock
+               arg1 = address of mutex lock
+mutex_block   - probe for resume from _possible_ mutex block event
+              arg1 = address of mutex lock
+mutex_entry   - probe for entry to the pthread_mutex_lock function
+              arg1 = address of mutex lock
+mutex_release - probe for pthread_mutex_unlock after the successful release of a
+                mutex lock
+              arg1 = address of mutex lock
+mutex_destroy - probe for pthread_mutex_destroy
+              arg1 = address of mutex lock
+
+wrlock_entry - probe for entry to the pthread_rwlock_wrlock function
+             arg1 = address of rw lock
+rdlock_entry - probe for entry to the pthread_rwlock_rdlock function
+             arg1 = address of rw lock
+
+rwlock_destroy - probe for pthread_rwlock_destroy
+               arg1 = address of rw lock
+rwlock_acquire_write - probe for pthread_rwlock_wrlock (after getting the lock)
+                     arg1 = address of rw lock
+rdlock_acquire_read - probe for pthread_rwlock_rdlock after successfully getting
+                      the lock
+                    arg1 = address of rw lock
+rwlock_unlock - probe for pthread_rwlock_unlock
+              arg1 = address of rw lock
+
+lll_lock_wait - probe in low-level (assembly language) locking code, only fired
+                when futex/FUTEX_WAIT is called (i.e. when trying to acquire a
+                contented lock)
+              arg1 = pointer to futex
+              arg2 = flags passed to the futex system call
+lll_lock_wait_private - probe in low-level (assembly language) locking code,
+                        only fired when futex/FUTEX_WAIT is called (i.e. when
+                        trying to acquire a contented lock)
+                      arg1 = pointer to futex
+                      
+lll_futex_wake - probe in low-level (assembly language) locking code, only fired
+                 when futex (FUTEX_WAKE) is called
+               arg1 = pointer to futex
+               arg2 = number of processes to wake
+               arg3 = additional flags
+             
+
+Condition variable Probes
+=========================
+cond_init - probe for pthread_cond_init
+          arg1 = condition
+          arg2 = attr
+cond_destroy - probe for pthread_cond_destroy
+             arg1 = cond
+cond_wait - probe for pthread_cond_wait
+          arg1 = condition
+          arg2 = mutex lock
+cond_timedwait - probe for pthread_cond_timedwait
+               arg1 = condition
+               arg2 = mutex lock
+               arg3 = timespec
+cond_signal - probe for pthread_cond_signal
+            arg1 = condition
+cond_broadcast - probe for pthread_cond_broadcast
+               arg1 = condition
diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
index 22523c2..a2e462f 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
@@ -32,6 +33,8 @@ int
 __pthread_cond_broadcast (cond)
      pthread_cond_t *cond;
 {
+  LIBC_PROBE (cond_broadcast, 1, cond);
+
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
   /* Make sure we are alone.  */
diff --git a/nptl/pthread_cond_destroy.c b/nptl/pthread_cond_destroy.c
index 35135a6..a28d061 100644
--- a/nptl/pthread_cond_destroy.c
+++ b/nptl/pthread_cond_destroy.c
@@ -20,6 +20,7 @@
 #include <errno.h>
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -29,6 +30,8 @@ __pthread_cond_destroy (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_destroy, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
index 65c01b1..caec6ca 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -20,6 +20,7 @@
 
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -42,6 +43,8 @@ __pthread_cond_init (cond, cond_attr)
 			  ? NULL : (void *) ~0l);
   cond->__data.__broadcast_seq = 0;
 
+  LIBC_PROBE (cond_init, 2, cond, cond_attr);
+
   return 0;
 }
 versioned_symbol (libpthread, __pthread_cond_init,
diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
index 023bbb5..414e6bc 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -26,6 +26,7 @@
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
+#include <stap-probe.h>
 
 
 int
@@ -35,6 +36,8 @@ __pthread_cond_signal (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_signal, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 467a03a..3d4c583 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -25,6 +25,7 @@
 #include <pthreadP.h>
 
 #include <shlib-compat.h>
+#include <stap-probe.h>
 
 
 struct _condvar_cleanup_buffer
@@ -101,6 +102,8 @@ __pthread_cond_wait (cond, mutex)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
   		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_wait, 2, cond, mutex);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 4075dd9..d3f5f06 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
   /* Pass the descriptor to the caller.  */
   *newthread = (pthread_t) pd;
 
+  LIBC_PROBE (pthread_create, 4, newthread, attr, start_routine, arg);
+
   /* Start the thread.  */
   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
 }
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 6a87a8b..609e2cf 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -23,6 +23,8 @@
 #include <atomic.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 static void
 cleanup (void *arg)
@@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
   struct pthread *self = THREAD_SELF;
   int result = 0;
 
+  LIBC_PROBE (pthread_join, 1, threadid);
+
   /* During the wait we change to asynchronous cancellation.  If we
      are canceled the thread we are waiting for must be marked as
      un-wait-ed for again.  */
@@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
       __free_tcb (pd);
     }
 
+  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
+
   return result;
 }
diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
index e2c9f8a..45c80b8 100644
--- a/nptl/pthread_mutex_destroy.c
+++ b/nptl/pthread_mutex_destroy.c
@@ -20,11 +20,15 @@
 #include <errno.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 int
 __pthread_mutex_destroy (mutex)
      pthread_mutex_t *mutex;
 {
+  LIBC_PROBE (mutex_destroy, 1, mutex);
+
   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
       && mutex->__data.__nusers != 0)
     return EBUSY;
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index d9b1ef0..a025a38 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -24,6 +24,8 @@
 #include <kernel-features.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 static const struct pthread_mutexattr default_attr =
   {
     /* Default is a normal mutex, not shared between processes.  */
@@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
   // mutex->__spins = 0;	already done by memset
   // mutex->__next = NULL;	already done by memset
 
+  LIBC_PROBE (mutex_init, 1, mutex);
+
   return 0;
 }
 strong_alias (__pthread_mutex_init, pthread_mutex_init)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 50dc188..010d43e 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -24,6 +24,7 @@
 #include <not-cancel.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 
 #ifndef LLL_MUTEX_LOCK
@@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
 
   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
+
+  LIBC_PROBE (mutex_entry, 1, mutex);
+
   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
     return __pthread_mutex_lock_full (mutex);
 
@@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
       /* Normal mutex.  */
       LLL_MUTEX_LOCK (mutex);
       assert (mutex->__data.__owner == 0);
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
     {
@@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
 
       assert (mutex->__data.__owner == 0);
       mutex->__data.__count = 1;
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
     {
@@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
 	    }
 	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 
+          LIBC_PROBE (mutex_block, 1, mutex);
+
 	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
 	}
       assert (mutex->__data.__owner == 0);
@@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 
@@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  }
 	while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
 
+        LIBC_PROBE (mutex_block, 1, mutex);
+
 	assert (mutex->__data.__owner == 0);
 	mutex->__data.__count = 1;
       }
@@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 #ifndef __pthread_mutex_lock
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index f9fe10b..479e500 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 static int
 internal_function
@@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       /* Unlock.  */
       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return 0;
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
@@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
 			PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return __pthread_tpp_change_priority (oldprio, -1);
 
     default:
@@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
       return EINVAL;
     }
 
+  LIBC_PROBE (mutex_release, 1, mutex);
   return 0;
 }
 
diff --git a/nptl/pthread_rwlock_destroy.c b/nptl/pthread_rwlock_destroy.c
index 28fd24b..84aa693 100644
--- a/nptl/pthread_rwlock_destroy.c
+++ b/nptl/pthread_rwlock_destroy.c
@@ -18,12 +18,15 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
 __pthread_rwlock_destroy (rwlock)
      pthread_rwlock_t *rwlock;
 {
+  LIBC_PROBE (rwlock_destroy, 1, rwlock);
+
   /* Nothing to be done.  For now.  */
   return 0;
 }
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 2feac57..09bad14 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire read lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_rdlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (rdlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -49,6 +52,8 @@ __pthread_rwlock_rdlock (rwlock)
 	      --rwlock->__data.__nr_readers;
 	      result = EAGAIN;
 	    }
+          else
+            LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
 
 	  break;
 	}
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
index a7ef71a..a6e8d87 100644
--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -22,11 +22,14 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 /* Unlock RWLOCK.  */
 int
 __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
 {
+  LIBC_PROBE (rwlock_unlock, 1, rwlock);
+
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
   if (rwlock->__data.__writer)
     rwlock->__data.__writer = 0;
diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
index 9d5f135..974fec5 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire write lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (wrlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -41,6 +44,8 @@ __pthread_rwlock_wrlock (rwlock)
 	{
 	  /* Mark self as writer.  */
 	  rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
+
+          LIBC_PROBE (rwlock_acquire_write, 1, rwlock);
 	  break;
 	}
 
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
index 3195db2..ea132f5 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
@@ -22,6 +22,8 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#include <stap-probe.h>
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +93,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE (lll_lock_wait_private, 1, %rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
@@ -130,7 +133,8 @@ __lll_lock_wait:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE (lll_lock_wait, 2, %rdi, %rsi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index 9b15bfb..da7b019 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -20,6 +20,8 @@
 #ifndef _LOWLEVELLOCK_H
 #define _LOWLEVELLOCK_H	1
 
+#include <stap-probe.h>
+
 #ifndef __ASSEMBLER__
 # include <time.h>
 # include <sys/param.h>
@@ -227,6 +229,7 @@ LLL_STUB_UNWIND_INFO_END
   do {									      \
     int __ignore;							      \
     register __typeof (nr) _nr __asm ("edx") = (nr);			      \
+    LIBC_PROBE (lll_futex_wake, 3, futex, nr, private);                       \
     __asm __volatile ("syscall"						      \
 		      : "=a" (__ignore)					      \
 		      : "0" (SYS_futex), "D" (futex),			      \
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
index 224a560..6037449 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
@@ -25,7 +25,7 @@
 #include <kernel-features.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -35,6 +35,8 @@
 	.align	16
 __pthread_cond_broadcast:
 
+        LIBC_PROBE (cond_broadcast, 1, %rdi)
+
 	/* Get internal lock.  */
 	movl	$1, %esi
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
index d1d83a8..80a61a4 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
@@ -24,6 +24,7 @@
 #include <pthread-pi-defines.h>
 #include <kernel-features.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 
 	.text
@@ -34,6 +35,8 @@
 	.align	16
 __pthread_cond_signal:
 
+        LIBC_PROBE (cond_signal, 1, %rdi)
+
 	/* Get internal lock.  */
 	movq	%rdi, %r8
 	movl	$1, %esi
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
index e6535fb..dad8ad4 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -41,6 +42,9 @@
 __pthread_cond_timedwait:
 .LSTARTCODE:
 	cfi_startproc
+
+        LIBC_PROBE (cond_timedwait, 3, %rdi, %rsi, %rdx)
+
 #ifdef SHARED
 	cfi_personality(DW_EH_PE_pcrel | DW_EH_PE_sdata4 | DW_EH_PE_indirect,
 			DW.ref.__gcc_personality_v0)
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
index f5b929e..09884bb 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <tcb-offsets.h>
 #include <pthread-pi-defines.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -34,6 +35,9 @@
 	.type	__pthread_cond_wait, @function
 	.align	16
 __pthread_cond_wait:
+
+        LIBC_PROBE (cond_wait, 2, %rdi, %rsi)
+
 .LSTARTCODE:
 	cfi_startproc
 #ifdef SHARED
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
index 35eb09c..1e794a9 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,9 @@
 	.align	16
 __pthread_rwlock_rdlock:
 	cfi_startproc
+
+        LIBC_PROBE (rdlock_entry, 1, %rdi)
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
index be6b8d8..9cc1197 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,9 @@
 	.align	16
 __pthread_rwlock_wrlock:
 	cfi_startproc
+
+        LIBC_PROBE (wrlock_entry, 1, %rdi)
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */

==========================================================

Rayson




On Tue, 2011-02-08 at 14:48 -0800, Roland McGrath wrote:
> I fixed the assembly case in a cleaner way.  Good catch there.
> Defining IN_LIB for the rtld cases is indeed correct and I added those.
> 
> Thanks,
> Roland


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-09 17:09                               ` Rayson Ho
@ 2011-02-09 18:00                                 ` Bert Wesarg
  2011-02-09 18:31                                   ` Rayson Ho
  2011-02-09 18:35                                   ` Rayson Ho
  0 siblings, 2 replies; 32+ messages in thread
From: Bert Wesarg @ 2011-02-09 18:00 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Roland McGrath, libc-alpha, systemtap

On Wed, Feb 9, 2011 at 18:09, Rayson Ho <rho@redhat.com> wrote:
> Thanks again, Roland! I've applied my changes on top of your latest
> branch refresh, and it works with & without those probes enabled.
>
> So here's my pthread probing patch again:
>
> diff --git a/nptl/DESIGN-systemtap-probes.txt b/nptl/DESIGN-systemtap-probes.txt
> index e69de29..5485ce4 100644
> --- a/nptl/DESIGN-systemtap-probes.txt
> +++ b/nptl/DESIGN-systemtap-probes.txt
> @@ -0,0 +1,88 @@

> +
> +wrlock_entry - probe for entry to the pthread_rwlock_wrlock function
> +             arg1 = address of rw lock
> +rdlock_entry - probe for entry to the pthread_rwlock_rdlock function
> +             arg1 = address of rw lock
> +
> +rwlock_destroy - probe for pthread_rwlock_destroy
> +               arg1 = address of rw lock
> +rwlock_acquire_write - probe for pthread_rwlock_wrlock (after getting the lock)
> +                     arg1 = address of rw lock

IMHO, this is a typo. Because for 'read' you have
'rdlock_acquire_read' and 'rdlock_entry'. So judging from this
pattern, I would expect, that this probe would be
'wrlock_acquire_write', like 'wrlock_entry'. Also note, that in the
'acquire' probe names are redundancies for 'read' and 'write' (after
fixing the typo), while in the 'entry' names not.

Bert

> +rdlock_acquire_read - probe for pthread_rwlock_rdlock after successfully getting
> +                      the lock
> +                    arg1 = address of rw lock
> +rwlock_unlock - probe for pthread_rwlock_unlock
> +              arg1 = address of rw lock
> +

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-09 18:00                                 ` Bert Wesarg
@ 2011-02-09 18:31                                   ` Rayson Ho
  2011-02-09 19:08                                     ` Roland McGrath
  2011-02-09 18:35                                   ` Rayson Ho
  1 sibling, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-02-09 18:31 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Roland McGrath, libc-alpha, systemtap

On Wed, 2011-02-09 at 19:00 +0100, Bert Wesarg wrote:
> IMHO, this is a typo. Because for 'read' you have
> 'rdlock_acquire_read' and 'rdlock_entry'. So judging from this
> pattern, I would expect, that this probe would be
> 'wrlock_acquire_write', like 'wrlock_entry'. Also note, that in the
> 'acquire' probe names are redundancies for 'read' and 'write' (after
> fixing the typo), while in the 'entry' names not.

Thanks Bert for providing comments since the beginning of the release of
this patch. I think changing wrlock_acquire_write makes sense -
originally I wanted to use "rw" to indicate it's a read-write lock, and
use "wr" to indicate it's "write". But in this case it does not make
things consistent.

However, I think it makes sense to have the 'acquire' probe names - but
if you really think there are redundancies, can you explain it in a bit
more detail??

Revised patch:
nptl/Changlog changes:

2011-02-09  Rayson Ho  <rho@redhat.com>

	* DESIGN-systemtap-probes.txt: New file.
	* pthread_cond_broadcast.c: SystemTap probes.
	* pthread_cond_init.c: Likewise.
	* pthread_cond_signal.c: Likewise.
	* pthread_cond_wait.c: Likewise.
	* pthread_cond_destroy.c: Likewise.
	* pthread_create.c: Likewise.
	* pthread_join.c: Likewise.
	* pthread_mutex_destroy.c: Likewise.
	* pthread_mutex_init.c: Likewise.
	* pthread_mutex_lock.c: Likewise.
	* pthread_mutex_unlock.c: Likewise.
	* pthread_rwlock_destroy.c: Likewise.
	* pthread_rwlock_rdlock.c: Likewise.
	* pthread_rwlock_unlock.c: Likewise.
	* pthread_rwlock_wrlock.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S: Likewise.


diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
index 22523c2..a2e462f 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
@@ -32,6 +33,8 @@ int
 __pthread_cond_broadcast (cond)
      pthread_cond_t *cond;
 {
+  LIBC_PROBE (cond_broadcast, 1, cond);
+
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
   /* Make sure we are alone.  */
diff --git a/nptl/pthread_cond_destroy.c b/nptl/pthread_cond_destroy.c
index 35135a6..a28d061 100644
--- a/nptl/pthread_cond_destroy.c
+++ b/nptl/pthread_cond_destroy.c
@@ -20,6 +20,7 @@
 #include <errno.h>
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -29,6 +30,8 @@ __pthread_cond_destroy (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_destroy, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
index 65c01b1..caec6ca 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -20,6 +20,7 @@
 
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
@@ -42,6 +43,8 @@ __pthread_cond_init (cond, cond_attr)
 			  ? NULL : (void *) ~0l);
   cond->__data.__broadcast_seq = 0;
 
+  LIBC_PROBE (cond_init, 2, cond, cond_attr);
+
   return 0;
 }
 versioned_symbol (libpthread, __pthread_cond_init,
diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
index 023bbb5..414e6bc 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -26,6 +26,7 @@
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
+#include <stap-probe.h>
 
 
 int
@@ -35,6 +36,8 @@ __pthread_cond_signal (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_signal, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 467a03a..3d4c583 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -25,6 +25,7 @@
 #include <pthreadP.h>
 
 #include <shlib-compat.h>
+#include <stap-probe.h>
 
 
 struct _condvar_cleanup_buffer
@@ -101,6 +102,8 @@ __pthread_cond_wait (cond, mutex)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
   		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_wait, 2, cond, mutex);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 4075dd9..d3f5f06 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
   /* Pass the descriptor to the caller.  */
   *newthread = (pthread_t) pd;
 
+  LIBC_PROBE (pthread_create, 4, newthread, attr, start_routine, arg);
+
   /* Start the thread.  */
   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
 }
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 6a87a8b..609e2cf 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -23,6 +23,8 @@
 #include <atomic.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 static void
 cleanup (void *arg)
@@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
   struct pthread *self = THREAD_SELF;
   int result = 0;
 
+  LIBC_PROBE (pthread_join, 1, threadid);
+
   /* During the wait we change to asynchronous cancellation.  If we
      are canceled the thread we are waiting for must be marked as
      un-wait-ed for again.  */
@@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
       __free_tcb (pd);
     }
 
+  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
+
   return result;
 }
diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
index e2c9f8a..45c80b8 100644
--- a/nptl/pthread_mutex_destroy.c
+++ b/nptl/pthread_mutex_destroy.c
@@ -20,11 +20,15 @@
 #include <errno.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 int
 __pthread_mutex_destroy (mutex)
      pthread_mutex_t *mutex;
 {
+  LIBC_PROBE (mutex_destroy, 1, mutex);
+
   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
       && mutex->__data.__nusers != 0)
     return EBUSY;
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index d9b1ef0..a025a38 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -24,6 +24,8 @@
 #include <kernel-features.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 static const struct pthread_mutexattr default_attr =
   {
     /* Default is a normal mutex, not shared between processes.  */
@@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
   // mutex->__spins = 0;	already done by memset
   // mutex->__next = NULL;	already done by memset
 
+  LIBC_PROBE (mutex_init, 1, mutex);
+
   return 0;
 }
 strong_alias (__pthread_mutex_init, pthread_mutex_init)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 50dc188..010d43e 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -24,6 +24,7 @@
 #include <not-cancel.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 
 #ifndef LLL_MUTEX_LOCK
@@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
 
   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
+
+  LIBC_PROBE (mutex_entry, 1, mutex);
+
   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
     return __pthread_mutex_lock_full (mutex);
 
@@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
       /* Normal mutex.  */
       LLL_MUTEX_LOCK (mutex);
       assert (mutex->__data.__owner == 0);
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
     {
@@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
 
       assert (mutex->__data.__owner == 0);
       mutex->__data.__count = 1;
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
     {
@@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
 	    }
 	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 
+          LIBC_PROBE (mutex_block, 1, mutex);
+
 	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
 	}
       assert (mutex->__data.__owner == 0);
@@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 
@@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  }
 	while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
 
+        LIBC_PROBE (mutex_block, 1, mutex);
+
 	assert (mutex->__data.__owner == 0);
 	mutex->__data.__count = 1;
       }
@@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 #ifndef __pthread_mutex_lock
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index f9fe10b..479e500 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 static int
 internal_function
@@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       /* Unlock.  */
       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return 0;
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
@@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
 			PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return __pthread_tpp_change_priority (oldprio, -1);
 
     default:
@@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
       return EINVAL;
     }
 
+  LIBC_PROBE (mutex_release, 1, mutex);
   return 0;
 }
 
diff --git a/nptl/pthread_rwlock_destroy.c b/nptl/pthread_rwlock_destroy.c
index 28fd24b..84aa693 100644
--- a/nptl/pthread_rwlock_destroy.c
+++ b/nptl/pthread_rwlock_destroy.c
@@ -18,12 +18,15 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 
 
 int
 __pthread_rwlock_destroy (rwlock)
      pthread_rwlock_t *rwlock;
 {
+  LIBC_PROBE (rwlock_destroy, 1, rwlock);
+
   /* Nothing to be done.  For now.  */
   return 0;
 }
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 2feac57..09bad14 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 
 /* Acquire read lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_rdlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (rdlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -49,6 +52,8 @@ __pthread_rwlock_rdlock (rwlock)
 	      --rwlock->__data.__nr_readers;
 	      result = EAGAIN;
 	    }
+          else
+            LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
 
 	  break;
 	}
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
index a7ef71a..a6e8d87 100644
--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -22,11 +22,14 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 /* Unlock RWLOCK.  */
 int
 __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
 {
+  LIBC_PROBE (rwlock_unlock, 1, rwlock);
+
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
   if (rwlock->__data.__writer)
     rwlock->__data.__writer = 0;

 diff --git a/nptl/pthread_rwlock_wrlock.c b/nptl/pthread_rwlock_wrlock.c
index 9d5f135..c099852 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>


 /* Acquire write lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
 {
   int result = 0;

+  LIBC_PROBE (wrlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);

@@ -41,6 +44,8 @@ __pthread_rwlock_wrlock (rwlock)
        {
          /* Mark self as writer.  */
          rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
+
+          LIBC_PROBE (wrlock_acquire_write, 1, rwlock);
          break;
        }


diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
index 3195db2..ea132f5 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
@@ -22,6 +22,8 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#include <stap-probe.h>
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +93,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE (lll_lock_wait_private, 1, %rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
@@ -130,7 +133,8 @@ __lll_lock_wait:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE (lll_lock_wait, 2, %rdi, %rsi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index 9b15bfb..da7b019 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -20,6 +20,8 @@
 #ifndef _LOWLEVELLOCK_H
 #define _LOWLEVELLOCK_H	1
 
+#include <stap-probe.h>
+
 #ifndef __ASSEMBLER__
 # include <time.h>
 # include <sys/param.h>
@@ -227,6 +229,7 @@ LLL_STUB_UNWIND_INFO_END
   do {									      \
     int __ignore;							      \
     register __typeof (nr) _nr __asm ("edx") = (nr);			      \
+    LIBC_PROBE (lll_futex_wake, 3, futex, nr, private);                       \
     __asm __volatile ("syscall"						      \
 		      : "=a" (__ignore)					      \
 		      : "0" (SYS_futex), "D" (futex),			      \
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
index 224a560..6037449 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
@@ -25,7 +25,7 @@
 #include <kernel-features.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -35,6 +35,8 @@
 	.align	16
 __pthread_cond_broadcast:
 
+        LIBC_PROBE (cond_broadcast, 1, %rdi)
+
 	/* Get internal lock.  */
 	movl	$1, %esi
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
index d1d83a8..80a61a4 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
@@ -24,6 +24,7 @@
 #include <pthread-pi-defines.h>
 #include <kernel-features.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 
 	.text
@@ -34,6 +35,8 @@
 	.align	16
 __pthread_cond_signal:
 
+        LIBC_PROBE (cond_signal, 1, %rdi)
+
 	/* Get internal lock.  */
 	movq	%rdi, %r8
 	movl	$1, %esi
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
index e6535fb..dad8ad4 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -41,6 +42,9 @@
 __pthread_cond_timedwait:
 .LSTARTCODE:
 	cfi_startproc
+
+        LIBC_PROBE (cond_timedwait, 3, %rdi, %rsi, %rdx)
+
 #ifdef SHARED
 	cfi_personality(DW_EH_PE_pcrel | DW_EH_PE_sdata4 | DW_EH_PE_indirect,
 			DW.ref.__gcc_personality_v0)
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
index f5b929e..09884bb 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <tcb-offsets.h>
 #include <pthread-pi-defines.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -34,6 +35,9 @@
 	.type	__pthread_cond_wait, @function
 	.align	16
 __pthread_cond_wait:
+
+        LIBC_PROBE (cond_wait, 2, %rdi, %rsi)
+
 .LSTARTCODE:
 	cfi_startproc
 #ifdef SHARED
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
index 35eb09c..1e794a9 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,9 @@
 	.align	16
 __pthread_rwlock_rdlock:
 	cfi_startproc
+
+        LIBC_PROBE (rdlock_entry, 1, %rdi)
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
index be6b8d8..9cc1197 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,9 @@
 	.align	16
 __pthread_rwlock_wrlock:
 	cfi_startproc
+
+        LIBC_PROBE (wrlock_entry, 1, %rdi)
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */

Thanks,
Rayson








> 
> Bert
> 
> > +rdlock_acquire_read - probe for pthread_rwlock_rdlock after successfully getting
> > +                      the lock
> > +                    arg1 = address of rw lock
> > +rwlock_unlock - probe for pthread_rwlock_unlock
> > +              arg1 = address of rw lock
> > +


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-09 18:00                                 ` Bert Wesarg
  2011-02-09 18:31                                   ` Rayson Ho
@ 2011-02-09 18:35                                   ` Rayson Ho
  2011-02-09 18:57                                     ` Bert Wesarg
  2011-02-10 15:27                                     ` Tom Tromey
  1 sibling, 2 replies; 32+ messages in thread
From: Rayson Ho @ 2011-02-09 18:35 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Roland McGrath, libc-alpha, systemtap

(Didn't include the diff for DESIGN-systemtap-probes.txt in the email I
sent 1 min ago, resending...)

On Wed, 2011-02-09 at 19:00 +0100, Bert Wesarg wrote:
> IMHO, this is a typo. Because for 'read' you have
> 'rdlock_acquire_read' and 'rdlock_entry'. So judging from this
> pattern, I would expect, that this probe would be
> 'wrlock_acquire_write', like 'wrlock_entry'. Also note, that in the
> 'acquire' probe names are redundancies for 'read' and 'write' (after
> fixing the typo), while in the 'entry' names not.

Thanks Bert for providing comments since the beginning of the release of
this patch. I think changing wrlock_acquire_write makes sense -
originally I wanted to use "rw" to indicate it's a read-write lock, and
use "wr" to indicate it's "write". But in this case it does not make
things consistent.

However, I think it makes sense to have the 'acquire' probe names - but
if you really think there are redundancies, can you explain it in a bit
more detail??

Revised patch:
nptl/Changlog changes:

2011-02-09  Rayson Ho  <rho@redhat.com>

	* DESIGN-systemtap-probes.txt: New file.
	* pthread_cond_broadcast.c: SystemTap probes.
	* pthread_cond_init.c: Likewise.
	* pthread_cond_signal.c: Likewise.
	* pthread_cond_wait.c: Likewise.
	* pthread_cond_destroy.c: Likewise.
	* pthread_create.c: Likewise.
	* pthread_join.c: Likewise.
	* pthread_mutex_destroy.c: Likewise.
	* pthread_mutex_init.c: Likewise.
	* pthread_mutex_lock.c: Likewise.
	* pthread_mutex_unlock.c: Likewise.
	* pthread_rwlock_destroy.c: Likewise.
	* pthread_rwlock_rdlock.c: Likewise.
	* pthread_rwlock_unlock.c: Likewise.
	* pthread_rwlock_wrlock.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S: Likewise.

diff --git a/nptl/DESIGN-systemtap-probes.txt
b/nptl/DESIGN-systemtap-probes.txt
index e69de29..9aa7c02 100644
--- a/nptl/DESIGN-systemtap-probes.txt
+++ b/nptl/DESIGN-systemtap-probes.txt
@@ -0,0 +1,88 @@
+Systemtap is a dynamic tracing/instrumenting tool available on Linux.
Probes
+that are not fired at run time have close to zero overhead.
+
+The following probes are available for NPTL:
+
+Thread creation & Join Probes
+=============================
+pthread_create - probe for pthread_create
+               arg1 = pointer (pthread_t*) to thread
+               arg2 = pointer (pthread_attr_t*) to attr
+               arg3 = pointer (void *) to start_routine
+               arg4 = arguments to start_routine
+pthread_start - probe for actual thread creation
+              arg1 = struct pthread (members include thread ID, process
ID)
+              arg2 = address of start_routine
+              arg3 = pointer to the list of arguments
+pthread_join - probe for pthread_join
+             arg1 = thread ID
+pthread_join_ret - probe for pthread_join return
+                 arg1 = thread ID
+                 arg2 = return value
+
+Lock-related Probes
+===================
+mutex_init    - probe for pthread_mutex_init
+              arg1 = address of mutex lock
+mutex_acquired - probe for pthread_mutex_lock
+               arg1 = address of mutex lock
+mutex_block   - probe for resume from _possible_ mutex block event
+              arg1 = address of mutex lock
+mutex_entry   - probe for entry to the pthread_mutex_lock function
+              arg1 = address of mutex lock
+mutex_release - probe for pthread_mutex_unlock after the successful
release of a
+                mutex lock
+              arg1 = address of mutex lock
+mutex_destroy - probe for pthread_mutex_destroy
+              arg1 = address of mutex lock
+
+wrlock_entry - probe for entry to the pthread_rwlock_wrlock function
+             arg1 = address of rw lock
+rdlock_entry - probe for entry to the pthread_rwlock_rdlock function
+             arg1 = address of rw lock
+
+rwlock_destroy - probe for pthread_rwlock_destroy
+               arg1 = address of rw lock
+wrlock_acquire_write - probe for pthread_rwlock_wrlock (after getting
the lock)
+                     arg1 = address of rw lock
+rdlock_acquire_read - probe for pthread_rwlock_rdlock after
successfully getting
+                      the lock
+                    arg1 = address of rw lock
+rwlock_unlock - probe for pthread_rwlock_unlock
+              arg1 = address of rw lock
+
+lll_lock_wait - probe in low-level (assembly language) locking code,
only fired
+                when futex/FUTEX_WAIT is called (i.e. when trying to
acquire a
+                contented lock)
+              arg1 = pointer to futex
+              arg2 = flags passed to the futex system call
+lll_lock_wait_private - probe in low-level (assembly language) locking
code,
+                        only fired when futex/FUTEX_WAIT is called
(i.e. when
+                        trying to acquire a contented lock)
+                      arg1 = pointer to futex
+                      
+lll_futex_wake - probe in low-level (assembly language) locking code,
only fired
+                 when futex (FUTEX_WAKE) is called
+               arg1 = pointer to futex
+               arg2 = number of processes to wake
+               arg3 = additional flags
+             
+
+Condition variable Probes
+=========================
+cond_init - probe for pthread_cond_init
+          arg1 = condition
+          arg2 = attr
+cond_destroy - probe for pthread_cond_destroy
+             arg1 = cond
+cond_wait - probe for pthread_cond_wait
+          arg1 = condition
+          arg2 = mutex lock
+cond_timedwait - probe for pthread_cond_timedwait
+               arg1 = condition
+               arg2 = mutex lock
+               arg3 = timespec
+cond_signal - probe for pthread_cond_signal
+            arg1 = condition
+cond_broadcast - probe for pthread_cond_broadcast
+               arg1 = condition
diff --git a/nptl/pthread_cond_broadcast.c
b/nptl/pthread_cond_broadcast.c
index 22523c2..a2e462f 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -23,6 +23,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
@@ -32,6 +33,8 @@ int
 __pthread_cond_broadcast (cond)
      pthread_cond_t *cond;
 {
+  LIBC_PROBE (cond_broadcast, 1, cond);
+
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
   /* Make sure we are alone.  */
diff --git a/nptl/pthread_cond_destroy.c b/nptl/pthread_cond_destroy.c
index 35135a6..a28d061 100644
--- a/nptl/pthread_cond_destroy.c
+++ b/nptl/pthread_cond_destroy.c
@@ -20,6 +20,7 @@
 #include <errno.h>
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 

 int
@@ -29,6 +30,8 @@ __pthread_cond_destroy (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_destroy, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
index 65c01b1..caec6ca 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -20,6 +20,7 @@
 
 #include <shlib-compat.h>
 #include "pthreadP.h"
+#include <stap-probe.h>
 

 int
@@ -42,6 +43,8 @@ __pthread_cond_init (cond, cond_attr)
 			  ? NULL : (void *) ~0l);
   cond->__data.__broadcast_seq = 0;
 
+  LIBC_PROBE (cond_init, 2, cond, cond_attr);
+
   return 0;
 }
 versioned_symbol (libpthread, __pthread_cond_init,
diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
index 023bbb5..414e6bc 100644
--- a/nptl/pthread_cond_signal.c
+++ b/nptl/pthread_cond_signal.c
@@ -26,6 +26,7 @@
 
 #include <shlib-compat.h>
 #include <kernel-features.h>
+#include <stap-probe.h>
 

 int
@@ -35,6 +36,8 @@ __pthread_cond_signal (cond)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
 		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_signal, 1, cond);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 467a03a..3d4c583 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -25,6 +25,7 @@
 #include <pthreadP.h>
 
 #include <shlib-compat.h>
+#include <stap-probe.h>
 

 struct _condvar_cleanup_buffer
@@ -101,6 +102,8 @@ __pthread_cond_wait (cond, mutex)
   int pshared = (cond->__data.__mutex == (void *) ~0l)
   		? LLL_SHARED : LLL_PRIVATE;
 
+  LIBC_PROBE (cond_wait, 2, cond, mutex);
+
   /* Make sure we are alone.  */
   lll_lock (cond->__data.__lock, pshared);
 
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 4075dd9..d3f5f06 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr,
start_routine, arg)
   /* Pass the descriptor to the caller.  */
   *newthread = (pthread_t) pd;
 
+  LIBC_PROBE (pthread_create, 4, newthread, attr, start_routine, arg);
+
   /* Start the thread.  */
   return create_thread (pd, iattr, STACK_VARIABLES_ARGS);
 }
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 6a87a8b..609e2cf 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -23,6 +23,8 @@
 #include <atomic.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 static void
 cleanup (void *arg)
@@ -55,6 +57,8 @@ pthread_join (threadid, thread_return)
   struct pthread *self = THREAD_SELF;
   int result = 0;
 
+  LIBC_PROBE (pthread_join, 1, threadid);
+
   /* During the wait we change to asynchronous cancellation.  If we
      are canceled the thread we are waiting for must be marked as
      un-wait-ed for again.  */
@@ -110,5 +114,7 @@ pthread_join (threadid, thread_return)
       __free_tcb (pd);
     }
 
+  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
+
   return result;
 }
diff --git a/nptl/pthread_mutex_destroy.c b/nptl/pthread_mutex_destroy.c
index e2c9f8a..45c80b8 100644
--- a/nptl/pthread_mutex_destroy.c
+++ b/nptl/pthread_mutex_destroy.c
@@ -20,11 +20,15 @@
 #include <errno.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 
 int
 __pthread_mutex_destroy (mutex)
      pthread_mutex_t *mutex;
 {
+  LIBC_PROBE (mutex_destroy, 1, mutex);
+
   if ((mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP) == 0
       && mutex->__data.__nusers != 0)
     return EBUSY;
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index d9b1ef0..a025a38 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -24,6 +24,8 @@
 #include <kernel-features.h>
 #include "pthreadP.h"
 
+#include <stap-probe.h>
+
 static const struct pthread_mutexattr default_attr =
   {
     /* Default is a normal mutex, not shared between processes.  */
@@ -135,6 +137,8 @@ __pthread_mutex_init (mutex, mutexattr)
   // mutex->__spins = 0;	already done by memset
   // mutex->__next = NULL;	already done by memset
 
+  LIBC_PROBE (mutex_init, 1, mutex);
+
   return 0;
 }
 strong_alias (__pthread_mutex_init, pthread_mutex_init)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 50dc188..010d43e 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -24,6 +24,7 @@
 #include <not-cancel.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 

 #ifndef LLL_MUTEX_LOCK
@@ -48,6 +49,9 @@ __pthread_mutex_lock (mutex)
   assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
 
   unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
+
+  LIBC_PROBE (mutex_entry, 1, mutex);
+
   if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
     return __pthread_mutex_lock_full (mutex);
 
@@ -60,6 +64,8 @@ __pthread_mutex_lock (mutex)
       /* Normal mutex.  */
       LLL_MUTEX_LOCK (mutex);
       assert (mutex->__data.__owner == 0);
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
     {
@@ -83,6 +89,8 @@ __pthread_mutex_lock (mutex)
 
       assert (mutex->__data.__owner == 0);
       mutex->__data.__count = 1;
+
+      LIBC_PROBE (mutex_block, 1, mutex);
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
     {
@@ -108,6 +116,8 @@ __pthread_mutex_lock (mutex)
 	    }
 	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 
+          LIBC_PROBE (mutex_block, 1, mutex);
+
 	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
 	}
       assert (mutex->__data.__owner == 0);
@@ -127,6 +137,8 @@ __pthread_mutex_lock (mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 
@@ -451,6 +463,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  }
 	while ((oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK) != ceilval);
 
+        LIBC_PROBE (mutex_block, 1, mutex);
+
 	assert (mutex->__data.__owner == 0);
 	mutex->__data.__count = 1;
       }
@@ -467,6 +481,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
   ++mutex->__data.__nusers;
 #endif
 
+  LIBC_PROBE (mutex_acquired, 1, mutex);
+
   return 0;
 }
 #ifndef __pthread_mutex_lock
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index f9fe10b..479e500 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include "pthreadP.h"
 #include <lowlevellock.h>
+#include <stap-probe.h>
 
 static int
 internal_function
@@ -50,6 +51,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       /* Unlock.  */
       lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return 0;
     }
   else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
@@ -272,6 +276,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
int decr)
 			PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
+
+      LIBC_PROBE (mutex_release, 1, mutex);
+
       return __pthread_tpp_change_priority (oldprio, -1);
 
     default:
@@ -279,6 +286,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex,
int decr)
       return EINVAL;
     }
 
+  LIBC_PROBE (mutex_release, 1, mutex);
   return 0;
 }
 
diff --git a/nptl/pthread_rwlock_destroy.c
b/nptl/pthread_rwlock_destroy.c
index 28fd24b..84aa693 100644
--- a/nptl/pthread_rwlock_destroy.c
+++ b/nptl/pthread_rwlock_destroy.c
@@ -18,12 +18,15 @@
    02111-1307 USA.  */
 
 #include "pthreadP.h"
+#include <stap-probe.h>
 

 int
 __pthread_rwlock_destroy (rwlock)
      pthread_rwlock_t *rwlock;
 {
+  LIBC_PROBE (rwlock_destroy, 1, rwlock);
+
   /* Nothing to be done.  For now.  */
   return 0;
 }
diff --git a/nptl/pthread_rwlock_rdlock.c b/nptl/pthread_rwlock_rdlock.c
index 2feac57..09bad14 100644
--- a/nptl/pthread_rwlock_rdlock.c
+++ b/nptl/pthread_rwlock_rdlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 

 /* Acquire read lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_rdlock (rwlock)
 {
   int result = 0;
 
+  LIBC_PROBE (rdlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
 
@@ -49,6 +52,8 @@ __pthread_rwlock_rdlock (rwlock)
 	      --rwlock->__data.__nr_readers;
 	      result = EAGAIN;
 	    }
+          else
+            LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
 
 	  break;
 	}
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c
index a7ef71a..a6e8d87 100644
--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -22,11 +22,14 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>
 
 /* Unlock RWLOCK.  */
 int
 __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
 {
+  LIBC_PROBE (rwlock_unlock, 1, rwlock);
+
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);
   if (rwlock->__data.__writer)
     rwlock->__data.__writer = 0;

 diff --git a/nptl/pthread_rwlock_wrlock.c
b/nptl/pthread_rwlock_wrlock.c
index 9d5f135..c099852 100644
--- a/nptl/pthread_rwlock_wrlock.c
+++ b/nptl/pthread_rwlock_wrlock.c
@@ -22,6 +22,7 @@
 #include <lowlevellock.h>
 #include <pthread.h>
 #include <pthreadP.h>
+#include <stap-probe.h>


 /* Acquire write lock for RWLOCK.  */
@@ -31,6 +32,8 @@ __pthread_rwlock_wrlock (rwlock)
 {
   int result = 0;

+  LIBC_PROBE (wrlock_entry, 1, rwlock);
+
   /* Make sure we are alone.  */
   lll_lock (rwlock->__data.__lock, rwlock->__data.__shared);

@@ -41,6 +44,8 @@ __pthread_rwlock_wrlock (rwlock)
        {
          /* Mark self as writer.  */
          rwlock->__data.__writer = THREAD_GETMEM (THREAD_SELF, tid);
+
+          LIBC_PROBE (wrlock_acquire_write, 1, rwlock);
          break;
        }


diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
index 3195db2..ea132f5 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S
@@ -22,6 +22,8 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#include <stap-probe.h>
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +93,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE (lll_lock_wait_private, 1, %rdi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
@@ -130,7 +133,8 @@ __lll_lock_wait:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne	2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE (lll_lock_wait, 2, %rdi, %rsi)
+	movl	$SYS_futex, %eax
 	syscall
 
 2:	movl	%edx, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index 9b15bfb..da7b019 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -20,6 +20,8 @@
 #ifndef _LOWLEVELLOCK_H
 #define _LOWLEVELLOCK_H	1
 
+#include <stap-probe.h>
+
 #ifndef __ASSEMBLER__
 # include <time.h>
 # include <sys/param.h>
@@ -227,6 +229,7 @@ LLL_STUB_UNWIND_INFO_END
   do {									      \
     int __ignore;							      \
     register __typeof (nr) _nr __asm ("edx") = (nr);			      \
+    LIBC_PROBE (lll_futex_wake, 3, futex, nr, private);
\
     __asm __volatile ("syscall"						      \
 		      : "=a" (__ignore)					      \
 		      : "0" (SYS_futex), "D" (futex),			      \
diff --git
a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
index 224a560..6037449 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S
@@ -25,7 +25,7 @@
 #include <kernel-features.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -35,6 +35,8 @@
 	.align	16
 __pthread_cond_broadcast:
 
+        LIBC_PROBE (cond_broadcast, 1, %rdi)
+
 	/* Get internal lock.  */
 	movl	$1, %esi
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
index d1d83a8..80a61a4 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S
@@ -24,6 +24,7 @@
 #include <pthread-pi-defines.h>
 #include <kernel-features.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 

 	.text
@@ -34,6 +35,8 @@
 	.align	16
 __pthread_cond_signal:
 
+        LIBC_PROBE (cond_signal, 1, %rdi)
+
 	/* Get internal lock.  */
 	movq	%rdi, %r8
 	movl	$1, %esi
diff --git
a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
index e6535fb..dad8ad4 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -41,6 +42,9 @@
 __pthread_cond_timedwait:
 .LSTARTCODE:
 	cfi_startproc
+
+        LIBC_PROBE (cond_timedwait, 3, %rdi, %rsi, %rdx)
+
 #ifdef SHARED
 	cfi_personality(DW_EH_PE_pcrel | DW_EH_PE_sdata4 | DW_EH_PE_indirect,
 			DW.ref.__gcc_personality_v0)
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
index f5b929e..09884bb 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
@@ -23,6 +23,7 @@
 #include <lowlevelcond.h>
 #include <tcb-offsets.h>
 #include <pthread-pi-defines.h>
+#include <stap-probe.h>
 
 #include <kernel-features.h>
 
@@ -34,6 +35,9 @@
 	.type	__pthread_cond_wait, @function
 	.align	16
 __pthread_cond_wait:
+
+        LIBC_PROBE (cond_wait, 2, %rdi, %rsi)
+
 .LSTARTCODE:
 	cfi_startproc
 #ifdef SHARED
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
index 35eb09c..1e794a9 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_rdlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,9 @@
 	.align	16
 __pthread_rwlock_rdlock:
 	cfi_startproc
+
+        LIBC_PROBE (rdlock_entry, 1, %rdi)
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
index be6b8d8..9cc1197 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S
@@ -22,7 +22,7 @@
 #include <lowlevelrwlock.h>
 #include <pthread-errnos.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -31,6 +31,9 @@
 	.align	16
 __pthread_rwlock_wrlock:
 	cfi_startproc
+
+        LIBC_PROBE (wrlock_entry, 1, %rdi)
+
 	xorq	%r10, %r10
 
 	/* Get the lock.  */

Thanks,
Rayson








> 
> Bert
> 
> > +rdlock_acquire_read - probe for pthread_rwlock_rdlock after
successfully getting
> > +                      the lock
> > +                    arg1 = address of rw lock
> > +rwlock_unlock - probe for pthread_rwlock_unlock
> > +              arg1 = address of rw lock
> > +



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-09 18:35                                   ` Rayson Ho
@ 2011-02-09 18:57                                     ` Bert Wesarg
  2011-02-10 15:27                                     ` Tom Tromey
  1 sibling, 0 replies; 32+ messages in thread
From: Bert Wesarg @ 2011-02-09 18:57 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Roland McGrath, libc-alpha, systemtap

On Wed, Feb 9, 2011 at 19:34, Rayson Ho <rho@redhat.com> wrote:
> (Didn't include the diff for DESIGN-systemtap-probes.txt in the email I
> sent 1 min ago, resending...)
>
> On Wed, 2011-02-09 at 19:00 +0100, Bert Wesarg wrote:
>> IMHO, this is a typo. Because for 'read' you have
>> 'rdlock_acquire_read' and 'rdlock_entry'. So judging from this
>> pattern, I would expect, that this probe would be
>> 'wrlock_acquire_write', like 'wrlock_entry'. Also note, that in the
>> 'acquire' probe names are redundancies for 'read' and 'write' (after
>> fixing the typo), while in the 'entry' names not.
>
> Thanks Bert for providing comments since the beginning of the release of
> this patch. I think changing wrlock_acquire_write makes sense -
> originally I wanted to use "rw" to indicate it's a read-write lock, and
> use "wr" to indicate it's "write". But in this case it does not make
> things consistent.
>
> However, I think it makes sense to have the 'acquire' probe names - but
> if you really think there are redundancies, can you explain it in a bit
> more detail??

Sure. You have now 'wrlock_acquire_write'. The prefix 'wr' tells the
reader that its about a write lock, so the suffix 'write' is
redundant, isn't ist? You haven't this redundancy in the
'wrlock_entry' case, else it would have the name 'wrlock_entry_write'.

Bert

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-09 18:31                                   ` Rayson Ho
@ 2011-02-09 19:08                                     ` Roland McGrath
  0 siblings, 0 replies; 32+ messages in thread
From: Roland McGrath @ 2011-02-09 19:08 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Bert Wesarg, libc-alpha, systemtap

This patch omits the new DESIGN-systemtap-probes.txt file.
It also fails to apply because it got detabified or something.
Why don't you just use git format-patch to make a submission that
includes the ChangeLog diff and send that to me privately?
I'll apply it to my branch now.

Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-09 18:35                                   ` Rayson Ho
  2011-02-09 18:57                                     ` Bert Wesarg
@ 2011-02-10 15:27                                     ` Tom Tromey
  2011-02-10 15:36                                       ` Tom Tromey
  1 sibling, 1 reply; 32+ messages in thread
From: Tom Tromey @ 2011-02-10 15:27 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Bert Wesarg, Roland McGrath, libc-alpha, systemtap

>>>>> "Rayson" == Rayson Ho <rho@redhat.com> writes:

Rayson> +mutex_acquired - probe for pthread_mutex_lock
Rayson> +               arg1 = address of mutex lock
Rayson> +mutex_block   - probe for resume from _possible_ mutex block event
Rayson> +              arg1 = address of mutex lock
Rayson> +mutex_entry   - probe for entry to the pthread_mutex_lock function
Rayson> +              arg1 = address of mutex lock

mutex_block seems misnamed to me.  I would suggest something like
mutex_unblock or mutex_unblocked instead.

Also, mutex_block and mutex_acquired seem fairly close in semantics --
almost redundant.  So another approach would be to just drop
mutex_block.

The piece that is missing for my scenario is a probe that fires just
before glibc blocks on a mutex.  I couldn't think of a way to get this
information with the current set of probes.

The rationale for such a probe is to make it possible to implement a
deadlock detector in gdb.  Maybe this could be done with the current
probes, assuming that debuginfo is available.  But it seems like it
would be simpler with an "about to block" probe.

Tom

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)
  2011-02-10 15:27                                     ` Tom Tromey
@ 2011-02-10 15:36                                       ` Tom Tromey
  2011-02-16 18:27                                         ` RFC: i386 port of the pthread library probes Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Tom Tromey @ 2011-02-10 15:36 UTC (permalink / raw)
  To: Rayson Ho; +Cc: Bert Wesarg, Roland McGrath, libc-alpha, systemtap

Tom> The piece that is missing for my scenario is a probe that fires just
Tom> before glibc blocks on a mutex.  I couldn't think of a way to get this
Tom> information with the current set of probes.

Also, I noticed that the patch does not modify pthread_mutex_timedlock.c.
I think this means that some mutex acquisitions will go unnoticed.

Tom

^ permalink raw reply	[flat|nested] 32+ messages in thread

* RFC: i386 port of the pthread library probes
  2011-02-10 15:36                                       ` Tom Tromey
@ 2011-02-16 18:27                                         ` Rayson Ho
  2011-02-16 18:57                                           ` Roland McGrath
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-02-16 18:27 UTC (permalink / raw)
  To: libc-alpha; +Cc: systemtap

The original patch was developed on x64 and thus the low-level probes
were missing on other architectures -- this is the patch for the machine
dependent part for 32-bit x86. Please note that while every file changed
by this patch compiles successfully on my machine (and I checked that
the probes were there with stap -L), I did not get the full glibc tested
because other parts not related to the systemtap probes failed to cross
compile.

However, before I fully test the 32-bit code, I just want to see if
others are OK with the i386 changes, which I think are quite straight
forward as they are functionally similar to the x64 counterparts (the
function argument passing convention is totally different in i386 but
that's another story...)

========================================================

diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/lowlevellock.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/lowlevellock.S
index 2198ccf..92437f0 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/lowlevellock.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/lowlevellock.S
@@ -22,6 +22,8 @@
 #include <kernel-features.h>
 #include <lowlevellock.h>
 
+#include <stap-probe.h>
+
 	.text
 
 #ifdef __ASSUME_PRIVATE_FUTEX
@@ -91,7 +93,8 @@ __lll_lock_wait_private:
 	cmpl	%edx, %eax	/* NB:	 %edx == 2 */
 	jne 2f
 
-1:	movl	$SYS_futex, %eax
+1:	LIBC_PROBE (lll_lock_wait_private, 1, %ebx)
+	movl	$SYS_futex, %eax
 	ENTER_KERNEL
 
 2:	movl	%edx, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_broadcast.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_broadcast.S
index a7ca78f..a0e2944 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_broadcast.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_broadcast.S
@@ -24,6 +24,7 @@
 #include <kernel-features.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
+#include <stap-probe.h>
 
 	.text
 
@@ -49,6 +50,8 @@ __pthread_cond_broadcast:
 
 	movl	20(%esp), %ebx
 
+	LIBC_PROBE (cond_broadcast, 1, %edx) 
+
 	/* Get internal lock.  */
 	movl	$1, %edx
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_signal.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_signal.S
index 05cda25..96b5928 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_signal.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_signal.S
@@ -24,7 +24,7 @@
 #include <kernel-features.h>
 #include <pthread-pi-defines.h>
 #include <pthread-errnos.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -45,6 +45,8 @@ __pthread_cond_signal:
 
 	movl	12(%esp), %edi
 
+	LIBC_PROBE (cond_signal, 1, %edi)
+
 	/* Get internal lock.  */
 	movl	$1, %edx
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
index dee73f0..3762abc 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
@@ -24,7 +24,7 @@
 #include <pthread-errnos.h>
 #include <pthread-pi-defines.h>
 #include <kernel-features.h>
-
+#include <stap-probe.h>
 
 	.text
 
@@ -61,6 +61,8 @@ __pthread_cond_timedwait:
 	movl	20(%esp), %ebx
 	movl	28(%esp), %ebp
 
+	LIBC_PROBE (cond_timedwait, 3, %ebx, 24(%esp), %ebp)
+
 	cmpl	$1000000000, 4(%ebp)
 	movl	$EINVAL, %eax
 	jae	18f
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
index 53970d7..2185cf6 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
@@ -25,6 +25,7 @@
 #include <pthread-errnos.h>
 #include <pthread-pi-defines.h>
 #include <kernel-features.h>
+#include <stap-probe.h>
 
 
 	.text
@@ -61,6 +62,8 @@ __pthread_cond_wait:
 	xorl	%esi, %esi
 	movl	20(%esp), %ebx
 
+	LIBC_PROBE (cond_wait, 2, 24(%esp), %ebx)
+
 	/* Get internal lock.  */
 	movl	$1, %edx
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_rdlock.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_rdlock.S
index 4e5f0c5..050266e 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_rdlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_rdlock.S
@@ -23,6 +23,7 @@
 #include <pthread-errnos.h>
 #include <kernel-features.h>
 
+#include <stap-probe.h>
 
 	.text
 
@@ -41,6 +42,8 @@ __pthread_rwlock_rdlock:
 	xorl	%esi, %esi
 	movl	12(%esp), %ebx
 
+	LIBC_PROBE (rdlock_entry, 1, %ebx)
+
 	/* Get the lock.  */
 	movl	$1, %edx
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_wrlock.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_wrlock.S
index 1007364..3ff0ac3 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_wrlock.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_wrlock.S
@@ -23,6 +23,7 @@
 #include <pthread-errnos.h>
 #include <kernel-features.h>
 
+#include <stap-probe.h>
 
 	.text
 
@@ -41,6 +42,8 @@ __pthread_rwlock_wrlock:
 	xorl	%esi, %esi
 	movl	12(%esp), %ebx
 
+	LIBC_PROBE (wrlock_entry, 1, %ebx)
+
 	/* Get the lock.  */
 	movl	$1, %edx
 	xorl	%eax, %eax
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/i386/lowlevellock.h
index 4bb585a..34e28bd 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/lowlevellock.h
+++ b/nptl/sysdeps/unix/sysv/linux/i386/lowlevellock.h
@@ -20,6 +20,8 @@
 #ifndef _LOWLEVELLOCK_H
 #define _LOWLEVELLOCK_H	1
 
+#include <stap-probe.h>
+
 #ifndef __ASSEMBLER__
 # include <time.h>
 # include <sys/param.h>
@@ -226,6 +228,7 @@ LLL_STUB_UNWIND_INFO_END
   do {									      \
     int __ignore;							      \
     register __typeof (nr) _nr asm ("edx") = (nr);			      \
+    LIBC_PROBE (lll_futex_wake, 3, futex, nr, private);                       \
     __asm __volatile (LLL_EBX_LOAD					      \
 		      LLL_ENTER_KERNEL					      \
 		      LLL_EBX_LOAD					      \

========================================================

Also, thanks Tom for the comment, I've updated the patch I just sent to
Roland - I removed mutex_block totally as it was there before the
low-level probes (the asm ones) were added, and then I recycled the name
for the release probe. For pthread_mutex_timedlock(), I added a few
simple probes:

========================================================

diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index 8d0db79..55d1802 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -24,6 +24,8 @@
 #include <lowlevellock.h>
 #include <not-cancel.h>
 
+#include <stap-probe.h>
+
 
 int
 pthread_mutex_timedlock (mutex, abstime)
@@ -34,6 +36,8 @@ pthread_mutex_timedlock (mutex, abstime)
   pid_t id = THREAD_GETMEM (THREAD_SELF, tid);
   int result = 0;
 
+  LIBC_PROBE (mutex_timedlock_entry, 2, mutex, abstime);
+
   /* We must not check ABSTIME here.  If the thread does not block
      abstime must not be checked for a valid value.  */
 
@@ -172,6 +176,8 @@ pthread_mutex_timedlock (mutex, abstime)
 
 		  ++mutex->__data.__count;
 
+                  LIBC_PROBE (mutex_timedlock_acquired, 1, mutex);
+
 		  return 0;
 		}
 	    }
@@ -242,6 +248,8 @@ pthread_mutex_timedlock (mutex, abstime)
 
 		++mutex->__data.__count;
 
+                LIBC_PROBE (mutex_timedlock_acquired, 1, mutex);
+
 		return 0;
 	      }
 	  }
@@ -377,6 +385,8 @@ pthread_mutex_timedlock (mutex, abstime)
 
 		++mutex->__data.__count;
 
+                LIBC_PROBE (mutex_timedlock_acquired, 1, mutex);
+
 		return 0;
 	      }
 	  }
@@ -477,6 +487,8 @@ pthread_mutex_timedlock (mutex, abstime)
       /* Record the ownership.  */
       mutex->__data.__owner = id;
       ++mutex->__data.__nusers;
+
+      LIBC_PROBE (mutex_timedlock_acquired, 1, mutex);
     }
 
  out:

========================================================

Thanks
Rayson


On Thu, 2011-02-10 at 08:36 -0700, Tom Tromey wrote:
> Tom> The piece that is missing for my scenario is a probe that fires just
> Tom> before glibc blocks on a mutex.  I couldn't think of a way to get this
> Tom> information with the current set of probes.
> 
> Also, I noticed that the patch does not modify pthread_mutex_timedlock.c.
> I think this means that some mutex acquisitions will go unnoticed.
> 
> Tom


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: RFC: i386 port of the pthread library probes
  2011-02-16 18:27                                         ` RFC: i386 port of the pthread library probes Rayson Ho
@ 2011-02-16 18:57                                           ` Roland McGrath
  2011-02-16 19:05                                             ` Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Roland McGrath @ 2011-02-16 18:57 UTC (permalink / raw)
  To: Rayson Ho; +Cc: libc-alpha, systemtap

The i386 code looks fine to me on its face.  I didn't check all the
assembly code to see if the probe parameters are the right things.
But that can be tested later by writing a stap test suite that makes
use of all the probe arguments and verifies their expected values.  I
also didn't check if you got every .S and .h file that you should, but
you can double-check that.  Note there are i486, i586, and i686
variants of several such files.  If you send me that patch on its own
with ChangeLog entries, I will merge that when I merge the x86_64 code.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: RFC: i386 port of the pthread library probes
  2011-02-16 18:57                                           ` Roland McGrath
@ 2011-02-16 19:05                                             ` Rayson Ho
  2011-02-22  7:24                                               ` Rayson Ho
  0 siblings, 1 reply; 32+ messages in thread
From: Rayson Ho @ 2011-02-16 19:05 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-alpha, systemtap

On Wed, 2011-02-16 at 10:57 -0800, Roland McGrath wrote:
> The i386 code looks fine to me on its face.  I didn't check all the
> assembly code to see if the probe parameters are the right things.
> But that can be tested later by writing a stap test suite that makes
> use of all the probe arguments and verifies their expected values.  I
> also didn't check if you got every .S and .h file that you should, but
> you can double-check that.  Note there are i486, i586, and i686
> variants of several such files.  If you send me that patch on its own
> with ChangeLog entries, I will merge that when I merge the x86_64 code.

Thanks Roland for the quick response!

(I need to double check) When I was adding the probes for i386 I looked
at the organizational structure -- the i686 variant for example just
includes the i486 version.

I will check that and also will add a changelog and send the patch to
the list again.

Rayson




> 
> 
> Thanks,
> Roland


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: RFC: i386 port of the pthread library probes
  2011-02-16 19:05                                             ` Rayson Ho
@ 2011-02-22  7:24                                               ` Rayson Ho
  0 siblings, 0 replies; 32+ messages in thread
From: Rayson Ho @ 2011-02-22  7:24 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-alpha, systemtap

On Wed, 2011-02-16 at 14:04 -0500, Rayson Ho wrote:
> Thanks Roland for the quick response!
> 
> (I need to double check) When I was adding the probes for i386 I looked
> at the organizational structure -- the i686 variant for example just
> includes the i486 version.
> 
> I will check that and also will add a changelog and send the patch to
> the list again.

Hi,

I verified that there is only 1 set of .S files for i386 in the nptl
code that has the SystemTap probes - like I mentioned above, the i686
variant just uses a #include to pick up the i486 version.

Last week, I finally found a 32-bit machine and successfully compiled
and ran a few tests, at least no regression in terms of compilation or
linking was introduced by this patch -- I will provide its own test
suite that covers all the probes like you mentioned in another patch
later.

So the ChnageLog entries are:

2011-02-22  Rayson Ho  <rho@redhat.com>

	* sysdeps/unix/sysv/linux/i386/lowlevellock.h: Low-level SystemTap
	probes for i386.
	* sysdeps/unix/sysv/linux/i386/i486/lowlevellock.S: Likewise.
	* sysdeps/unix/sysv/linux/i386/i486/lowlevellock.S: Likewise.
	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_broadcast.S: Likewise.
	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_signal.S: Likewise.
	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S: Likewise.
	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S: Likewise.
	* sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_rdlock.S: Likewise.
	* sysdeps/unix/sysv/linux/i386/i486/pthread_rwlock_wrlock.S: Likewise.

Thanks,
Rayson



> 
> Rayson
> 
> 
> 
> 
> > 
> > 
> > Thanks,
> > Roland
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2011-02-22  7:24 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-12 17:54 [PATCH] Adding systemtap probe points in pthread library (slightly revised again) Rayson Ho
2011-01-12 18:18 ` Roland McGrath
2011-01-12 18:50   ` Rayson Ho
2011-01-12 20:07     ` Bert Wesarg
2011-01-19 18:03       ` Rayson Ho
2011-01-19 18:36         ` Roland McGrath
2011-01-19 18:46           ` Rayson Ho
2011-01-19 18:56             ` Roland McGrath
2011-01-26 18:24               ` Rayson Ho
2011-01-26 18:51                 ` Bert Wesarg
2011-01-26 18:54                 ` Roland McGrath
2011-01-27  9:11                   ` Rayson Ho
2011-01-28  7:20                     ` Roland McGrath
2011-02-02  8:12                       ` Rayson Ho
2011-02-04 19:04                         ` Roland McGrath
2011-02-08 22:18                           ` Rayson Ho
2011-02-08 22:48                             ` Roland McGrath
2011-02-09 17:09                               ` Rayson Ho
2011-02-09 18:00                                 ` Bert Wesarg
2011-02-09 18:31                                   ` Rayson Ho
2011-02-09 19:08                                     ` Roland McGrath
2011-02-09 18:35                                   ` Rayson Ho
2011-02-09 18:57                                     ` Bert Wesarg
2011-02-10 15:27                                     ` Tom Tromey
2011-02-10 15:36                                       ` Tom Tromey
2011-02-16 18:27                                         ` RFC: i386 port of the pthread library probes Rayson Ho
2011-02-16 18:57                                           ` Roland McGrath
2011-02-16 19:05                                             ` Rayson Ho
2011-02-22  7:24                                               ` Rayson Ho
2011-01-19 18:38         ` [PATCH] Adding systemtap probe points in pthread library (slightly revised again) Bert Wesarg
2011-01-19 18:54           ` Roland McGrath
2011-01-19 19:04             ` Bert Wesarg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).