From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
"Metzger, Markus T" <markus.t.metzger@intel.com>,
Roland McGrath <roland@redhat.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH 4/4] reparent/untrace: do nothing if no childs/tracees
Date: Wed, 11 Feb 2009 22:12:24 +0100 [thread overview]
Message-ID: <20090211211224.GA16860@redhat.com> (raw)
forget_original_parent() and exit_ptrace() can avoid taking the global
tasklist_lock if there are no childs/tracees. But I failed to invent
the comment to explain why/when this is safe to do, that is why the
separate patch/changelog.
The problem is, we can race with the concurrent release_task() which
can remove the last child form our ->children/ptraced list. This means
that list_empty() can return the "false" positive, it is possible that
release_task() is still in progress, it can use the caller's task_struct
somehow, and it is even possible that list_del(sibling/ptrace_entry)
has not yet completed.
But this is fine, before our task_struct will be released we will take
tasklist_lock at least once in release_task(), this will synchronize us
with the possible release_task/ptrace_unlink in flight.
However, forget_original_parent() has another problem. We can race with
another thread which has already picked us for reparenting before we set
PF_EXITING, so this patch also checks thread_group_empty().
It is possible to be more clever, we can take tasklist for reading, or
ensure that ->thread_group.prev is not PF_EXITING, but this is nasty.
Perhaps even this optimization is too ugly.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
--- 6.29-rc3/kernel/exit.c~4_TASKLIST 2009-02-11 07:20:54.000000000 +0100
+++ 6.29-rc3/kernel/exit.c 2009-02-11 21:25:35.000000000 +0100
@@ -803,6 +803,16 @@ static void forget_original_parent(struc
struct task_struct *p, *n, *reaper;
LIST_HEAD(dead_childs);
+ if (thread_group_empty(father)) {
+ /*
+ * Make sure no other thread can reparent to
+ * us after the list_empty(->children) check.
+ */
+ smp_rmb();
+ if (list_empty(&father->children))
+ return;
+ }
+
write_lock_irq(&tasklist_lock);
reaper = find_new_reaper(father);
--- 6.29-rc3/kernel/ptrace.c~4_TASKLIST 2009-02-11 04:04:17.000000000 +0100
+++ 6.29-rc3/kernel/ptrace.c 2009-02-11 08:27:41.000000000 +0100
@@ -323,6 +323,9 @@ void exit_ptrace(struct task_struct *tra
struct task_struct *p, *n;
LIST_HEAD(ptrace_dead);
+ if (list_empty(&tracer->ptraced))
+ return;
+
write_lock_irq(&tasklist_lock);
list_for_each_entry_safe(p, n, &tracer->ptraced, ptrace_entry) {
if (__ptrace_detach(tracer, p))
next reply other threads:[~2009-02-11 21:16 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-11 21:12 Oleg Nesterov [this message]
2009-02-11 21:14 ` [PATCH 4/4] reparent/untrace: do nothing if no childs/tracees Oleg Nesterov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090211211224.GA16860@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=markus.t.metzger@intel.com \
--cc=roland@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.