Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Sun, 17 Jul 2016 20:49:38 +0200
Jiri Olsa  wrote:

> 
> I haven't checked other distro's kernel packages
> but I guess it'll be similar
> 
> this way we export traceevent lib for other users and its
> source stays in the kernel
> 
> thoughts? ;-)

I say go for it! :-)

-- Steve

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Sun, 17 Jul 2016 20:49:38 +0200
Jiri Olsa  wrote:

> 
> I haven't checked other distro's kernel packages
> but I guess it'll be similar
> 
> this way we export traceevent lib for other users and its
> source stays in the kernel
> 
> thoughts? ;-)

I say go for it! :-)

-- Steve

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Jiri Olsa]

On Wed, Jul 13, 2016 at 10:39:09PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 13, 2016 at 09:25:50PM -0400, Steven Rostedt escreveu:
> > On Wed, 13 Jul 2016 17:01:48 -0300 Arnaldo Carvalho de Melo 
> >  wrote:
> > > > perf, trace-cmd/kernelshark, powertop and ras-daemon  
> > > 
> > > Cool, so this is all just about tools/lib/traceevent/ after all, sure,
> > > go ahead, you're the "external source" after all :-)
> > 
> > Great! We're on the same page then ;-)
> 
> It took a while, now it seems so :-)

heya,
after discussing this with Arnaldo I checked the possibility
of adding traceevent as a fedora kernel rpm subpackage..

and to my surprise fedora already has following rpms:
(see attached rpm -ql output for list of their files)

  kernel-tools
  kernel-tools-libs
  kernel-tools-libs-devel

I wonder we could solve all this by adding traceevent
lib into kernel-libs and kernel-libs-devel packages.

I tried and with easy change to fedora kernel.spec made following rpm:

---
[jolsa@krava fedora]$ rpm -ql -p 
/home/jolsa/rpmbuild/RPMS/x86_64/kernel-tools-libs-devel-4.7.0-0.rc7.git3.1.fc22.x86_64.rpm
/usr/include/cpufreq.h
/usr/lib64/libcpupower.so
/usr/lib64/libtraceevent.a
/usr/lib64/libtraceevent.so
/usr/lib64/traceevent/plugins/plugin_cfg80211.so
/usr/lib64/traceevent/plugins/plugin_function.so
/usr/lib64/traceevent/plugins/plugin_hrtimer.so
/usr/lib64/traceevent/plugins/plugin_jbd2.so
/usr/lib64/traceevent/plugins/plugin_kmem.so
/usr/lib64/traceevent/plugins/plugin_kvm.so
/usr/lib64/traceevent/plugins/plugin_mac80211.so
/usr/lib64/traceevent/plugins/plugin_sched_switch.so
/usr/lib64/traceevent/plugins/plugin_scsi.so
/usr/lib64/traceevent/plugins/plugin_xen.so
---

we'll need to make some enhancements to install targets
to provide header files and *.so.VERSION files.. and some
other minor things

the kernel-tools-libs file could looks like (haven't made this one):
---
[jolsa@krava fedora]$ rpm -ql kernel-tools-libs
/usr/lib64/libcpupower.so.0
/usr/lib64/libcpupower.so.0.0.0
/usr/lib64/libtraceevent.so.0
/usr/lib64/libtraceevent.so.0.0.0
---

I haven't checked other distro's kernel packages
but I guess it'll be similar

this way we export traceevent lib for other users and its
source stays in the kernel

thoughts? ;-)
jirka



---
[jolsa@krava fedora]$ rpm -qa kernel-tools
kernel-tools-4.4.14-200.fc22.x86_64
[jolsa@krava fedora]$ rpm -ql kernel-tools
/etc/sysconfig/cpupower
/usr/bin/centrino-decode
/usr/bin/cpupower
/usr/bin/powernow-k8-decode
/usr/bin/tmon
/usr/bin/turbostat
/usr/bin/x86_energy_perf_policy
/usr/lib/systemd/system/cpupower.service
/usr/share/locale/cs/LC_MESSAGES/cpupower.mo
/usr/share/locale/de/LC_MESSAGES/cpupower.mo
/usr/share/locale/fr/LC_MESSAGES/cpupower.mo
/usr/share/locale/it/LC_MESSAGES/cpupower.mo
/usr/share/locale/pt/LC_MESSAGES/cpupower.mo
/usr/share/man/man1/cpupower-frequency-info.1.gz
/usr/share/man/man1/cpupower-frequency-set.1.gz
/usr/share/man/man1/cpupower-idle-info.1.gz
/usr/share/man/man1/cpupower-idle-set.1.gz
/usr/share/man/man1/cpupower-info.1.gz
/usr/share/man/man1/cpupower-monitor.1.gz
/usr/share/man/man1/cpupower-set.1.gz
/usr/share/man/man1/cpupower.1.gz
/usr/share/man/man8/turbostat.8.gz
/usr/share/man/man8/x86_energy_perf_policy.8.gz
[jolsa@krava fedora]$ rpm -ql kernel-tools-libs
/usr/lib64/libcpupower.so.0
/usr/lib64/libcpupower.so.0.0.0
[jolsa@krava fedora]$ rpm -ql kernel-tools-libs-devel
/usr/include/cpufreq.h
/usr/lib64/libcpupower.so
[jolsa@krava fedora]$ 

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Jiri Olsa]

On Wed, Jul 13, 2016 at 10:39:09PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 13, 2016 at 09:25:50PM -0400, Steven Rostedt escreveu:
> > On Wed, 13 Jul 2016 17:01:48 -0300 Arnaldo Carvalho de Melo 
> >  wrote:
> > > > perf, trace-cmd/kernelshark, powertop and ras-daemon  
> > > 
> > > Cool, so this is all just about tools/lib/traceevent/ after all, sure,
> > > go ahead, you're the "external source" after all :-)
> > 
> > Great! We're on the same page then ;-)
> 
> It took a while, now it seems so :-)

heya,
after discussing this with Arnaldo I checked the possibility
of adding traceevent as a fedora kernel rpm subpackage..

and to my surprise fedora already has following rpms:
(see attached rpm -ql output for list of their files)

  kernel-tools
  kernel-tools-libs
  kernel-tools-libs-devel

I wonder we could solve all this by adding traceevent
lib into kernel-libs and kernel-libs-devel packages.

I tried and with easy change to fedora kernel.spec made following rpm:

---
[jolsa@krava fedora]$ rpm -ql -p 
/home/jolsa/rpmbuild/RPMS/x86_64/kernel-tools-libs-devel-4.7.0-0.rc7.git3.1.fc22.x86_64.rpm
/usr/include/cpufreq.h
/usr/lib64/libcpupower.so
/usr/lib64/libtraceevent.a
/usr/lib64/libtraceevent.so
/usr/lib64/traceevent/plugins/plugin_cfg80211.so
/usr/lib64/traceevent/plugins/plugin_function.so
/usr/lib64/traceevent/plugins/plugin_hrtimer.so
/usr/lib64/traceevent/plugins/plugin_jbd2.so
/usr/lib64/traceevent/plugins/plugin_kmem.so
/usr/lib64/traceevent/plugins/plugin_kvm.so
/usr/lib64/traceevent/plugins/plugin_mac80211.so
/usr/lib64/traceevent/plugins/plugin_sched_switch.so
/usr/lib64/traceevent/plugins/plugin_scsi.so
/usr/lib64/traceevent/plugins/plugin_xen.so
---

we'll need to make some enhancements to install targets
to provide header files and *.so.VERSION files.. and some
other minor things

the kernel-tools-libs file could looks like (haven't made this one):
---
[jolsa@krava fedora]$ rpm -ql kernel-tools-libs
/usr/lib64/libcpupower.so.0
/usr/lib64/libcpupower.so.0.0.0
/usr/lib64/libtraceevent.so.0
/usr/lib64/libtraceevent.so.0.0.0
---

I haven't checked other distro's kernel packages
but I guess it'll be similar

this way we export traceevent lib for other users and its
source stays in the kernel

thoughts? ;-)
jirka



---
[jolsa@krava fedora]$ rpm -qa kernel-tools
kernel-tools-4.4.14-200.fc22.x86_64
[jolsa@krava fedora]$ rpm -ql kernel-tools
/etc/sysconfig/cpupower
/usr/bin/centrino-decode
/usr/bin/cpupower
/usr/bin/powernow-k8-decode
/usr/bin/tmon
/usr/bin/turbostat
/usr/bin/x86_energy_perf_policy
/usr/lib/systemd/system/cpupower.service
/usr/share/locale/cs/LC_MESSAGES/cpupower.mo
/usr/share/locale/de/LC_MESSAGES/cpupower.mo
/usr/share/locale/fr/LC_MESSAGES/cpupower.mo
/usr/share/locale/it/LC_MESSAGES/cpupower.mo
/usr/share/locale/pt/LC_MESSAGES/cpupower.mo
/usr/share/man/man1/cpupower-frequency-info.1.gz
/usr/share/man/man1/cpupower-frequency-set.1.gz
/usr/share/man/man1/cpupower-idle-info.1.gz
/usr/share/man/man1/cpupower-idle-set.1.gz
/usr/share/man/man1/cpupower-info.1.gz
/usr/share/man/man1/cpupower-monitor.1.gz
/usr/share/man/man1/cpupower-set.1.gz
/usr/share/man/man1/cpupower.1.gz
/usr/share/man/man8/turbostat.8.gz
/usr/share/man/man8/x86_energy_perf_policy.8.gz
[jolsa@krava fedora]$ rpm -ql kernel-tools-libs
/usr/lib64/libcpupower.so.0
/usr/lib64/libcpupower.so.0.0.0
[jolsa@krava fedora]$ rpm -ql kernel-tools-libs-devel
/usr/include/cpufreq.h
/usr/lib64/libcpupower.so
[jolsa@krava fedora]$ 

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Wed, Jul 13, 2016 at 09:25:50PM -0400, Steven Rostedt escreveu:
> On Wed, 13 Jul 2016 17:01:48 -0300 Arnaldo Carvalho de Melo  
> wrote:
> > > perf, trace-cmd/kernelshark, powertop and ras-daemon  
> > 
> > Cool, so this is all just about tools/lib/traceevent/ after all, sure,
> > go ahead, you're the "external source" after all :-)
> 
> Great! We're on the same page then ;-)

It took a while, now it seems so :-)

- Arnaldo

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Wed, Jul 13, 2016 at 09:25:50PM -0400, Steven Rostedt escreveu:
> On Wed, 13 Jul 2016 17:01:48 -0300 Arnaldo Carvalho de Melo  
> wrote:
> > > perf, trace-cmd/kernelshark, powertop and ras-daemon  
> > 
> > Cool, so this is all just about tools/lib/traceevent/ after all, sure,
> > go ahead, you're the "external source" after all :-)
> 
> Great! We're on the same page then ;-)

It took a while, now it seems so :-)

- Arnaldo

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Wed, 13 Jul 2016 17:01:48 -0300
Arnaldo Carvalho de Melo  wrote:


> > perf, trace-cmd/kernelshark, powertop and ras-daemon  
> 
> Cool, so this is all just about tools/lib/traceevent/ after all, sure,
> go ahead, you're the "external source" after all :-)

Great! We're on the same page then ;-)

-- Steve

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Wed, 13 Jul 2016 17:01:48 -0300
Arnaldo Carvalho de Melo  wrote:


> > perf, trace-cmd/kernelshark, powertop and ras-daemon  
> 
> Cool, so this is all just about tools/lib/traceevent/ after all, sure,
> go ahead, you're the "external source" after all :-)

Great! We're on the same page then ;-)

-- Steve

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Wed, Jul 13, 2016 at 03:17:00PM -0400, Steven Rostedt escreveu:
> On Wed, 13 Jul 2016 10:51:25 -0300
> Arnaldo Carvalho de Melo  wrote:
> 
> > Em Wed, Jul 13, 2016 at 08:30:01AM -0400, Steven Rostedt escreveu:
> > > On Tue, 12 Jul 2016 20:32:18 -0300
> > > Arnaldo Carvalho de Melo  wrote:  
> > > > > > Forgot about the out of tree copy :-\
> > 
> > > > > Yeah, we really need to make this into a real library. I haven't had
> > > > > the time to do that. Hopefully in August I can talk with some people 
> > > > > at
> > 
> > > > What exactly do you mean by that? To grab a copy of what is in tools/
> > > > and have it turned into a library somewhere else?  
> > 
> > > > Or to freeze its interfaces and create an .so with a committed to ABI?  
> > 
> > > Yes, this is what we need to do.  
> > 
> > > > I kind of like the way it is now... :-)  
> > 
> > > You like the current ABI? Or the fact that we have 4 tools with each  
> > 
> > I like the lack of an ABI, i.e. I like to use the same rationale for
> > when a out of tree driver breaks because we improve kernel internals. As
> > soon as we commit to an ABI for those libraries, that becomes way more
> > difficult.
> 
> Yes, ABI is hard. But it can be done. Let's not get lazy. The code came

Hey, that is what I call enthusiasm, expecting the patches!

> from external sources to begin with. I've been working hard to try to

What code "that come from external sources" are you talking about?

> keep the ABI fixed. There's still a few things I would like to change
> before doing it officially, and that's because this code is quite
> mature now, and I have an idea of what to change.

Ah, you're talking about just tools/lib/traceevent/ ?
 
> One thing I want to do is change the event_format structure to
> "pevent_event" and the format_field to "pevent_field" just to be more
> consistent. There's probably a few other changes as well. But once they
> are done, I think we should get this out into a library.

> > And since we're still adding more and more stuff (write_backward, eBPF
> > for kernel and userspace, etc, etc) I fear lots are still very much in
> > flux to commit to something like that.
 
> We can always add new functions. The old ones will have to be
> maintained. Anyway, I'm only looking to what is being used externally
> from perf. We can slowly add functionality. The .so releases should
> only increment the version if something new is added. I refuse to
> follow the gtk crap that they find ways to remove or modify existing
> functions every single release.

> > So if we know what are the needs of those out of the tree tools (can't
> > they be brought in tree? I'm digressing, couldn't resist, sorry ;-)),
 
> Well, I Cc'd some of the maintainers. We can see what they think.
 
> > then we could, if absolutely required, try, without much enthusiasm,
> > relutanctly, see what could be done besides what has been already
> > underway (moving stuff out of tools/perf and into tools/lib).
> 
> Note, I'm only looking at making a library out of what is currently in
> tools/lib. We don't need to move stuff out from perf unless you want to.

Hey, if it is only rom tools/lib/traceevent, I have no worries, you're
the one proposing turning it into a committed to ABI and you're the main
author, also willing to do the work of turning it into a .so, fair enough.

As time goes by and the changes take place that requires we to write
tools/build/feature/ entries to cope with changes in its ABI, we'll just
do it, it will be just another external library we need and can't do
changes in tandem with tools/perf/, we'll have to suport old versions,
do feature checks to see if the one present in a particular
distro/release has some new feature, etc, etc.
 
> There's a trace-cmd library I want to have as well, which I would love
> to have in the kernel tools/lib directory too. I'd call it
> libftrace.so. This would include the ways to parse the ftrace ring
> buffer, as well as parsing and creating trace.dat files.
 
> > It would add more burden to me for perhaps reducing the burden to who
> > would use those libraries.
 
> Well the event-parse.* code has already been pretty stable. I do want
> to do the renames as I mentioned above before making it official. But
> once that is done, I would want us to get a libtraceevent.so that
> distros can provide.

I was confused, I was thinking you were talking about code other than
tools/lib/traceevent/.

> > > their own copies of the library? (and maybe even more)  

> > Which ones?
 
> perf, trace-cmd/kernelshark, powertop and ras-daemon

Cool, so this is all just about tools/lib/traceevent/ after all, sure,
go ahead, you're the "external source" after all :-)

