> Not only UTRACE_SINGLESTEP... UTRACE_REPORT can't be lost, we have
> utrace_report->reports. But what about UTRACE_INTERRUPT ?
>
> IOW, should any callback ever return (say) UTRACE_INTERRUPT instead
> of utrace_control(UTRACE_INTERRUPT) ?
Indeed I think this is handled wrong in the current code. The intent is
that if you return UTRACE_INTERRUPT, then after a resumption you will
indeed get the interrupt effect (consistent with what I just told Srikar).
This change replaces the one I just posted in response to Srikar's message.
It compiles, but I haven't tested it or anything.
I think this might be the right fix. First, we change the order so that
UTRACE_INTERRUPT prevails over UTRACE_REPORT. (I'm really not sure why I
ever had it the other way around.) Next, we keep track of not only whether
one engine wanted a report when another wanted stop, but of the prevailing
resume action (i.e. least value still > UTRACE_STOP). After stopping and
resuming, we apply UTRACE_INTERRUPT or UTRACE_REPORT as needed to make sure
we get the kind of resume report we need.
What do you think?
Thanks,
Roland
diff --git a/include/linux/utrace.h b/include/linux/utrace.h
index 6b62c05..0000000 100644
--- a/include/linux/utrace.h
+++ b/include/linux/utrace.h
@@ -183,8 +183,8 @@ void task_utrace_proc_status(struct seq_
/**
* enum utrace_resume_action - engine's choice of action for a traced task
* @UTRACE_STOP: Stay quiescent after callbacks.
- * @UTRACE_REPORT: Make some callback soon.
* @UTRACE_INTERRUPT: Make @report_signal() callback soon.
+ * @UTRACE_REPORT: Make some callback soon.
* @UTRACE_SINGLESTEP: Resume in user mode for one instruction.
* @UTRACE_BLOCKSTEP: Resume in user mode until next branch.
* @UTRACE_RESUME: Resume normally in user mode.
@@ -199,8 +199,8 @@ void task_utrace_proc_status(struct seq_
*/
enum utrace_resume_action {
UTRACE_STOP,
- UTRACE_REPORT,
UTRACE_INTERRUPT,
+ UTRACE_REPORT,
UTRACE_SINGLESTEP,
UTRACE_BLOCKSTEP,
UTRACE_RESUME,
diff --git a/kernel/utrace.c b/kernel/utrace.c
index 954f9fe..0000000 100644
--- a/kernel/utrace.c
+++ b/kernel/utrace.c
@@ -377,7 +377,7 @@ static inline bool finish_utrace_stop(st
* engine may still want us to stay stopped.
*/
static bool utrace_stop(struct task_struct *task, struct utrace *utrace,
- bool report)
+ enum utrace_resume_action action)
{
bool killed;
@@ -401,7 +401,14 @@ static bool utrace_stop(struct task_stru
return true;
}
- if (report) {
+ if (action == UTRACE_INTERRUPT) {
+ /*
+ * Ensure a %UTRACE_SIGNAL_REPORT reporting pass when we're
+ * resumed. The recalc_sigpending() call below will see
+ * this flag and set TIF_SIGPENDING.
+ */
+ utrace->interrupt = 1;
+ } else if (action < UTRACE_RESUME) {
/*
* Ensure a reporting pass when we're resumed.
*/
@@ -1259,17 +1266,17 @@ EXPORT_SYMBOL_GPL(utrace_barrier);
* This is local state used for reporting loops, perhaps optimized away.
*/
struct utrace_report {
- enum utrace_resume_action action;
u32 result;
+ enum utrace_resume_action action;
+ enum utrace_resume_action resume_action;
bool detaches;
- bool reports;
bool takers;
bool killed;
};
#define INIT_REPORT(var) \
- struct utrace_report var = { UTRACE_RESUME, 0, \
- false, false, false, false }
+ struct utrace_report var = { 0, UTRACE_RESUME, UTRACE_RESUME, \
+ false, false, false }
/*
* We are now making the report, so clear the flag saying we need one.
@@ -1298,14 +1305,14 @@ static void finish_report(struct utrace_
{
bool clean = (report->takers && !report->detaches);
- if (report->action <= UTRACE_REPORT && !utrace->report) {
- spin_lock(&utrace->lock);
- utrace->report = 1;
- set_tsk_thread_flag(task, TIF_NOTIFY_RESUME);
- } else if (report->action == UTRACE_INTERRUPT && !utrace->interrupt) {
+ if (report->action == UTRACE_INTERRUPT && !utrace->interrupt) {
spin_lock(&utrace->lock);
utrace->interrupt = 1;
set_tsk_thread_flag(task, TIF_SIGPENDING);
+ } else if (report->action <= UTRACE_REPORT && !utrace->report) {
+ spin_lock(&utrace->lock);
+ utrace->report = 1;
+ set_tsk_thread_flag(task, TIF_NOTIFY_RESUME);
} else if (clean) {
return;
} else {
@@ -1348,8 +1355,8 @@ static bool finish_callback(struct utrac
spin_unlock(&utrace->lock);
}
} else {
- if (action == UTRACE_REPORT)
- report->reports = true;
+ if (action < report->resume_action)
+ report->resume_action = action;
if (engine_wants_stop(engine)) {
spin_lock(&utrace->lock);
@@ -1709,7 +1716,8 @@ static void finish_resume_report(struct
switch (report->action) {
case UTRACE_STOP:
- report->killed = utrace_stop(task, utrace, report->reports);
+ report->killed = utrace_stop(task, utrace,
+ report->resume_action);
break;
case UTRACE_INTERRUPT:
