Repopulate symbol/type info.

6 files changed, 154 insertions, 129 deletions

diff --git a/ChangeLog b/ChangeLog
index 6c130644..d151735f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,14 @@
 
 	* elaborate.cxx (add_global_var_display): Repopulate symbol/type info.
 
+2008-08-29  David Smith  <dsmith@redhat.com>
+
+	* tapsets.cxx (utrace_derived_probe_group::emit_probe_decl):
+	Pushed quiesce logic down into the task_finder layer.
+	(utrace_derived_probe_group::emit_module_decls): Removed quiesce
+	handler routines.  Syscall probe handler detaches if systemtap
+	state isn't correct.
+
 2008-08-29  Wenji Huang <wenji.huang@oracle.com>
 
 	* main.cxx (printscript): Print variable name and type for listing mode.
diff --git a/runtime/ChangeLog b/runtime/ChangeLog
index 926e05d9..b7e44a0f 100644
--- a/runtime/ChangeLog
+++ b/runtime/ChangeLog
@@ -1,3 +1,11 @@
+2008-08-29  David Smith  <dsmith@redhat.com>
+
+	* task_finder.c (__stp_utrace_attach_match_filename): Don't call
+	the callback when the interesting thread is found, call it when
+	the tread is quiesced.
+	(stap_start_task_finder): Ditto.
+	(__stp_utrace_task_finder_target_quiesce): Call the callback.
+
 2008-08-26  David Smith  <dsmith@redhat.com>
 
 	* autoconf-d_path-path.c: New file.
diff --git a/runtime/task_finder.c b/runtime/task_finder.c
index af2345be..f1c2c41b 100644
--- a/runtime/task_finder.c
+++ b/runtime/task_finder.c
@@ -341,23 +341,41 @@ __stp_get_mm_path(struct mm_struct *mm, char *buf, int buflen)
 	return rc;
 }
 
+/*
+ * All user threads get an engine with __STP_TASK_FINDER_EVENTS events
+ * attached to it so the task_finder layer can monitor new thread
+ * creation/death.
+ */
 #define __STP_TASK_FINDER_EVENTS (UTRACE_EVENT(CLONE)		\
 				  | UTRACE_EVENT(EXEC)		\
 				  | UTRACE_EVENT(DEATH))
 
