On 11/18, Roland McGrath wrote:
>
> So maybe you will look at this and merge them before I do.
Somehow I can't really understand this patch. I hope more or less
I can see what it does, but the resulting code looks even more
subtle to me.
OK, probably I need to re-read this patch tomorrow, I can never
understand the code after the first reading.
With this patch, apply_resume_action() is always called after
utrace_stop(). Well, except for utrace_report_exit(), but I guess
it could do apply_resume_action() too.
Now it is not clear why utrace_control(SINGLESTEP) sets
TIF_NOTIFY_RESUME, it is not needed after apply_resume_action()
processes ->resume. Yes, we have jctl stops, but we can move
this code into utrace_finish_stop().
It is not clear to me why apply_resume_action(UTRACE_INTERRUPT) does
set_tsk_thread_flag(TIF_SIGPENDING). In fact I don't understand why
apply_resume_action() checks UTRACE_INTERRUPT at all. If it is called
after utrace_stop(), action == start_report() which never resets
UTRACE_INTERRUPT.
And since we process ->resume after stop, it is not clear why we
set ->resume = report->resume_action before stop, apply_resume_action()
could use min(report->resume_action, utrace->resume). Yes, yes,
we have the nasty utrace->vfork_stop case, I mean it is not easy to
understand the logic behind all these ->resume changes.
And afaics, this can't help to fix the tracehook_report_syscall_exit()
&& TIF_SINGLESTEP problems in the multitracing case, UTRACE_INTERRUPT
"destroys" UTRACE_SINGLESTEP. Ptrace can reassert it later, but this
will be too late, the trace has already passed this tracehook.
> static void utrace_stop(struct task_struct *task, struct utrace *utrace,
> - enum utrace_resume_action action)
> + enum utrace_resume_action action, bool skip_notify)
> {
> ...
> - else
> + else if (action == UTRACE_REPORT || !skip_notify)
> set_thread_flag(TIF_NOTIFY_RESUME);
I don't understand this skip_notify, utrace_stop() is always called
with skip_notify == true?
Oleg.