- Arnaldo

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Wed, Jul 13, 2016 at 03:17:00PM -0400, Steven Rostedt escreveu:
> On Wed, 13 Jul 2016 10:51:25 -0300
> Arnaldo Carvalho de Melo  wrote:
> 
> > Em Wed, Jul 13, 2016 at 08:30:01AM -0400, Steven Rostedt escreveu:
> > > On Tue, 12 Jul 2016 20:32:18 -0300
> > > Arnaldo Carvalho de Melo  wrote:  
> > > > > > Forgot about the out of tree copy :-\
> > 
> > > > > Yeah, we really need to make this into a real library. I haven't had
> > > > > the time to do that. Hopefully in August I can talk with some people 
> > > > > at
> > 
> > > > What exactly do you mean by that? To grab a copy of what is in tools/
> > > > and have it turned into a library somewhere else?  
> > 
> > > > Or to freeze its interfaces and create an .so with a committed to ABI?  
> > 
> > > Yes, this is what we need to do.  
> > 
> > > > I kind of like the way it is now... :-)  
> > 
> > > You like the current ABI? Or the fact that we have 4 tools with each  
> > 
> > I like the lack of an ABI, i.e. I like to use the same rationale for
> > when a out of tree driver breaks because we improve kernel internals. As
> > soon as we commit to an ABI for those libraries, that becomes way more
> > difficult.
> 
> Yes, ABI is hard. But it can be done. Let's not get lazy. The code came

Hey, that is what I call enthusiasm, expecting the patches!

> from external sources to begin with. I've been working hard to try to

What code "that come from external sources" are you talking about?

> keep the ABI fixed. There's still a few things I would like to change
> before doing it officially, and that's because this code is quite
> mature now, and I have an idea of what to change.

Ah, you're talking about just tools/lib/traceevent/ ?
 
> One thing I want to do is change the event_format structure to
> "pevent_event" and the format_field to "pevent_field" just to be more
> consistent. There's probably a few other changes as well. But once they
> are done, I think we should get this out into a library.

> > And since we're still adding more and more stuff (write_backward, eBPF
> > for kernel and userspace, etc, etc) I fear lots are still very much in
> > flux to commit to something like that.
 
> We can always add new functions. The old ones will have to be
> maintained. Anyway, I'm only looking to what is being used externally
> from perf. We can slowly add functionality. The .so releases should
> only increment the version if something new is added. I refuse to
> follow the gtk crap that they find ways to remove or modify existing
> functions every single release.

> > So if we know what are the needs of those out of the tree tools (can't
> > they be brought in tree? I'm digressing, couldn't resist, sorry ;-)),
 
> Well, I Cc'd some of the maintainers. We can see what they think.
 
> > then we could, if absolutely required, try, without much enthusiasm,
> > relutanctly, see what could be done besides what has been already
> > underway (moving stuff out of tools/perf and into tools/lib).
> 
> Note, I'm only looking at making a library out of what is currently in
> tools/lib. We don't need to move stuff out from perf unless you want to.

Hey, if it is only rom tools/lib/traceevent, I have no worries, you're
the one proposing turning it into a committed to ABI and you're the main
author, also willing to do the work of turning it into a .so, fair enough.

As time goes by and the changes take place that requires we to write
tools/build/feature/ entries to cope with changes in its ABI, we'll just
do it, it will be just another external library we need and can't do
changes in tandem with tools/perf/, we'll have to suport old versions,
do feature checks to see if the one present in a particular
distro/release has some new feature, etc, etc.
 
> There's a trace-cmd library I want to have as well, which I would love
> to have in the kernel tools/lib directory too. I'd call it
> libftrace.so. This would include the ways to parse the ftrace ring
> buffer, as well as parsing and creating trace.dat files.
 
> > It would add more burden to me for perhaps reducing the burden to who
> > would use those libraries.
 
> Well the event-parse.* code has already been pretty stable. I do want
> to do the renames as I mentioned above before making it official. But
> once that is done, I would want us to get a libtraceevent.so that
> distros can provide.

I was confused, I was thinking you were talking about code other than
tools/lib/traceevent/.

> > > their own copies of the library? (and maybe even more)  

> > Which ones?
 
> perf, trace-cmd/kernelshark, powertop and ras-daemon

Cool, so this is all just about tools/lib/traceevent/ after all, sure,
go ahead, you're the "external source" after all :-)

- Arnaldo

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Wed, 13 Jul 2016 10:51:25 -0300
Arnaldo Carvalho de Melo  wrote:

> Em Wed, Jul 13, 2016 at 08:30:01AM -0400, Steven Rostedt escreveu:
> > On Tue, 12 Jul 2016 20:32:18 -0300
> > Arnaldo Carvalho de Melo  wrote:  
> > > > > Forgot about the out of tree copy :-\
> 
> > > > Yeah, we really need to make this into a real library. I haven't had
> > > > the time to do that. Hopefully in August I can talk with some people at 
> > > >
> 
> > > What exactly do you mean by that? To grab a copy of what is in tools/
> > > and have it turned into a library somewhere else?  
> 
> > > Or to freeze its interfaces and create an .so with a committed to ABI?  
> 
> > Yes, this is what we need to do.  
> 
> > > I kind of like the way it is now... :-)  
> 
> > You like the current ABI? Or the fact that we have 4 tools with each  
> 
> I like the lack of an ABI, i.e. I like to use the same rationale for
> when a out of tree driver breaks because we improve kernel internals. As
> soon as we commit to an ABI for those libraries, that becomes way more
> difficult.

Yes, ABI is hard. But it can be done. Let's not get lazy. The code came
from external sources to begin with. I've been working hard to try to
keep the ABI fixed. There's still a few things I would like to change
before doing it officially, and that's because this code is quite
mature now, and I have an idea of what to change.

One thing I want to do is change the event_format structure to
"pevent_event" and the format_field to "pevent_field" just to be more
consistent. There's probably a few other changes as well. But once they
are done, I think we should get this out into a library.


> 
> And since we're still adding more and more stuff (write_backward, eBPF
> for kernel and userspace, etc, etc) I fear lots are still very much in
> flux to commit to something like that.

