[RFC 0/4] tools lib traceevent: Install fixes

[RFC 0/4] tools lib traceevent: Install fixes

[Jiri Olsa]

hi,
sending traceevent changes to make this lib installable
under rpm spec.

Basically adding support to:
  - install header files
  - install version links

Having this patchset applied over the fedora source,
I could built following rpms:

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

with added libtraceevent stuff:

  $ rpm -ql kernel-tools-libs
  /usr/lib64/libcpupower.so.0
  /usr/lib64/libcpupower.so.0.0.0
  /usr/lib64/libtraceevent.so.1
  /usr/lib64/libtraceevent.so.1.1.0

  $ rpm -ql kernel-tools-libs-devel
  /usr/include/cpufreq.h
  /usr/include/traceevent
  /usr/include/traceevent/event-parse.h
  /usr/include/traceevent/event-utils.h
  /usr/include/traceevent/kbuffer.h
  /usr/lib64/libcpupower.so
  /usr/lib64/libtraceevent.a
  /usr/lib64/libtraceevent.so

and could build following ex.c outside the kernel tree:

  $ cat ex.c 
  #include <traceevent/event-parse.h>

  int main(void)
  {
          struct pevent *pevent = pevent_alloc();
          printf("krava %p\n", pevent);
          return 0;
  }
  $ gcc -o ex ex.c -ltraceevent -ldl
  $ ./ex
  krava 0x10c6010
  $


I'll send out fedora rpm build changes once we decide
on this patchset.

Also available in:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  traceevent/install

thanks,
jirka


---
Jiri Olsa (4):
      tools lib traceevent: Add install_headers target
      tools lib traceevent: Add do_install_mkdir Makefile function
      tools lib traceevent: Rename LIB_FILE to LIB_TARGET
      tools lib traceevent: Add version for traceevent shared object

 tools/lib/traceevent/Makefile | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

[PATCH 1/4] tools lib traceevent: Add install_headers target

[Jiri Olsa]

Adding install_headers target to install all headers
under 'include/traceevent' path, like:

  $ make DESTDIR=/tmp/krava prefix=/usr install_headers
  $ find /tmp/krava/ -type f
  /tmp/krava/usr/include/traceevent/kbuffer.h
  /tmp/krava/usr/include/traceevent/event-utils.h
  /tmp/krava/usr/include/traceevent/event-parse.h

Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/n/tip-if70lj3zhdc3csdqm5webjvc@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/traceevent/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index 7851df1490e0..8e44bea646ee 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -240,7 +240,7 @@ define do_install
 	if [ ! -d '$(DESTDIR_SQ)$2' ]; then		\
 		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2';	\
 	fi;						\
-	$(INSTALL) $1 '$(DESTDIR_SQ)$2'
+	$(INSTALL) $(if $3,-m $3,) $1 '$(DESTDIR_SQ)$2'
 endef
 
 define do_install_plugins
@@ -264,6 +264,12 @@ install_plugins: $(PLUGINS)
 	$(call QUIET_INSTALL, trace_plugins) \
 		$(call do_install_plugins, $(PLUGINS))
 
+install_headers:
+	$(call QUIET_INSTALL, headers) \
+		$(call do_install,event-parse.h,$(prefix)/include/traceevent,644); \
+		$(call do_install,event-utils.h,$(prefix)/include/traceevent,644); \
+		$(call do_install,kbuffer.h,$(prefix)/include/traceevent,644)
+
 install: install_lib
 
 clean:
-- 
2.4.11

[PATCH 2/4] tools lib traceevent: Add do_install_mkdir Makefile function

[Jiri Olsa]

Decompose the do_install function to ease up
the following patch a little.

Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/n/tip-zzs19yx8seyors532vuer37w@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/traceevent/Makefile | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index 8e44bea646ee..deeae5201ec9 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -236,10 +236,14 @@ TAGS:	force
 	find . -name '*.[ch]' | xargs etags \
 	--regex='/_PE(\([^,)]*\).*/PEVENT_ERRNO__\1/'
 
+define do_install_mkdir
+	if [ ! -d '$(DESTDIR_SQ)$1' ]; then		\
+		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1';	\
+	fi
+endef
+
 define do_install
-	if [ ! -d '$(DESTDIR_SQ)$2' ]; then		\
-		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2';	\
-	fi;						\
+	$(call do_install_mkdir,$2);			\
 	$(INSTALL) $(if $3,-m $3,) $1 '$(DESTDIR_SQ)$2'
 endef
 
-- 
2.4.11

[PATCH 3/4] tools lib traceevent: Rename LIB_FILE to LIB_TARGET

[Jiri Olsa]

To ease up following patch.

Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/n/tip-zpv5gd8y7clwrhh6dq03ucd5@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/traceevent/Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index deeae5201ec9..0d7e1725a0f8 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -99,7 +99,7 @@ libdir_SQ = $(subst ','\'',$(libdir))
 libdir_relative_SQ = $(subst ','\'',$(libdir_relative))
 plugin_dir_SQ = $(subst ','\'',$(plugin_dir))
 
