Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] trace-cmd: Re-enable the build of KernelShark v1
@ 2020-12-16 13:07 Yordan Karadzhov (VMware)
  2020-12-16 20:26 ` Steven Rostedt
  2020-12-16 20:28 ` Steven Rostedt
  0 siblings, 2 replies; 5+ messages in thread
From: Yordan Karadzhov (VMware) @ 2020-12-16 13:07 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, Yordan Karadzhov (VMware)

libtraceevent and libtracefs are now stand-alone libraries, independent
from trace-cmd, but nevertheless trace-cmd still provides legacy/obsolete
versions of this libraries. Since we are "freezing" KernelShark v1 and
all active development is now focused on  KernelShark v2, we will keep
v1 use the legacy/obsolete versions of the libraries.

An additional problem is that trace-filter-hash.h is no longer a public
header of libtracecmd. For this reason we are adding the path to the
tarce-cmd private headers to the list of header file locations.

We also remove the inclusion of event-utils.h in parse-utils.c in order
to fix a compilation error when building libtraceevent.a.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 Makefile                              | 16 ++++++++-------
 kernel-shark/CMakeLists.txt           |  1 +
 kernel-shark/build/FindTraceCmd.cmake | 29 ++++++---------------------
 kernel-shark/src/libkshark.c          | 14 ++++++++-----
 kernel-shark/src/libkshark.h          |  2 +-
 lib/traceevent/parse-utils.c          |  2 --
 6 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/Makefile b/Makefile
index ab91ae7..a2a41ae 100644
--- a/Makefile
+++ b/Makefile
@@ -356,13 +356,6 @@ $(PKG_CONFIG_FILE) : ${PKG_CONFIG_SOURCE_FILE}.template
 $(kshark-dir)/build/Makefile: $(kshark-dir)/CMakeLists.txt
 	$(Q) cd $(kshark-dir)/build && $(CMAKE_COMMAND) -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) -D_INSTALL_PREFIX=$(prefix) -D_LIBDIR=$(libdir) ..
 
-gui: force
-	$(MAKE) $(CMD_TARGETS)
-	$(MAKE) $(kshark-dir)/build/Makefile
-	$(Q)$(MAKE) $(S) -C $(kshark-dir)/build
-	@echo "gui build complete"
-	@echo "  kernelshark located at $(kshark-dir)/bin"
-
 trace-cmd: force $(LIBTRACEEVENT_STATIC_BUILD) $(LIBTRACECMD_STATIC) $(LIBTRACEFS_STATIC_BUILD) \
 	force $(obj)/lib/trace-cmd/plugins/tracecmd_plugin_dir
 	$(Q)$(MAKE) -C $(src)/tracecmd $(obj)/tracecmd/$@
@@ -387,6 +380,15 @@ libtracefs.a: $(LIBTRACEFS_STATIC)
 
 libs: $(LIBTRACECMD_SHARED) $(LIBTRACEEVENT_STATIC_BUILD) $(LIBTRACEFS_STATIC_BUILD)
 
+gui: force
+	$(MAKE) $(CMD_TARGETS)
+	$(MAKE) $(LIBTRACEEVENT_STATIC)
+	$(MAKE) $(LIBTRACEFS_STATIC)
+	$(MAKE) $(kshark-dir)/build/Makefile
+	$(Q)$(MAKE) $(S) -C $(kshark-dir)/build
+	@echo "gui build complete"
+	@echo "  kernelshark located at $(kshark-dir)/bin"
+
 test: force $(LIBTRACEEVENT_STATIC_BUILD) $(LIBTRACEFS_STATIC_BUILD) $(LIBTRACECMD_STATIC)
 ifneq ($(CUNIT_INSTALLED),1)
 	$(error CUnit framework not installed, cannot build unit tests))
diff --git a/kernel-shark/CMakeLists.txt b/kernel-shark/CMakeLists.txt
index c95249e..0c0b825 100644
--- a/kernel-shark/CMakeLists.txt
+++ b/kernel-shark/CMakeLists.txt
@@ -81,6 +81,7 @@ include_directories(${KS_DIR}/src/
                     ${KS_DIR}/build/src/
                     ${JSONC_INCLUDE_DIR}
                     ${TRACECMD_INCLUDE_DIR}
+                    ${TRACECMD_PRIVATE_INCLUDE_DIR}
                     ${TRACEFS_INCLUDE_DIR})
 
 message("")
