All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Americo Wang <xiyou.wangcong@gmail.com>,
	Balbir Singh <balbir@in.ibm.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>
Cc: linux-kernel@vger.kernel.org
Subject: [RFC,PATCH 2/2] cputimers/proc: do_task_stat()->thread_group_times() is racy and O(n) under ->siglock
Date: Wed, 24 Mar 2010 21:45:54 +0100	[thread overview]
Message-ID: <20100324204554.GA31780@redhat.com> (raw)

Nowadays ->siglock is overloaded, it would be really nice to change
do_task_stat() to walk through the list of threads lockless. And note
that we are doing while_each_thread() twice!

while_each_thread() is rcu-safe, but thread_group_times() also needs
->siglock to serialize the modifications of signal_struct->prev_Xtime
members.

(however, please note that currently do_task_stat() can race with
 wait_task_zombie() which calls thread_group_times() without siglock).

This patch changes the code back to use thread_group_cputime(), as we
did before 0cf55e1e "sched, cputime: Introduce thread_group_times()".

Of course, this makes the output from /proc/pid/stat less accurate, but
otoh this allows us to make do_task_stat() (apart from ->tty bits).

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

 fs/proc/array.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- 34-rc1/fs/proc/array.c~PROC_5_DTS_TGTS_IS_PITA	2010-03-24 20:47:19.000000000 +0100
+++ 34-rc1/fs/proc/array.c	2010-03-24 20:47:51.000000000 +0100
@@ -403,6 +403,7 @@ static int do_task_stat(struct seq_file 
 
 	if (lock_task_sighand(task, &flags)) {
 		struct signal_struct *sig = task->signal;
+		struct task_cputime cputime;
 
 		if (sig->tty) {
 			struct pid *pgrp = tty_get_pgrp(sig->tty);
@@ -433,8 +434,11 @@ static int do_task_stat(struct seq_file 
 
 			min_flt += sig->min_flt;
 			maj_flt += sig->maj_flt;
-			thread_group_times(task, &utime, &stime);
+
 			gtime = cputime_add(gtime, sig->gtime);
+			thread_group_cputime(task, &cputime);
+			utime = cputime.utime;
+			stime = cputime.stime;
 		}
 
 		sid = task_session_nr_ns(task, ns);


             reply	other threads:[~2010-03-24 20:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-24 20:45 Oleg Nesterov [this message]
2010-03-24 20:54 ` [RFC,PATCH 2/2] cputimers/proc: do_task_stat()->thread_group_times() is racy and O(n) under ->siglock Peter Zijlstra
2010-03-25 12:12   ` Oleg Nesterov
2010-03-25 12:19     ` Peter Zijlstra
2010-03-26  7:59     ` Stanislaw Gruszka

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=20100324204554.GA31780@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=balbir@in.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.