linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
       [not found] <CAGZ=bq+mJ-9pf0Y22b9LLey0Em2Y7SAA5FnQ5cPsde6GB_aqgw@mail.gmail.com>
@ 2011-08-06 21:40 ` Steven Rostedt
  2011-08-06 23:14   ` Kyle Moffett
  2011-08-08  9:34   ` Américo Wang
  0 siblings, 2 replies; 18+ messages in thread
From: Steven Rostedt @ 2011-08-06 21:40 UTC (permalink / raw)
  To: Kyle Moffett
  Cc: Arnaldo Carvalho de Melo, Frederic Weisbecker, David Ahern,
	Arjan van de Ven, Borislav Petkov, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, linux-kernel

On Sat, 2011-08-06 at 14:47 -0400, Kyle Moffett wrote:
> On Aug 6, 2011 11:35 AM, "Steven Rostedt" <rostedt@goodmis.org> wrote:
> >
> > On Sat, 2011-08-06 at 09:16 -0600, David Ahern wrote:
> >
> > > If the library is just for parsing trace events why not call it
> > > libtraceparse or libtrace? This isn't perf specific functionality.
> >
> > Actually, I'm thinking libevents would probably be most appropriate.
> It
> > has less to do with perf in general, and more about events. Even
> reading
> > the events goes under this category.
> 
> Ooh, please don't... "libevent" is already an epoll() event-based
> application helper library and this would be overly confusing.

OK, what about libkevent, as this has to do with kernel events.

-- Steve



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-06 21:40 ` [RFC][PATCH 0/8] Having perf use libparsevent.a Steven Rostedt
@ 2011-08-06 23:14   ` Kyle Moffett
  2011-08-08  9:34   ` Américo Wang
  1 sibling, 0 replies; 18+ messages in thread
From: Kyle Moffett @ 2011-08-06 23:14 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Arnaldo Carvalho de Melo, Frederic Weisbecker, David Ahern,
	Arjan van de Ven, Borislav Petkov, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, linux-kernel

On Sat, Aug 6, 2011 at 17:40, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sat, 2011-08-06 at 14:47 -0400, Kyle Moffett wrote:
>> On Aug 6, 2011 11:35 AM, "Steven Rostedt" <rostedt@goodmis.org> wrote:
>>> On Sat, 2011-08-06 at 09:16 -0600, David Ahern wrote:
>>>> If the library is just for parsing trace events why not call it
>>>> libtraceparse or libtrace? This isn't perf specific functionality.
>>>
>>> Actually, I'm thinking libevents would probably be most appropriate.
>>> It has less to do with perf in general, and more about events. Even
>>> reading the events goes under this category.
>>
>> Ooh, please don't... "libevent" is already an epoll() event-based
>> application helper library and this would be overly confusing.
>
> OK, what about libkevent, as this has to do with kernel events.

Hmm, well "kevent()" is a BSD syscall that does the same kinds of things
as "epoll()" does on Linux.  It's used by libevent, in fact.

Maybe "libkernevent"?  A quick google doesn't find anything problematic
there...  Other options with minimal chance of confusion are:
  libkernelevent
  liblinuxevent
  libinuxevent (Linked with option "-linuxevent")

To be honest I don't really see anything wrong with a name involving
"trace"...  Perhaps something like libevtrace/libtraceevent/etc?

Cheers,
Kyle Moffett

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-06 21:40 ` [RFC][PATCH 0/8] Having perf use libparsevent.a Steven Rostedt
  2011-08-06 23:14   ` Kyle Moffett
@ 2011-08-08  9:34   ` Américo Wang
  2011-08-08 13:42     ` Steven Rostedt
  1 sibling, 1 reply; 18+ messages in thread
From: Américo Wang @ 2011-08-08  9:34 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Kyle Moffett, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	David Ahern, Arjan van de Ven, Borislav Petkov, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, linux-kernel

