New utrace fixes.

2008-09-25  David Smith  <dsmith@redhat.com>

	* task_finder.c (__stp_utrace_attach): Added action flag to know
	to request the thread to be stopped or not.
	(stap_utrace_attach): Now just calls __stp_utrace_attach().
	(__stp_utrace_task_finder_target_quiesce): Handles
	utrace_set_events() errors properly.

	* utrace_compatibility.h (enum utrace_resume_action): Added
	utrace_resume_action enum.
	(utrace_control): Added UTRACE_STOP support.
	(utrace_engine_put): New.
	(utrace_barrier): New.

3 files changed, 121 insertions, 29 deletions

diff --git a/runtime/ChangeLog b/runtime/ChangeLog
index e5594783..78ffbb3d 100644
--- a/runtime/ChangeLog
+++ b/runtime/ChangeLog
@@ -1,3 +1,17 @@
+2008-09-25  David Smith  <dsmith@redhat.com>
+
+	* task_finder.c (__stp_utrace_attach): Added action flag to know
+	to request the thread to be stopped or not.
+	(stap_utrace_attach): Now just calls __stp_utrace_attach().
+	(__stp_utrace_task_finder_target_quiesce): Handles
+	utrace_set_events() errors properly.
+	
+	* utrace_compatibility.h (enum utrace_resume_action): Added
+	utrace_resume_action enum.
+	(utrace_control): Added UTRACE_STOP support.
+	(utrace_engine_put): New.
+	(utrace_barrier): New.
+
 2008-09-17  Frank Ch. Eigler  <fche@elastic.org>
 
 	PR 6487, 6504.
diff --git a/runtime/task_finder.c b/runtime/task_finder.c
index 493ca6f7..75c706ec 100644
--- a/runtime/task_finder.c
+++ b/runtime/task_finder.c
@@ -89,7 +89,7 @@ struct stap_task_finder_target {
 	size_t pathlen;
 
 /* public: */
-    	const char *pathname;
+	const char *pathname;
 	pid_t pid;
 	stap_task_finder_callback callback;
 	stap_task_finder_vm_callback vm_callback;
@@ -148,8 +148,8 @@ static int
 stap_register_task_finder_target(struct stap_task_finder_target *new_tgt)
 {
 	// Since this __stp_task_finder_list is (currently) only
-        // written to in one big setup operation before the task
-        // finder process is started, we don't need to lock it.
+	// written to in one big setup operation before the task
+	// finder process is started, we don't need to lock it.
 	struct list_head *node;
 	struct stap_task_finder_target *tgt = NULL;
 	int found_node = 0;
@@ -258,6 +258,7 @@ stap_utrace_detach(struct task_struct *tsk,
 				   rc, tsk->pid);
 			break;
 		}
+		utrace_engine_put(engine);
 	}
 	return rc;
 }
@@ -394,9 +395,10 @@ __stp_get_mm_path(struct mm_struct *mm, char *buf, int buflen)
 	 : __STP_TASK_VM_BASE_EVENTS)
 
 static int
