commit b9b055e2f6e7db5197fe90c4b95ae52dd84a5994 (HEAD, TMP)
Author: David Smith <dsmith@redhat.com>
Date: Fri Jul 18 15:49:39 2014 -0500
Fixed PR17181 by making utrace handle interrupting processes better.
* runtime/stp_utrace.c (utrace_init): Use kallsyms_lookup_name() to lookup
"wake_up_state" if needed.
(stp_task_notify_resume): New function to handle the details of adding a
resume handler.
(utrace_cleanup): Only output debug printk's if STP_TF_DEBUG is defined.
(utrace_set_events): Improve check.
(utrace_do_stop): Call stp_task_notify_resume() instead of inline code.
(utrace_wakeup): Call stp_wake_up_state() instead of wake_up_process()
to avoid a WARN(). Call stp_task_notify_resume() instead of inline
code.
(utrace_control): Call stp_task_notify_resume() instead of inline code.
(finish_report): Ditto.
(finish_resume_report): Add UTRACE_INTERRUPT support.
(utrace_resume): Handle UTRACE_INTERRUPT.
* runtime/linux/task_finder2.c (stap_task_finder_post_init): Go back to
sending UTRACE_INTERRUPT to all tasks.
* buildrun.cxx (compile_pass): Add export tests for 'wake_up_state' and
'try_to_wake_up'.
* runtime/linux/runtime.h: Added 'kallsyms_wake_up_state' declaration when
necessary.
* testsuite/systemtap.base/process_resume.c: New file.
* testsuite/systemtap.base/process_resume.exp: New file.
diff --git a/buildrun.cxx b/buildrun.cxx
index a2f2197e4609..87b946a27621 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -404,6 +404,8 @@ compile_pass (systemtap_session& s)
// used by runtime/stp_utrace.c
output_exportconf(s, o, "task_work_add", "STAPCONF_TASK_WORK_ADD_EXPORTED");
+ output_exportconf(s, o, "wake_up_state", "STAPCONF_WAKE_UP_STATE_EXPORTED");
+ output_exportconf(s, o, "try_to_wake_up", "STAPCONF_TRY_TO_WAKE_UP_EXPORTED");
output_exportconf(s, o, "signal_wake_up_state", "STAPCONF_SIGNAL_WAKE_UP_STATE_EXPORTED");
output_exportconf(s, o, "signal_wake_up", "STAPCONF_SIGNAL_WAKE_UP_EXPORTED");
output_exportconf(s, o, "__lock_task_sighand", "STAPCONF___LOCK_TASK_SIGHAND_EXPORTED");
diff --git a/runtime/linux/runtime.h b/runtime/linux/runtime.h
index 76dbea4fd24d..281ce41af72e 100644
--- a/runtime/linux/runtime.h
+++ b/runtime/linux/runtime.h
@@ -180,6 +180,9 @@ static void *kallsyms_task_work_add;
static void *kallsyms_task_work_cancel;
#endif
+#if !defined(STAPCONF_TRY_TO_WAKE_UP_EXPORTED) && !defined(STAPCONF_WAKE_UP_STATE_EXPORTED)
+static void *kallsyms_wake_up_state;
+#endif
#if !defined(STAPCONF_SIGNAL_WAKE_UP_STATE_EXPORTED)
static void *kallsyms_signal_wake_up_state;
#endif
diff --git a/runtime/linux/task_finder2.c b/runtime/linux/task_finder2.c
index e88543a81039..64f263319c0c 100644
--- a/runtime/linux/task_finder2.c
+++ b/runtime/linux/task_finder2.c
@@ -1805,10 +1805,12 @@ stap_task_finder_post_init(void)
int rc = utrace_control(tsk, engine,
UTRACE_INTERRUPT);
/* If utrace_control() returns
- * EINPROGRESS when we're trying to
- * stop/interrupt, that means the task
- * hasn't stopped quite yet, but will
- * soon. Ignore this error. */
+ * EINPROGRESS when we're
+ * trying to stop/interrupt,
+ * that means the task hasn't
+ * stopped quite yet, but will
+ * soon. Ignore this
+ * error. */
if (rc != 0 && rc != -EINPROGRESS) {
_stp_error("utrace_control returned error %d on pid %d",
rc, (int)tsk->pid);
diff --git a/runtime/stp_utrace.c b/runtime/stp_utrace.c
index 0c9d0eba3969..acbe9366ed83 100644
--- a/runtime/stp_utrace.c
+++ b/runtime/stp_utrace.c
@@ -109,6 +109,22 @@ static void utrace_report_exec(void *cb_data __attribute__ ((unused)),
#define __UTRACE_REGISTERED 1
static atomic_t utrace_state = ATOMIC_INIT(__UTRACE_UNREGISTERED);
+// If wake_up_state() is exported, use it.
+#if defined(STAPCONF_WAKE_UP_STATE_EXPORTED)
+#define stp_wake_up_state wake_up_state
+// Otherwise, try to use try_to_wake_up(). The wake_up_state()
+// function is just a wrapper around try_to_wake_up().
+#elif defined(STAPCONF_TRY_TO_WAKE_UP_EXPORTED)
+static inline int stp_wake_up_state(struct task_struct *p, unsigned int state)
+{
+ return try_to_wake_up(p, state, 0);
+}
+// Otherwise, we'll have to look up wake_up_state() with kallsyms.
+#else
+typedef typeof(&wake_up_state) wake_up_state_fn;
+#define stp_wake_up_state (* (wake_up_state_fn)kallsyms_wake_up_state)
+#endif
+
#if !defined(STAPCONF_SIGNAL_WAKE_UP_STATE_EXPORTED)
// Sigh. On kernel's without signal_wake_up_state(), there is no
// declaration to use in 'typeof(&signal_wake_up_state)'. So, we'll
@@ -188,6 +204,14 @@ static int utrace_init(void)
INIT_HLIST_HEAD(&task_utrace_table[i]);
}
+#if !defined(STAPCONF_TRY_TO_WAKE_UP_EXPORTED) \
+ && !defined(STAPCONF_WAKE_UP_STATE_EXPORTED)
+ kallsyms_wake_up_state = (void *)kallsyms_lookup_name("wake_up_state");
+ if (kallsyms_wake_up_state == NULL) {
+ _stp_error("Can't resolve wake_up_state!");
+ goto error;
+ }
+#endif
#if !defined(STAPCONF_SIGNAL_WAKE_UP_STATE_EXPORTED)
/* The signal_wake_up_state() function (which replaces
* signal_wake_up() in newer kernels) isn't exported. Look up
@@ -316,6 +340,29 @@ static int utrace_exit(void)
return 0;
}
+/*
+ * stp_task_notify_resume() is our version of
+ * set_notify_resume(). When called, the task_work infrastructure will
+ * cause utrace_resume() to get called.
+ */
+static void
+stp_task_notify_resume(struct task_struct *target, struct utrace *utrace)
+{
+ if (! utrace->task_work_added) {
+ int rc = stp_task_work_add(target, &utrace->work);
+ if (rc == 0) {
+ utrace->task_work_added = 1;
+ }
+ /* stp_task_work_add() returns -ESRCH if the task has
+ * already passed exit_task_work(). Just ignore this
+ * error. */
+ else if (rc != -ESRCH) {
+ printk(KERN_ERR "%s:%d - task_work_add() returned %d\n",
+ __FUNCTION__, __LINE__, rc);
+ }
+ }
+}
+
static void utrace_resume(struct task_work *work);
static void utrace_report_work(struct task_work *work);
@@ -348,21 +395,29 @@ static void utrace_cleanup(struct utrace *utrace)
}
if (utrace->task_work_added) {
+#ifdef STP_TF_DEBUG
if (stp_task_work_cancel(utrace->task, &utrace_resume) == NULL)
printk(KERN_ERR "%s:%d - task_work_cancel() failed? task %p, %d, %s\n",
__FUNCTION__, __LINE__, utrace->task,
utrace->task->tgid,
(utrace->task->comm ? utrace->task->comm
: "UNKNOWN"));
+#else
+ stp_task_work_cancel(utrace->task, &utrace_resume);
+#endif
utrace->task_work_added = 0;
}
if (utrace->report_work_added) {
+#ifdef STP_TF_DEBUG
if (stp_task_work_cancel(utrace->task, &utrace_report_work) == NULL)
printk(KERN_ERR "%s:%d - task_work_cancel() failed? task %p, %d, %s\n",
__FUNCTION__, __LINE__, utrace->task,
utrace->task->tgid,
(utrace->task->comm ? utrace->task->comm
: "UNKNOWN"));
+#else
+ stp_task_work_cancel(utrace->task, &utrace_report_work);
+#endif
utrace->report_work_added = 0;
}
spin_unlock(&utrace->lock);
@@ -945,7 +1000,11 @@ static int utrace_set_events(struct task_struct *target,
* If utrace_report_death() is already progress now,
* it's too late to clear the death event bits.
*/
- if (((old_flags & ~events) & _UTRACE_DEATH_EVENTS) && utrace->death)
+ if (target->exit_state &&
+ (((events & ~old_flags) & _UTRACE_DEATH_EVENTS) ||
+ (utrace->death &&
+ ((old_flags & ~events) & _UTRACE_DEATH_EVENTS)) ||
+ (utrace->reap && ((old_flags & ~events) & UTRACE_EVENT(REAP)))))
goto unlock;
/*
@@ -1035,20 +1094,7 @@ static bool utrace_do_stop(struct task_struct *target, struct utrace *utrace)
spin_unlock_irq(&target->sighand->siglock);
} else if (utrace->resume > UTRACE_REPORT) {
utrace->resume = UTRACE_REPORT;
- if (! utrace->task_work_added) {
- int rc = stp_task_work_add(target, &utrace->work);
- if (rc == 0) {
- utrace->task_work_added = 1;
- }
- /* stp_task_work_add() returns -ESRCH if the task
- * has already passed exit_task_work(). Just
- * ignore this error. */
- else if (rc != -ESRCH) {
- printk(KERN_ERR
- "%s:%d - task_work_add() returned %d\n",
- __FUNCTION__, __LINE__, rc);
- }
- }
+ stp_task_notify_resume(target, utrace);
}
return task_is_traced(target);
@@ -1066,10 +1112,7 @@ static void utrace_wakeup(struct task_struct *target, struct utrace *utrace)
target->signal->group_stop_count)
target->state = TASK_STOPPED;
else
- /* FIXME: Had to change wake_up_state() to
- * wake_up_process() here (since wake_up_state() isn't
- * exported). Reasonable? */
- wake_up_process(target);
+ stp_wake_up_state(target, __TASK_TRACED);
spin_unlock_irq(&target->sighand->siglock);
}
@@ -1175,20 +1218,9 @@ static void utrace_stop(struct task_struct *task, struct utrace *utrace,
* Ensure a reporting pass when we're resumed.
*/
utrace->resume = action;
- if (! utrace->task_work_added) {
- int rc = stp_task_work_add(task, &utrace->work);
- if (rc == 0) {
- utrace->task_work_added = 1;
- }
- /* stp_task_work_add() returns -ESRCH if the task
- * has already passed exit_task_work(). Just
- * ignore this error. */
- else if (rc != -ESRCH) {
- printk(KERN_ERR
- "%s:%d - stp_task_work_add() returned %d\n",
- __FUNCTION__, __LINE__, rc);
- }
- }
+ stp_task_notify_resume(task, utrace);
+ if (action == UTRACE_INTERRUPT)
+ set_thread_flag(TIF_SIGPENDING);
}
/*
@@ -1540,21 +1572,7 @@ static int utrace_control(struct task_struct *target,
clear_engine_wants_stop(engine);
if (action < utrace->resume) {
utrace->resume = action;
- if (! utrace->task_work_added) {
- ret = stp_task_work_add(target, &utrace->work);
- if (ret == 0) {
- utrace->task_work_added = 1;
- }
- /* stp_task_work_add() returns -ESRCH if
- * the task has already passed
- * exit_task_work(). Just ignore this
- * error. */
- else if (ret != -ESRCH) {
- printk(KERN_ERR
- "%s:%d - stp_task_work_add() returned %d\n",
- __FUNCTION__, __LINE__, ret);
- }
- }
+ stp_task_notify_resume(target, utrace);
}
break;
@@ -1574,6 +1592,7 @@ static int utrace_control(struct task_struct *target,
* we've serialized any later recalc_sigpending() after our
* setting of utrace->resume to force it on.
*/
+ stp_task_notify_resume(target, utrace);
if (reset) {
/*
* This is really just to keep the invariant that
@@ -1583,35 +1602,12 @@ static int utrace_control(struct task_struct *target,
*/
set_tsk_thread_flag(target, TIF_SIGPENDING);
} else {
- int rc = 0;
-
- if (! utrace->task_work_added) {
- rc = stp_task_work_add(target, &utrace->work);
- /* stp_task_work_add() returns -ESRCH
- * if the task has already passed
- * exit_task_work(). Just ignore this
- * error. */
- if (rc == 0 || rc == -ESRCH) {
- utrace->task_work_added = 1;
- rc = 0;
- }
- else {
- printk(KERN_ERR
- "%s:%d - task_work_add() returned %d\n",
- __FUNCTION__, __LINE__, rc);
- }
- }
-
- if (likely(rc == 0)) {
- struct sighand_struct *sighand;
- unsigned long irqflags;
-
- sighand = stp_lock_task_sighand(target,
- &irqflags);
- if (likely(sighand)) {
- stp_signal_wake_up(target, 0);
- unlock_task_sighand(target, &irqflags);
- }
+ struct sighand_struct *sighand;
+ unsigned long irqflags;
+ sighand = stp_lock_task_sighand(target, &irqflags);
+ if (likely(sighand)) {
+ stp_signal_wake_up(target, 0);
+ unlock_task_sighand(target, &irqflags);
}
}
break;
@@ -1764,20 +1760,9 @@ static void finish_report(struct task_struct *task, struct utrace *utrace,
if (resume < utrace->resume) {
spin_lock(&utrace->lock);
utrace->resume = resume;
- if (! utrace->task_work_added) {
- int rc = stp_task_work_add(task, &utrace->work);
- if (rc == 0) {
- utrace->task_work_added = 1;
- }
- /* stp_task_work_add() returns -ESRCH if the task
- * has already passed exit_task_work(). Just
- * ignore this error. */
- else if (rc != -ESRCH) {
- printk(KERN_ERR
- "%s:%d - task_work_add() returned %d\n",
- __FUNCTION__, __LINE__, rc);
- }
- }
+ stp_task_notify_resume(task, utrace);
+ if (resume == UTRACE_INTERRUPT)
+ set_tsk_thread_flag(task, TIF_SIGPENDING);
spin_unlock(&utrace->lock);
}
@@ -2323,12 +2308,16 @@ static void finish_resume_report(struct task_struct *task,
utrace_stop(task, utrace, report->resume_action);
break;
+ case UTRACE_INTERRUPT:
+ if (!signal_pending(task)) {
+ stp_task_notify_resume(task, utrace);
+ set_tsk_thread_flag(task, TIF_SIGPENDING);
+ }
+ break;
+
case UTRACE_REPORT:
case UTRACE_RESUME:
default:
-#if 0
- user_disable_single_step(task);
-#endif
break;
}
}
@@ -2385,6 +2374,14 @@ static void utrace_resume(struct task_work *work)
/* Remember that this task_work_func is finished. */
stp_task_work_func_done();
return;
+ case UTRACE_INTERRUPT:
+ /*
+ * Note that UTRACE_INTERRUPT reporting was handled by
+ * utrace_get_signal() in original utrace. In this
+ * utrace version, we'll handle it here like UTRACE_REPORT.
+ *
+ * Fallthrough...
+ */
case UTRACE_REPORT:
if (unlikely(!(utrace->utrace_flags & UTRACE_EVENT(QUIESCE))))
break;
diff --git a/testsuite/systemtap.base/process_resume.c b/testsuite/systemtap.base/process_resume.c
new file mode 100644
index 000000000000..ddf8b1eaf795
--- /dev/null
+++ b/testsuite/systemtap.base/process_resume.c
@@ -0,0 +1,10 @@
+#include <sys/types.h>
+#include <unistd.h>
+#include <signal.h>
+
+int main()
+{
+ kill(getpid(), SIGSTOP);
+ getpid();
+ return 0;
+}
diff --git a/testsuite/systemtap.base/process_resume.exp b/testsuite/systemtap.base/process_resume.exp
new file mode 100644
index 000000000000..0cb4a4322efe
--- /dev/null
+++ b/testsuite/systemtap.base/process_resume.exp
@@ -0,0 +1,82 @@
+# This test answers the following questions:
+#
+# - Can we attach to stopped processes correctly? Before PR?????? fix,
+# stap would cause a kernel warn.
+#
+# - Does resuming work on those processes? Before PR?????? fix,
+# processes would get stuck.
+
+set test "process_resume"
+
+# Only run on make installcheck and if we have utrace
+if {![installtest_p]} { untested "$test"; return }
+if {![utrace_p]} { untested "$test : no kernel utrace support found"; return }
+
+# "load" generation function for stap_run. It waits a bit, tells the
+# test program to continue and then waits a bit.
+proc resume_load {} {
+ global pid
+
+ wait_n_secs 2
+ kill -cont $pid 0
+ wait_n_secs 2
+ return 0
+}
+
+set getpid_script {
+ global getpid_calls = 0
+ probe begin { printf("systemtap starting probe\n") }
+ probe syscall.getpid { getpid_calls++ }
+ probe end { printf("systemtap ending probe\n")
+ printf("getpid calls = %d\n", getpid_calls) }
+}
+# Notice we don't care how many getpid() calls we see.
+set getpid_script_output "getpid calls = \[0-9\]\r\n"
+
+set end_script {
+ global end_probes_fired = 0
+ probe begin { printf("systemtap starting probe\n") }
+ probe process("%s").end { end_probes_fired++ }
+ probe end { printf("systemtap ending probe\n")
+ printf("end probes = %%d\n", end_probes_fired) }
+}
+set end_script_output "end probes = 1\r\n"
+
+# Compile test program.
+set res [target_compile $srcdir/$subdir/${test}.c ${test} executable \
+ "additional_flags=-O2 additional_flags=-g"]
+if { $res != "" } {
+ verbose "target_compile failed: $res" 2
+ fail "${test}: unable to compile ${test}.c"
+ return
+}
+
+# Start the application and put it in the background before each
+# test. It will stop itself and be resumed by the 'resume_load' proc.
+
+set TEST_NAME "${test}-getpid"
+set pid [exec ./$test &]
+#spawn "./$test"
+#set id $spawn_id
+#set pid [exp_pid -i $spawn_id]
+stap_run $TEST_NAME resume_load $getpid_script_output -e $getpid_script
+if {[file isdirectory /proc/$pid]} {
+ kill -KILL $pid 0
+ fail "$TEST_NAME: process didn't resume properly"
+} else {
+ pass "$TEST_NAME: process resumed properly"
+}
+
+set TEST_NAME "${test}-end"
+set pid [exec ./$test &]
+set script [format $end_script $test]
+stap_run $TEST_NAME resume_load $end_script_output -e $script
+if {[file isdirectory /proc/$pid]} {
+ kill -KILL $pid 0
+ fail "$TEST_NAME: process didn't resume properly"
+} else {
+ pass "$TEST_NAME: process resumed properly"
+}
+
+# Cleanup
+if { $verbose == 0 } { catch { exec rm -f $test } }