On Sun, Aug 7, 2011 at 5:40 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sat, 2011-08-06 at 14:47 -0400, Kyle Moffett wrote:
>> On Aug 6, 2011 11:35 AM, "Steven Rostedt" <rostedt@goodmis.org> wrote:
>> >
>> > On Sat, 2011-08-06 at 09:16 -0600, David Ahern wrote:
>> >
>> > > If the library is just for parsing trace events why not call it
>> > > libtraceparse or libtrace? This isn't perf specific functionality.
>> >
>> > Actually, I'm thinking libevents would probably be most appropriate.
>> It
>> > has less to do with perf in general, and more about events. Even
>> reading
>> > the events goes under this category.
>>
>> Ooh, please don't... "libevent" is already an epoll() event-based
>> application helper library and this would be overly confusing.
>
> OK, what about libkevent, as this has to do with kernel events.
>

libkevent is still too generic, how about libtraceevent?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-08  9:34   ` Américo Wang
@ 2011-08-08 13:42     ` Steven Rostedt
  0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2011-08-08 13:42 UTC (permalink / raw)
  To: Américo Wang
  Cc: Kyle Moffett, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	David Ahern, Arjan van de Ven, Borislav Petkov, Peter Zijlstra,
	Thomas Gleixner, Ingo Molnar, linux-kernel

On Mon, 2011-08-08 at 17:34 +0800, Américo Wang wrote:

> libkevent is still too generic, how about libtraceevent?

Hmm, I wanted to make it more generic than tracing, as it can be
profiling as well. But they do hook to tracepoints in the kernel, so
maybe libtracevent may be fine. Or libtracepoint.

-- Steve




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-06  6:51     ` Ingo Molnar
@ 2011-08-08 21:30       ` Steven Rostedt
  0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2011-08-08 21:30 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven

On Sat, 2011-08-06 at 08:51 +0200, Ingo Molnar wrote:
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > Ingo, I was trying to do as you said, but to do so would require a 
> > lot of restructuring of the perf code base. I started talking with 
> > Arnaldo, as he's doing a lot of the work in the tools/perf code, 
> > and he's the one that suggested that I do it this way. It made 
> > things a lot easier.
> 
> Could you guys please talk some more and clear it up? There's 
> absolutely no technical reason why tools/perf/lib/ (or 
> tools/perf/libperf/) should be harder than tools/lib/.

The issue was trying to use the Makefile within perf for the building.
There was issues with the warning flags and also the deps that are
automatically made with libparsevent.

But you are correct, if I were to just move the pointers over to
tools/perf/lib/events instead of tools/lib/events and had it be a
separate entity in the build process, then it would not be any different
between the two locations.

But there are two non-technical issues with that. Is it wise to have a
different type of build process within the tools/perf directory. The
libparsevents follows the Linux build system more than the git build
system.

The other being, is the parse events a perf only thing? Maybe having a
tools/lib would be nice even if it is. Because we can put things in
tools/lib that will go as a separate package. perf could depend on
tools/lib but there's no reason that tools/lib should depend on perf.

-- Steve



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
       [not found]         ` <CAKYOsXw+Q+h2D++LxAoCUJ3tFVEhczBgDWNjwXzuJ0mNDav_Rw@mail.gmail.com>
  2011-08-06 15:18           ` Frederic Weisbecker
@ 2011-08-06 15:35           ` Steven Rostedt
  1 sibling, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2011-08-06 15:35 UTC (permalink / raw)
  To: David Ahern
  Cc: Frederic Weisbecker, Ingo Molnar, linux-kernel, Thomas Gleixner,
	Peter Zijlstra, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven

On Sat, 2011-08-06 at 09:16 -0600, David Ahern wrote:

> If the library is just for parsing trace events why not call it
> libtraceparse or libtrace? This isn't perf specific functionality.

Actually, I'm thinking libevents would probably be most appropriate. It
has less to do with perf in general, and more about events. Even reading
the events goes under this category.

-- Steve

> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-05 20:59 Steven Rostedt
  2011-08-05 21:24 ` Ingo Molnar
  2011-08-06  0:07 ` David Ahern
@ 2011-08-06 15:23 ` Colin Walters
  2 siblings, 0 replies; 18+ messages in thread