-LIB_FILE = libtraceevent.a libtraceevent.so
+LIB_TARGET = libtraceevent.a libtraceevent.so
 
 CONFIG_INCLUDES = 
 CONFIG_LIBS	=
@@ -156,11 +156,11 @@ PLUGINS += plugin_cfg80211.so
 PLUGINS    := $(addprefix $(OUTPUT),$(PLUGINS))
 PLUGINS_IN := $(PLUGINS:.so=-in.o)
 
-TE_IN    := $(OUTPUT)libtraceevent-in.o
-LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
+TE_IN      := $(OUTPUT)libtraceevent-in.o
+LIB_TARGET := $(addprefix $(OUTPUT),$(LIB_TARGET))
 DYNAMIC_LIST_FILE := $(OUTPUT)libtraceevent-dynamic-list
 
-CMD_TARGETS = $(LIB_FILE) $(PLUGINS) $(DYNAMIC_LIST_FILE)
+CMD_TARGETS = $(LIB_TARGET) $(PLUGINS) $(DYNAMIC_LIST_FILE)
 
 TARGETS = $(CMD_TARGETS)
 
@@ -261,8 +261,8 @@ define do_generate_dynamic_list_file
 endef
 
 install_lib: all_cmd install_plugins
-	$(call QUIET_INSTALL, $(LIB_FILE)) \
-		$(call do_install,$(LIB_FILE),$(libdir_SQ))
+	$(call QUIET_INSTALL, $(LIB_TARGET)) \
+		$(call do_install,$(LIB_TARGET),$(libdir_SQ))
 
 install_plugins: $(PLUGINS)
 	$(call QUIET_INSTALL, trace_plugins) \
-- 
2.4.11

[PATCH 4/4] tools lib traceevent: Add version for traceevent shared object

[Jiri Olsa]

Adding version support for libtraceevent.so object.

Using the existing EVENT_PARSE_VERSION variable to construct
the .so object version string, which now consists of:

  $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)

Looks like it was created for this purpose anyway.

The build will now produce following traeceevent libraries:

  $ ll libtraceevent*
  libtraceevent.a
  libtraceevent.so -> libtraceevent.so.1.1.0
  libtraceevent.so.1 -> libtraceevent.so.1.1.0
  libtraceevent.so.1.1.0

Also the install target will carry them:

  $ make DESTDIR=/tmp/krava prefix=/usr install
  INSTALL  trace_plugins
  INSTALL  libtraceevent.a
  INSTALL  libtraceevent.so.1.1.0

  $ find /tmp/krava/ | xargs ls -l
  ...
  /tmp/krava/usr/lib64:
  total 572
  libtraceevent.a
  libtraceevent.so -> libtraceevent.so.1.1.0
  libtraceevent.so.1 -> libtraceevent.so.1.1.0
  libtraceevent.so.1.1.0
  ...

Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/n/tip-v64z62fh0dwt0ueie5usrnac@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/traceevent/Makefile | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index 0d7e1725a0f8..c76012ebdb9c 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -99,8 +99,6 @@ libdir_SQ = $(subst ','\'',$(libdir))
 libdir_relative_SQ = $(subst ','\'',$(libdir_relative))
 plugin_dir_SQ = $(subst ','\'',$(plugin_dir))
 
-LIB_TARGET = libtraceevent.a libtraceevent.so
-
 CONFIG_INCLUDES = 
 CONFIG_LIBS	=
 CONFIG_FLAGS	=
@@ -114,6 +112,9 @@ N		=
 
 EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
 
+LIB_TARGET  = libtraceevent.a libtraceevent.so.$(EVENT_PARSE_VERSION)
+LIB_INSTALL = libtraceevent.a libtraceevent.so*
+
 INCLUDES = -I. -I $(srctree)/tools/include $(CONFIG_INCLUDES)
 
 # Set compile option CFLAGS
@@ -171,8 +172,10 @@ all_cmd: $(CMD_TARGETS)
 $(TE_IN): force
 	$(Q)$(MAKE) $(build)=libtraceevent
 
-$(OUTPUT)libtraceevent.so: $(TE_IN)
-	$(QUIET_LINK)$(CC) --shared $^ -o $@
+$(OUTPUT)libtraceevent.so.$(EVENT_PARSE_VERSION): $(TE_IN)
+	$(QUIET_LINK)$(CC) --shared $^ -Wl,-soname,libtraceevent.so.$(EP_VERSION) -o $@
+	@ln -sf $(@F) $(OUTPUT)libtraceevent.so
+	@ln -sf $(@F) $(OUTPUT)libtraceevent.so.$(EP_VERSION)
 
 $(OUTPUT)libtraceevent.a: $(TE_IN)
 	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
@@ -262,7 +265,8 @@ endef
 
 install_lib: all_cmd install_plugins
 	$(call QUIET_INSTALL, $(LIB_TARGET)) \
-		$(call do_install,$(LIB_TARGET),$(libdir_SQ))
+		$(call do_install_mkdir,$(libdir_SQ)); \
+		cp -fpR $(LIB_INSTALL) $(DESTDIR)$(libdir_SQ)
 
 install_plugins: $(PLUGINS)
 	$(call QUIET_INSTALL, trace_plugins) \
-- 
2.4.11

Re: [PATCH 1/4] tools lib traceevent: Add install_headers target

[Namhyung Kim]

Hi Jiri,

On Mon, Aug 01, 2016 at 07:41:29PM +0200, Jiri Olsa wrote:
> Adding install_headers target to install all headers
> under 'include/traceevent' path, like:
> 
>   $ make DESTDIR=/tmp/krava prefix=/usr install_headers
>   $ find /tmp/krava/ -type f
>   /tmp/krava/usr/include/traceevent/kbuffer.h
>   /tmp/krava/usr/include/traceevent/event-utils.h
>   /tmp/krava/usr/include/traceevent/event-parse.h
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Link: http://lkml.kernel.org/n/tip-if70lj3zhdc3csdqm5webjvc@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/traceevent/Makefile | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
> index 7851df1490e0..8e44bea646ee 100644
> --- a/tools/lib/traceevent/Makefile
> +++ b/tools/lib/traceevent/Makefile
> @@ -240,7 +240,7 @@ define do_install
>  	if [ ! -d '$(DESTDIR_SQ)$2' ]; then		\
>  		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2';	\
>  	fi;						\
> -	$(INSTALL) $1 '$(DESTDIR_SQ)$2'
> +	$(INSTALL) $(if $3,-m $3,) $1 '$(DESTDIR_SQ)$2'
>  endef
>  
>  define do_install_plugins
> @@ -264,6 +264,12 @@ install_plugins: $(PLUGINS)
>  	$(call QUIET_INSTALL, trace_plugins) \
>  		$(call do_install_plugins, $(PLUGINS))
>  
> +install_headers:
> +	$(call QUIET_INSTALL, headers) \
> +		$(call do_install,event-parse.h,$(prefix)/include/traceevent,644); \
> +		$(call do_install,event-utils.h,$(prefix)/include/traceevent,644); \
> +		$(call do_install,kbuffer.h,$(prefix)/include/traceevent,644)

For possible future changes, what about making it to use the wildcard
somehow?  Or else, we could define HEADER_FILES variable..

Thanks,
Namhyung


> +
>  install: install_lib
>  
>  clean:
> -- 
> 2.4.11
> 

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Namhyung Kim]

On Mon, Aug 01, 2016 at 07:41:28PM +0200, Jiri Olsa wrote:
> hi,
> sending traceevent changes to make this lib installable
> under rpm spec.
> 
> Basically adding support to:
>   - install header files
>   - install version links
> 
> Having this patchset applied over the fedora source,
> I could built following rpms:
> 
>   kernel-tools-libs
>   kernel-tools-libs-devel
> 
> with added libtraceevent stuff:
> 
>   $ rpm -ql kernel-tools-libs
>   /usr/lib64/libcpupower.so.0
>   /usr/lib64/libcpupower.so.0.0.0
>   /usr/lib64/libtraceevent.so.1
>   /usr/lib64/libtraceevent.so.1.1.0
> 
>   $ rpm -ql kernel-tools-libs-devel
>   /usr/include/cpufreq.h
>   /usr/include/traceevent
>   /usr/include/traceevent/event-parse.h
>   /usr/include/traceevent/event-utils.h
>   /usr/include/traceevent/kbuffer.h
>   /usr/lib64/libcpupower.so
>   /usr/lib64/libtraceevent.a
>   /usr/lib64/libtraceevent.so
> 
> and could build following ex.c outside the kernel tree:
> 
>   $ cat ex.c 
>   #include <traceevent/event-parse.h>
> 
>   int main(void)
>   {
>           struct pevent *pevent = pevent_alloc();
>           printf("krava %p\n", pevent);
>           return 0;
>   }
>   $ gcc -o ex ex.c -ltraceevent -ldl
>   $ ./ex
>   krava 0x10c6010
>   $

On my system, building ex.c with libtraceevent failed:

$ gcc -I ~/.local/include/ ex.c -L ~/.local/lib64 -ltraceevent -ldl
/home/namhyung/.local/lib64/libtraceevent.so: undefined reference to `str_error_r'
collect2: error: ld returned 1 exit status


Also I think it'd be better for libtraceevent has dependency to libdl
explicitly so that we can get rid of -ldl at the end.


>From ac7dc027274cb31f5860c4cf6219ea7584611e17 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Tue, 2 Aug 2016 12:03:00 +0900
Subject: [PATCH] tools lib traceevent: Add dependency to libdl

The libtraceevent has dependency to libdl due to plugins.  So if
external program wants to link libtraceevent it also needs to add -ldl
to the compiler command line.  Make it explicit so that external
programs doesn't care about the internel dependency of libtraceevent
anymore.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index c76012ebdb9c..bae0c090c638 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -173,7 +173,7 @@ $(TE_IN): force
 	$(Q)$(MAKE) $(build)=libtraceevent
 
 $(OUTPUT)libtraceevent.so.$(EVENT_PARSE_VERSION): $(TE_IN)
-	$(QUIET_LINK)$(CC) --shared $^ -Wl,-soname,libtraceevent.so.$(EP_VERSION) -o $@
+	$(QUIET_LINK)$(CC) --shared $^ -Wl,-soname,libtraceevent.so.$(EP_VERSION) -o $@ -ldl
 	@ln -sf $(@F) $(OUTPUT)libtraceevent.so
 	@ln -sf $(@F) $(OUTPUT)libtraceevent.so.$(EP_VERSION)
 
-- 
2.9.2

[PATCH 1/2] tools lib traceevent: Ignore generated library files

[Namhyung Kim]

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/traceevent/.gitignore b/tools/lib/traceevent/.gitignore
index 3c60335fe7be..9e9f25fb1922 100644
--- a/tools/lib/traceevent/.gitignore
+++ b/tools/lib/traceevent/.gitignore
@@ -1,2 +1,3 @@
 TRACEEVENT-CFLAGS
 libtraceevent-dynamic-list
+libtraceevent.so.*
-- 
2.9.2

[PATCH 2/2] tools lib traceevent: Add str_error_r()

[Namhyung Kim]

The libtraceevent uses str_error_r() but its implementation is outside
of the library.  So if an external program wants to link the
libtraceevent it'd fail due to missing str_error_r.  Add it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/Build | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build
index c681d0575d16..a4efdf7876ce 100644
--- a/tools/lib/traceevent/Build
+++ b/tools/lib/traceevent/Build
@@ -4,6 +4,10 @@ libtraceevent-y += trace-seq.o
 libtraceevent-y += parse-filter.o
 libtraceevent-y += parse-utils.o
 libtraceevent-y += kbuffer-parse.o
+libtraceevent-y += str_error_r.o
+
+str_error_r.o: ../str_error_r.c
+	$(call if_changed_dep,cc_o_c)
 
 plugin_jbd2-y         += plugin_jbd2.o
 plugin_hrtimer-y      += plugin_hrtimer.o
-- 
2.9.2

Re: [PATCH 1/4] tools lib traceevent: Add install_headers target

[Jiri Olsa]

On Tue, Aug 02, 2016 at 11:19:08AM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Mon, Aug 01, 2016 at 07:41:29PM +0200, Jiri Olsa wrote:
> > Adding install_headers target to install all headers
> > under 'include/traceevent' path, like:
> > 
> >   $ make DESTDIR=/tmp/krava prefix=/usr install_headers
> >   $ find /tmp/krava/ -type f
> >   /tmp/krava/usr/include/traceevent/kbuffer.h
> >   /tmp/krava/usr/include/traceevent/event-utils.h
> >   /tmp/krava/usr/include/traceevent/event-parse.h
> > 
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Link: http://lkml.kernel.org/n/tip-if70lj3zhdc3csdqm5webjvc@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/traceevent/Makefile | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
> > index 7851df1490e0..8e44bea646ee 100644
> > --- a/tools/lib/traceevent/Makefile
> > +++ b/tools/lib/traceevent/Makefile
> > @@ -240,7 +240,7 @@ define do_install
> >  	if [ ! -d '$(DESTDIR_SQ)$2' ]; then		\
> >  		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2';	\
> >  	fi;						\
> > -	$(INSTALL) $1 '$(DESTDIR_SQ)$2'
> > +	$(INSTALL) $(if $3,-m $3,) $1 '$(DESTDIR_SQ)$2'
> >  endef
> >  
> >  define do_install_plugins
> > @@ -264,6 +264,12 @@ install_plugins: $(PLUGINS)
> >  	$(call QUIET_INSTALL, trace_plugins) \
> >  		$(call do_install_plugins, $(PLUGINS))
> >  
> > +install_headers:
> > +	$(call QUIET_INSTALL, headers) \
> > +		$(call do_install,event-parse.h,$(prefix)/include/traceevent,644); \
> > +		$(call do_install,event-utils.h,$(prefix)/include/traceevent,644); \
> > +		$(call do_install,kbuffer.h,$(prefix)/include/traceevent,644)
> 
> For possible future changes, what about making it to use the wildcard
> somehow?  Or else, we could define HEADER_FILES variable..

right we can.. but I dont expect increase of header files any time soon ;-)

jirka

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Jiri Olsa]

On Tue, Aug 02, 2016 at 12:10:55PM +0900, Namhyung Kim wrote:
> On Mon, Aug 01, 2016 at 07:41:28PM +0200, Jiri Olsa wrote:
> > hi,
> > sending traceevent changes to make this lib installable
> > under rpm spec.
> > 
> > Basically adding support to:
> >   - install header files
> >   - install version links
> > 
> > Having this patchset applied over the fedora source,
> > I could built following rpms:
> > 
> >   kernel-tools-libs
> >   kernel-tools-libs-devel
> > 
> > with added libtraceevent stuff:
> > 
> >   $ rpm -ql kernel-tools-libs
> >   /usr/lib64/libcpupower.so.0
> >   /usr/lib64/libcpupower.so.0.0.0
> >   /usr/lib64/libtraceevent.so.1
> >   /usr/lib64/libtraceevent.so.1.1.0
> > 
> >   $ rpm -ql kernel-tools-libs-devel
> >   /usr/include/cpufreq.h
> >   /usr/include/traceevent
> >   /usr/include/traceevent/event-parse.h
> >   /usr/include/traceevent/event-utils.h
> >   /usr/include/traceevent/kbuffer.h
> >   /usr/lib64/libcpupower.so
> >   /usr/lib64/libtraceevent.a
> >   /usr/lib64/libtraceevent.so
> > 
> > and could build following ex.c outside the kernel tree:
> > 
> >   $ cat ex.c 
> >   #include <traceevent/event-parse.h>
> > 
> >   int main(void)
> >   {
> >           struct pevent *pevent = pevent_alloc();
> >           printf("krava %p\n", pevent);
> >           return 0;
> >   }
> >   $ gcc -o ex ex.c -ltraceevent -ldl
> >   $ ./ex
> >   krava 0x10c6010
> >   $
> 
> On my system, building ex.c with libtraceevent failed:
> 
> $ gcc -I ~/.local/include/ ex.c -L ~/.local/lib64 -ltraceevent -ldl
> /home/namhyung/.local/lib64/libtraceevent.so: undefined reference to `str_error_r'
> collect2: error: ld returned 1 exit status
> 
> 
> Also I think it'd be better for libtraceevent has dependency to libdl
> explicitly so that we can get rid of -ldl at the end.

agreed, I'll queue your patch if v2 is needed

thanks,
jirka

> 
> 
> From ac7dc027274cb31f5860c4cf6219ea7584611e17 Mon Sep 17 00:00:00 2001
> From: Namhyung Kim <namhyung@kernel.org>
> Date: Tue, 2 Aug 2016 12:03:00 +0900
> Subject: [PATCH] tools lib traceevent: Add dependency to libdl
> 
> The libtraceevent has dependency to libdl due to plugins.  So if
> external program wants to link libtraceevent it also needs to add -ldl
> to the compiler command line.  Make it explicit so that external
> programs doesn't care about the internel dependency of libtraceevent
> anymore.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/lib/traceevent/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
> index c76012ebdb9c..bae0c090c638 100644
> --- a/tools/lib/traceevent/Makefile
> +++ b/tools/lib/traceevent/Makefile
> @@ -173,7 +173,7 @@ $(TE_IN): force
>  	$(Q)$(MAKE) $(build)=libtraceevent
>  
>  $(OUTPUT)libtraceevent.so.$(EVENT_PARSE_VERSION): $(TE_IN)
> -	$(QUIET_LINK)$(CC) --shared $^ -Wl,-soname,libtraceevent.so.$(EP_VERSION) -o $@
> +	$(QUIET_LINK)$(CC) --shared $^ -Wl,-soname,libtraceevent.so.$(EP_VERSION) -o $@ -ldl
>  	@ln -sf $(@F) $(OUTPUT)libtraceevent.so
>  	@ln -sf $(@F) $(OUTPUT)libtraceevent.so.$(EP_VERSION)
>  
> -- 
> 2.9.2
> 

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Steven Rostedt]

On Tue, 2 Aug 2016 11:31:34 +0200
Jiri Olsa <jolsa@redhat.com> wrote:

> On Tue, Aug 02, 2016 at 12:10:55PM +0900, Namhyung Kim wrote:
> > On Mon, Aug 01, 2016 at 07:41:28PM +0200, Jiri Olsa wrote:  
> > > hi,
> > > sending traceevent changes to make this lib installable
> > > under rpm spec.
> > > 
> > > Basically adding support to:
> > >   - install header files
> > >   - install version links
> > > 
> > > Having this patchset applied over the fedora source,
> > > I could built following rpms:
> > > 
> > >   kernel-tools-libs
> > >   kernel-tools-libs-devel
> > > 
> > > with added libtraceevent stuff:
> > > 
> > >   $ rpm -ql kernel-tools-libs
> > >   /usr/lib64/libcpupower.so.0
> > >   /usr/lib64/libcpupower.so.0.0.0
> > >   /usr/lib64/libtraceevent.so.1
> > >   /usr/lib64/libtraceevent.so.1.1.0
> > > 
> > >   $ rpm -ql kernel-tools-libs-devel
> > >   /usr/include/cpufreq.h
> > >   /usr/include/traceevent
> > >   /usr/include/traceevent/event-parse.h
> > >   /usr/include/traceevent/event-utils.h
> > >   /usr/include/traceevent/kbuffer.h
> > >   /usr/lib64/libcpupower.so
> > >   /usr/lib64/libtraceevent.a
> > >   /usr/lib64/libtraceevent.so
> > > 
> > > and could build following ex.c outside the kernel tree:
> > > 
> > >   $ cat ex.c 
> > >   #include <traceevent/event-parse.h>
> > > 
> > >   int main(void)
> > >   {
> > >           struct pevent *pevent = pevent_alloc();
> > >           printf("krava %p\n", pevent);
> > >           return 0;
> > >   }
> > >   $ gcc -o ex ex.c -ltraceevent -ldl
> > >   $ ./ex
> > >   krava 0x10c6010
> > >   $  
> > 
> > On my system, building ex.c with libtraceevent failed:
> > 
> > $ gcc -I ~/.local/include/ ex.c -L ~/.local/lib64 -ltraceevent -ldl
> > /home/namhyung/.local/lib64/libtraceevent.so: undefined reference to `str_error_r'
> > collect2: error: ld returned 1 exit status
> > 
> > 
> > Also I think it'd be better for libtraceevent has dependency to libdl
> > explicitly so that we can get rid of -ldl at the end.  
> 
> agreed, I'll queue your patch if v2 is needed
> 

BTW, before we start making this ready for their own libraries, I'd
like to make some changes with the naming convention. Mainly with
event_format and format_field.

Perhaps we should change them to pevent_event and pevent_field?

-- Steve

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Jiri Olsa]

On Tue, Aug 02, 2016 at 09:48:23AM -0400, Steven Rostedt wrote:

SNIP

> > > 
> > > 
> > > Also I think it'd be better for libtraceevent has dependency to libdl
> > > explicitly so that we can get rid of -ldl at the end.  
> > 
> > agreed, I'll queue your patch if v2 is needed
> > 
> 
> BTW, before we start making this ready for their own libraries, I'd
> like to make some changes with the naming convention. Mainly with
> event_format and format_field.
> 
> Perhaps we should change them to pevent_event and pevent_field?

right, it does not have any global prefix for public symbols

I'd actualy expect something like 'traceevent_',
but 'pevent' is ok as well I guess

jirka

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Namhyung Kim]

On Tue, Aug 02, 2016 at 04:01:44PM +0200, Jiri Olsa wrote:
> On Tue, Aug 02, 2016 at 09:48:23AM -0400, Steven Rostedt wrote:
> 
> SNIP
> 
> > > > 
> > > > 
> > > > Also I think it'd be better for libtraceevent has dependency to libdl
> > > > explicitly so that we can get rid of -ldl at the end.  
> > > 
> > > agreed, I'll queue your patch if v2 is needed
> > > 
> > 
> > BTW, before we start making this ready for their own libraries, I'd
> > like to make some changes with the naming convention. Mainly with
> > event_format and format_field.
> > 
> > Perhaps we should change them to pevent_event and pevent_field?
> 
> right, it does not have any global prefix for public symbols
> 
> I'd actualy expect something like 'traceevent_',
> but 'pevent' is ok as well I guess

The 'pevent' prefix would be more consistent to similar APIs IMHO.

Maybe it's worth splitting public header and internal header files?

Thanks,
Namhyung

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Steven Rostedt]

On Tue, 2 Aug 2016 16:01:44 +0200
Jiri Olsa <jolsa@redhat.com> wrote:


> I'd actualy expect something like 'traceevent_',
> but 'pevent' is ok as well I guess
> 

Hmm, I like pevent because it is short (less typing ;-), but as this is
going to be in libtracevent, I'm wondering if we want to rename it to
traceevent_ or maybe tevent? or what about kevent?

-- Steve

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Arnaldo Carvalho de Melo]

Em Tue, Aug 02, 2016 at 11:14:11PM +0900, Namhyung Kim escreveu:
> On Tue, Aug 02, 2016 at 04:01:44PM +0200, Jiri Olsa wrote:
> > On Tue, Aug 02, 2016 at 09:48:23AM -0400, Steven Rostedt wrote:
> > 
> > SNIP
> > 
> > > > > 
> > > > > 
> > > > > Also I think it'd be better for libtraceevent has dependency to libdl
> > > > > explicitly so that we can get rid of -ldl at the end.  
> > > > 
> > > > agreed, I'll queue your patch if v2 is needed
> > > > 
> > > 
> > > BTW, before we start making this ready for their own libraries, I'd
> > > like to make some changes with the naming convention. Mainly with
> > > event_format and format_field.
> > > 
> > > Perhaps we should change them to pevent_event and pevent_field?
> > 
> > right, it does not have any global prefix for public symbols
> > 
> > I'd actualy expect something like 'traceevent_',
> > but 'pevent' is ok as well I guess
> 
> The 'pevent' prefix would be more consistent to similar APIs IMHO.
> 
> Maybe it's worth splitting public header and internal header files?

I'd suggest exporting just what is already used by tools, exposing more
as the need arises.

- Arnaldo

Re: [PATCH 2/2] tools lib traceevent: Add str_error_r()

[Namhyung Kim]

On Tue, Aug 2, 2016 at 2:01 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> The libtraceevent uses str_error_r() but its implementation is outside
> of the library.  So if an external program wants to link the
> libtraceevent it'd fail due to missing str_error_r.  Add it.

Please ignore this as it breaks perf build due to multiple definition. :-/

Thanks,
Namhyung


>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/lib/traceevent/Build | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build
> index c681d0575d16..a4efdf7876ce 100644
> --- a/tools/lib/traceevent/Build
> +++ b/tools/lib/traceevent/Build
> @@ -4,6 +4,10 @@ libtraceevent-y += trace-seq.o
>  libtraceevent-y += parse-filter.o
>  libtraceevent-y += parse-utils.o
>  libtraceevent-y += kbuffer-parse.o
> +libtraceevent-y += str_error_r.o
> +
> +str_error_r.o: ../str_error_r.c
> +       $(call if_changed_dep,cc_o_c)
>
>  plugin_jbd2-y         += plugin_jbd2.o
>  plugin_hrtimer-y      += plugin_hrtimer.o
> --
> 2.9.2
>



-- 
Thanks,
Namhyung

[tip:perf/urgent] tools lib traceevent: Ignore generated library files

[tip-bot for Namhyung Kim]

Commit-ID:  979a70a237efb68e15b1cee36f9f92037e71d1fd
Gitweb:     http://git.kernel.org/tip/979a70a237efb68e15b1cee36f9f92037e71d1fd
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Tue, 2 Aug 2016 14:01:47 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 2 Aug 2016 12:16:13 -0300

tools lib traceevent: Ignore generated library files

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/20160802050148.3413-1-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/traceevent/.gitignore b/tools/lib/traceevent/.gitignore
index 3c60335..9e9f25f 100644
--- a/tools/lib/traceevent/.gitignore
+++ b/tools/lib/traceevent/.gitignore
@@ -1,2 +1,3 @@
 TRACEEVENT-CFLAGS
 libtraceevent-dynamic-list
+libtraceevent.so.*

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Steven Rostedt]

On Tue, 2 Aug 2016 16:01:44 +0200
Jiri Olsa <jolsa@redhat.com> wrote:

> > BTW, before we start making this ready for their own libraries, I'd
> > like to make some changes with the naming convention. Mainly with
> > event_format and format_field.
> > 
> > Perhaps we should change them to pevent_event and pevent_field?  
> 
> right, it does not have any global prefix for public symbols
> 
> I'd actualy expect something like 'traceevent_',
> but 'pevent' is ok as well I guess

OK, what's the consensus here? Should we keep "pevent_" or switch to
the longer "traceevent_"?

I want to make the patch that gets this ready to ship!

-- Steve

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Arnaldo Carvalho de Melo]

Em Fri, Aug 05, 2016 at 10:13:52AM -0400, Steven Rostedt escreveu:
> On Tue, 2 Aug 2016 16:01:44 +0200
> Jiri Olsa <jolsa@redhat.com> wrote:
> > > BTW, before we start making this ready for their own libraries, I'd
> > > like to make some changes with the naming convention. Mainly with
> > > event_format and format_field.
> > > 
> > > Perhaps we should change them to pevent_event and pevent_field?  
> > 
> > right, it does not have any global prefix for public symbols
> > 
> > I'd actualy expect something like 'traceevent_',
> > but 'pevent' is ok as well I guess
> 
> OK, what's the consensus here? Should we keep "pevent_" or switch to
> the longer "traceevent_"?

traceevent_ matches libtraceevent, so I think would be more obvious the
association of code with the library.

- Arnaldo
 
> I want to make the patch that gets this ready to ship!
> 
> -- Steve

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Jiri Olsa]

On Fri, Aug 05, 2016 at 11:42:41AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Aug 05, 2016 at 10:13:52AM -0400, Steven Rostedt escreveu:
> > On Tue, 2 Aug 2016 16:01:44 +0200
> > Jiri Olsa <jolsa@redhat.com> wrote:
> > > > BTW, before we start making this ready for their own libraries, I'd
> > > > like to make some changes with the naming convention. Mainly with
> > > > event_format and format_field.
> > > > 
> > > > Perhaps we should change them to pevent_event and pevent_field?  
> > > 
> > > right, it does not have any global prefix for public symbols
> > > 
> > > I'd actualy expect something like 'traceevent_',
> > > but 'pevent' is ok as well I guess
> > 
> > OK, what's the consensus here? Should we keep "pevent_" or switch to
> > the longer "traceevent_"?
> 
> traceevent_ matches libtraceevent, so I think would be more obvious the
> association of code with the library.

+1 for traceevent_ ... makes more sense to me

jirka

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Jiri Olsa]

On Tue, Aug 02, 2016 at 09:48:23AM -0400, Steven Rostedt wrote:
> On Tue, 2 Aug 2016 11:31:34 +0200
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > On Tue, Aug 02, 2016 at 12:10:55PM +0900, Namhyung Kim wrote:
> > > On Mon, Aug 01, 2016 at 07:41:28PM +0200, Jiri Olsa wrote:  
> > > > hi,
> > > > sending traceevent changes to make this lib installable
> > > > under rpm spec.
> > > > 
> > > > Basically adding support to:
> > > >   - install header files
> > > >   - install version links
> > > > 
> > > > Having this patchset applied over the fedora source,
> > > > I could built following rpms:
> > > > 
> > > >   kernel-tools-libs
> > > >   kernel-tools-libs-devel
> > > > 
> > > > with added libtraceevent stuff:
> > > > 
> > > >   $ rpm -ql kernel-tools-libs
> > > >   /usr/lib64/libcpupower.so.0
> > > >   /usr/lib64/libcpupower.so.0.0.0
> > > >   /usr/lib64/libtraceevent.so.1
> > > >   /usr/lib64/libtraceevent.so.1.1.0
> > > > 
> > > >   $ rpm -ql kernel-tools-libs-devel
> > > >   /usr/include/cpufreq.h
> > > >   /usr/include/traceevent
> > > >   /usr/include/traceevent/event-parse.h
> > > >   /usr/include/traceevent/event-utils.h
> > > >   /usr/include/traceevent/kbuffer.h
> > > >   /usr/lib64/libcpupower.so
> > > >   /usr/lib64/libtraceevent.a
> > > >   /usr/lib64/libtraceevent.so
> > > > 
> > > > and could build following ex.c outside the kernel tree:
> > > > 
> > > >   $ cat ex.c 
> > > >   #include <traceevent/event-parse.h>
> > > > 
> > > >   int main(void)
> > > >   {
> > > >           struct pevent *pevent = pevent_alloc();
> > > >           printf("krava %p\n", pevent);
> > > >           return 0;
> > > >   }
> > > >   $ gcc -o ex ex.c -ltraceevent -ldl
> > > >   $ ./ex
> > > >   krava 0x10c6010
> > > >   $  
> > > 
> > > On my system, building ex.c with libtraceevent failed:
> > > 
> > > $ gcc -I ~/.local/include/ ex.c -L ~/.local/lib64 -ltraceevent -ldl
> > > /home/namhyung/.local/lib64/libtraceevent.so: undefined reference to `str_error_r'
> > > collect2: error: ld returned 1 exit status
> > > 
> > > 
> > > Also I think it'd be better for libtraceevent has dependency to libdl
> > > explicitly so that we can get rid of -ldl at the end.  
> > 
> > agreed, I'll queue your patch if v2 is needed
> > 
> 
> BTW, before we start making this ready for their own libraries, I'd
> like to make some changes with the naming convention. Mainly with
> event_format and format_field.
> 
> Perhaps we should change them to pevent_event and pevent_field?