diff --git a/kernel-shark/build/FindTraceCmd.cmake b/kernel-shark/build/FindTraceCmd.cmake
index c29b779..a40f70e 100644
--- a/kernel-shark/build/FindTraceCmd.cmake
+++ b/kernel-shark/build/FindTraceCmd.cmake
@@ -14,25 +14,15 @@
 # MESSAGE(" Looking for trace-cmd ...")
 
 # First search in the user provided paths.
-if (CMAKE_BUILD_TYPE MATCHES Debug)
-
-  find_program(TRACECMD_EXECUTABLE   NAMES  trace-cmd
-                                     PATHS  $ENV{TRACE_CMD}/tracecmd/
-                                            ${CMAKE_SOURCE_DIR}/../tracecmd/
-                                     NO_DEFAULT_PATH)
-
-endif (CMAKE_BUILD_TYPE MATCHES Debug)
-
-if (NOT TRACECMD_EXECUTABLE)
-
-  set(TRACECMD_EXECUTABLE "${_INSTALL_PREFIX}/bin/trace-cmd")
-
-endif (NOT TRACECMD_EXECUTABLE)
 
 find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h
                                 PATHS  $ENV{TRACE_CMD}/include/
                                        ${CMAKE_SOURCE_DIR}/../include/
                                 NO_DEFAULT_PATH)
+find_path(TRACECMD_PRIVATE_INCLUDE_DIR  NAMES private/trace-filter-hash.h
+                                PATHS  $ENV{TRACE_CMD}/lib/trace-cmd/include/
+                                       ${CMAKE_SOURCE_DIR}/../lib/trace-cmd/include/
+                                NO_DEFAULT_PATH)
 find_path(TRACEFS_INCLUDE_DIR   NAMES  tracefs/tracefs.h
                                 PATHS  $ENV{TRACE_CMD}/include/
                                        ${CMAKE_SOURCE_DIR}/../include/
@@ -53,20 +43,13 @@ find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.a
                                        ${CMAKE_SOURCE_DIR}/../lib/
                                 NO_DEFAULT_PATH)
 
-# If not found, search in the default system paths. Note that if the previous
-# search was successful "find_path" will do nothing this time.
 find_program(TRACECMD_EXECUTABLE   NAMES  trace-cmd)
-find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h)
-find_path(TRACEFS_INCLUDE_DIR   NAMES  tracefs/tracefs.h)
-find_library(TRACECMD_LIBRARY   NAMES  trace-cmd/libtracecmd.so)
-find_library(TRACEFS_LIBRARY    NAMES  tracefs/libtracefs.so)
-find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.so)
 
-IF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY)
+IF (TRACECMD_INCLUDE_DIR AND TRACECMD_PRIVATE_INCLUDE_DIR AND TRACECMD_LIBRARY)
 
   SET(TRACECMD_FOUND TRUE)
 
-ENDIF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY)
+ENDIF (TRACECMD_INCLUDE_DIR AND TRACECMD_PRIVATE_INCLUDE_DIR AND TRACECMD_LIBRARY)
 
 IF (TRACECMD_FOUND)
 
diff --git a/kernel-shark/src/libkshark.c b/kernel-shark/src/libkshark.c
index 52aacd3..4e625a2 100644
--- a/kernel-shark/src/libkshark.c
+++ b/kernel-shark/src/libkshark.c
@@ -141,10 +141,14 @@ bool kshark_open(struct kshark_context *kshark_ctx, const char *file)
 
 	kshark_free_task_list(kshark_ctx);
 
-	handle = tracecmd_open(file);
+	handle = tracecmd_open_head(file);
 	if (!handle)
 		return false;
 