From: Colin Walters @ 2011-08-06 15:23 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven

On Fri, Aug 5, 2011 at 4:59 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Note, because perf installs in the user's ~/bin and I did not want to
> make it a requirement to add ~/bin to LD_LIBRARY_PATH, I kept the
> library as a .a that is linked directly into perf.

Note you can add an rpath to avoid having to modify LD_LIBRARY_PATH;
rpath is just better for this kind of thing.  As a general rule of
thumb, if you're installing somewhere outside of the target system's
/etc/ld.so.conf, add an rpath.  And for this purpose ignore e.g.
Fedora's broken tendency to put private libraries in
/etc/ld.so.conf.d.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
       [not found]         ` <CAKYOsXw+Q+h2D++LxAoCUJ3tFVEhczBgDWNjwXzuJ0mNDav_Rw@mail.gmail.com>
@ 2011-08-06 15:18           ` Frederic Weisbecker
  2011-08-06 15:35           ` Steven Rostedt
  1 sibling, 0 replies; 18+ messages in thread
From: Frederic Weisbecker @ 2011-08-06 15:18 UTC (permalink / raw)
  To: David Ahern
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel, Thomas Gleixner,
	Peter Zijlstra, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven

On Sat, Aug 06, 2011 at 09:16:58AM -0600, David Ahern wrote:
> On Sat, Aug 6, 2011 at 8:56 AM, Frederic Weisbecker <fweisbec@gmail.com>wrote:
> 
> > On Sat, Aug 06, 2011 at 08:48:47AM +0200, Ingo Molnar wrote:
> > > That is why i suggested libperf.so - this will handle the cases you
> > > mention, plus any future case - while still allow more flexible code
> > > sharing between libperf and the perf tools themselves.
> >
> > But what would you want inside libperf.o, further the trace event
> > library?
> >
> 
> 
> If the library is just for parsing trace events why not call it
> libtraceparse or libtrace? This isn't perf specific functionality.
> 
> David

This is what I don't understand yeah.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-06  6:48     ` Ingo Molnar
@ 2011-08-06 14:56       ` Frederic Weisbecker
       [not found]         ` <CAKYOsXw+Q+h2D++LxAoCUJ3tFVEhczBgDWNjwXzuJ0mNDav_Rw@mail.gmail.com>
  0 siblings, 1 reply; 18+ messages in thread
From: Frederic Weisbecker @ 2011-08-06 14:56 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Steven Rostedt, linux-kernel, Thomas Gleixner, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Borislav Petkov, Arjan van de Ven

On Sat, Aug 06, 2011 at 08:48:47AM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > On Fri, Aug 05, 2011 at 11:24:09PM +0200, Ingo Molnar wrote:
> > > 
> > > * Steven Rostedt <rostedt@goodmis.org> wrote:
> > > 
> > > > By keeping the code separate from perf, made the transition from 
> > > > trace-cmd to tools much easier. I've wasted too many days trying to 
> > > > get other ways working, and I don't want to rewrite perf to do so.
> > > 
> > > But we want to move tools together, not further apart. Every code 
> > > activity i see from you is trying to tear apart instrumentation 
> > > tooling - while previously you agreed that it should be unified. So 
> > > why not do tools/perf/lib/ as you agreed before?
> > > 
> > > I'm really not interested in seeing the libdrm/libdri mess repeated. 
> > > Libraries have their uses when there's some very important external 
> > > interface, but here it's actively harmful as it complicates and 
> > > hardcodes APIs into ABIs that are clearly not finished yet.
> > > 
> > > Really, lets not be stupid here.
> > > 
> > > Thanks,
> > > 
> > > 	Ingo
> > 
> > The trace events format is a general interface that not only perf 
> > and trace-cmd use but also powertop and pytimechart, and may be 
> > others?
> > 
> > And given the breakage we had with powertop, for example, that 
> > broke because it was relying on an ad-hoc static layout of the 
> > trace event, or pytimechart that relies(ed?) on the event string 
> > output, I think that library is needed outside perf.
> 
> That is why i suggested libperf.so - this will handle the cases you 
> mention, plus any future case - while still allow more flexible code 
> sharing between libperf and the perf tools themselves.

But what would you want inside libperf.o, further the trace event
library?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-06  1:01   ` Steven Rostedt
  2011-08-06  6:51     ` Ingo Molnar
@ 2011-08-06  9:14     ` Borislav Petkov
  1 sibling, 0 replies; 18+ messages in thread
From: Borislav Petkov @ 2011-08-06  9:14 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Ingo Molnar, linux-kernel, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Arjan van de Ven

On Fri, Aug 05, 2011 at 09:01:28PM -0400, Steven Rostedt wrote:
> On Fri, 2011-08-05 at 23:24 +0200, Ingo Molnar wrote:
> > * Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > By keeping the code separate from perf, made the transition from 
> > > trace-cmd to tools much easier. I've wasted too many days trying to 
> > > get other ways working, and I don't want to rewrite perf to do so.
> > 
> > But we want to move tools together, not further apart. Every code 
> > activity i see from you is trying to tear apart instrumentation 
> 
> Ouch!
> 
> Ingo, I was trying to do as you said, but to do so would require a lot
> of restructuring of the perf code base. I started talking with Arnaldo,
> as he's doing a lot of the work in the tools/perf code, and he's the one
> that suggested that I do it this way. It made things a lot easier.
> 
> Seriously, I wasted Mon-Wed trying to see how it would work in
> tools/perf. I went through iteration after iteration, and each time I
> either rewrote the tools/perf/Makefile, or hacked apart the libparsevent
> code to be strictly perf specific, and not very useful for anything
> else.

Now you feel exactly how I felt when preparing the RAS daemon stuff :-)

I think we should first agree on the design of libperf or whatever other
libraries and then start working towards it. Then we'd all know where
we're going so coding it up would be almost a routine task. And then,
other people who've done it for a couple of times already (hint: me)
could help so Steve won't have to do it alone.

Thanks.

-- 
Regards/Gruss,
    Boris.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-06  1:01   ` Steven Rostedt
@ 2011-08-06  6:51     ` Ingo Molnar
  2011-08-08 21:30       ` Steven Rostedt
  2011-08-06  9:14     ` Borislav Petkov
  1 sibling, 1 reply; 18+ messages in thread
From: Ingo Molnar @ 2011-08-06  6:51 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven


* Steven Rostedt <rostedt@goodmis.org> wrote:

> Ingo, I was trying to do as you said, but to do so would require a 
> lot of restructuring of the perf code base. I started talking with 
> Arnaldo, as he's doing a lot of the work in the tools/perf code, 
> and he's the one that suggested that I do it this way. It made 
> things a lot easier.

Could you guys please talk some more and clear it up? There's 
absolutely no technical reason why tools/perf/lib/ (or 
tools/perf/libperf/) should be harder than tools/lib/.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-06  0:43   ` Frederic Weisbecker
@ 2011-08-06  6:48     ` Ingo Molnar
  2011-08-06 14:56       ` Frederic Weisbecker
  0 siblings, 1 reply; 18+ messages in thread
From: Ingo Molnar @ 2011-08-06  6:48 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Steven Rostedt, linux-kernel, Thomas Gleixner, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Borislav Petkov, Arjan van de Ven


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Fri, Aug 05, 2011 at 11:24:09PM +0200, Ingo Molnar wrote:
> > 
> > * Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > By keeping the code separate from perf, made the transition from 
> > > trace-cmd to tools much easier. I've wasted too many days trying to 
> > > get other ways working, and I don't want to rewrite perf to do so.
> > 
> > But we want to move tools together, not further apart. Every code 
> > activity i see from you is trying to tear apart instrumentation 
> > tooling - while previously you agreed that it should be unified. So 
> > why not do tools/perf/lib/ as you agreed before?
> > 
> > I'm really not interested in seeing the libdrm/libdri mess repeated. 
> > Libraries have their uses when there's some very important external 
> > interface, but here it's actively harmful as it complicates and 
> > hardcodes APIs into ABIs that are clearly not finished yet.
> > 
> > Really, lets not be stupid here.
> > 
> > Thanks,
> > 
> > 	Ingo
> 
> The trace events format is a general interface that not only perf 
> and trace-cmd use but also powertop and pytimechart, and may be 
> others?
> 
> And given the breakage we had with powertop, for example, that 
> broke because it was relying on an ad-hoc static layout of the 
> trace event, or pytimechart that relies(ed?) on the event string 
> output, I think that library is needed outside perf.

