Moved details of utrace detach to stap_utrace_detach().

2008-08-08  David Smith  <dsmith@redhat.com>

	* tapsets.cxx (utrace_derived_probe_group::emit_module_decls):
	Calls stap_utrace_detach() to perform detach.
2008-08-08  David Smith  <dsmith@redhat.com>

	* task_finder.c (stap_utrace_detach): New function.
	(stap_utrace_detach_ops): Calls stap_utrace_detach().
	(__stp_utrace_attach_match_filename): Ditto.

4 files changed, 78 insertions, 38 deletions

diff --git a/ChangeLog b/ChangeLog
index 5583ca1b..4b922ba0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2008-08-08  David Smith  <dsmith@redhat.com>
+
+	* tapsets.cxx (utrace_derived_probe_group::emit_module_decls):
+	Calls stap_utrace_detach() to perform detach.
+
 2008-08-08  Mark Wielaard  <mwielaard@redhat.com>
 
 	* Makefile.am (EXAMPLE_BUILD_DIR): Removed. Examples indexes are
diff --git a/runtime/ChangeLog b/runtime/ChangeLog
index fdfb5691..7dfade1c 100644
--- a/runtime/ChangeLog
+++ b/runtime/ChangeLog
@@ -1,5 +1,9 @@
 2008-08-08  David Smith  <dsmith@redhat.com>
 
+	* task_finder.c (stap_utrace_detach): New function.
+	(stap_utrace_detach_ops): Calls stap_utrace_detach().
+	(__stp_utrace_attach_match_filename): Ditto.
+
 	* task_finder.c (__stp_tf_vm_cb): Added calls to save/delete vma
 	information.
 	* task_finder_vma.c (__stp_tf_vma_map_hash): New function.
diff --git a/runtime/task_finder.c b/runtime/task_finder.c
index 1db7d836..1832c795 100644
--- a/runtime/task_finder.c
+++ b/runtime/task_finder.c
@@ -150,12 +150,72 @@ stap_register_task_finder_target(struct stap_task_finder_target *new_tgt)
 	return 0;
 }
 
+static int
+stap_utrace_detach(struct task_struct *tsk,
+		   const struct utrace_engine_ops *ops)
+{
+	struct utrace_attached_engine *engine;
+	struct mm_struct *mm;
+	int rc = 0;
+
+	// Ignore init
+	if (tsk == NULL || tsk->pid <= 1)
+		return 0;
+
+	// Notice we're not calling get_task_mm() here.  Normally we
+	// avoid tasks with no mm, because those are kernel threads.
+	// So, why is this function different?  When a thread is in
+	// the process of dying, its mm gets freed.  Then, later the
+	// thread gets in the dying state and the thread's DEATH event
+	// handler gets called (if any).
+	//
+	// If a thread is in this "mortally wounded" state - no mm
+	// but not dead - and at that moment this function is called,
+	// we'd miss detaching from it if we were checking to see if
+	// it had an mm.
+
+	engine = utrace_attach(tsk, UTRACE_ATTACH_MATCH_OPS, ops, 0);
+	if (IS_ERR(engine)) {
+		rc = -PTR_ERR(engine);
+		if (rc != ENOENT) {
+			_stp_error("utrace_attach returned error %d on pid %d",
+				   rc, tsk->pid);
+		}
+		else {
+			rc = 0;
+		}
+	}
+	else if (unlikely(engine == NULL)) {
+		_stp_error("utrace_attach returned NULL on pid %d",
+			   (int)tsk->pid);
+		rc = EFAULT;
+	}
+	else {
+		rc = utrace_detach(tsk, engine);
+		switch (rc) {
+		case 0:			/* success */
+			debug_task_finder_detach();
+			break;
+		case -ESRCH:	    /* REAP callback already begun */
+		case -EALREADY:	    /* DEATH callback already begun */
+			rc = 0;	    /* ignore these errors*/
+			break;
+		default:
+			rc = -rc;
+			_stp_error("utrace_detach returned error %d on pid %d",
+				   rc, tsk->pid);
+			break;
+		}
+	}
+	return rc;
+}
+
 static void
 stap_utrace_detach_ops(struct utrace_engine_ops *ops)
 {
 	struct task_struct *grp, *tsk;
 	struct utrace_attached_engine *engine;
-	long error = 0;
+	int rc = 0;
 	pid_t pid = 0;
 
 	// Notice we're not calling get_task_mm() in this loop. In
@@ -173,31 +233,12 @@ stap_utrace_detach_ops(struct utrace_engine_ops *ops)
 
 	rcu_read_lock();
 	do_each_thread(grp, tsk) {
-		if (tsk == NULL || tsk->pid <= 1)
-			continue;
-
-		engine = utrace_attach(tsk, UTRACE_ATTACH_MATCH_OPS,
-				       ops, 0);
-		if (IS_ERR(engine)) {
-			error = -PTR_ERR(engine);
-			if (error != ENOENT) {
-				pid = tsk->pid;
-				goto udo_err;
-			}
-			error = 0;
-		}
-		else if (engine != NULL) {
-			utrace_detach(tsk, engine);
-			debug_task_finder_detach();
-		}
+		rc = stap_utrace_detach(tsk, ops);
+		if (rc != 0)
+			goto udo_err;
 	} while_each_thread(grp, tsk);
 udo_err:
 	rcu_read_unlock();
-
-	if (error != 0) {
-		_stp_error("utrace_attach returned error %d on pid %d",
-			   error, pid);
-	}
 	debug_task_finder_report();
 }
 
@@ -381,14 +422,10 @@ __stp_utrace_attach_match_filename(struct task_struct *tsk,
 				cb_tgt->engine_attached = 1;
 			}
 			else {
-				struct utrace_attached_engine *engine;
-				engine = utrace_attach(tsk,
-						       UTRACE_ATTACH_MATCH_OPS,
-						       &cb_tgt->ops, 0);
-				if (! IS_ERR(engine) && engine != NULL) {
-					utrace_detach(tsk, engine);
-					debug_task_finder_detach();
-				}
+				rc = stap_utrace_detach(tsk, &cb_tgt->ops);
+				if (rc != 0)
+					break;
+				cb_tgt->engine_attached = 0;
 			}
 		}
 	}
diff --git a/tapsets.cxx b/tapsets.cxx
index 71c92470..c5679043 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -6290,13 +6290,7 @@ utrace_derived_probe_group::emit_module_decls (systemtap_session& s)
       s.op->newline() << "case UDPF_SYSCALL:";
       s.op->newline() << "case UDPF_SYSCALL_RETURN:";
       s.op->indent(1);
-      s.op->newline() << "engine = utrace_attach(tsk, UTRACE_ATTACH_MATCH_OPS, &p->ops, 0);";
-      s.op->newline() << "if (! IS_ERR(engine) && engine != NULL) {";
-      s.op->indent(1);
-      s.op->newline() << "utrace_detach(tsk, engine);";
-      s.op->newline() << "debug_task_finder_detach();";
-
-      s.op->newline(-1) << "}";
+      s.op->newline() << "stap_utrace_detach(tsk, &p->ops);";
       s.op->newline() << "break;";
       s.op->indent(-1);
     }