1. SIGKILL can't be blocked, remove this check from sigkill_pending().
2. When ptrace_stop() sees sigkill_pending() == T, it can just return.

   Kill "int killed" and simplify the code. This also is more correct,

   the tracer shouldn't see us in TASK_TRACED if we are not going to

   stop.
I strongly believe this code needs further changes. We should do the

"was this task killed" check unconditionally, currently it depends on

arch_ptrace_stop_needed(). On the other hand, sigkill_pending() isn't

very clever. If the task was killed tkill(SIGKILL), the signal can be

already dequeued if the caller is do_exit().
Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
--- 26-rc2/kernel/signal.c~PT_KILL	2008-06-28 17:06:58.000000000 +0400

+++ 26-rc2/kernel/signal.c	2008-07-06 19:29:27.000000000 +0400

@@ -1498,9 +1498,8 @@ static inline int may_ptrace_stop(void)

  */

 static int sigkill_pending(struct task_struct *tsk)

 {

-	return ((sigismember(&tsk->pending.signal, SIGKILL) ||

-		 sigismember(&tsk->signal->shared_pending.signal, SIGKILL)) &&

-		!unlikely(sigismember(&tsk->blocked, SIGKILL)));

+	return	sigismember(&tsk->pending.signal, SIGKILL) ||

+		sigismember(&tsk->signal->shared_pending.signal, SIGKILL);

 }
 /*

@@ -1516,8 +1515,6 @@ static int sigkill_pending(struct task_s

  */

 static void ptrace_stop(int exit_code, int clear_code, siginfo_t *info)

 {

-	int killed = 0;

-

 	if (arch_ptrace_stop_needed(exit_code, info)) {

 		/*

 		 * The arch code has something special to do before a

@@ -1533,7 +1530,8 @@ static void ptrace_stop(int exit_code, i

 		spin_unlock_irq(&current->sighand->siglock);

 		arch_ptrace_stop(exit_code, info);

 		spin_lock_irq(&current->sighand->siglock);

-		killed = sigkill_pending(current);

+		if (sigkill_pending(current))

+			return;

 	}
 	/*

@@ -1550,7 +1548,7 @@ static void ptrace_stop(int exit_code, i

 	__set_current_state(TASK_TRACED);

 	spin_unlock_irq(&current-&gt...
[ message continues ]

Sorry I haven't replied sooner to your related patches about this recently.
I would rather not futz around the margins with the arch_ptrace_stop_needed

case.  That code path exists only on ia64.  Let's not worry about the

details of its current form.  We can just hash out the whole subject and

this bit will wind up irrelevant or quite different.
To give some perspective, the arch_ptrace_stop_needed path was added

(recently) for ia64 where there was a need to unlock the siglock where it

wasn't unlocked before.  Unlike the pure signals races, this new ia64 case

potentially works/blocks for a relatively long time (doing page faults)

while leaving the siglock unlocked in the path that always before held it

throughout.  The bad situation with a missed SIGKILL actually came up on

ia64 when someone tried adding the unlock+arch_ptrace_stop+lock sequence

without the extra check.  So the "killed" logic was added with a narrow

focus to redress just that case, and keep ia64 no worse than the status quo

of many years.  (The ia64 path can have real blocks, whereas any other

problems on this path have never been possible in uniprocessor no-preempt

kernels, for example.)
When you sent your earlier patches, something I didn't find immediately

clear was the exact list of problem scenarios you were considering.  So,

let's look at the whole picture.
The two paths into ptrace_stop() are ptrace_signal() and ptrace_notify().
In the ptrace_signal() path, the siglock is held throughout.  If someone

comes along later to post a SIGKILL, it will happen after ptrace_stop()

enters TASK_TRACED and signal_wake_up() will do the right thing.  The

problem scenario is when the SIGKILL was already posted before we took the

siglock, but ptrace_signal() gets called on a different signal first, i.e.

one numbered < 9 is dequeued first.
In the ptrace_notify() path, we just took the siglock before ptrace_stop().

There can have been a SIGKILL pending before that, and we'll stop anyway.

The paths into ptrace_notif...
[ message continues ]

Hmm. Yes, the execing thread will block, but why this is wrong? It waits
Just to be sure I got this right, you mean the fatal signal but not SIGKILL,
This differs from the case above just because the signal is
Agreed, I also thought about that. Not that it matters, but we can safely

do this after the group_stop_count check, or we can change dequeue_signal()
Well, this is OK for ptrace (but see below), but doesn't look right

in general. complete_signal() sets this flag only for SIGKILL, what

about other fatal signals which cause SIGNAL_GROUP_EXIT? In that

case fatal_signal_pending() must be true as well.
The same for sys_exit_group(), it also must set this flag. So I can't

understand how SIGNAL_GROUP_KILLED can differ from signal_group_exit().

Apart from optimization, of course.
But signal_group_exit() doesn't suit for ptrace_stop() case.
(off-topic: any sleep after exit_notify() is not good, CPU_DEAD can't
Agreed. In fact I have already thought about changing fatal_signal_pending()

to use signal_group_exit(), see http://marc.info/?l=linux-kernel&m=121267953524149

This also fixes the issues with /sbin/init. But again, this is not good

for PT_TRACE_EXIT.
And I was worried about schedule in TASK_KILLABLE after exit_notify().
Well yes, but PT_TRACE_EXIT is special again. signal_pending() can be
Completely agreed, I was going to do this from the very beginning.

And __down_common() too.
Oleg.
--