All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Aaron Tomlin <atomlin@redhat.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Rik van Riel <riel@redhat.com>,
	Sterling Alexander <stalexan@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/5] exit: wait: don't use zombie->real_parent
Date: Fri, 14 Nov 2014 02:38:18 +0100	[thread overview]
Message-ID: <20141114013818.GA5119@redhat.com> (raw)
In-Reply-To: <20141114013750.GA4697@redhat.com>

1. wait_task_zombie() uses p->real_parent to get psig/siglock. This is
   correct but needs tasklist_lock, ->real_parent can exit.

   We can use "current" instead. This is our natural child, its parent
   must be our sub-thread.

2. Read psig/sig outside of ->siglock, ->signal is no longer protected
   by this lock.

3. Fix the outdated comments about tasklist_lock. We can not race with
   __exit_signal(), the whole thread group is dead, nobody but us can
   call it.

   Also clarify the usage of ->stats_lock and ->siglock.

Note: thread_group_cputime_adjusted() is sub-optimal in this case, we
probably want to export cputime_adjust() to avoid thread_group_cputime().
The comment says "all threads" but there are no other threads.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/exit.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 0511f1d..40d53de 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1008,8 +1008,8 @@ static int wait_task_zombie(struct wait_opts *wo, struct task_struct *p)
 	 * Check thread_group_leader() to exclude the traced sub-threads.
 	 */
 	if (state == EXIT_DEAD && thread_group_leader(p)) {
-		struct signal_struct *psig;
-		struct signal_struct *sig;
+		struct signal_struct *sig = p->signal;
+		struct signal_struct *psig = current->signal;
 		unsigned long maxrss;
 		cputime_t tgutime, tgstime;
 
@@ -1021,21 +1021,20 @@ static int wait_task_zombie(struct wait_opts *wo, struct task_struct *p)
 		 * accumulate in the parent's signal_struct c* fields.
 		 *
 		 * We don't bother to take a lock here to protect these
-		 * p->signal fields, because they are only touched by
-		 * __exit_signal, which runs with tasklist_lock
-		 * write-locked anyway, and so is excluded here.  We do
-		 * need to protect the access to parent->signal fields,
-		 * as other threads in the parent group can be right
-		 * here reaping other children at the same time.
+		 * p->signal fields because the whole thread group is dead
+		 * and nobody can change them.
+		 *
+		 * psig->stats_lock also protects us from our sub-theads
+		 * which can reap other children at the same time. Until
+		 * we change k_getrusage()-like users to rely on this lock
+		 * we have to take ->siglock as well.
 		 *
 		 * We use thread_group_cputime_adjusted() to get times for
 		 * the thread group, which consolidates times for all threads
 		 * in the group including the group leader.
 		 */
 		thread_group_cputime_adjusted(p, &tgutime, &tgstime);
-		spin_lock_irq(&p->real_parent->sighand->siglock);
-		psig = p->real_parent->signal;
-		sig = p->signal;
+		spin_lock_irq(&current->sighand->siglock);
 		write_seqlock(&psig->stats_lock);
 		psig->cutime += tgutime + sig->cutime;
 		psig->cstime += tgstime + sig->cstime;
@@ -1060,7 +1059,7 @@ static int wait_task_zombie(struct wait_opts *wo, struct task_struct *p)
 		task_io_accounting_add(&psig->ioac, &p->ioac);
 		task_io_accounting_add(&psig->ioac, &sig->ioac);
 		write_sequnlock(&psig->stats_lock);
-		spin_unlock_irq(&p->real_parent->sighand->siglock);
+		spin_unlock_irq(&current->sighand->siglock);
 	}
 
 	/*
-- 
1.5.5.1


  parent reply	other threads:[~2014-11-14  1:39 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-07 20:14 [PATCH 0/4] proc: deuglify task_state() Oleg Nesterov
2014-11-07 20:14 ` [PATCH 1/4] proc: task_state: read cred->group_info outside of task_lock() Oleg Nesterov
2014-11-07 20:14 ` [PATCH 2/4] proc: task_state: deuglify the max_fds calculation Oleg Nesterov
2014-11-07 20:14 ` [PATCH 3/4] proc: task_state: move the main seq_printf() outside of rcu_read_lock() Oleg Nesterov
2014-11-13 18:04   ` Paul E. McKenney
2014-11-07 20:14 ` [PATCH 4/4] proc: task_state: ptrace_parent() doesn't need pid_alive() check Oleg Nesterov
2014-11-10 21:59 ` [PATCH 0/5] exit: reparent zombie fix + cleanups/optimizations Oleg Nesterov
2014-11-10 22:00   ` [PATCH 1/5] sched_show_task: fix unsafe usage of ->real_parent Oleg Nesterov
2014-11-11 10:39     ` Peter Zijlstra
2014-11-10 22:00   ` [PATCH 2/5] exit: reparent: use ->ptrace_entry rather than ->sibling for EXIT_DEAD tasks Oleg Nesterov
2014-11-10 22:00   ` [PATCH 3/5] exit: reparent: cleanup the changing of ->parent Oleg Nesterov
2014-11-10 22:00   ` [PATCH 4/5] exit: reparent: cleanup the usage of reparent_leader() Oleg Nesterov
2014-11-10 22:00   ` [PATCH 5/5] exit: ptrace: shift "reap dead" code from exit_ptrace() to forget_original_parent() Oleg Nesterov
2014-11-14  1:37 ` [PATCH 0/5] exit: more cleanups/optimizations Oleg Nesterov
2014-11-14  1:38   ` [PATCH 1/5] exit: wait: cleanup the ptrace_reparented() checks Oleg Nesterov
2014-11-14  1:38   ` Oleg Nesterov [this message]
2014-11-14  1:38   ` [PATCH 3/5] exit: wait: drop tasklist_lock before psig->c* accounting Oleg Nesterov
2014-11-14  1:38   ` [PATCH 4/5] exit: release_task: fix the comment about group leader accounting Oleg Nesterov
2014-11-14  1:38   ` [PATCH 5/5] exit: proc: don't try to flush /proc/tgid/task/tgid Oleg Nesterov
2014-11-18 21:29 ` [PATCH 0/6] exit: find_new_reaper() fixes/cleanups Oleg Nesterov
2014-11-18 21:30   ` [PATCH 1/6] exit: reparent: fix the dead-parent PR_SET_CHILD_SUBREAPER reparenting Oleg Nesterov
2014-11-18 21:30   ` [PATCH 2/6] exit: reparent: fix the cross-namespace " Oleg Nesterov
2014-11-18 21:30   ` [PATCH 3/6] exit: reparent: s/while_each_thread/for_each_thread/ in find_new_reaper() Oleg Nesterov
2014-11-18 21:30   ` [PATCH 4/6] exit: reparent: document the ->has_child_subreaper checks Oleg Nesterov
2014-11-18 21:30   ` [PATCH 5/6] exit: reparent: introduce find_child_reaper() Oleg Nesterov
2014-11-18 21:30   ` [PATCH 6/6] exit: reparent: introduce find_alive_thread() Oleg Nesterov
2014-11-20 18:34 ` [PATCH 0/3] exit: avoid O(n ** 2) thread-list scan on group-exit if possible Oleg Nesterov
2014-11-20 18:34   ` [PATCH -mm 1/3] exit: reparent: avoid find_new_reaper() if no children Oleg Nesterov
2014-11-20 22:37     ` Andrew Morton
2014-11-21 20:01       ` Oleg Nesterov
2014-11-20 18:34   ` [PATCH -mm 2/3] exit: reparent: call forget_original_parent() under tasklist_lock Oleg Nesterov
2014-11-20 18:34   ` [PATCH -mm 3/3] exit: exit_notify: re-use "dead" list to autoreap current Oleg Nesterov
2014-11-24 20:06 ` [PATCH 0/2] exit/pid_ns: comments + simple fix Oleg Nesterov
2014-11-24 20:06   ` [PATCH 1/2] exit: reparent: fix/update the comments in zap_pid_ns_processes() Oleg Nesterov
2014-11-24 20:14     ` Oleg Nesterov
2014-11-24 22:07     ` Eric W. Biederman
2014-11-25 16:57       ` Oleg Nesterov
2014-11-25 17:17         ` Oleg Nesterov
2014-11-24 20:06   ` [PATCH 2/2] exit: pidns: alloc_pid() leaks pid_namespace if child_reaper is exiting Oleg Nesterov
2014-11-24 21:46     ` Eric W. Biederman
2014-11-25 17:07       ` Oleg Nesterov
2014-11-25 17:50         ` Eric W. Biederman
2014-11-25 18:15           ` Oleg Nesterov
2014-11-25 18:43             ` Eric W. Biederman
2014-11-25 18:59               ` Oleg Nesterov
2014-11-24 21:27   ` [PATCH 0/2] exit/pid_ns: comments + simple fix Eric W. Biederman
2014-11-24 21:38     ` Oleg Nesterov
2014-11-24 21:48   ` Eric W. Biederman
2014-11-25 16:57     ` Oleg Nesterov
2014-11-26 23:54   ` [PATCH v2 " Oleg Nesterov
2014-11-26 23:54     ` [PATCH v2 1/2] exit: pidns: alloc_pid() leaks pid_namespace if child_reaper is exiting Oleg Nesterov
2014-11-27 15:44       ` Eric W. Biederman
2014-11-26 23:54     ` [PATCH v2 2/2] exit: pidns: fix/update the comments in zap_pid_ns_processes() Oleg Nesterov
2014-12-01 22:39       ` Andrew Morton
2014-12-01 23:24         ` 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=20141114013818.GA5119@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=atomlin@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=riel@redhat.com \
    --cc=stalexan@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.