We can always add new functions. The old ones will have to be
maintained. Anyway, I'm only looking to what is being used externally
from perf. We can slowly add functionality. The .so releases should
only increment the version if something new is added. I refuse to
follow the gtk crap that they find ways to remove or modify existing
functions every single release.


> 
> So if we know what are the needs of those out of the tree tools (can't
> they be brought in tree? I'm digressing, couldn't resist, sorry ;-)),

Well, I Cc'd some of the maintainers. We can see what they think.

> then we could, if absolutely required, try, without much enthusiasm,
> relutanctly, see what could be done besides what has been already
> underway (moving stuff out of tools/perf and into tools/lib).

Note, I'm only looking at making a library out of what is currently in
tools/lib. We don't need to move stuff out from perf unless you want to.

There's a trace-cmd library I want to have as well, which I would love
to have in the kernel tools/lib directory too. I'd call it
libftrace.so. This would include the ways to parse the ftrace ring
buffer, as well as parsing and creating trace.dat files.

> 
> It would add more burden to me for perhaps reducing the burden to who
> would use those libraries.

Well the event-parse.* code has already been pretty stable. I do want
to do the renames as I mentioned above before making it official. But
once that is done, I would want us to get a libtraceevent.so that
distros can provide.

> 
> > their own copies of the library? (and maybe even more)  
> 
> Which ones?

perf, trace-cmd/kernelshark, powertop and ras-daemon

-- Steve

> 
> - Arnaldo
>  
> > > > LinuxCon to see the best way to go about doing that.  
> 
> > > > Anyway, I may just take that file and port it to trace-cmd.
> 
> > > Yeah, that would solve this specific case.  
> 
> > OK, I'll do this.  

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Wed, 13 Jul 2016 10:51:25 -0300
Arnaldo Carvalho de Melo  wrote:

> Em Wed, Jul 13, 2016 at 08:30:01AM -0400, Steven Rostedt escreveu:
> > On Tue, 12 Jul 2016 20:32:18 -0300
> > Arnaldo Carvalho de Melo  wrote:  
> > > > > Forgot about the out of tree copy :-\
> 
> > > > Yeah, we really need to make this into a real library. I haven't had
> > > > the time to do that. Hopefully in August I can talk with some people at 
> > > >
> 
> > > What exactly do you mean by that? To grab a copy of what is in tools/
> > > and have it turned into a library somewhere else?  
> 
> > > Or to freeze its interfaces and create an .so with a committed to ABI?  
> 
> > Yes, this is what we need to do.  
> 
> > > I kind of like the way it is now... :-)  
> 
> > You like the current ABI? Or the fact that we have 4 tools with each  
> 
> I like the lack of an ABI, i.e. I like to use the same rationale for
> when a out of tree driver breaks because we improve kernel internals. As
> soon as we commit to an ABI for those libraries, that becomes way more
> difficult.

Yes, ABI is hard. But it can be done. Let's not get lazy. The code came
from external sources to begin with. I've been working hard to try to
keep the ABI fixed. There's still a few things I would like to change
before doing it officially, and that's because this code is quite
mature now, and I have an idea of what to change.

One thing I want to do is change the event_format structure to
"pevent_event" and the format_field to "pevent_field" just to be more
consistent. There's probably a few other changes as well. But once they
are done, I think we should get this out into a library.


> 
> And since we're still adding more and more stuff (write_backward, eBPF
> for kernel and userspace, etc, etc) I fear lots are still very much in
> flux to commit to something like that.

We can always add new functions. The old ones will have to be
maintained. Anyway, I'm only looking to what is being used externally
from perf. We can slowly add functionality. The .so releases should
only increment the version if something new is added. I refuse to
follow the gtk crap that they find ways to remove or modify existing
functions every single release.


> 
> So if we know what are the needs of those out of the tree tools (can't
> they be brought in tree? I'm digressing, couldn't resist, sorry ;-)),

Well, I Cc'd some of the maintainers. We can see what they think.

> then we could, if absolutely required, try, without much enthusiasm,
> relutanctly, see what could be done besides what has been already
> underway (moving stuff out of tools/perf and into tools/lib).

Note, I'm only looking at making a library out of what is currently in
tools/lib. We don't need to move stuff out from perf unless you want to.

There's a trace-cmd library I want to have as well, which I would love
to have in the kernel tools/lib directory too. I'd call it
libftrace.so. This would include the ways to parse the ftrace ring
buffer, as well as parsing and creating trace.dat files.

> 
> It would add more burden to me for perhaps reducing the burden to who
> would use those libraries.

Well the event-parse.* code has already been pretty stable. I do want
to do the renames as I mentioned above before making it official. But
once that is done, I would want us to get a libtraceevent.so that
distros can provide.

> 
> > their own copies of the library? (and maybe even more)  
> 
> Which ones?

perf, trace-cmd/kernelshark, powertop and ras-daemon

-- Steve

> 
> - Arnaldo
>  
> > > > LinuxCon to see the best way to go about doing that.  
> 
> > > > Anyway, I may just take that file and port it to trace-cmd.
> 
> > > Yeah, that would solve this specific case.  
> 
> > OK, I'll do this.  

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Wed, Jul 13, 2016 at 08:30:01AM -0400, Steven Rostedt escreveu:
> On Tue, 12 Jul 2016 20:32:18 -0300
> Arnaldo Carvalho de Melo  wrote:
> > > > Forgot about the out of tree copy :-\  

> > > Yeah, we really need to make this into a real library. I haven't had
> > > the time to do that. Hopefully in August I can talk with some people at  

> > What exactly do you mean by that? To grab a copy of what is in tools/
> > and have it turned into a library somewhere else?

> > Or to freeze its interfaces and create an .so with a committed to ABI?

> Yes, this is what we need to do.

> > I kind of like the way it is now... :-)

> You like the current ABI? Or the fact that we have 4 tools with each

I like the lack of an ABI, i.e. I like to use the same rationale for
when a out of tree driver breaks because we improve kernel internals. As
soon as we commit to an ABI for those libraries, that becomes way more
difficult.

And since we're still adding more and more stuff (write_backward, eBPF
for kernel and userspace, etc, etc) I fear lots are still very much in
flux to commit to something like that.

So if we know what are the needs of those out of the tree tools (can't
they be brought in tree? I'm digressing, couldn't resist, sorry ;-)),
then we could, if absolutely required, try, without much enthusiasm,
relutanctly, see what could be done besides what has been already
underway (moving stuff out of tools/perf and into tools/lib).

It would add more burden to me for perhaps reducing the burden to who
would use those libraries.

> their own copies of the library? (and maybe even more)

Which ones?

- Arnaldo
 
> > > LinuxCon to see the best way to go about doing that.

> > > Anyway, I may just take that file and port it to trace-cmd.  

> > Yeah, that would solve this specific case.

> OK, I'll do this.

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Wed, Jul 13, 2016 at 08:30:01AM -0400, Steven Rostedt escreveu:
> On Tue, 12 Jul 2016 20:32:18 -0300
> Arnaldo Carvalho de Melo  wrote:
> > > > Forgot about the out of tree copy :-\  

> > > Yeah, we really need to make this into a real library. I haven't had
> > > the time to do that. Hopefully in August I can talk with some people at  

> > What exactly do you mean by that? To grab a copy of what is in tools/
> > and have it turned into a library somewhere else?

> > Or to freeze its interfaces and create an .so with a committed to ABI?

> Yes, this is what we need to do.

> > I kind of like the way it is now... :-)

> You like the current ABI? Or the fact that we have 4 tools with each

I like the lack of an ABI, i.e. I like to use the same rationale for
when a out of tree driver breaks because we improve kernel internals. As
soon as we commit to an ABI for those libraries, that becomes way more
difficult.

And since we're still adding more and more stuff (write_backward, eBPF
for kernel and userspace, etc, etc) I fear lots are still very much in
flux to commit to something like that.

So if we know what are the needs of those out of the tree tools (can't
they be brought in tree? I'm digressing, couldn't resist, sorry ;-)),
then we could, if absolutely required, try, without much enthusiasm,
relutanctly, see what could be done besides what has been already
underway (moving stuff out of tools/perf and into tools/lib).

It would add more burden to me for perhaps reducing the burden to who
would use those libraries.

> their own copies of the library? (and maybe even more)

Which ones?

- Arnaldo
 
> > > LinuxCon to see the best way to go about doing that.

> > > Anyway, I may just take that file and port it to trace-cmd.  

> > Yeah, that would solve this specific case.

> OK, I'll do this.

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Tue, 12 Jul 2016 20:32:18 -0300
Arnaldo Carvalho de Melo  wrote:


> > > Forgot about the out of tree copy :-\  
>  
> > Yeah, we really need to make this into a real library. I haven't had
> > the time to do that. Hopefully in August I can talk with some people at  
> 
> What exactly do you mean by that? To grab a copy of what is in tools/
> and have it turned into a library somewhere else?
> 
> Or to freeze its interfaces and create an .so with a committed to ABI?

Yes, this is what we need to do.

> 
> I kind of like the way it is now... :-)

You like the current ABI? Or the fact that we have 4 tools with each
their own copies of the library? (and maybe even more)

> 
> > LinuxCon to see the best way to go about doing that.
> > 
> > Anyway, I may just take that file and port it to trace-cmd.  
> 
> Yeah, that would solve this specific case.

OK, I'll do this.

-- Steve

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Tue, 12 Jul 2016 20:32:18 -0300
Arnaldo Carvalho de Melo  wrote:


> > > Forgot about the out of tree copy :-\  
>  
> > Yeah, we really need to make this into a real library. I haven't had
> > the time to do that. Hopefully in August I can talk with some people at  
> 
> What exactly do you mean by that? To grab a copy of what is in tools/
> and have it turned into a library somewhere else?
> 
> Or to freeze its interfaces and create an .so with a committed to ABI?

Yes, this is what we need to do.

> 
> I kind of like the way it is now... :-)

You like the current ABI? Or the fact that we have 4 tools with each
their own copies of the library? (and maybe even more)

> 
> > LinuxCon to see the best way to go about doing that.
> > 
> > Anyway, I may just take that file and port it to trace-cmd.  
> 
> Yeah, that would solve this specific case.

OK, I'll do this.

-- Steve

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Tue, 12 Jul 2016 20:14:24 -0300
Arnaldo Carvalho de Melo  wrote:

> Em Tue, Jul 12, 2016 at 07:11:08PM -0400, Steven Rostedt escreveu:
> > On Tue, 12 Jul 2016 19:40:04 -0300 Arnaldo Carvalho de Melo 
> >  wrote:  
> > > To make it portable to non-glibc systems, that follow the XSI variant
> > > instead of the GNU specific one that gets in place when _GNU_SOURCE is
> > > defined.  
> 
> > >  #include "event-parse.h"
> > > @@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent 
> > > __maybe_unused,
> > > + str_error_r(errnum, buf, buflen);
> > >   return 0;  
> > 
> > What library is used with this? When I port this over to trace-cmd
> > (which is still needed as I develop this there), it fails to build.
> > "undefined reference to str_error_r"  
> 
> tools/lib/str_error_r.c
> 
> Forgot about the out of tree copy :-\
> 

Yeah, we really need to make this into a real library. I haven't had
the time to do that. Hopefully in August I can talk with some people at
LinuxCon to see the best way to go about doing that.

Anyway, I may just take that file and port it to trace-cmd.

-- Steve

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Tue, Jul 12, 2016 at 07:25:19PM -0400, Steven Rostedt escreveu:
> On Tue, 12 Jul 2016 20:14:24 -0300
> Arnaldo Carvalho de Melo  wrote:
> 
> > Em Tue, Jul 12, 2016 at 07:11:08PM -0400, Steven Rostedt escreveu:
> > > On Tue, 12 Jul 2016 19:40:04 -0300 Arnaldo Carvalho de Melo 
> > >  wrote:  
> > > > To make it portable to non-glibc systems, that follow the XSI variant
> > > > instead of the GNU specific one that gets in place when _GNU_SOURCE is
> > > > defined.  
> > 
> > > >  #include "event-parse.h"
> > > > @@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent 
> > > > __maybe_unused,
> > > > +   str_error_r(errnum, buf, buflen);
> > > > return 0;  

> > > What library is used with this? When I port this over to trace-cmd
> > > (which is still needed as I develop this there), it fails to build.
> > > "undefined reference to str_error_r"  

> > tools/lib/str_error_r.c

> > Forgot about the out of tree copy :-\
 
> Yeah, we really need to make this into a real library. I haven't had
> the time to do that. Hopefully in August I can talk with some people at

What exactly do you mean by that? To grab a copy of what is in tools/
and have it turned into a library somewhere else?

Or to freeze its interfaces and create an .so with a committed to ABI?

I kind of like the way it is now... :-)

> LinuxCon to see the best way to go about doing that.
> 
> Anyway, I may just take that file and port it to trace-cmd.

Yeah, that would solve this specific case.

- Arnaldo

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Tue, Jul 12, 2016 at 07:25:19PM -0400, Steven Rostedt escreveu:
> On Tue, 12 Jul 2016 20:14:24 -0300
> Arnaldo Carvalho de Melo  wrote:
> 
> > Em Tue, Jul 12, 2016 at 07:11:08PM -0400, Steven Rostedt escreveu:
> > > On Tue, 12 Jul 2016 19:40:04 -0300 Arnaldo Carvalho de Melo 
> > >  wrote:  
> > > > To make it portable to non-glibc systems, that follow the XSI variant
> > > > instead of the GNU specific one that gets in place when _GNU_SOURCE is
> > > > defined.  
> > 
> > > >  #include "event-parse.h"
> > > > @@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent 
> > > > __maybe_unused,
> > > > +   str_error_r(errnum, buf, buflen);
> > > > return 0;  

> > > What library is used with this? When I port this over to trace-cmd
> > > (which is still needed as I develop this there), it fails to build.
> > > "undefined reference to str_error_r"  

> > tools/lib/str_error_r.c

> > Forgot about the out of tree copy :-\
 
> Yeah, we really need to make this into a real library. I haven't had
> the time to do that. Hopefully in August I can talk with some people at

What exactly do you mean by that? To grab a copy of what is in tools/
and have it turned into a library somewhere else?

Or to freeze its interfaces and create an .so with a committed to ABI?

I kind of like the way it is now... :-)

> LinuxCon to see the best way to go about doing that.
> 
> Anyway, I may just take that file and port it to trace-cmd.

Yeah, that would solve this specific case.

- Arnaldo

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Tue, 12 Jul 2016 20:14:24 -0300
Arnaldo Carvalho de Melo  wrote:

> Em Tue, Jul 12, 2016 at 07:11:08PM -0400, Steven Rostedt escreveu:
> > On Tue, 12 Jul 2016 19:40:04 -0300 Arnaldo Carvalho de Melo 
> >  wrote:  
> > > To make it portable to non-glibc systems, that follow the XSI variant
> > > instead of the GNU specific one that gets in place when _GNU_SOURCE is
> > > defined.  
> 
> > >  #include "event-parse.h"
> > > @@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent 
> > > __maybe_unused,
> > > + str_error_r(errnum, buf, buflen);
> > >   return 0;  
> > 
> > What library is used with this? When I port this over to trace-cmd
> > (which is still needed as I develop this there), it fails to build.
> > "undefined reference to str_error_r"  
> 
> tools/lib/str_error_r.c
> 
> Forgot about the out of tree copy :-\
> 

Yeah, we really need to make this into a real library. I haven't had
the time to do that. Hopefully in August I can talk with some people at
LinuxCon to see the best way to go about doing that.

Anyway, I may just take that file and port it to trace-cmd.

-- Steve

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Tue, 12 Jul 2016 19:40:04 -0300
Arnaldo Carvalho de Melo  wrote:

> From: Arnaldo Carvalho de Melo 
> 
> To make it portable to non-glibc systems, that follow the XSI variant
> instead of the GNU specific one that gets in place when _GNU_SOURCE is
> defined.
> 
> Cc: Adrian Hunter 
> Cc: David Ahern 
> Cc: Jiri Olsa 
> Cc: Namhyung Kim 
> Cc: Steven Rostedt 
> Cc: Wang Nan 
> Link: http://lkml.kernel.org/n/tip-c1gn8x978qfop65m510wy...@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo 
> ---
>  tools/lib/traceevent/event-parse.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c 
> b/tools/lib/traceevent/event-parse.c
> index a8b6357d1ffe..3a7bd175f73c 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include "event-parse.h"
> @@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent 
> __maybe_unused,
>   const char *msg;
>  
>   if (errnum >= 0) {
> - msg = strerror_r(errnum, buf, buflen);
> - if (msg != buf) {
> - size_t len = strlen(msg);
> - memcpy(buf, msg, min(buflen - 1, len));
> - *(buf + min(buflen - 1, len)) = '\0';
> - }
> + str_error_r(errnum, buf, buflen);
>   return 0;

What library is used with this? When I port this over to trace-cmd
(which is still needed as I develop this there), it fails to build.
"undefined reference to str_error_r"

-- Steve

>   }
>  

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Steven Rostedt]

On Tue, 12 Jul 2016 19:40:04 -0300
Arnaldo Carvalho de Melo  wrote:

> From: Arnaldo Carvalho de Melo 
> 
> To make it portable to non-glibc systems, that follow the XSI variant
> instead of the GNU specific one that gets in place when _GNU_SOURCE is
> defined.
> 
> Cc: Adrian Hunter 
> Cc: David Ahern 
> Cc: Jiri Olsa 
> Cc: Namhyung Kim 
> Cc: Steven Rostedt 
> Cc: Wang Nan 
> Link: http://lkml.kernel.org/n/tip-c1gn8x978qfop65m510wy...@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo 
> ---
>  tools/lib/traceevent/event-parse.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c 
> b/tools/lib/traceevent/event-parse.c
> index a8b6357d1ffe..3a7bd175f73c 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include "event-parse.h"
> @@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent 
> __maybe_unused,
>   const char *msg;
>  
>   if (errnum >= 0) {
> - msg = strerror_r(errnum, buf, buflen);
> - if (msg != buf) {
> - size_t len = strlen(msg);
> - memcpy(buf, msg, min(buflen - 1, len));
> - *(buf + min(buflen - 1, len)) = '\0';
> - }
> + str_error_r(errnum, buf, buflen);
>   return 0;

What library is used with this? When I port this over to trace-cmd
(which is still needed as I develop this there), it fails to build.
"undefined reference to str_error_r"

-- Steve

>   }
>  

[PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo 

To make it portable to non-glibc systems, that follow the XSI variant
instead of the GNU specific one that gets in place when _GNU_SOURCE is
defined.

Cc: Adrian Hunter 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Steven Rostedt 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-c1gn8x978qfop65m510wy...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/lib/traceevent/event-parse.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c 
b/tools/lib/traceevent/event-parse.c
index a8b6357d1ffe..3a7bd175f73c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include "event-parse.h"
@@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent __maybe_unused,
const char *msg;
 
if (errnum >= 0) {
-   msg = strerror_r(errnum, buf, buflen);
-   if (msg != buf) {
-   size_t len = strlen(msg);
-   memcpy(buf, msg, min(buflen - 1, len));
-   *(buf + min(buflen - 1, len)) = '\0';
-   }
+   str_error_r(errnum, buf, buflen);
return 0;
}
 
-- 
2.7.4

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Tue, Jul 12, 2016 at 07:11:08PM -0400, Steven Rostedt escreveu:
> On Tue, 12 Jul 2016 19:40:04 -0300 Arnaldo Carvalho de Melo  
> wrote:
> > To make it portable to non-glibc systems, that follow the XSI variant
> > instead of the GNU specific one that gets in place when _GNU_SOURCE is
> > defined.

> >  #include "event-parse.h"
> > @@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent 
> > __maybe_unused,
> > +   str_error_r(errnum, buf, buflen);
> > return 0;
> 
> What library is used with this? When I port this over to trace-cmd
> (which is still needed as I develop this there), it fails to build.
> "undefined reference to str_error_r"

tools/lib/str_error_r.c

Forgot about the out of tree copy :-\

- Arnaldo

[PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo 

To make it portable to non-glibc systems, that follow the XSI variant
instead of the GNU specific one that gets in place when _GNU_SOURCE is
defined.

Cc: Adrian Hunter 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Steven Rostedt 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-c1gn8x978qfop65m510wy...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/lib/traceevent/event-parse.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c 
b/tools/lib/traceevent/event-parse.c
index a8b6357d1ffe..3a7bd175f73c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include "event-parse.h"
@@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent __maybe_unused,
const char *msg;
 
if (errnum >= 0) {
-   msg = strerror_r(errnum, buf, buflen);
-   if (msg != buf) {
-   size_t len = strlen(msg);
-   memcpy(buf, msg, min(buflen - 1, len));
-   *(buf + min(buflen - 1, len)) = '\0';
-   }
+   str_error_r(errnum, buf, buflen);
return 0;
}
 
-- 
2.7.4

Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Tue, Jul 12, 2016 at 07:11:08PM -0400, Steven Rostedt escreveu:
> On Tue, 12 Jul 2016 19:40:04 -0300 Arnaldo Carvalho de Melo  
> wrote:
> > To make it portable to non-glibc systems, that follow the XSI variant
> > instead of the GNU specific one that gets in place when _GNU_SOURCE is
> > defined.

> >  #include "event-parse.h"
> > @@ -6131,12 +6132,7 @@ int pevent_strerror(struct pevent *pevent 
> > __maybe_unused,
> > +   str_error_r(errnum, buf, buflen);
> > return 0;
> 
> What library is used with this? When I port this over to trace-cmd
> (which is still needed as I develop this there), it fails to build.
> "undefined reference to str_error_r"

tools/lib/str_error_r.c

Forgot about the out of tree copy :-\

- Arnaldo