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);
next 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.