-#define __STP_ATTACHED_TASK_BASE_EVENTS (UTRACE_EVENT(DEATH))
-
-#define __STP_ATTACHED_TASK_VM_BASE_EVENTS (__STP_ATTACHED_TASK_BASE_EVENTS \
-					    | UTRACE_EVENT(SYSCALL_ENTRY) \
-					    | UTRACE_EVENT(SYSCALL_EXIT))
-
-#define __STP_ATTACHED_TASK_VM_EVENTS (__STP_ATTACHED_TASK_BASE_EVENTS	\
-				       | UTRACE_STOP			\
-				       | UTRACE_EVENT(QUIESCE))
-
-#define __STP_ATTACHED_TASK_EVENTS(tgt) \
-	((((tgt)->vm_callback) == NULL) ? __STP_ATTACHED_TASK_BASE_EVENTS \
-	 : __STP_ATTACHED_TASK_VM_EVENTS)
+/* 
+ * __STP_TASK_BASE_EVENTS: base events for stap_task_finder_target's
+ * without a vm_callback
+ *
+ * __STP_TASK_VM_BASE_EVENTS: base events for
+ * stap_task_finder_target's with a vm_callback
+ */
+#define __STP_TASK_BASE_EVENTS	(UTRACE_EVENT(DEATH))
+
+#define __STP_TASK_VM_BASE_EVENTS (__STP_TASK_BASE_EVENTS	\
+				   | UTRACE_EVENT(SYSCALL_ENTRY)\
+				   | UTRACE_EVENT(SYSCALL_EXIT))
+
+/*
+ * All "interesting" threads get an engine with
+ * __STP_ATTACHED_TASK_EVENTS events attached to it.  After the thread
+ * quiesces, we reset the events to __STP_ATTACHED_TASK_BASE_EVENTS
+ * events.
+ */
+#define __STP_ATTACHED_TASK_EVENTS (__STP_TASK_BASE_EVENTS	\
+				    | UTRACE_STOP		\
+				    | UTRACE_EVENT(QUIESCE))
+
+#define __STP_ATTACHED_TASK_BASE_EVENTS(tgt) \
+	((((tgt)->vm_callback) == NULL) ? __STP_TASK_BASE_EVENTS \
+	 : __STP_TASK_VM_BASE_EVENTS)
 
 static int
 stap_utrace_attach(struct task_struct *tsk,
@@ -444,31 +462,40 @@ __stp_utrace_attach_match_filename(struct task_struct *tsk,
 			if (cb_tgt == NULL)
 				continue;
 
-			if (cb_tgt->callback != NULL) {
-				int rc = cb_tgt->callback(cb_tgt, tsk,
-							  register_p,
-							  process_p);
-				if (rc != 0) {
-					_stp_error("callback for %d failed: %d",
-						   (int)tsk->pid, rc);
-					break;
-				}
-			}
-
 			// Set up events we need for attached tasks.
+			// When register_p is set, we won't actually
+			// call the callback here - we'll call it when
+			// the thread gets quiesced.  When register_p
+			// isn't set, we can go ahead and call the
+			// callback.
 			if (register_p) {
 				rc = stap_utrace_attach(tsk, &cb_tgt->ops,
 							cb_tgt,
-							__STP_ATTACHED_TASK_EVENTS(cb_tgt));
+							__STP_ATTACHED_TASK_EVENTS);
 				if (rc != 0 && rc != EPERM)
 					break;
 				cb_tgt->engine_attached = 1;
 			}
 			else {
+				if (cb_tgt->callback != NULL) {
+					rc = cb_tgt->callback(cb_tgt, tsk,
+							      register_p,
+							      process_p);
+					if (rc != 0) {
+						_stp_error("callback for %d failed: %d",
+							   (int)tsk->pid, rc);
+						break;
+					}
+				}
+
 				rc = stap_utrace_detach(tsk, &cb_tgt->ops);
 				if (rc != 0)
 					break;
-				cb_tgt->engine_attached = 0;
+
+				// Note that we don't want to set
+				// engine_attached to 0 here - only
+				// when *all* threads using this
+				// engine have been detached.
 			}
 		}
 	}
@@ -685,18 +712,32 @@ __stp_utrace_task_finder_target_quiesce(enum utrace_resume_action action,
 	struct stap_task_finder_target *tgt = engine->data;
 	int rc;
 
-	// Turn off quiesce handling (and turn on syscall handling).
-	rc = utrace_set_events(tsk, engine, __STP_ATTACHED_TASK_VM_BASE_EVENTS);
+	if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING) {
+		debug_task_finder_detach();
+		return UTRACE_DETACH;
+	}
+
+	if (tgt == NULL)
+		return UTRACE_DETACH;
+
+	// Turn off quiesce handling
+	rc = utrace_set_events(tsk, engine,
+			       __STP_ATTACHED_TASK_BASE_EVENTS(tgt));
 	if (rc != 0)
 		_stp_error("utrace_set_events returned error %d on pid %d",
 			   rc, (int)tsk->pid);
 
-	if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING) {
-		debug_task_finder_detach();
-		return UTRACE_DETACH;
+	if (tgt->callback != NULL) {
+		/* Call the callback.  Assume that if the thread is a
+		 * thread group leader, it is a process. */
+		rc = tgt->callback(tgt, tsk, 1, (tsk->pid == tsk->tgid));
+		if (rc != 0) {
+			_stp_error("callback for %d failed: %d",
+				   (int)tsk->pid, rc);
+		}
 	}
 
-	if (tgt != NULL && tgt->vm_callback != NULL) {
+	if (tgt->vm_callback != NULL) {
 		struct mm_struct *mm;
 		char *mmpath_buf;
 		char *mmpath;
@@ -1148,24 +1189,11 @@ stap_start_task_finder(void)
 						    callback_list);
 				if (cb_tgt == NULL)
 					continue;
-					
-				// Call the callback.  Assume that if
-				// the thread is a thread group
-				// leader, it is a process.
-				if (cb_tgt->callback != NULL) {
-					rc = cb_tgt->callback(cb_tgt, tsk, 1,
-							      (tsk->pid == tsk->tgid));
-					if (rc != 0) {
-						_stp_error("attach callback for %d failed: %d",
-							   (int)tsk->pid, rc);
-						goto stf_err;
-					}
-				}
 
 				// Set up events we need for attached tasks.
 				rc = stap_utrace_attach(tsk, &cb_tgt->ops,
 							cb_tgt,
-							__STP_ATTACHED_TASK_EVENTS(cb_tgt));
+							__STP_ATTACHED_TASK_EVENTS);
 				if (rc != 0 && rc != EPERM)
 					goto stf_err;
 				cb_tgt->engine_attached = 1;
diff --git a/tapsets.cxx b/tapsets.cxx
index 289c5fac..aa6a611d 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -6073,17 +6073,13 @@ utrace_derived_probe_group::emit_probe_decl (systemtap_session& s,
   // Handle flags
   switch (p->flags)
     {
-    // Look in _stp_utrace_probe_cb for description of why quiesce is
-    // used here.
+    // Notice that we'll just call the probe directly when we get
+    // notified, since the task_finder layer stops the thread for us.
     case UDPF_BEGIN:				// process begin
       s.op->line() << " .flags=(UDPF_BEGIN),";
-      s.op->line() << " .ops={ .report_quiesce=stap_utrace_probe_quiesce },";
-      s.op->line() << " .events=(UTRACE_STOP|UTRACE_EVENT(QUIESCE)),";
       break;
     case UDPF_THREAD_BEGIN:			// thread begin
       s.op->line() << " .flags=(UDPF_THREAD_BEGIN),";
-      s.op->line() << " .ops={ .report_quiesce=stap_utrace_probe_quiesce },";
-      s.op->line() << " .events=(UTRACE_STOP|UTRACE_EVENT(QUIESCE)),";
       break;
 
     // Notice we're not setting up a .ops/.report_death handler for
@@ -6098,17 +6094,17 @@ utrace_derived_probe_group::emit_probe_decl (systemtap_session& s,
 
     // For UDPF_SYSCALL/UDPF_SYSCALL_RETURN probes, the .report_death
     // handler isn't strictly necessary.  However, it helps to keep
-    // our attaches/detaches symmetrical.  Notice we're using quiesce
-    // as a workaround for bug 6841.
+    // our attaches/detaches symmetrical.  Since the task_finder layer
+    // stops the thread, that works around bug 6841.
     case UDPF_SYSCALL:
       s.op->line() << " .flags=(UDPF_SYSCALL),";
-      s.op->line() << " .ops={ .report_syscall_entry=stap_utrace_probe_syscall,  .report_death=stap_utrace_task_finder_report_death, .report_quiesce=stap_utrace_probe_syscall_quiesce },";
-      s.op->line() << " .events=(UTRACE_STOP|UTRACE_EVENT(QUIESCE)|UTRACE_EVENT(DEATH)),";
+      s.op->line() << " .ops={ .report_syscall_entry=stap_utrace_probe_syscall,  .report_death=stap_utrace_task_finder_report_death },";
+      s.op->line() << " .events=(UTRACE_EVENT(SYSCALL_ENTRY)|UTRACE_EVENT(DEATH)),";
       break;
     case UDPF_SYSCALL_RETURN:
       s.op->line() << " .flags=(UDPF_SYSCALL_RETURN),";
-      s.op->line() << " .ops={ .report_syscall_exit=stap_utrace_probe_syscall, .report_death=stap_utrace_task_finder_report_death, .report_quiesce=stap_utrace_probe_syscall_quiesce },";
-      s.op->line() << " .events=(UTRACE_STOP|UTRACE_EVENT(QUIESCE)|UTRACE_EVENT(DEATH)),";
+      s.op->line() << " .ops={ .report_syscall_exit=stap_utrace_probe_syscall, .report_death=stap_utrace_task_finder_report_death },";
+      s.op->line() << " .events=(UTRACE_EVENT(SYSCALL_EXIT)|UTRACE_EVENT(DEATH)),";
       break;
 
     case UDPF_NONE:
@@ -6191,32 +6187,10 @@ utrace_derived_probe_group::emit_module_decls (systemtap_session& s)
   s.op->newline(-1) << "};";
 
 
-  // Output handler function for UDPF_BEGIN and UDPF_THREAD_BEGIN
-  if (flags_seen[UDPF_BEGIN] || flags_seen[UDPF_THREAD_BEGIN])
-    {
-      s.op->newline() << "#ifdef UTRACE_ORIG_VERSION";
-      s.op->newline() << "static u32 stap_utrace_probe_quiesce(struct utrace_attached_engine *engine, struct task_struct *tsk) {";
-      s.op->newline() << "#else";
-      s.op->newline() << "static u32 stap_utrace_probe_quiesce(enum utrace_resume_action action, struct utrace_attached_engine *engine, struct task_struct *tsk, unsigned long event) {";
-      s.op->newline() << "#endif";
-      s.op->indent(1);
-      s.op->newline() << "struct stap_utrace_probe *p = (struct stap_utrace_probe *)engine->data;";
-
-      common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING");
-      s.op->newline() << "c->probe_point = p->pp;";
-
-      // call probe function
-      s.op->newline() << "(*p->ph) (c);";
-      common_probe_entryfn_epilogue (s.op);
-
-      // we're detaching, so utrace automatically restarts the thread.
-      s.op->newline() << "debug_task_finder_detach();";
-      s.op->newline() << "return UTRACE_DETACH;";
-      s.op->newline(-1) << "}";
-    }
-
-  // Output handler function for UDPF_END and UDPF_THREAD_END
-  if (flags_seen[UDPF_END] || flags_seen[UDPF_THREAD_END])
+  // Output handler function for UDPF_BEGIN, UDPF_THREAD_BEGIN,
+  // UDPF_END, and UDPF_THREAD_END
+  if (flags_seen[UDPF_BEGIN] || flags_seen[UDPF_THREAD_BEGIN]
+      || flags_seen[UDPF_END] || flags_seen[UDPF_THREAD_END])
     {
       s.op->newline() << "static void stap_utrace_probe_handler(struct task_struct *tsk, struct stap_utrace_probe *p) {";
       s.op->indent(1);
@@ -6236,33 +6210,6 @@ utrace_derived_probe_group::emit_module_decls (systemtap_session& s)
   if (flags_seen[UDPF_SYSCALL] || flags_seen[UDPF_SYSCALL_RETURN])
     {
       s.op->newline() << "#ifdef UTRACE_ORIG_VERSION";
-      s.op->newline() << "static u32 stap_utrace_probe_syscall_quiesce(struct utrace_attached_engine *engine, struct task_struct *tsk) {";
-      s.op->newline() << "#else";
-      s.op->newline() << "static u32 stap_utrace_probe_syscall_quiesce(enum utrace_resume_action action, struct utrace_attached_engine *engine, struct task_struct *tsk, unsigned long event) {";
-      s.op->newline() << "#endif";
-      s.op->indent(1);
-      s.op->newline() << "struct stap_utrace_probe *p = (struct stap_utrace_probe *)engine->data;";
-      s.op->newline() << "int rc = 0;";
-
-      // Turn off quiesce handling and turn on either syscall entry
-      // or exit events.
-      s.op->newline() << "if (p->flags == UDPF_SYSCALL)";
-      s.op->indent(1);
-      s.op->newline() << "rc = utrace_set_events(tsk, engine, UTRACE_EVENT(SYSCALL_ENTRY)|UTRACE_EVENT(DEATH));";
-      s.op->indent(-1);
-      s.op->newline() << "else if (p->flags == UDPF_SYSCALL_RETURN)";
-      s.op->indent(1);
-      s.op->newline() << "rc = utrace_set_events(tsk, engine, UTRACE_EVENT(SYSCALL_EXIT)|UTRACE_EVENT(DEATH));";
-      s.op->indent(-1);
-      s.op->newline() << "if (rc != 0)";
-      s.op->indent(1);
-      s.op->newline() << "_stp_error(\"utrace_set_events returned error %d on pid %d\", rc, (int)tsk->pid);";
-      s.op->indent(-1);
-
-      s.op->newline() << "return UTRACE_RESUME;";
-      s.op->newline(-1) << "}";
-
-      s.op->newline() << "#ifdef UTRACE_ORIG_VERSION";
       s.op->newline() << "static u32 stap_utrace_probe_syscall(struct utrace_attached_engine *engine, struct task_struct *tsk, struct pt_regs *regs) {";
       s.op->newline() << "#else";
       s.op->newline() << "static u32 stap_utrace_probe_syscall(enum utrace_resume_action action, struct utrace_attached_engine *engine, struct task_struct *tsk, struct pt_regs *regs) {";
@@ -6279,6 +6226,11 @@ utrace_derived_probe_group::emit_module_decls (systemtap_session& s)
       s.op->newline() << "(*p->ph) (c);";
       common_probe_entryfn_epilogue (s.op);
 
+      s.op->newline() << "if ((atomic_read (&session_state) != STAP_SESSION_STARTING) && (atomic_read (&session_state) != STAP_SESSION_RUNNING)) {";
+      s.op->indent(1);
+      s.op->newline() << "debug_task_finder_detach();";
+      s.op->newline() << "return UTRACE_DETACH;";
+      s.op->newline(-1) << "}";
       s.op->newline() << "return UTRACE_RESUME;";
       s.op->newline(-1) << "}";
     }
@@ -6307,11 +6259,7 @@ utrace_derived_probe_group::emit_module_decls (systemtap_session& s)
       s.op->indent(1);
       s.op->newline() << "if (process_p) {";
       s.op->indent(1);
-      s.op->newline() << "rc = stap_utrace_attach(tsk, &p->ops, p, p->events);";
-      s.op->newline() << "if (rc == 0) {";
-      s.op->indent(1);
-      s.op->newline() << "p->engine_attached = 1;";
-      s.op->newline(-1) << "}";
+      s.op->newline() << "stap_utrace_probe_handler(tsk, p);";
       s.op->newline(-1) << "}";
       s.op->newline() << "break;";
       s.op->indent(-1);
@@ -6322,11 +6270,7 @@ utrace_derived_probe_group::emit_module_decls (systemtap_session& s)
       s.op->indent(1);
       s.op->newline() << "if (! process_p) {";
       s.op->indent(1);
-      s.op->newline() << "rc = stap_utrace_attach(tsk, &p->ops, p, p->events);";
-      s.op->newline() << "if (rc == 0) {";
-      s.op->indent(1);
-      s.op->newline() << "p->engine_attached = 1;";
-      s.op->newline(-1) << "}";
+      s.op->newline() << "stap_utrace_probe_handler(tsk, p);";
       s.op->newline(-1) << "}";
       s.op->newline() << "break;";
       s.op->indent(-1);
@@ -6367,7 +6311,8 @@ utrace_derived_probe_group::emit_module_decls (systemtap_session& s)
   s.op->newline(-1) << "}";
 
   // Since this engine could be attached to multiple threads, don't
-  // call stap_utrace_detach_ops() here.
+  // call stap_utrace_detach_ops() here, only call
+  // stap_utrace_detach() as necessary.
   s.op->newline() << "else {";
   s.op->indent(1);
   s.op->newline() << "switch (p->flags) {";
@@ -6396,15 +6341,18 @@ utrace_derived_probe_group::emit_module_decls (systemtap_session& s)
       s.op->indent(-1);
     }
 
-  // For begin/thread_begin probes, at deregistration time we'll try
-  // to detach.  This will only be necessary if the new thread/process
-  // got killed before the probe got run in the UTRACE_EVENT(QUIESCE)
-  // handler.
-  if (flags_seen[UDPF_BEGIN] || flags_seen[UDPF_THREAD_BEGIN]
-      || flags_seen[UDPF_SYSCALL] || flags_seen[UDPF_SYSCALL_RETURN])
-    {
+  // For begin/thread_begin probes, we don't need to do anything.
+  if (flags_seen[UDPF_BEGIN] || flags_seen[UDPF_THREAD_BEGIN])
+  {
       s.op->newline() << "case UDPF_BEGIN:";
       s.op->newline() << "case UDPF_THREAD_BEGIN:";
+      s.op->indent(1);
+      s.op->newline() << "break;";
+      s.op->indent(-1);
+  }
+      
+  if (flags_seen[UDPF_SYSCALL] || flags_seen[UDPF_SYSCALL_RETURN])
+    {
       s.op->newline() << "case UDPF_SYSCALL:";
       s.op->newline() << "case UDPF_SYSCALL_RETURN:";
       s.op->indent(1);
diff --git a/testsuite/ChangeLog b/testsuite/ChangeLog
index e6649a36..a0e19521 100644
--- a/testsuite/ChangeLog
+++ b/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2008-08-29  David Smith  <dsmith@redhat.com>
+
+	PR6841
+	* systemtap.base/utrace_p5.exp: Added system-wide syscall test for
+	bug 6841 fix.
+
 2008-08-27  Stan Cox  <scox@redhat.com>
 
 	* systemtap.base/global_end.exp: New.
diff --git a/testsuite/systemtap.base/utrace_p5.exp b/testsuite/systemtap.base/utrace_p5.exp
index fcd617fe..33281350 100644
--- a/testsuite/systemtap.base/utrace_p5.exp
+++ b/testsuite/systemtap.base/utrace_p5.exp
@@ -73,6 +73,23 @@ set thread_end_script {
 }
 set thread_end_script_output "thread_ends = \\d+\r\n"
 
+# Script that tests the bug 6841 fix.
+set bz6841_script {
+    global proc,name
+    probe begin { printf("systemtap starting probe\n") }
+    probe process.syscall {
+	proc[pid()] <<< 1
+	name[pid()] = execname()
+    }
+    probe end { printf("systemtap ending probe\n")
+	foreach(p+ in proc) {
+	    printf("%s(%d) issues syscall %d times\n",
+		   name[p], p, @sum(proc[p]))
+	}
+    }
+}
+set bz6841_script_output ".+ issues syscall \\d+ times\r\n"
+
 # Try to find utrace_attach symbol in /proc/kallsyms
 set path "/proc/kallsyms"
 if {! [catch {exec grep -q utrace_attach $path} dummy]} {
@@ -182,5 +199,15 @@ if {$utrace_support_found == 0} {
 	-e $script
 }
 
+set TEST_NAME "UTRACE_P5_07"
+if {$utrace_support_found == 0} {
+    untested "$TEST_NAME : no kernel utrace support found"
+} elseif {![installtest_p]} {
+    untested "$TEST_NAME"
+} else {
+    stap_run $TEST_NAME run_utrace_p5_multi $bz6841_script_output \
+	-e $bz6841_script
+}
+
 # Cleanup
 exec rm -f $exepath $multi_exepath