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 would be better.
>
> As long as we have akpm drop/update the any conflicting ones, sure.
OK, please find the patch below. changes: add the comment as you suggested.
> > Confused, signals-tracehook_notify_jctl-change.patch already updated the
> > comment?
>
> I was looking at the Linus tree, not the -mm tree (I always do).
> If that prerequisite patch (or merged rejiggered patch or whatever)
> updates the comments that is fine.
Yes it does (and just in case, this patch is applied to your
linux-2.6-utrace.git).
> I was also just noticing the other
> callers of tracehook_notify_jctl, which should all be rejiggered as makes
> most sense for the locking change (maybe that pending patch already did that).
Yes, it did.
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().
Should I send utrace part to Andrew too? This is not strictly needed.
Oleg.
---------------------------------------------------------------------
[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 | 40 ++++++++++++++++++----------------------
kernel/utrace.c | 20 --------------------
2 files changed, 18 insertions(+), 42 deletions(-)
--- __UTRACE/kernel/signal.c~JCTL 2009-07-28 01:37:58.000000000 +0200
+++ __UTRACE/kernel/signal.c 2009-07-28 21:39:31.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,28 @@ 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.
+ * 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);
+ /*
+ * tracehook_notify_jctl() can drop and reacquire siglock, so
+ * we keep ->group_stop_count != 0 before the call. If SIGCONT
+ * or SIGKILL comes in between ->group_stop_count == 0.
+ */
+ 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-07-28 20:55:01.000000000 +0200
+++ __UTRACE/kernel/utrace.c 2009-07-28 20:56:32.000000000 +0200
@@ -1607,18 +1607,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);
/*
@@ -1647,16 +1636,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;
- }
}
/*
