* Re: + ptrace-simplify-ptrace_exit-ignoring_children-path.patch added to -mm tree
[not found] <200901290843.n0T8hpmi002940@imap1.linux-foundation.org>
@ 2009-01-29 16:40 ` Oleg Nesterov
0 siblings, 0 replies; only message in thread
From: Oleg Nesterov @ 2009-01-29 16:40 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, dvlasenk, jmarchan, roland
On 01/29, Andrew Morton wrote:
>
> The patch titled
> ptrace: simplify ptrace_exit()->ignoring_children() path
> has been added to the -mm tree. Its filename is
> ptrace-simplify-ptrace_exit-ignoring_children-path.patch
> ------------------------------------------------------
> Subject: ptrace: simplify ptrace_exit()->ignoring_children() path
> From: Oleg Nesterov <oleg@redhat.com>
>
> ignoring_children() takes parent->sighand->siglock and checks
> k_sigaction[SIGCHLD] atomically. But this buys nothing, we can't get the
> "really" wrong result even if we race with sigaction(SIGCHLD). If we read
> the "stale" sa_handler/sa_flags we can pretend it was changed right after
> the check.
I am stupid.
Somehow I didn't even try to think about false _negative_, and it is
possible without ->siglock.
If we race with sigaction() which changes ->action[SIGCHLD]
from: { .sa_handler = SIG_IGN, .sa_flags = 0 }
to: { .sa_handler = handler, .sa_flags = SA_NOCLDWAIT }
we can read the stores out of order, and ignoring_children() may
return the wrong "false". Nothing really bad can happen, and the
race is mostly theoretical, but still this is not right.
Andrew, please replace
ptrace-simplify-ptrace_exit-ignoring_children-path.patch
with the new version below.
I'll also re-send the next one,
ptrace-reintroduce-__ptrace_detach-as-a-callee-of-ptrace_exit.patch
because it should be re-diffed, and because I forgot to update the comments.
Other patches I sent can be applied cleanly.
Sorry for inconvenience!
--------------------------------------------------------------------
[PATCH 2/4] ptrace: simplify ptrace_exit()->ignoring_children() path
In order to avoid the unnecessary ignoring_children() calls, ptrace_exit()
uses "int ign" to cache the returned value.
This makes sense because ptrace_exit() works with the list of tracees, but
the next patche changes this code to work with individual threads.
Remove this optimization, I don't think it is common to exit with a lot of
attached EXIT_ZOMBIE tracees. And this makes the code a bit simpler.
Also remove _irqsave/_irqrestore, this code always runs with irqs disabled.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
--- 6.29-rc3/kernel/exit.c~2_IGN_SIGCHLD 2009-01-19 10:44:33.000000000 +0100
+++ 6.29-rc3/kernel/exit.c 2009-01-29 16:40:38.000000000 +0100
@@ -729,19 +729,15 @@ static void exit_mm(struct task_struct *
}
/*
- * Return nonzero if @parent's children should reap themselves.
- *
- * Called with write_lock_irq(&tasklist_lock) held.
+ * Called with irqs disabled, returns true if childs should reap themselves.
*/
-static int ignoring_children(struct task_struct *parent)
+static int ignoring_children(struct sighand_struct *sigh)
{
int ret;
- struct sighand_struct *psig = parent->sighand;
- unsigned long flags;
- spin_lock_irqsave(&psig->siglock, flags);
- ret = (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
- (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT));
- spin_unlock_irqrestore(&psig->siglock, flags);
+ spin_lock(&sigh->siglock);
+ ret = (sigh->action[SIGCHLD-1].sa.sa_handler == SIG_IGN) ||
+ (sigh->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT);
+ spin_unlock(&sigh->siglock);
return ret;
}
@@ -754,7 +750,6 @@ static int ignoring_children(struct task
static void ptrace_exit(struct task_struct *parent, struct list_head *dead)
{
struct task_struct *p, *n;
- int ign = -1;
list_for_each_entry_safe(p, n, &parent->ptraced, ptrace_entry) {
__ptrace_unlink(p);
@@ -776,12 +771,8 @@ static void ptrace_exit(struct task_stru
if (!task_detached(p) && thread_group_empty(p)) {
if (!same_thread_group(p->real_parent, parent))
do_notify_parent(p, p->exit_signal);
- else {
- if (ign < 0)
- ign = ignoring_children(parent);
- if (ign)
- p->exit_signal = -1;
- }
+ else if (ignoring_children(parent->sighand))
+ p->exit_signal = -1;
}
if (task_detached(p)) {
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2009-01-29 16:43 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <200901290843.n0T8hpmi002940@imap1.linux-foundation.org>
2009-01-29 16:40 ` + ptrace-simplify-ptrace_exit-ignoring_children-path.patch added to -mm tree Oleg Nesterov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).