+	/* Read the tracing data from the file. */
+	if (tracecmd_init_data(handle) < 0)
+		return false;
+
 	if (pthread_mutex_init(&kshark_ctx->input_mutex, NULL) != 0) {
 		tracecmd_close(handle);
 		return false;
@@ -696,7 +700,7 @@ static ssize_t get_records(struct kshark_context *kshark_ctx,
 	int pid;
 	int cpu;
 
-	n_cpus = tracecmd_cpus(kshark_ctx->handle);
+	n_cpus = tep_get_cpus(kshark_ctx->pevent);;
 	cpu_list = calloc(n_cpus, sizeof(*cpu_list));
 	if (!cpu_list)
 		return -ENOMEM;
@@ -867,7 +871,7 @@ ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 	if (total < 0)
 		goto fail;
 
-	n_cpus = tracecmd_cpus(kshark_ctx->handle);
+	n_cpus = tep_get_cpus(kshark_ctx->pevent);;
 
 	rows = calloc(total, sizeof(struct kshark_entry *));
 	if (!rows)
@@ -923,7 +927,7 @@ ssize_t kshark_load_data_records(struct kshark_context *kshark_ctx,
 	if (total < 0)
 		goto fail;
 
-	n_cpus = tracecmd_cpus(kshark_ctx->handle);
+	n_cpus = tep_get_cpus(kshark_ctx->pevent);;
 
 	rows = calloc(total, sizeof(struct tep_record *));
 	if (!rows)
@@ -1047,7 +1051,7 @@ size_t kshark_load_data_matrix(struct kshark_context *kshark_ctx,
 	if (total < 0)
 		goto fail;
 
-	n_cpus = tracecmd_cpus(kshark_ctx->handle);
+	n_cpus = tep_get_cpus(kshark_ctx->pevent);;
 
 	status = data_matrix_alloc(total, offset_array,
 					  cpu_array,
diff --git a/kernel-shark/src/libkshark.h b/kernel-shark/src/libkshark.h
index 0d6c50d..a9cba05 100644
--- a/kernel-shark/src/libkshark.h
+++ b/kernel-shark/src/libkshark.h
@@ -26,7 +26,7 @@ extern "C" {
 
 // trace-cmd
 #include "trace-cmd/trace-cmd.h"
-#include "trace-cmd/trace-filter-hash.h"
+#include "private/trace-filter-hash.h"
 #include "traceevent/event-parse.h"
 #include "tracefs/tracefs.h"
 
diff --git a/lib/traceevent/parse-utils.c b/lib/traceevent/parse-utils.c
index 150cee8..62d2789 100644
--- a/lib/traceevent/parse-utils.c
+++ b/lib/traceevent/parse-utils.c
@@ -9,8 +9,6 @@
 #include <stdarg.h>
 #include <errno.h>
 
-#include "event-utils.h"
-
 #define __weak __attribute__((weak))
 
 void __vwarning(const char *fmt, va_list ap)
-- 
2.25.1


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

* Re: [PATCH v2] trace-cmd: Re-enable the build of KernelShark v1
  2020-12-16 13:07 [PATCH v2] trace-cmd: Re-enable the build of KernelShark v1 Yordan Karadzhov (VMware)
@ 2020-12-16 20:26 ` Steven Rostedt
  2020-12-16 20:28 ` Steven Rostedt
  1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-12-16 20:26 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel

On Wed, 16 Dec 2020 15:07:15 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> libtraceevent and libtracefs are now stand-alone libraries, independent
> from trace-cmd, but nevertheless trace-cmd still provides legacy/obsolete
> versions of this libraries. Since we are "freezing" KernelShark v1 and
> all active development is now focused on  KernelShark v2, we will keep
> v1 use the legacy/obsolete versions of the libraries.
> 
> An additional problem is that trace-filter-hash.h is no longer a public
> header of libtracecmd. For this reason we are adding the path to the
> tarce-cmd private headers to the list of header file locations.
> 
> We also remove the inclusion of event-utils.h in parse-utils.c in order
> to fix a compilation error when building libtraceevent.a.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  Makefile                              | 16 ++++++++-------
>  kernel-shark/CMakeLists.txt           |  1 +
>  kernel-shark/build/FindTraceCmd.cmake | 29 ++++++---------------------
>  kernel-shark/src/libkshark.c          | 14 ++++++++-----
>  kernel-shark/src/libkshark.h          |  2 +-
>  lib/traceevent/parse-utils.c          |  2 --
>  6 files changed, 26 insertions(+), 38 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index ab91ae7..a2a41ae 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -356,13 +356,6 @@ $(PKG_CONFIG_FILE) : ${PKG_CONFIG_SOURCE_FILE}.template
>  $(kshark-dir)/build/Makefile: $(kshark-dir)/CMakeLists.txt
>  	$(Q) cd $(kshark-dir)/build && $(CMAKE_COMMAND) -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) -D_INSTALL_PREFIX=$(prefix) -D_LIBDIR=$(libdir) ..
>  
> -gui: force
> -	$(MAKE) $(CMD_TARGETS)
> -	$(MAKE) $(kshark-dir)/build/Makefile
> -	$(Q)$(MAKE) $(S) -C $(kshark-dir)/build
> -	@echo "gui build complete"
> -	@echo "  kernelshark located at $(kshark-dir)/bin"
> -
>  trace-cmd: force $(LIBTRACEEVENT_STATIC_BUILD) $(LIBTRACECMD_STATIC) $(LIBTRACEFS_STATIC_BUILD) \
>  	force $(obj)/lib/trace-cmd/plugins/tracecmd_plugin_dir
>  	$(Q)$(MAKE) -C $(src)/tracecmd $(obj)/tracecmd/$@
> @@ -387,6 +380,15 @@ libtracefs.a: $(LIBTRACEFS_STATIC)
>  
>  libs: $(LIBTRACECMD_SHARED) $(LIBTRACEEVENT_STATIC_BUILD) $(LIBTRACEFS_STATIC_BUILD)
>  
> +gui: force
> +	$(MAKE) $(CMD_TARGETS)
> +	$(MAKE) $(LIBTRACEEVENT_STATIC)
> +	$(MAKE) $(LIBTRACEFS_STATIC)
> +	$(MAKE) $(kshark-dir)/build/Makefile
> +	$(Q)$(MAKE) $(S) -C $(kshark-dir)/build
> +	@echo "gui build complete"
> +	@echo "  kernelshark located at $(kshark-dir)/bin"
> +
>

The above didn't seem to work, so I changed it to:

  gui: force $(CMD_TARGETS) $(LIBTRACEEVENT_STATIC) $(LIBTRACEFS_STATIC)
	$(MAKE) $(kshark-dir)/build/Makefile
	$(Q)$(MAKE) $(S) -C $(kshark-dir)/build
	@echo "gui build complete"
	@echo "  kernelshark located at $(kshark-dir)/bin"


And let the make dependencies do the work for us. And this appears to have
worked.

Want to send a v3?

Thanks,

-- Steve

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

* Re: [PATCH v2] trace-cmd: Re-enable the build of KernelShark v1
  2020-12-16 13:07 [PATCH v2] trace-cmd: Re-enable the build of KernelShark v1 Yordan Karadzhov (VMware)
  2020-12-16 20:26 ` Steven Rostedt
@ 2020-12-16 20:28 ` Steven Rostedt
  2020-12-16 22:19   ` Steven Rostedt
  1 sibling, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2020-12-16 20:28 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel

On Wed, 16 Dec 2020 15:07:15 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> --- a/lib/traceevent/parse-utils.c
> +++ b/lib/traceevent/parse-utils.c
> @@ -9,8 +9,6 @@
>  #include <stdarg.h>
>  #include <errno.h>
>  
> -#include "event-utils.h"
> -
>  #define __weak __attribute__((weak))
>  
>  void __vwarning(const char *fmt, va_list ap)
> -- 

Is this needed?

I removed this hunk, and it still builds fine for me. After I did the
dependency change mentioned in the other email.

-- Steve

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

* Re: [PATCH v2] trace-cmd: Re-enable the build of KernelShark v1
  2020-12-16 20:28 ` Steven Rostedt
@ 2020-12-16 22:19   ` Steven Rostedt
  2020-12-16 23:00     ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2020-12-16 22:19 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel

On Wed, 16 Dec 2020 15:28:50 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 16 Dec 2020 15:07:15 +0200
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
> > --- a/lib/traceevent/parse-utils.c
> > +++ b/lib/traceevent/parse-utils.c
> > @@ -9,8 +9,6 @@
> >  #include <stdarg.h>
> >  #include <errno.h>
> >  
> > -#include "event-utils.h"
> > -
> >  #define __weak __attribute__((weak))
> >  
> >  void __vwarning(const char *fmt, va_list ap)
> > --   
> 
> Is this needed?
> 
> I removed this hunk, and it still builds fine for me. After I did the
> dependency change mentioned in the other email.

I was building without libtraceevent installed, and it worked for me. But I
see why you did this. After installing libtraceevent on my host, I get:

( cc -c  -g -Wall -DVSOCK -D_GNU_SOURCE -DWARN_NO_AUDIT -DNO_AUDIT -I/work/git/trace-cmd.git/include -I/work/git/trace-cmd.git/../../include -I/work/git/trace-cmd.git/include/trace-cmd -I/work/git/trace-cmd.git/lib/trace-cmd/include -I/work/git/trace-cmd.git/lib/trace-cmd/include/private -I/work/git/trace-cmd.git/tracecmd/include -I/usr/local/include/traceevent  -I/usr/local/include/tracefs  -DVAR_DIR="/var" '-DPLUGIN_TRACEEVENT_DIR="/usr/local/lib64/traceevent/plugins"' '-DPLUGIN_TRACECMD_DIR=""'   -DHAVE_BLK_TC_FLUSH -I/work/git/trace-cmd.git/lib/traceevent/include -std=gnu99 -fPIC parse-utils.c -o /work/git/trace-cmd.git/lib/traceevent/parse-utils.o)
parse-utils.c:16:6: error: conflicting types for ‘__vwarning’
   16 | void __vwarning(const char *fmt, va_list ap)
      |      ^~~~~~~~~~
In file included from parse-utils.c:12:
/usr/local/include/traceevent/event-utils.h:20:6: note: previous declaration of ‘__vwarning’ was here
   20 | void __vwarning(const char *fmt, ...);
      |      ^~~~~~~~~~
parse-utils.c:41:6: error: conflicting types for ‘__vpr_stat’
   41 | void __vpr_stat(const char *fmt, va_list ap)
      |      ^~~~~~~~~~
In file included from parse-utils.c:12:
/usr/local/include/traceevent/event-utils.h:21:6: note: previous declaration of ‘__vpr_stat’ was here
   21 | void __vpr_stat(const char *fmt, ...);
      |      ^~~~~~~~~~
make[1]: *** [Makefile:50: /work/git/trace-cmd.git/lib/traceevent/parse-utils.o] Error 1
make: *** [Makefile:363: /work/git/trace-cmd.git/lib/traceevent/libtraceevent.a] Error 2


Ideally, we don't want to include the libtraceevent or libtracefs system
includes when building locally. Let me see if I can fix the make file here.

-- Steve

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

* Re: [PATCH v2] trace-cmd: Re-enable the build of KernelShark v1
  2020-12-16 22:19   ` Steven Rostedt
@ 2020-12-16 23:00     ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-12-16 23:00 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel

On Wed, 16 Dec 2020 17:19:44 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Ideally, we don't want to include the libtraceevent or libtracefs system
> includes when building locally. Let me see if I can fix the make file here.

Can you apply this? It should fix the build problem with libtraceevent,
without having to touch any of its files.

  https://patchwork.kernel.org/project/linux-trace-devel/patch/20201216175138.2d1a9b34@gandalf.local.home/

-- Steve


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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-16 13:07 [PATCH v2] trace-cmd: Re-enable the build of KernelShark v1 Yordan Karadzhov (VMware)
2020-12-16 20:26 ` Steven Rostedt
2020-12-16 20:28 ` Steven Rostedt
2020-12-16 22:19   ` Steven Rostedt
2020-12-16 23:00     ` Steven Rostedt

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git