That is why i suggested libperf.so - this will handle the cases you 
mention, plus any future case - while still allow more flexible code 
sharing between libperf and the perf tools themselves.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-06  0:07 ` David Ahern
@ 2011-08-06  1:05   ` Steven Rostedt
  0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2011-08-06  1:05 UTC (permalink / raw)
  To: David Ahern
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven

On Fri, 2011-08-05 at 18:07 -0600, David Ahern wrote:
> On 08/05/2011 02:59 PM, Steven Rostedt wrote:
> > Hi All,
> > 
> > I've spent the week working on getting libparsevent.a working with
> > perf. After several rewrites, hacking in both perf and the trace-cmd
> > code, and breaking both beyond repair, I finally got to a point that
> > it just works.
> 
> Does this version allow use of the trace-cmd plugins with perf?
> Specifically, kvm tracing is not compatible with perf (perf barfs on the
> kvm_exit traces); I suspect the plugins would fix that - as well as have
> the pretty format for the exit reasons.
> 
> I want to try out the patch set, but will not have time until the week
> of the kvm forum/linuxcon.
> 

I posted this patch set as an RFC as I don't have time to waste on doing
more work, and I knew it would cause some controversy.

That said, this patch set does not yet add the plugins, but it lays the
ground work for adding plugins to perf in the next step. I probably
could get plugins working with perf by next week. If we keep going in
this direction, otherwise, probably not till next year.

-- Steve



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-05 21:24 ` Ingo Molnar
  2011-08-06  0:43   ` Frederic Weisbecker
@ 2011-08-06  1:01   ` Steven Rostedt
  2011-08-06  6:51     ` Ingo Molnar
  2011-08-06  9:14     ` Borislav Petkov
  1 sibling, 2 replies; 18+ messages in thread
From: Steven Rostedt @ 2011-08-06  1:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven

On Fri, 2011-08-05 at 23:24 +0200, Ingo Molnar wrote:
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > By keeping the code separate from perf, made the transition from 
> > trace-cmd to tools much easier. I've wasted too many days trying to 
> > get other ways working, and I don't want to rewrite perf to do so.
> 
> But we want to move tools together, not further apart. Every code 
> activity i see from you is trying to tear apart instrumentation 

Ouch!

Ingo, I was trying to do as you said, but to do so would require a lot
of restructuring of the perf code base. I started talking with Arnaldo,
as he's doing a lot of the work in the tools/perf code, and he's the one
that suggested that I do it this way. It made things a lot easier.

Seriously, I wasted Mon-Wed trying to see how it would work in
tools/perf. I went through iteration after iteration, and each time I
either rewrote the tools/perf/Makefile, or hacked apart the libparsevent
code to be strictly perf specific, and not very useful for anything
else.

It took me half of Thursday to get something working with this method.
It made sense and was easy to do. I don't have much more time to spend
on this. Red Hat does not pay me to do tracing/perf work. There's other
things that I need to focus on, and there's other aspects of ftrace that
needs to get done too (-mfentry for one).


> tooling - while previously you agreed that it should be unified. So 
> why not do tools/perf/lib/ as you agreed before?

Because Arnaldo and Frederic convinced me the tools/lib made more sense.

> 
> I'm really not interested in seeing the libdrm/libdri mess repeated. 
> Libraries have their uses when there's some very important external 
> interface, but here it's actively harmful as it complicates and 
> hardcodes APIs into ABIs that are clearly not finished yet.

What API/ABI is being hardcoded here?

-- Steve



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-05 21:24 ` Ingo Molnar
@ 2011-08-06  0:43   ` Frederic Weisbecker
  2011-08-06  6:48     ` Ingo Molnar
  2011-08-06  1:01   ` Steven Rostedt
  1 sibling, 1 reply; 18+ messages in thread
From: Frederic Weisbecker @ 2011-08-06  0:43 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Steven Rostedt, linux-kernel, Thomas Gleixner, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Borislav Petkov, Arjan van de Ven

On Fri, Aug 05, 2011 at 11:24:09PM +0200, Ingo Molnar wrote:
> 
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > By keeping the code separate from perf, made the transition from 
> > trace-cmd to tools much easier. I've wasted too many days trying to 
> > get other ways working, and I don't want to rewrite perf to do so.
> 
> But we want to move tools together, not further apart. Every code 
> activity i see from you is trying to tear apart instrumentation 
> tooling - while previously you agreed that it should be unified. So 
> why not do tools/perf/lib/ as you agreed before?
> 
> I'm really not interested in seeing the libdrm/libdri mess repeated. 
> Libraries have their uses when there's some very important external 
> interface, but here it's actively harmful as it complicates and 
> hardcodes APIs into ABIs that are clearly not finished yet.
> 
> Really, lets not be stupid here.
> 
> Thanks,
> 
> 	Ingo

The trace events format is a general interface that not only
perf and trace-cmd use but also powertop and pytimechart, and
may be others?

And given the breakage we had with powertop, for example, that broke
because it was relying on an ad-hoc static layout of the trace event,
or pytimechart that relies(ed?) on the event string output, I think that library
is needed outside perf.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-05 20:59 Steven Rostedt
  2011-08-05 21:24 ` Ingo Molnar
