All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Balbir Singh <balbir@linux.vnet.ibm.com>,
	Americo Wang <xiyou.wangcong@gmail.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>,
	Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <peterz@infradead.org>,
	Roland McGrath <roland@redhat.com>,
	Spencer Candland <spencer@bluehost.com>,
	Stanislaw Gruszka <sgruszka@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH -mm 1/4] cputimers: thread_group_cputime: cleanup rcu/signal stuff
Date: Mon, 29 Mar 2010 20:12:38 +0200	[thread overview]
Message-ID: <20100329181238.GB16356@redhat.com> (raw)
In-Reply-To: <20100329181204.GA16356@redhat.com>

- With the recent changes tsk->signal is stable, we can read it
  first and avoid the initialization from INIT_CPUTIME.

- Even if tsk->signal is always valid, we still have to check it
  is safe to use next_thread() under rcu_read_lock(). Currently
  the code checks ->sighand != NULL, change it to use pid_alive()
  which is commonly used to ensure the task wasn't unhashed before
  we take rcu_read_lock().

- Change the main loop to use the while_each_thread() helper.

This change itself doesn't add the functional changes, currently
it is always called under tasklist/siglock or when we know that
this thread group is already dead (wait, coredump).

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 kernel/posix-cpu-timers.c |   21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

--- 34-rc1/kernel/posix-cpu-timers.c~cpuacct_1_thread_group_cputime_cleanup_rcu	2010-03-29 18:08:14.000000000 +0200
+++ 34-rc1/kernel/posix-cpu-timers.c	2010-03-29 18:09:15.000000000 +0200
@@ -233,31 +233,24 @@ static int cpu_clock_sample(const clocki
 
 void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
 {
-	struct sighand_struct *sighand;
-	struct signal_struct *sig;
+	struct signal_struct *sig = tsk->signal;
 	struct task_struct *t;
 
-	*times = INIT_CPUTIME;
+	times->utime = sig->utime;
+	times->stime = sig->stime;
+	times->sum_exec_runtime = sig->sum_sched_runtime;
 
 	rcu_read_lock();
-	sighand = rcu_dereference(tsk->sighand);
-	if (!sighand)
+	/* make sure we can trust tsk->thread_group list */
+	if (!likely(pid_alive(tsk)))
 		goto out;
 
-	sig = tsk->signal;
-
 	t = tsk;
 	do {
 		times->utime = cputime_add(times->utime, t->utime);
 		times->stime = cputime_add(times->stime, t->stime);
 		times->sum_exec_runtime += t->se.sum_exec_runtime;
-
-		t = next_thread(t);
-	} while (t != tsk);
-
-	times->utime = cputime_add(times->utime, sig->utime);
-	times->stime = cputime_add(times->stime, sig->stime);
-	times->sum_exec_runtime += sig->sum_sched_runtime;
+	} while_each_thread(tsk, t);
 out:
 	rcu_read_unlock();
 }


  reply	other threads:[~2010-03-29 18:15 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-24 20:45 [RFC,PATCH 1/2] cputimers/proc: do_task_stat()->task_times() can race with getrusage() Oleg Nesterov
2010-03-26  3:53 ` Balbir Singh
2010-03-26  7:37   ` Stanislaw Gruszka
2010-03-26 16:12     ` Stanislaw Gruszka
2010-03-26 21:49   ` Oleg Nesterov
2010-03-29 11:17     ` Stanislaw Gruszka
2010-03-29 12:54       ` Oleg Nesterov
2010-03-29 18:12         ` [PATCH -mm 0/4] cputimers/proc: do_task_stat: don't walk through the thread list under ->siglock Oleg Nesterov
2010-03-29 18:12           ` Oleg Nesterov [this message]
2010-03-29 18:13           ` [PATCH -mm 2/4] cputimers: make sure thread_group_cputime() can't count the same thread twice lockless Oleg Nesterov
2010-03-30 11:01             ` Stanislaw Gruszka
2010-03-30 13:43               ` Oleg Nesterov
2010-03-29 18:13           ` [PATCH -mm 3/4] cputimers: thread_group_times: make it rcu-safe Oleg Nesterov
2010-03-29 18:14           ` [PATCH -mm 1/4] cputimers: do_task_stat: avoid ->siglock for while_each_thread() Oleg Nesterov
2010-03-29 18:16             ` 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=20100329181238.GB16356@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=roland@redhat.com \
    --cc=seto.hidetoshi@jp.fujitsu.com \
    --cc=sgruszka@redhat.com \
    --cc=spencer@bluehost.com \
    --cc=xiyou.wangcong@gmail.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.