resume from another engine results in loss of singlestep req.

[Roland McGrath]

> 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: