On 07/27, Roland McGrath wrote:
Ah. I forgot signals-tracehook_notify_jctl-change.patch is still pending in
-mm.
Perhaps we should just rejigger all these together into one new patch (or
two, whatever) before akpm submits any of them.
Or you can just merge these 2 patches, perhaps this
Perhaps we need more changes here, but I think this one is most important.
Agreed, we definitely do. For the generic case it needs to complete what
do_signal_stop would do. That is, do_notify_parent_cldstop and maybe also:
current-exit_code = sig-group_exit_code;
Though it might be now
Questions: should I send the change in signal.c to Andrew right now?
It can be applied separetely, it doesn't break utrace_report_jctl().
Yes, send it now. It might be best just to send a replacement
for signals-tracehook_notify_jctl-change.patch that rolls this in.
I wouldn't mention
Introduce the empty inline tracehook_finish_jctl() helper called by
do_signal_stop() after wakeup.
Currently we lack the ability to report this state change.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
include/linux/tracehook.h |9 +
kernel/signal.c |2 ++
2
Now that we always clear -stopped in utrace_finish_jctl(), we can cleanup
utrace_report_jctl() and utrace_get_signal(), they do not need to worry
about -stopped == T any longer.
Note that the change in utrace_report_jctl() removes start_report()'s work
too. Contrary to what the comment says,
utrace_do_stop() sets utrace-stopped but leaves the tracee in TASK_STOPPED
state. This means SIGCONT can wake up the tracee and fool the tracer.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/utrace.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
---
Introduce the empty inline tracehook_finish_jctl() helper called by
do_signal_stop() after wakeup.
You could possibly roll this into the tracehook_notify_jctl rework too.
Or send it upstream separately, either way.
+/**
+ * tracehook_finish_jctl - report about return from job control stop
+ * While in TASK_STOPPED, we can be considered safely
+ * stopped by utrace_do_stop(). Clear -stopped if we
+ * were woken by signal.
s/signal/SIGKILL/, no? It's not really while in TASK_STOPPED any more,
it's that utrace_do_stop could have changed TASK_STOPPED to TASK_TRACED.
utrace_do_stop() sets utrace-stopped but leaves the tracee in TASK_STOPPED
state. This means SIGCONT can wake up the tracee and fool the tracer.
IMHO this one belongs before 2/4 and 3/4 and all the comments changed to
reflect that SIGKILL is the only case. But the incremental order really
On 07/28, Roland McGrath wrote:
+* While in TASK_STOPPED, we can be considered safely
+* stopped by utrace_do_stop(). Clear -stopped if we
+* were woken by signal.
s/signal/SIGKILL/, no? It's not really while in TASK_STOPPED any more,
it's that utrace_do_stop could have
On 07/28, Roland McGrath wrote:
utrace_do_stop() sets utrace-stopped but leaves the tracee in TASK_STOPPED
state. This means SIGCONT can wake up the tracee and fool the tracer.
IMHO this one belongs before 2/4 and 3/4 and all the comments changed to
reflect that SIGKILL is the only case.
I'd prefer to send it separately, just to avoid the how this hook is
connected to subtle changes in do_signal_stop questions/discussion.
Fine, it's up to you and Andrew.
Hmm... shouldn't we move this comment above schedule() ? And, perhaps,
/* Now we don't run again until continued
OK, I'll send the first patch upstream, then I re-send 2-4 with updated
comments to you.
Ok, good. When I get those I'll merge those upstream-bound ones into the
tracehook git branch (now just signals-tracehook_notify_jctl-change.patch)
and merge the utrace changes into the utrace branch.
On 07/28, Roland McGrath wrote:
Questions: should I send the change in signal.c to Andrew right now?
It can be applied separetely, it doesn't break utrace_report_jctl().
Yes, send it now.
OK, will do
It might be best just to send a replacement
for
On 07/28, Roland McGrath wrote:
OK, I'll send the first patch upstream, then I re-send 2-4 with updated
comments to you.
Ok, good. When I get those I'll merge those upstream-bound ones into the
tracehook git branch (now just signals-tracehook_notify_jctl-change.patch)
and merge the
Damn. Please ignore.
Somehow I missed the fact that the patch below changes both tracehooks
and utrace.c, it is not possible to send the tracehook part to Andrew.
But I think it would be really nice to do this cleanup before sending
the next version of utrace.
Oleg.
On 07/29, Oleg Nesterov
But I think it would be really nice to do this cleanup before sending
the next version of utrace.
Let's get through all the utrace cleanups we know we need before we get there.
Thanks,
Roland
Roland, I'd prefer to send this change separately. Otherwise it will
be really hard to review the unified patch.
Do it however you think is best for getting all the generic signal.c and
tracehook.h bits merged ASAP.
And I think this will complicate its way to Linus's tree. I _think_
that
Andrew, this is on top of
signals-tracehook_notify_jctl-change.patch
and in fact it can be folded into that patch.
Or I can send the unified patch as a replacement, whatever is more
convenient to you.
But, to clarify, the patch above is correct, this one just tries to
improve things,
do_signal_stop() can call tracehook_notify_jctl() before decrementing
-group_stop_count and setting TASK_STOPPED/SIGNAL_STOP_STOPPED.
This way the tracing hooks can drop and reacquire the siglock freely
and do any blocking hooks without potential SIGCONT races.
With this patch
(textually depends on signals-tracehook_notify_jctl-change.patch)
Introduce the empty inline tracehook_finish_jctl() helper called by
do_signal_stop() after wakeup.
Currently we lack the ability to report this state change.
Also fix the comment, it should be placed before schedule().
21 matches
Mail list logo
