Re: [PATCH] ftrace: Don't use tracing_record_cmdline() inworkqueue tracer

From: Ingo Molnar
Date: Tue Mar 10 2009 - 08:11:54 EST



* KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx> wrote:

> >
> > * 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.

ok, agreed - mind sending a patch for that?

> and, I find my last patch has one race window. fixing below.

Could you please send a delta patch against tip:master? Thanks,

Ingo
--
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/