[HACK] utrace: "fix" utrace_resume()->finish_resume_report() logic

[Roland McGrath]

> In short, it is just wrong to call finish_resume_report() in utrace_resume()
> without reporting loop, because utrace never clears TIF_NOTIFY_RESUME. 

It's not supposed to.  The arch code clears TIF_NOTIFY_RESUME before
calling tracehook_notify_resume().  This implies that utrace must keep its
own independent bookkeeping sufficient to know what to do when there is a
spurious call to utrace_resume().

> It is
> very possible we enter utrace_resume() with utrace->resume == UTRACE_RESUME,

Let's figure out exactly when this can happen.  Perhaps we need more state
bits in some form.  But perhaps it's just the case that we don't really
need to do anything at all for UTRACE_RESUME, i.e. we should just never
call finish_resume_report() in that case.

You cited the one most obvious case: utrace_get_signal() has just run, so
finish_resume_report() has just run and everything is already as we want.

What else?

* TIF_NOTIFY_RESUME spuriously set for some unrelated reason
  (arch purposes)
  -> want to do nothing

* utrace_signal_handler set it
  -> want to do nothing more after clearing utrace->signal_handler

* TIF_NOTIFY_RESUME just set by utrace_control()
  -> it also set utrace->resume != UTRACE_RESUME, so not this case

It feels like there should be more corners, but they are not coming to mind
immediately.  For just those cases, I think the change below would do it.

What do you think?


Thanks,
Roland


diff --git a/kernel/utrace.c b/kernel/utrace.c
index 34c990a..0000000 100644  
--- a/kernel/utrace.c
+++ b/kernel/utrace.c
@@ -1866,8 +1866,18 @@ void utrace_resume(struct task_struct *t
 	 */
 	report.action = start_report(utrace);
 
-	if (report.action == UTRACE_REPORT &&
-	    likely(task->utrace_flags & UTRACE_EVENT(QUIESCE))) {
+	switch (report.action) {
+	case UTRACE_RESUME:
+		/*
+		 * Anything we might have done was already handled by
+		 * utrace_get_signal(), or this is an entirely spurious
+		 * call.  (The arch might use TIF_NOTIFY_RESUME for other
+		 * purposes as well as calling us.)
+		 */
+		return;
+	case UTRACE_REPORT:
+		if (unlikely(!(task->utrace_flags & UTRACE_EVENT(QUIESCE))))
+			break;
 		/*
 		 * Do a simple reporting pass, with no specific
 		 * callback after report_quiesce.
@@ -1875,13 +1885,15 @@ void utrace_resume(struct task_struct *t
 		report.action = UTRACE_RESUME;
 		list_for_each_entry(engine, &utrace->attached, entry)
 			start_callback(utrace, &report, engine, task, 0);
-	} else {
+		break;
+	default:
 		/*
 		 * Even if this report was truly spurious, there is no need
 		 * for utrace_reset() now.  TIF_NOTIFY_RESUME was already
 		 * cleared--it doesn't stay spuriously set.
 		 */
 		report.spurious = false;
+		break;
 	}
 
 	/*