On 07/27, Oleg Nesterov wrote:
>
> On 07/24, Roland McGrath wrote:
> >
> > 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().
Please find the patch below, compile tested.
Assuming you agree with this change... I don't know how it should be merged.
Probably the change in signal.c should be sent separately, but this breaks
-mm tree.
------------------------------------------------------------------------------
[PATCH] simplify do_signal_stop() && utrace_report_jctl() interaction
do_signal_stop() can call utrace_report_jctl() before decrementing
->group_stop_count and setting TASK_STOPPED/SIGNAL_STOP_STOPPED.
This allows to greatly simplify utrace_report_jctl() and avoid playing
with group-stop bookkeeping twice.
Signed-off-by: Oleg Nesterov <o...@redhat.com>
----
kernel/signal.c | 29 +++++++++++------------------
kernel/utrace.c | 20 --------------------
2 files changed, 11 insertions(+), 38 deletions(-)
--- __UTRACE/kernel/signal.c~JCTL 2009-07-13 17:44:27.000000000 +0200
+++ __UTRACE/kernel/signal.c 2009-07-28 01:20:17.000000000 +0200
@@ -1682,16 +1682,9 @@ void ptrace_notify(int exit_code)
static int do_signal_stop(int signr)
{
struct signal_struct *sig = current->signal;
- int stop_count;
int notify;
- if (sig->group_stop_count > 0) {
- /*
- * There is a group stop in progress. We don't need to
- * start another one.
- */
- stop_count = --sig->group_stop_count;
- } else {
+ if (!sig->group_stop_count) {
struct task_struct *t;
if (!likely(sig->flags & SIGNAL_STOP_DEQUEUED) ||
@@ -1703,7 +1696,7 @@ static int do_signal_stop(int signr)
*/
sig->group_exit_code = signr;
- stop_count = 0;
+ sig->group_stop_count = 1;
for (t = next_thread(current); t != current; t = next_thread(t))
/*
* Setting state to TASK_STOPPED for a group
@@ -1712,25 +1705,25 @@ static int do_signal_stop(int signr)
*/
if (!(t->flags & PF_EXITING) &&
!task_is_stopped_or_traced(t)) {
- stop_count++;
+ sig->group_stop_count++;
signal_wake_up(t, 0);
}
- sig->group_stop_count = stop_count;
}
- if (stop_count == 0)
- sig->flags = SIGNAL_STOP_STOPPED;
- current->exit_code = sig->group_exit_code;
- __set_current_state(TASK_STOPPED);
-
/*
* If there are no other threads in the group, or if there is
* a group stop in progress and we are the last to stop,
* report to the parent. When ptraced, every thread reports itself.
*/
- notify = tracehook_notify_jctl(stop_count == 0 ? CLD_STOPPED : 0,
- CLD_STOPPED);
+ notify = sig->group_stop_count == 1 ? CLD_STOPPED : 0;
+ notify = tracehook_notify_jctl(notify, CLD_STOPPED);
+ if (sig->group_stop_count) {
+ if (!--sig->group_stop_count)
+ sig->flags = SIGNAL_STOP_STOPPED;
+ current->exit_code = sig->group_exit_code;
+ __set_current_state(TASK_STOPPED);
+ }
spin_unlock_irq(¤t->sighand->siglock);
if (notify) {
--- __UTRACE/kernel/utrace.c~JCTL 2009-06-11 17:46:26.000000000 +0200
+++ __UTRACE/kernel/utrace.c 2009-07-28 01:20:17.000000000 +0200
@@ -1606,18 +1606,7 @@ void utrace_report_jctl(int notify, int
struct task_struct *task = current;
struct utrace *utrace = task_utrace_struct(task);
INIT_REPORT(report);
- bool stop = task_is_stopped(task);
- /*
- * We have to come out of TASK_STOPPED in case the event report
- * hooks might block. Since we held the siglock throughout, it's
- * as if we were never in TASK_STOPPED yet at all.
- */
- if (stop) {
- __set_current_state(TASK_RUNNING);
- task->signal->flags &= ~SIGNAL_STOP_STOPPED;
- ++task->signal->group_stop_count;
- }
spin_unlock_irq(&task->sighand->siglock);
/*
@@ -1646,16 +1635,7 @@ void utrace_report_jctl(int notify, int
REPORT(task, utrace, &report, UTRACE_EVENT(JCTL),
report_jctl, what, notify);
- /*
- * Retake the lock, and go back into TASK_STOPPED
- * unless the stop was just cleared.
- */
spin_lock_irq(&task->sighand->siglock);
- if (stop && task->signal->group_stop_count > 0) {
- __set_current_state(TASK_STOPPED);
- if (--task->signal->group_stop_count == 0)
- task->signal->flags |= SIGNAL_STOP_STOPPED;
- }
}
/*
