linux-kernel - Re: [PATCH 12/38] perf tools: Introduce thread__comm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150306043806.GC7872@sejong>
Date:	Fri, 6 Mar 2015 13:38:06 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	Frederic Weisbecker <fweisbec@...il.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Jiri Olsa <jolsa@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Stephane Eranian <eranian@...gle.com>,
	Andi Kleen <andi@...stfloor.org>,
	David Ahern <dsahern@...il.com>
Subject: Re: [PATCH 12/38] perf tools: Introduce thread__comm_time() helpers

Hi Frederic and Arnaldo,

On Thu, Mar 05, 2015 at 05:08:56PM +0100, Frederic Weisbecker wrote:
> On Wed, Mar 04, 2015 at 09:02:55AM +0900, Namhyung Kim wrote:
> > Hi Frederic,
> > 
> > On Tue, Mar 03, 2015 at 05:28:40PM +0100, Frederic Weisbecker wrote:
> > > On Tue, Mar 03, 2015 at 12:07:24PM +0900, Namhyung Kim wrote:
> > > > When data file indexing is enabled, it processes all task, comm and mmap
> > > > events first and then goes to the sample events.  So all it sees is the
> > > > last comm of a thread although it has information at the time of sample.
> > > > 
> > > > Sort thread's comm by time so that it can find appropriate comm at the
> > > > sample time.  The thread__comm_time() will mostly work even if
> > > > PERF_SAMPLE_TIME bit is off since in that case, sample->time will be
> > > > -1 so it'll take the last comm anyway.
> > > > 
> > > > Cc: Frederic Weisbecker <fweisbec@...il.com>
> > > > Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> > > > ---
> > > >  tools/perf/util/thread.c | 33 ++++++++++++++++++++++++++++++++-
> > > >  tools/perf/util/thread.h |  2 ++
> > > >  2 files changed, 34 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> > > > index 9ebc8b1f9be5..ad96725105c2 100644
> > > > --- a/tools/perf/util/thread.c
> > > > +++ b/tools/perf/util/thread.c
> > > > @@ -103,6 +103,21 @@ struct comm *thread__exec_comm(const struct thread *thread)
> > > >  	return last;
> > > >  }
> > > >  
> > > > +struct comm *thread__comm_time(const struct thread *thread, u64 timestamp)
> > > 
> > > Usually thread__comm_foo() would suggest that we return the "foo" from a thread comm.
> > > For example thread__comm_len() returns the len of the last thread comm.
> > > thread__comm_str() returns the string of the last thread comm.
> > 
> > Ah, okay.
> 
> I mean, that's just an impression, others may have a different one :o)

Right.  Although I agree with your idea of function naming, I'm not
sure it's worth changing every function call site for this - and for
similar machine__find(new)_thread()_time.

Arnaldo, What do you think?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/