Re: [PATCH] ftrace: Don't use tracing_record_cmdline() in workqueue tracer
- From: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
- Date: Tue, 10 Mar 2009 09:57:02 +0900 (JST)
* KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx> wrote:
Currently, /sys/kernel/debug/tracing/trace_stat/workqueues can
display wrong and strang thread name.
Why?
Currently, ftrace has
tracing_record_cmdline()/trace_find_cmdline() convinience
function. they implement task->comm string cache. it can avoid
unnecessary memcpy overhead. and workqueue tracer use it.
However, in general, any trace stastics feature shouldn't use
tracing_record_cmdline(). A trace stastics can display very
old process. then comm cache can return wrong string because
recent process override the cache.
Fortunately, workqueue trace gerantee to live displayed
process. Then, we can search comm string from pid at display
time.
Applied, thanks!
We might need to improve the comm-cache - i've seen frequent
artifacts due to it. Displaying <...> is _far_ better than an
outright misleading string displayed.
I.e. the cache should be improved to be properly coherent with
reality.
Ingo
Doh!
My last mail's cc list didn't include lkml. that's unintensional silly
my mistake. very sorry.
and Yes, memcpy(buf, task->comm, 16) mean two movq instruction.
that is worthless for caching. I think we can remove this cache completely.
and, I find my last patch has one race window. fixing below.
================================
Subject: [PATCH] tracing: Don't use tracing_record_cmdline() in workqueue tracer fix
last "Don't use tracing_record_cmdline() in workqueue tracer" patch have
a race window.
find_task_by_vpid() require task_list_lock(). but the patch doesn't.
fixing here.
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
---
kernel/trace/trace_workqueue.c | 20 ++++++++++++++------
1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_workqueue.c b/kernel/trace/trace_workqueue.c
index 46c8dc8..e6c4d00 100644
--- a/kernel/trace/trace_workqueue.c
+++ b/kernel/trace/trace_workqueue.c
@@ -193,12 +193,20 @@ static int workqueue_stat_show(struct seq_file *s, void *p)
struct cpu_workqueue_stats *cws = p;
unsigned long flags;
int cpu = cws->cpu;
- struct task_struct *tsk = find_task_by_vpid(cws->pid);
-
- seq_printf(s, "%3d %6d %6u %s\n", cws->cpu,
- atomic_read(&cws->inserted),
- cws->executed,
- tsk ? tsk->comm : "<...>");
+ struct pid *pid;
+ struct task_struct *tsk;
+
+ pid = find_get_pid(cws->pid);
+ if (pid) {
+ tsk = get_pid_task(pid, PIDTYPE_PID);
+ if (tsk) {
+ seq_printf(s, "%3d %6d %6u %s\n", cws->cpu,
+ atomic_read(&cws->inserted), cws->executed,
+ tsk->comm);
+ put_task_struct(tsk);
+ }
+ put_pid(pid);
+ }
spin_lock_irqsave(&workqueue_cpu_stat(cpu)->lock, flags);
if (&cws->list == workqueue_cpu_stat(cpu)->list.next)
--
1.6.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Follow-Ups:
- Prev by Date: Re: [RFC][PATCH 1/2] tracing/ftrace: syscall tracing infrastructure
- Next by Date: RE: [ipw3945-devel] iwlagn: Cannot allocate memory on 2.6.29-rc4-ikn-00001-gd5b5623
- Previous by thread: Absolute symbols in vmlinux_64.lds.S
- Next by thread: Re: [PATCH] ftrace: Don't use tracing_record_cmdline() in workqueue tracer
- Index(es):
Relevant Pages
|
