On 07/24, Roland McGrath wrote:
>
> > Roland, can we just remove all code which plays with ->stopped in
> > utrace_report_jctl() and utrace_get_signal(), and do something like
> [...]
> > @@ -1602,6 +1602,16 @@ static int do_signal_stop(int signr)
> > do {
> > schedule();
> > } while (try_to_freeze());
> > +
> > + // needs a tracehook helper
> > + if (utrace->stopped) {
>
> Hmm. I guess we could. I think my original thinking was just minimizing
> the number of places in the core kernel code where we inserted any
> tracehook call, keeping them to about exactly where the ptrace hooks had
> been. But now I'd say it is certainly fine to insert another hook in the
> signals code if it keeps the utrace code cleaner.
Good. In any case I believe utrace_get_signal() and utrace_report_jctl()
should not play with ->stopped. This really simplifies the code and
what ->stopped == T means.
> I recall you wanted to rework something about tracehook_notify_jctl earlier
> too. Now is the time to revisit all that and clean the code up however
> seems best. It shouldn't be a problem to rewrite the last half of
> do_signal_stop() and one (or two) tracehook_*_jctl interfaces to be optimal
> for utrace.
OK, I'll send the patch. _IIRC_, this patch should also fix some races
with SIGNAL_STOP_STOPPED which is temporary cleared by utrace_report_jctl().
> We should also think about having utrace_do_stop() change TASK_STOPPED into
> TASK_TRACED like the old ptrace_check_attach() does. That way SIGCONT just
> won't wake it up in the first place.
Yes, I thought about this too.
But until we kill the ->stopped flag as you suggested,
> It means exactly:
> ->state == TASK_TRACED ||
> (->exit_state && !(->utrace_flags & _UTRACE_DEATH_EVENTS))
we still have to clear ->stopped after wake up from TASK_STOPPED.
(this change also breaks ptrace_resume() but this is fixeable).
But personally I never liked the fact that ptrace_check_attach() does
s/TASK_STOPPED/TASK_TRACED/. Because I think this creates the unfixeable
ptrace && jctl-group-stop problems. The bookkeeping of ->group_stop_count
in utrace_stop() (and in ptrace_stop()) is just wrong and imho should be
removed. And do_signal_stop() should not take TASK_TRACED threads into
account.
This leads to another question which I was going to ask later, I'll
send another email.
Oleg.