@ 2011-08-06  0:07 ` David Ahern
  2011-08-06  1:05   ` Steven Rostedt
  2011-08-06 15:23 ` Colin Walters
  2 siblings, 1 reply; 18+ messages in thread
From: David Ahern @ 2011-08-06  0:07 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven

On 08/05/2011 02:59 PM, Steven Rostedt wrote:
> Hi All,
> 
> I've spent the week working on getting libparsevent.a working with
> perf. After several rewrites, hacking in both perf and the trace-cmd
> code, and breaking both beyond repair, I finally got to a point that
> it just works.

Does this version allow use of the trace-cmd plugins with perf?
Specifically, kvm tracing is not compatible with perf (perf barfs on the
kvm_exit traces); I suspect the plugins would fix that - as well as have
the pretty format for the exit reasons.

I want to try out the patch set, but will not have time until the week
of the kvm forum/linuxcon.

David

> 
> The library was originally going to be called libperf.so, but after
> talking with Arnaldo, he wants the functionality of the libraries
> separated more, not combined all in one binary. As this code deals
> only with the parsing the event formats and creating a way to read
> the binary data from it, we both agreed that it is best to keep its
> original name that was in trace-cmd and call it libparsevent.so.
> 
> Note, because perf installs in the user's ~/bin and I did not want to
> make it a requirement to add ~/bin to LD_LIBRARY_PATH, I kept the
> library as a .a that is linked directly into perf.
> 
> In the beginning, I tried to work with the library code within the
> tools/perf directory, and because of the CFLAGS of perf, it made
> compiling the library code very nasty (namely the -Wswitch-enum).
> I also talked with Arnaldo and Frederic about the placement of this
> code, and we all agreed that tools/lib/events is a good place to
> put it.
> 
> By keeping the code separate from perf, made the transition from
> trace-cmd to tools much easier. I've wasted too many days trying to
> get other ways working, and I don't want to rewrite perf to do so.
> 
> Thus, this is what I ended up with.
> 
> This patch set just gets the libparsevent.a/so working with perf.
> New code would be required to get it packaged for distributions to
> ship as a library itself. Once it gets there, I'll work on getting
> this code to work with code like latencytrace.
> 
> Anyway, this is still in the RFC phase. Lets hear what people have to
> say.
> 
> (some of the patches are quite large as they either insert full
> files or remove them, and probably will not make LKML due to the
> size constraint.)
> 
> This patches can be viewed from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> tip/perf/parse-events
> 
> Head SHA1: 781d261ff8241243173cf996ceb068cb19bd5874
> 
> 
> Julia Lawall (1):
>       perf/events: Correct size given to memset
> 
> Steven Rostedt (6):
>       perf: Separate out trace-cmd parse-events from perf files
>       tools/events: Add files to create libparsevent.a
>       perf: Build libparsevent.a
>       events: Update tools/lib/events to work with perf
>       perf: Have perf use the new libparsevent.a library
>       perf/events: Add flag to produce nsec output
> 
> Tom Zanussi (1):
>       perf/events: Add flag/symbol format_flags
> 
> ----
>  tools/lib/events/Makefile                          |  303 ++
>  tools/lib/events/event-parse.c                     | 4991 ++++++++++++++++++++
>  tools/lib/events/event-parse.h                     |  804 ++++
>  tools/lib/events/event-utils.h                     |   80 +
>  tools/lib/events/parse-filter.c                    | 2262 +++++++++
>  tools/lib/events/parse-utils.c                     |  110 +
>  tools/lib/events/trace-seq.c                       |  200 +
>  tools/perf/Makefile                                |   17 +-
>  tools/perf/builtin-kmem.c                          |    6 +-
>  tools/perf/builtin-lock.c                          |   26 +-
>  tools/perf/builtin-sched.c                         |   42 +-
>  tools/perf/builtin-script.c                        |    2 +-
>  .../util/scripting-engines/trace-event-python.c    |   16 +-
>  tools/perf/util/trace-event-info.c                 |    4 +-
>  tools/perf/util/trace-event-parse.c                | 3188 +------------
>  tools/perf/util/trace-event-read.c                 |   34 +-
>  tools/perf/util/trace-event.h                      |  268 +-
>  17 files changed, 9070 insertions(+), 3283 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC][PATCH 0/8] Having perf use libparsevent.a
  2011-08-05 20:59 Steven Rostedt
@ 2011-08-05 21:24 ` Ingo Molnar
  2011-08-06  0:43   ` Frederic Weisbecker
  2011-08-06  1:01   ` Steven Rostedt
  2011-08-06  0:07 ` David Ahern
  2011-08-06 15:23 ` Colin Walters
  2 siblings, 2 replies; 18+ messages in thread
