lkml.org 
[lkml]   [2015]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 35/40] perf record: Synthesize COMM event for a command line workload
Em Wed, May 20, 2015 at 08:56:58AM +0900, Namhyung Kim escreveu:
> On Tue, May 19, 2015 at 05:18:54PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, May 19, 2015 at 09:49:03PM +0200, Jiri Olsa escreveu:
> > > > > Humm, you're thinking about where you managed to reproduce the problem,
> > > > > I am thinking outside indexing, etc, i.e. by definition we either enable
> > > > > the event before we fork, so that we get the PERF_RECORD_FORK/COMM or we
> > > > > synthesize it either from /proc or directly (preferred) if we decide to
> > > > > do it after the fork/exec, right?

> > > > But as I said before, later COMM event will override thread->comm to a
> > > > proper string as long as it can find a matching thread. So I think it
> > > > has no problem in the current code.

> > > I can see the issue in the current script code and the patch cured it ;-)

> > Exactly, this is my point, this is not something new :-)

> Ah, okay. The perf script shows samples before processing comm events
> while perf report shows after processing all events.

I.e. 'perf script' behaves like 'perf trace' and 'perf top'. 'perf
report' is the odd one out, and I think it should be not, i.e. you
should try to think more about the non 'report' use cases when thinking
about how to improve report :-)

But I digress, lets get back to the question at hand...

> But to move it under generic place like perf_evlist__{prepare,start}_
> workload(), it seems we need to pass an additional callback and data.

Only if you want to do it with perf_event__synthesize_comm(). I
suggested writing a new synthesize routine that doesn't parses /proc, as
we have all that we need, no?

I think that just doing something like:

thread = machine__findnew_thread(evlist->workload.pid, evlist->workload.pid);
if (thread)
thread__set_comm(thread, argv[0], timestamp);

Should be enough, no? I.e. no need for setting up a PERF_RECORD_FORK and
a PERF_RECORD_COMM, read /proc, etc, just do it directly with the info
we used to do the fork in perf_evlist__prepare_workload(), etc.

> I'll try to write a patch.

Great!

- Arnaldo


\
 
 \ /
  Last update: 2015-05-20 03:01    [W:0.419 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site