hi,
I checked on this one and was surprised last email is from 2016 ;-)
so we did not move much with this.. is there still will to do that?

I think we were kind of waiting for the namespace changes in
traceevent library.. like to have some common prefix for public
functions/struct, like 'traceevent_' ?

thoughts? thanks,
jirka

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Steven Rostedt]

On Fri, 19 Jan 2018 11:37:13 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> hi,
> I checked on this one and was surprised last email is from 2016 ;-)
> so we did not move much with this.. is there still will to do that?
> 
> I think we were kind of waiting for the namespace changes in
> traceevent library.. like to have some common prefix for public
> functions/struct, like 'traceevent_' ?
> 
> thoughts? thanks,

Yes, actually this is back on my radar. I now have full time staff
working on trace-cmd and kernelshark. One of the up coming changes is
to start pushing libtraceevent as a real library which means changing
this.

Thus, do we think "traceevent_" is the proper naming? Could we shorten
it to "tevent_"? Need to get this bike-shedding exercise out of the way
first ;-)

-- Steve

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Jiri Olsa]

On Fri, Jan 19, 2018 at 10:07:28AM -0500, Steven Rostedt wrote:
> On Fri, 19 Jan 2018 11:37:13 +0100
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > hi,
> > I checked on this one and was surprised last email is from 2016 ;-)
> > so we did not move much with this.. is there still will to do that?
> > 
> > I think we were kind of waiting for the namespace changes in
> > traceevent library.. like to have some common prefix for public
> > functions/struct, like 'traceevent_' ?
> > 
> > thoughts? thanks,
> 
> Yes, actually this is back on my radar. I now have full time staff
> working on trace-cmd and kernelshark. One of the up coming changes is
> to start pushing libtraceevent as a real library which means changing
> this.
> 
> Thus, do we think "traceevent_" is the proper naming? Could we shorten
> it to "tevent_"? Need to get this bike-shedding exercise out of the way
> first ;-)

tevent is ok with me.. one character far from the current one ;-)

jirka

Re: [RFC 0/4] tools lib traceevent: Install fixes

[Namhyung Kim]

Hi Jiri and Steve,

On Fri, Jan 19, 2018 at 04:15:02PM +0100, Jiri Olsa wrote:
> On Fri, Jan 19, 2018 at 10:07:28AM -0500, Steven Rostedt wrote:
> > On Fri, 19 Jan 2018 11:37:13 +0100
> > Jiri Olsa <jolsa@redhat.com> wrote:
> > 
> > > hi,
> > > I checked on this one and was surprised last email is from 2016 ;-)
> > > so we did not move much with this.. is there still will to do that?
> > > 
> > > I think we were kind of waiting for the namespace changes in
> > > traceevent library.. like to have some common prefix for public
> > > functions/struct, like 'traceevent_' ?
> > > 
> > > thoughts? thanks,
> > 
> > Yes, actually this is back on my radar. I now have full time staff
> > working on trace-cmd and kernelshark. One of the up coming changes is
> > to start pushing libtraceevent as a real library which means changing
> > this.
> > 
> > Thus, do we think "traceevent_" is the proper naming? Could we shorten
> > it to "tevent_"? Need to get this bike-shedding exercise out of the way
> > first ;-)
> 
> tevent is ok with me.. one character far from the current one ;-)

I'd suggest 'trev'.

Thanks,
Namhyung