From: Ingo Molnar @ 2011-08-05 21:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven


* Steven Rostedt <rostedt@goodmis.org> wrote:

> By keeping the code separate from perf, made the transition from 
> trace-cmd to tools much easier. I've wasted too many days trying to 
> get other ways working, and I don't want to rewrite perf to do so.

But we want to move tools together, not further apart. Every code 
activity i see from you is trying to tear apart instrumentation 
tooling - while previously you agreed that it should be unified. So 
why not do tools/perf/lib/ as you agreed before?

I'm really not interested in seeing the libdrm/libdri mess repeated. 
Libraries have their uses when there's some very important external 
interface, but here it's actively harmful as it complicates and 
hardcodes APIs into ABIs that are clearly not finished yet.

Really, lets not be stupid here.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC][PATCH 0/8] Having perf use libparsevent.a
@ 2011-08-05 20:59 Steven Rostedt
  2011-08-05 21:24 ` Ingo Molnar
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Steven Rostedt @ 2011-08-05 20:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Arnaldo Carvalho de Melo, Borislav Petkov,
	Arjan van de Ven

Hi All,

I've spent the week working on getting libparsevent.a working with
perf. After several rewrites, hacking in both perf and the trace-cmd
code, and breaking both beyond repair, I finally got to a point that
it just works.

The library was originally going to be called libperf.so, but after
talking with Arnaldo, he wants the functionality of the libraries
separated more, not combined all in one binary. As this code deals
only with the parsing the event formats and creating a way to read
the binary data from it, we both agreed that it is best to keep its
original name that was in trace-cmd and call it libparsevent.so.

Note, because perf installs in the user's ~/bin and I did not want to
make it a requirement to add ~/bin to LD_LIBRARY_PATH, I kept the
library as a .a that is linked directly into perf.

In the beginning, I tried to work with the library code within the
tools/perf directory, and because of the CFLAGS of perf, it made
compiling the library code very nasty (namely the -Wswitch-enum).
I also talked with Arnaldo and Frederic about the placement of this
code, and we all agreed that tools/lib/events is a good place to
put it.

By keeping the code separate from perf, made the transition from
trace-cmd to tools much easier. I've wasted too many days trying to
get other ways working, and I don't want to rewrite perf to do so.

Thus, this is what I ended up with.

This patch set just gets the libparsevent.a/so working with perf.
New code would be required to get it packaged for distributions to
ship as a library itself. Once it gets there, I'll work on getting
this code to work with code like latencytrace.

Anyway, this is still in the RFC phase. Lets hear what people have to
say.

(some of the patches are quite large as they either insert full
files or remove them, and probably will not make LKML due to the
size constraint.)

This patches can be viewed from:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/parse-events

Head SHA1: 781d261ff8241243173cf996ceb068cb19bd5874


Julia Lawall (1):
      perf/events: Correct size given to memset

Steven Rostedt (6):
      perf: Separate out trace-cmd parse-events from perf files
      tools/events: Add files to create libparsevent.a
      perf: Build libparsevent.a
      events: Update tools/lib/events to work with perf
      perf: Have perf use the new libparsevent.a library
      perf/events: Add flag to produce nsec output

Tom Zanussi (1):
      perf/events: Add flag/symbol format_flags

----
 tools/lib/events/Makefile                          |  303 ++
 tools/lib/events/event-parse.c                     | 4991 ++++++++++++++++++++
 tools/lib/events/event-parse.h                     |  804 ++++
 tools/lib/events/event-utils.h                     |   80 +
 tools/lib/events/parse-filter.c                    | 2262 +++++++++
 tools/lib/events/parse-utils.c                     |  110 +
 tools/lib/events/trace-seq.c                       |  200 +
 tools/perf/Makefile                                |   17 +-
 tools/perf/builtin-kmem.c                          |    6 +-
 tools/perf/builtin-lock.c                          |   26 +-
 tools/perf/builtin-sched.c                         |   42 +-
 tools/perf/builtin-script.c                        |    2 +-
 .../util/scripting-engines/trace-event-python.c    |   16 +-
 tools/perf/util/trace-event-info.c                 |    4 +-
 tools/perf/util/trace-event-parse.c                | 3188 +------------
 tools/perf/util/trace-event-read.c                 |   34 +-
 tools/perf/util/trace-event.h                      |  268 +-
 17 files changed, 9070 insertions(+), 3283 deletions(-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2011-08-08 21:30 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAGZ=bq+mJ-9pf0Y22b9LLey0Em2Y7SAA5FnQ5cPsde6GB_aqgw@mail.gmail.com>
2011-08-06 21:40 ` [RFC][PATCH 0/8] Having perf use libparsevent.a Steven Rostedt
2011-08-06 23:14   ` Kyle Moffett
2011-08-08  9:34   ` Américo Wang
2011-08-08 13:42     ` Steven Rostedt
2011-08-05 20:59 Steven Rostedt
2011-08-05 21:24 ` Ingo Molnar
2011-08-06  0:43   ` Frederic Weisbecker
2011-08-06  6:48     ` Ingo Molnar
2011-08-06 14:56       ` Frederic Weisbecker
     [not found]         ` <CAKYOsXw+Q+h2D++LxAoCUJ3tFVEhczBgDWNjwXzuJ0mNDav_Rw@mail.gmail.com>
2011-08-06 15:18           ` Frederic Weisbecker
2011-08-06 15:35           ` Steven Rostedt
2011-08-06  1:01   ` Steven Rostedt
2011-08-06  6:51     ` Ingo Molnar
2011-08-08 21:30       ` Steven Rostedt
2011-08-06  9:14     ` Borislav Petkov
2011-08-06  0:07 ` David Ahern
2011-08-06  1:05   ` Steven Rostedt
2011-08-06 15:23 ` Colin Walters

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).