-stap_utrace_attach(struct task_struct *tsk,
-		   const struct utrace_engine_ops *ops, void *data,
-		   unsigned long event_flags)
+__stp_utrace_attach(struct task_struct *tsk,
+		    const struct utrace_engine_ops *ops, void *data,
+		    unsigned long event_flags,
+		    enum utrace_resume_action action)
 {
 	struct utrace_attached_engine *engine;
 	struct mm_struct *mm;
@@ -428,15 +430,51 @@ stap_utrace_attach(struct task_struct *tsk,
 	}
 	else {
 		rc = utrace_set_events(tsk, engine, event_flags);
-		if (rc == 0)
+		if (rc == -EINPROGRESS) {
+			/*
+			 * It's running our callback, so we have to
+			 * synchronize.  We can't keep rcu_read_lock,
+			 * so the task pointer might die.  But it's
+			 * safe to call utrace_barrier() even with a
+			 * stale task pointer, if we have an engine
+			 * ref.
+			 */
+			rc = utrace_barrier(tsk, engine);
+			if (rc != 0)
+				_stp_error("utrace_barrier returned error %d on pid %d",
+					   rc, (int)tsk->pid);
+		}
+		if (rc == 0) {
 			debug_task_finder_attach();
+
+			if (action != UTRACE_RESUME) {
+				rc = utrace_control(tsk, engine, UTRACE_STOP);
+				/* EINPROGRESS means we must wait for
+				 * a callback, which is what we want. */
+				if (rc != 0 && rc != -EINPROGRESS)
+					_stp_error("utrace_control returned error %d on pid %d",
+						   rc, (int)tsk->pid);
+				else
+					rc = 0;
+			}
+
+		}
 		else
-			_stp_error("utrace_set_events returned error %d on pid %d",
+			_stp_error("utrace_set_events2 returned error %d on pid %d",
 				   rc, (int)tsk->pid);
+		utrace_engine_put(engine);
 	}
 	return rc;
 }
 
+static int
+stap_utrace_attach(struct task_struct *tsk,
+		   const struct utrace_engine_ops *ops, void *data,
+		   unsigned long event_flags)
+{
+	return __stp_utrace_attach(tsk, ops, data, event_flags, UTRACE_RESUME);
+}
+
 static inline void
 __stp_utrace_attach_match_filename(struct task_struct *tsk,
 				   const char * const filename,
@@ -485,9 +523,10 @@ __stp_utrace_attach_match_filename(struct task_struct *tsk,
 			// 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);
+				rc = __stp_utrace_attach(tsk, &cb_tgt->ops,
+							 cb_tgt,
+							 __STP_ATTACHED_TASK_EVENTS,
+							 UTRACE_STOP);
 				if (rc != 0 && rc != EPERM)
 					break;
 				cb_tgt->engine_attached = 1;
@@ -601,8 +640,8 @@ __stp_utrace_task_finder_report_clone(enum utrace_resume_action action,
 	__stp_tf_handler_start();
 
 	// On clone, attach to the child.
-	rc = stap_utrace_attach(child, engine->ops, 0,
-				__STP_TASK_FINDER_EVENTS);
+	rc = __stp_utrace_attach(child, engine->ops, 0,
+				 __STP_TASK_FINDER_EVENTS, UTRACE_RESUME);
 	if (rc != 0 && rc != EPERM) {
 		__stp_tf_handler_end();
 		return UTRACE_RESUME;
@@ -757,6 +796,22 @@ __stp_utrace_task_finder_target_quiesce(enum utrace_resume_action action,
 	// Turn off quiesce handling
 	rc = utrace_set_events(tsk, engine,
 			       __STP_ATTACHED_TASK_BASE_EVENTS(tgt));
+
+	if (rc == -EINPROGRESS) {
+		/*
+		 * It's running our callback, so we have to
+		 * synchronize.  We can't keep rcu_read_lock,
+		 * so the task pointer might die.  But it's
+		 * safe to call utrace_barrier() even with
+		 * a stale task pointer, if we have an engine ref.
+		 */
+		rc = utrace_barrier(tsk, engine);
+		if (rc != 0)
+			_stp_error("utrace_barrier returned error %d on pid %d",
+				   rc, (int)tsk->pid);
+		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);
@@ -1173,13 +1228,14 @@ stap_start_task_finder(void)
 		size_t mmpathlen;
 		struct list_head *tgt_node;
 
-                /* Skip over processes other than that specified with
-                   stap -c or -x. */
-                if (_stp_target && tsk->tgid != _stp_target)
-                  continue;
+		/* Skip over processes other than that specified with
+		 * stap -c or -x. */
+		if (_stp_target && tsk->tgid != _stp_target)
+			continue;
 
-		rc = stap_utrace_attach(tsk, &__stp_utrace_task_finder_ops, 0,
-					__STP_TASK_FINDER_EVENTS);
+		rc = __stp_utrace_attach(tsk, &__stp_utrace_task_finder_ops, 0,
+					 __STP_TASK_FINDER_EVENTS,
+					 UTRACE_RESUME);
 		if (rc == EPERM) {
 			/* Ignore EPERM errors, which mean this wasn't
 			 * a thread we can attach to. */
@@ -1242,16 +1298,17 @@ stap_start_task_finder(void)
 					continue;
 
 				// Set up events we need for attached tasks.
-				rc = stap_utrace_attach(tsk, &cb_tgt->ops,
-							cb_tgt,
-							__STP_ATTACHED_TASK_EVENTS);
+				rc = __stp_utrace_attach(tsk, &cb_tgt->ops,
+							 cb_tgt,
+							 __STP_ATTACHED_TASK_EVENTS,
+							 UTRACE_STOP);
 				if (rc != 0 && rc != EPERM)
 					goto stf_err;
 				cb_tgt->engine_attached = 1;
 			}
 		}
 	} while_each_thread(grp, tsk);
- stf_err:
+stf_err:
 	rcu_read_unlock();
 
 	_stp_kfree(mmpath_buf);
diff --git a/runtime/utrace_compatibility.h b/runtime/utrace_compatibility.h
index 80037015..27fca250 100644
--- a/runtime/utrace_compatibility.h
+++ b/runtime/utrace_compatibility.h
@@ -24,9 +24,11 @@
 
 #define UTRACE_ORIG_VERSION
 
-#define UTRACE_RESUME	UTRACE_ACTION_RESUME
-#define UTRACE_DETACH	UTRACE_ACTION_DETACH
-#define UTRACE_STOP	UTRACE_ACTION_QUIESCE
+enum utrace_resume_action {
+	UTRACE_STOP = UTRACE_ACTION_QUIESCE,
+	UTRACE_RESUME = UTRACE_ACTION_RESUME,
+	UTRACE_DETACH = UTRACE_ACTION_DETACH,
+};
 
 static inline struct utrace_attached_engine *
 utrace_attach_task(struct task_struct *target, int flags,
@@ -38,11 +40,17 @@ utrace_attach_task(struct task_struct *target, int flags,
 static inline int __must_check
 utrace_control(struct task_struct *target,
 	       struct utrace_attached_engine *engine,
-	       unsigned long action)
+	       enum utrace_resume_action action)
 {
-	if (action == UTRACE_DETACH)
+	switch (action) {
+	case UTRACE_DETACH:
 		return utrace_detach(target, engine);
-	return -EINVAL;
+	case UTRACE_STOP:
+		return utrace_set_flags(target, engine,
+					(engine->flags | UTRACE_ACTION_QUIESCE));
+	default:
+		return -EINVAL;
+	}
 }
 
 static inline int __must_check
@@ -52,6 +60,19 @@ utrace_set_events(struct task_struct *target,
 {
 	return utrace_set_flags(target, engine, eventmask);
 }
+
+static inline void
+utrace_engine_put(struct utrace_attached_engine *engine)
+{
+	return;
+}
+
+static inline int __must_check
+utrace_barrier(struct task_struct *target,
+	       struct utrace_attached_engine *engine)
+{
+	return 0;
+}
 #endif
 
 #endif	/* _UTRACE_COMPATIBILITY_H_ */