On 09/23, Roland McGrath wrote:
>
> > I think we should start with changing utrace_control(DETACH) anyway,
> > then try to improve. I'll ressend the one-liner I already showed.
>
> Ok.
Yes. I think we need a simple fix first, please see another email
I am going to send.
However, I am a bit confused. I was going to send the patch which
changes utrace_control(DETACH) as you suggested, but now I am not
sure. See below.
> The original theory on this was that we'd one day stop overloading
> user signals for debugger-induced traps.
> things like hardware stepping would generate a special new flavor
> of utrace event
>
> But we don't have any of that now, and don't yet know if we will
> really pursue any big improvements at this API level.
Yes! I thought about this too many times.
So. Let's discuss the alternatives for now.
The 1st patch I sent initially:
--- kstub/kernel/utrace.c~10_utrace_reset_should_clear_ss
2010-09-20 03:55:12.000000000 +0200
+++ kstub/kernel/utrace.c 2010-09-20 21:33:47.000000000 +0200
@@ -709,6 +709,7 @@ static bool utrace_reset(struct task_str
*/
utrace->resume = UTRACE_RESUME;
utrace->signal_handler = 0;
+ user_disable_single_step(task);
}
/*
It clears TIF_SINGLESTEP when the last engine detaches.
The 2nd one which changes utrace_control(DETACH),
@@ -1139,7 +1139,9 @@ int utrace_control(struct task_struct *t
target->utrace_flags &= ~ENGINE_STOP;
mark_engine_detached(engine);
reset = reset || utrace_do_stop(target, utrace);
- if (!reset) {
+ if (reset) {
+ user_disable_single_step(target);
+ } else {
/*
* As in utrace_set_events(), this barrier
ensures
* that our engine->flags changes have hit
before we
It doesn't cover the case when the TIF_SINGLESTEP tracee detaches
itself, but a) currently the are no such engines, and b) it looks
more clean.
However, I am starting to think that if you prefer this change we
have a better option. Instead of duplicating UTRACE_RESUME logic,
perhaps the patch below makes more sense?
Please tell me which fix you prefer? Or just commit the fix you
like more.
Oleg.
--- x/kernel/utrace.c
+++ x/kernel/utrace.c
@@ -716,8 +716,15 @@ static bool utrace_reset(struct task_str
/*
* If no more engines want it stopped, wake it up.
*/
- if (task_is_traced(task) && !(flags & ENGINE_STOP))
+ if (task_is_traced(task) && !(flags & ENGINE_STOP)) {
+ /*
+ * It just resumes, so make sure single-step
+ * is not left set.
+ */
+ if (utrace->resume == UTRACE_RESUME)
+ user_disable_single_step(task);
utrace_wakeup(task, utrace);
+ }
/*
* In theory spin_lock() doesn't imply rcu_read_lock().
@@ -1157,14 +1164,7 @@ int utrace_control(struct task_struct *t
break;
case UTRACE_RESUME:
- /*
- * This and all other cases imply resuming if stopped.
- * There might not be another report before it just
- * resumes, so make sure single-step is not left set.
- */
clear_engine_wants_stop(engine);
- if (likely(reset))
- user_disable_single_step(target);
break;
case UTRACE_BLOCKSTEP:
