[PATCH 0/5] Nodifications needed by the NumPy interface

[PATCH 0/5] Nodifications needed by the NumPy interface

[Yordan Karadzhov]

The patch-set contains various modifications needed in order to implement
a standalone version of the NumPy data interface. Although those changes
are not strictly required for KS 1.0 it will be great if at least
patches 1-3 can go upstream befor the release. 

Yordan Karadzhov (5):
  kernel-shark: Add new dataloading method to be used by the NumPu
    interface
  kernel-shark: Use full paths for non-standard library headers
  kernel-shark: Use the shared versions of the trace-cmd libraries.
  trace-cmd: Install public interface headers
  kernel-shark: Install public interface headers

 Makefile                              |  26 +++--
 include/trace-cmd/trace-cmd.h         |   2 +-
 include/traceevent/event-parse.h      |   2 +-
 kernel-shark/CMakeLists.txt           |   3 +-
 kernel-shark/README                   |   4 +
 kernel-shark/build/FindTraceCmd.cmake |  40 +++----
 kernel-shark/src/CMakeLists.txt       |  42 ++++++-
 kernel-shark/src/libkshark-plugin.h   |   2 +-
 kernel-shark/src/libkshark.c          | 155 ++++++++++++++++++++++++++
 kernel-shark/src/libkshark.h          |  16 ++-
 10 files changed, 250 insertions(+), 42 deletions(-)

-- 
2.20.1

[PATCH 1/5] kernel-shark: Add new dataloading method to be used by the NumPu interface

[Yordan Karadzhov]

The new function loads the content of the trace data file into a
table / matrix, made of columns / arrays of data having various integer
types. Later those arrays will be wrapped as NumPy arrays.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/libkshark.c | 155 +++++++++++++++++++++++++++++++++++
 kernel-shark/src/libkshark.h |   7 ++
 2 files changed, 162 insertions(+)

diff --git a/kernel-shark/src/libkshark.c b/kernel-shark/src/libkshark.c
index 175279c..ac634fd 100644
--- a/kernel-shark/src/libkshark.c
+++ b/kernel-shark/src/libkshark.c
@@ -957,6 +957,161 @@ ssize_t kshark_load_data_records(struct kshark_context *kshark_ctx,
 	return -ENOMEM;
 }
 
+static bool data_matrix_alloc(size_t n_rows, uint64_t **offset_array,
+					     uint8_t **cpu_array,
+					     uint64_t **ts_array,
+					     uint16_t **pid_array,
+					     int **event_array)
+{
+	if (offset_array)
+		*offset_array = NULL;
+
+	if (cpu_array)
+		*cpu_array = NULL;
+
+	if (ts_array)
+		*ts_array = NULL;
+
+	if (pid_array)
+		*pid_array = NULL;
+
+	if (event_array)
+		*event_array = NULL;
+
+	if (offset_array) {
+		*offset_array = calloc(n_rows, sizeof(**offset_array));
+		if (!*offset_array)
+			goto free_all;
+	}
+
+	if (cpu_array) {
+		*cpu_array = calloc(n_rows, sizeof(**cpu_array));
+		if (!*cpu_array)
+			goto free_all;
+	}
+
+	if (ts_array) {
+		*ts_array = calloc(n_rows, sizeof(**ts_array));
+		if (!*ts_array)
+			goto free_all;
+	}
+
+	if (pid_array) {
+		*pid_array = calloc(n_rows, sizeof(**pid_array));
+		if (!*pid_array)
+			goto free_all;
+	}
+
+	if (event_array) {
+		*event_array = calloc(n_rows, sizeof(**event_array));
+		if (!*event_array)
+			goto free_all;
+	}
+
+	return true;
+
+ free_all:
+	fprintf(stderr, "Failed to allocate memory during data loading.\n");
+
+	if (offset_array)
+		free(*offset_array);
+
+	if (cpu_array)
+		free(*cpu_array);
+
+	if (ts_array)
+		free(*ts_array);
+
+	if (pid_array)
+		free(*pid_array);
+
+	if (event_array)
+		free(*event_array);
+
+	return false;
+}
+
+/**
+ * @brief Load the content of the trace data file into a table / matrix made
+ *	  of columns / arrays of data. The user is responsible for freeing the
+ *	  elements of the outputted array
+ *
+ * @param kshark_ctx: Input location for the session context pointer.
+ * @param offset_array: Output location for the array of record offsets.
+ * @param cpu_array: Output location for the array of CPU Ids.
+ * @param ts_array: Output location for the array of timestamps.
+ * @param pid_array: Output location for the array of Process Ids.
+ * @param event_array: Output location for the array of Event Ids.
+ *
+ * @returns The size of the outputted arrays in the case of success, or a
+ *	    negative error code on failure.
+ */
+size_t kshark_load_data_matrix(struct kshark_context *kshark_ctx,
+			       uint64_t **offset_array,
+			       uint8_t **cpu_array,
+			       uint64_t **ts_array,
+			       uint16_t **pid_array,
+			       int **event_array)
+{
+	enum rec_type type = REC_ENTRY;
+	struct rec_list **rec_list;
+	size_t count, total = 0;
+	bool status;
+	int n_cpus;
+
+	total = get_records(kshark_ctx, &rec_list, type);
+	if (total < 0)
+		goto fail;
+
+	n_cpus = tracecmd_cpus(kshark_ctx->handle);
+
+	status = data_matrix_alloc(total, offset_array,
+					  cpu_array,
+					  ts_array,
+					  pid_array,
+					  event_array);
+	if (!status)
+		goto fail_free;
+
+	for (count = 0; count < total; count++) {
+		int next_cpu;
+
+		next_cpu = pick_next_cpu(rec_list, n_cpus, type);
+		if (next_cpu >= 0) {
+			struct kshark_entry *e = &rec_list[next_cpu]->entry;
+
+			if (offset_array)
+				(*offset_array)[count] = e->offset;
+
+			if (cpu_array)
+				(*cpu_array)[count] = e->cpu;
+
+			if (ts_array)
+				(*ts_array)[count] = e->ts;
+
+			if (pid_array)
+				(*pid_array)[count] = e->pid;
+
+			if (event_array)
+				(*event_array)[count] = e->event_id;
+
+			rec_list[next_cpu] = rec_list[next_cpu]->next;
+			free(e);
+		}
+	}
+
+	/* There should be no entries left in rec_list. */
+	free_rec_list(rec_list, n_cpus, type);
+	return total;
+
+ fail_free:
+	free_rec_list(rec_list, n_cpus, type);
+
+ fail:
+	fprintf(stderr, "Failed to allocate memory during data loading.\n");
+	return -ENOMEM;
+}
+
 static const char *kshark_get_latency(struct tep_handle *pe,
 				      struct tep_record *record)
 {
diff --git a/kernel-shark/src/libkshark.h b/kernel-shark/src/libkshark.h
index c218b61..92ade41 100644
--- a/kernel-shark/src/libkshark.h
+++ b/kernel-shark/src/libkshark.h
@@ -149,6 +149,13 @@ ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 ssize_t kshark_load_data_records(struct kshark_context *kshark_ctx,
 				 struct tep_record ***data_rows);
 
+size_t kshark_load_data_matrix(struct kshark_context *kshark_ctx,
+			       uint64_t **offset_array,
+			       uint8_t **cpu_array,
+			       uint64_t **ts_array,
+			       uint16_t **pid_array,
+			       int **event_array);
+
 ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids);
 
 void kshark_close(struct kshark_context *kshark_ctx);
-- 
2.20.1

[PATCH 2/5] kernel-shark: Use full paths for non-standard library headers

[Yordan Karadzhov]

The all header files included in the public interfaces of the trace-cmd
and KernelShark libraries have to use full path when including other
non-standard library headers. This will be useful if someone wants to use
those public interfaces from there installation location.

You may need to clean Cmake's cache after aplaying this patch:

cd kernel-shark/build
./cmake_clean.sh
cmake ..

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 include/trace-cmd/trace-cmd.h         |  2 +-
 include/traceevent/event-parse.h      |  2 +-
 kernel-shark/CMakeLists.txt           |  3 +-
 kernel-shark/build/FindTraceCmd.cmake | 40 ++++++++++++---------------
 kernel-shark/src/libkshark-plugin.h   |  2 +-
 kernel-shark/src/libkshark.h          |  9 +++---
 6 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index ceb03f4..9320098 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -6,7 +6,7 @@
 #ifndef _TRACE_CMD_H
 #define _TRACE_CMD_H
 
-#include "event-parse.h"
+#include "traceevent/event-parse.h"
 
 #define ARRAY_SIZE(_a) (sizeof(_a) / sizeof((_a)[0]))
 #define __weak __attribute__((weak))
diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index 5e0fd19..65cabd9 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -12,7 +12,7 @@
 #include <regex.h>
 #include <string.h>
 
-#include "trace-seq.h"
+#include "traceevent/trace-seq.h"
 
 #ifndef __maybe_unused
 #define __maybe_unused __attribute__((unused))
diff --git a/kernel-shark/CMakeLists.txt b/kernel-shark/CMakeLists.txt
index 52e4a29..145b058 100644
--- a/kernel-shark/CMakeLists.txt
+++ b/kernel-shark/CMakeLists.txt
@@ -56,8 +56,7 @@ SET(CMAKE_BUILD_WITH_INSTALL_RPATH FALSE)
 include_directories(${KS_DIR}/src/
                     ${KS_DIR}/build/src/
                     ${JSONC_INCLUDE_DIR}
-                    ${TRACECMD_INCLUDE_DIR}
-                    ${TRACEEVENT_INCLUDE_DIR})
+                    ${TRACECMD_INCLUDE_DIR})
 
 message("")
 message(STATUS "C flags      : " ${CMAKE_C_FLAGS})
diff --git a/kernel-shark/build/FindTraceCmd.cmake b/kernel-shark/build/FindTraceCmd.cmake
index b09a11b..21315e7 100644
--- a/kernel-shark/build/FindTraceCmd.cmake
+++ b/kernel-shark/build/FindTraceCmd.cmake
@@ -2,7 +2,6 @@
 # This module finds an installed trace-cmd package.
 #
 # It sets the following variables:
-#  TRACEEVENT_INCLUDE_DIR, where to find traceevent header.
 #  TRACEEVENT_LIBRARY_DIR , where to find the traceevent library.
 #  TRACEEVENT_LIBRARY, traceevent the library.
 #  TRACEEVENT_FOUND, If false, do not try to use traceevent.
@@ -20,26 +19,32 @@ find_path(TRACECMD_BIN_DIR      NAMES  trace-cmd
                                        ${CMAKE_SOURCE_DIR}/../tracecmd/
                                 NO_DEFAULT_PATH)
 
-find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd.h
-                                PATHS  $ENV{TRACE_CMD}/include/trace-cmd/
-                                       ${CMAKE_SOURCE_DIR}/../include/trace-cmd/
+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_LIBRARY_DIR  NAMES  libtracecmd.a
-                                PATHS  $ENV{TRACE_CMD}/lib/trace-cmd/
-                                       ${CMAKE_SOURCE_DIR}/../lib/trace-cmd/
+find_path(TRACECMD_LIBRARY_DIR  NAMES  trace-cmd/libtracecmd.a
+                                PATHS  $ENV{TRACE_CMD}/lib/
+                                       ${CMAKE_SOURCE_DIR}/../lib/
                                 NO_DEFAULT_PATH)
 
+find_path(TRACEEVENT_LIBRARY_DIR  NAMES  traceevent/libtraceevent.a
+                                  PATHS  $ENV{TRACE_CMD}/lib/
+                                         ${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_path(TRACECMD_BIN_DIR      NAMES  trace-cmd)
-find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd.h)
-find_path(TRACECMD_LIBRARY_DIR  NAMES  libtracecmd.a)
+find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h)
+find_library(TRACECMD_LIBRARY_DIR  NAMES  trace-cmd/libtracecmd.a)
+find_library(TRACEEVENT_LIBRARY_DIR  NAMES  traceevent/libtraceevent.a)
 
 IF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY_DIR)
 
   SET(TRACECMD_FOUND TRUE)
-  SET(TRACECMD_LIBRARY "${TRACECMD_LIBRARY_DIR}/libtracecmd.a")
+  SET(TRACECMD_LIBRARY "${TRACECMD_LIBRARY_DIR}/trace-cmd/libtracecmd.a")
 
 ENDIF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY_DIR)
 
@@ -53,21 +58,12 @@ ELSE (TRACECMD_FOUND)
 
 ENDIF (TRACECMD_FOUND)
 
-
-find_path(TRACEEVENT_INCLUDE_DIR  NAMES  event-parse.h
-                                  PATHS  $ENV{TRACE_CMD}/include/traceevent/
-                                         ${CMAKE_SOURCE_DIR}/../include/traceevent/)
-
-find_path(TRACEEVENT_LIBRARY_DIR  NAMES  libtraceevent.a
-                                  PATHS  $ENV{TRACE_CMD}/lib/traceevent/
-                                         ${CMAKE_SOURCE_DIR}/../lib/traceevent/)
-
-IF (TRACEEVENT_INCLUDE_DIR AND TRACEEVENT_LIBRARY_DIR)
+IF (TRACEEVENT_LIBRARY_DIR)
 
   SET(TRACEEVENT_FOUND TRUE)
-  SET(TRACEEVENT_LIBRARY "${TRACEEVENT_LIBRARY_DIR}/libtraceevent.a")
+  SET(TRACEEVENT_LIBRARY "${TRACEEVENT_LIBRARY_DIR}/traceevent/libtraceevent.a")
 
-ENDIF (TRACEEVENT_INCLUDE_DIR AND TRACEEVENT_LIBRARY_DIR)
+ENDIF (TRACEEVENT_LIBRARY_DIR)
 
 IF (TRACEEVENT_FOUND)
 
diff --git a/kernel-shark/src/libkshark-plugin.h b/kernel-shark/src/libkshark-plugin.h
index 0cb677a..b3cf1c6 100644
--- a/kernel-shark/src/libkshark-plugin.h
+++ b/kernel-shark/src/libkshark-plugin.h
@@ -17,7 +17,7 @@ extern "C" {
 #endif // __cplusplus
 
 // trace-cmd
-#include "event-parse.h"
+#include "traceevent/event-parse.h"
 
 /* Quiet warnings over documenting simple structures */
 //! @cond Doxygen_Suppress
diff --git a/kernel-shark/src/libkshark.h b/kernel-shark/src/libkshark.h
index 92ade41..e4349c3 100644
--- a/kernel-shark/src/libkshark.h
+++ b/kernel-shark/src/libkshark.h
@@ -18,17 +18,16 @@
 #include <errno.h>
 
 // Json-C
-#include <json.h>
+#include <json-c/json.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 // trace-cmd
-#include "trace-cmd.h"
-#include "trace-filter-hash.h"
-#include "event-parse.h"
-#include "trace-filter-hash.h"
+#include "trace-cmd/trace-cmd.h"
+#include "trace-cmd/trace-filter-hash.h"
+#include "traceevent/event-parse.h"
 
 // KernelShark
 #include "libkshark-plugin.h"
-- 
2.20.1

[PATCH 3/5] kernel-shark: Use the shared versions of the trace-cmd libraries.

[Yordan Karadzhov]

You may need to clean Cmake's cache after aplaying this patch:

cd kernel-shark/build
./cmake_clean.sh
cmake ..

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/build/FindTraceCmd.cmake | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel-shark/build/FindTraceCmd.cmake b/kernel-shark/build/FindTraceCmd.cmake
index 21315e7..7ebf8da 100644
--- a/kernel-shark/build/FindTraceCmd.cmake
+++ b/kernel-shark/build/FindTraceCmd.cmake
@@ -24,12 +24,12 @@ find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h
                                        ${CMAKE_SOURCE_DIR}/../include/
                                 NO_DEFAULT_PATH)
 
-find_path(TRACECMD_LIBRARY_DIR  NAMES  trace-cmd/libtracecmd.a
+find_path(TRACECMD_LIBRARY_DIR  NAMES  trace-cmd/libtracecmd.so
                                 PATHS  $ENV{TRACE_CMD}/lib/
                                        ${CMAKE_SOURCE_DIR}/../lib/
                                 NO_DEFAULT_PATH)
 
-find_path(TRACEEVENT_LIBRARY_DIR  NAMES  traceevent/libtraceevent.a
+find_path(TRACEEVENT_LIBRARY_DIR  NAMES  traceevent/libtraceevent.so
                                   PATHS  $ENV{TRACE_CMD}/lib/
                                          ${CMAKE_SOURCE_DIR}/../lib/
                                   NO_DEFAULT_PATH)
@@ -38,13 +38,13 @@ find_path(TRACEEVENT_LIBRARY_DIR  NAMES  traceevent/libtraceevent.a
 # search was successful "find_path" will do nothing this time.
 find_path(TRACECMD_BIN_DIR      NAMES  trace-cmd)
 find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h)
-find_library(TRACECMD_LIBRARY_DIR  NAMES  trace-cmd/libtracecmd.a)
-find_library(TRACEEVENT_LIBRARY_DIR  NAMES  traceevent/libtraceevent.a)
+find_library(TRACECMD_LIBRARY_DIR  NAMES  trace-cmd/libtracecmd.so)
+find_library(TRACEEVENT_LIBRARY_DIR  NAMES  traceevent/libtraceevent.so)
 
 IF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY_DIR)
 
   SET(TRACECMD_FOUND TRUE)
-  SET(TRACECMD_LIBRARY "${TRACECMD_LIBRARY_DIR}/trace-cmd/libtracecmd.a")
+  SET(TRACECMD_LIBRARY "${TRACECMD_LIBRARY_DIR}/trace-cmd/libtracecmd.so")
 
 ENDIF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY_DIR)
 
@@ -61,7 +61,7 @@ ENDIF (TRACECMD_FOUND)
 IF (TRACEEVENT_LIBRARY_DIR)
 
   SET(TRACEEVENT_FOUND TRUE)
-  SET(TRACEEVENT_LIBRARY "${TRACEEVENT_LIBRARY_DIR}/traceevent/libtraceevent.a")
+  SET(TRACEEVENT_LIBRARY "${TRACEEVENT_LIBRARY_DIR}/traceevent/libtraceevent.so")
 
 ENDIF (TRACEEVENT_LIBRARY_DIR)
 
-- 
2.20.1

[PATCH 4/5] trace-cmd: Install public interface headers

[Yordan Karadzhov]

Installing the header files included in the public interfaces of
the trace-cmd libraries will make possible the libraries to be used
for development.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 Makefile | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index cde45f8..ac514b1 100644
--- a/Makefile
+++ b/Makefile
@@ -49,10 +49,18 @@ html_install = $(prefix)/share/kernelshark/html
 html_install_SQ = '$(subst ','\'',$(html_install))'
 img_install = $(prefix)/share/kernelshark/html/images
 img_install_SQ = '$(subst ','\'',$(img_install))'
-libdir ?= $(prefix)/lib
-libdir_SQ = '$(subst ','\'',$(libdir))'
-includedir = $(prefix)/include/trace-cmd
-includedir_SQ = '$(subst ','\'',$(includedir))'
+
+tcmd_libdir ?= $(prefix)/lib/trace-cmd
+tcmd_libdir_SQ = '$(subst ','\'',$(tcmd_libdir))'
+
+tevt_libdir ?= $(prefix)/lib/traceevent
+tevt_libdir_SQ = '$(subst ','\'',$(tevt_libdir))'
+
+tcmd_includedir = $(prefix)/include/trace-cmd
+tcmd_includedir_SQ = '$(subst ','\'',$(tcmd_includedir))'
+
+tevt_includedir = $(prefix)/include/traceevent
+tevt_includedir_SQ = '$(subst ','\'',$(tevt_includedir))'
 
 export man_dir man_dir_SQ html_install html_install_SQ INSTALL
 export img_install img_install_SQ
@@ -335,10 +343,12 @@ install_gui: install_cmd gui
 	$(Q)$(MAKE) $(S) -C $(kshark-dir)/build install
 
 install_libs: libs
-	$(Q)$(call do_install,$(LIBTRACECMD_SHARED),$(libdir_SQ))
-	$(Q)$(call do_install,$(LIBTRACEEVENT_SHARED),$(libdir_SQ))
-	$(Q)$(call do_install,$(src)/include/traceevent/event-parse.h,$(includedir_SQ))
-	$(Q)$(call do_install,$(src)/include/trace-cmd/trace-cmd.h,$(includedir_SQ))
+	$(Q)$(call do_install,$(LIBTRACECMD_SHARED),$(tcmd_libdir_SQ))
+	$(Q)$(call do_install,$(LIBTRACEEVENT_SHARED),$(tevt_libdir_SQ))
+	$(Q)$(call do_install,$(src)/include/traceevent/event-parse.h,$(tevt_includedir_SQ))
+	$(Q)$(call do_install,$(src)/include/traceevent/trace-seq.h,$(tevt_includedir_SQ))
+	$(Q)$(call do_install,$(src)/include/trace-cmd/trace-cmd.h,$(tcmd_includedir_SQ))
+	$(Q)$(call do_install,$(src)/include/trace-cmd/trace-filter-hash.h,$(tcmd_includedir_SQ))
 
 doc:
 	$(MAKE) -C $(src)/Documentation all
-- 
2.20.1

[PATCH 5/5] kernel-shark: Install public interface headers

[Yordan Karadzhov]

Installing the header files included in the public interfaces of
the KernelShark libraries will make possible the libraries to be used
for development.

A symbolic link that points to the latest versions of the libraries is
added to INSTALL_PREFIX/lib/kernelshark

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/README             |  4 ++++
 kernel-shark/src/CMakeLists.txt | 42 +++++++++++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/kernel-shark/README b/kernel-shark/README
index 379c390..573e279 100644
--- a/kernel-shark/README
+++ b/kernel-shark/README
@@ -44,6 +44,10 @@ as a CMake Command-Line option.
 2.1.3 By default, installation prefix is "/usr/local". It can be changed using
 -D_INSTALL_PREFIX= as a CMake Command-Line option.
 
+2.1.4 In order to install a development version (including headers e.t.c) add
+-D_DEVEL=1 as a CMake Command-Line option.
+
+
 Example:
     cmake -D_DOXYGEN_DOC=1 -D_DEBUG=1 -D_INSTALL_PREFIX=/usr ../
 
diff --git a/kernel-shark/src/CMakeLists.txt b/kernel-shark/src/CMakeLists.txt
index 6cbc00f..6dd9b5a 100644
--- a/kernel-shark/src/CMakeLists.txt
+++ b/kernel-shark/src/CMakeLists.txt
@@ -1,5 +1,13 @@
 message("\n src ...")
 
+macro(install_symlink filepath sympath)
+    install(CODE "execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink ${filepath} ${sympath})")
+    install(CODE "LIST(APPEND CMAKE_INSTALL_MANIFEST_FILES ${sympath})")
+    install(CODE "message(\"-- Created symlink: ${sympath} -> ${filepath}\")")
+endmacro(install_symlink)
+
+set(KS_INCLUDS_DESTINATION "${_INSTALL_PREFIX}/include/${KS_APP_NAME}")
+
 message(STATUS "libkshark")
 add_library(kshark SHARED libkshark.c
                           libkshark-model.c
@@ -14,6 +22,22 @@ target_link_libraries(kshark ${CMAKE_DL_LIBS}
 
 set_target_properties(kshark  PROPERTIES SUFFIX	".so.${KS_VERSION_STRING}")
 
+install(TARGETS kshark LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/${KS_APP_NAME})
+
+if (_DEVEL)
+
+    install_symlink("libkshark.so.${KS_VERSION_STRING}"
+                    "${_INSTALL_PREFIX}/lib/${KS_APP_NAME}/libkshark.so")
+
+    install(FILES "${KS_DIR}/src/libkshark.h"
+            DESTINATION ${KS_INCLUDS_DESTINATION})
+    install(FILES "${KS_DIR}/src/libkshark-plugin.h"
+            DESTINATION ${KS_INCLUDS_DESTINATION})
+    install(FILES "${KS_DIR}/src/libkshark-model.h"
+            DESTINATION ${KS_INCLUDS_DESTINATION})
+
+endif (_DEVEL)
+
 if (OPENGL_FOUND AND GLUT_FOUND)
 
     message(STATUS "libkshark-plot")
@@ -26,6 +50,20 @@ if (OPENGL_FOUND AND GLUT_FOUND)
 
     set_target_properties(kshark-plot PROPERTIES  SUFFIX ".so.${KS_VERSION_STRING}")
 
+    install(TARGETS kshark-plot LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/${KS_APP_NAME})
+
+    if (_DEVEL)
+
+        install_symlink("libkshark-plot.so.${KS_VERSION_STRING}"
+                        "${_INSTALL_PREFIX}/lib/${KS_APP_NAME}/libkshark-plot.so")
+
+        install(FILES "${KS_DIR}/src/KsPlotTools.hpp"
+                DESTINATION ${KS_INCLUDS_DESTINATION})
+        install(FILES "${KS_DIR}/src/libkshark-plot.h"
+                DESTINATION ${KS_INCLUDS_DESTINATION})
+
+    endif (_DEVEL)
+
 endif (OPENGL_FOUND AND GLUT_FOUND)
 
 if (Qt5Widgets_FOUND AND Qt5Network_FOUND)
@@ -77,9 +115,9 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND)
     add_executable(kshark-record        kshark-record.cpp)
     target_link_libraries(kshark-record kshark-gui)
 
-    install(TARGETS ${KS_APP_NAME} kshark-record kshark kshark-plot kshark-gui
+    install(TARGETS ${KS_APP_NAME} kshark-record kshark-gui
             RUNTIME DESTINATION ${_INSTALL_PREFIX}/bin/
-            LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/${KS_APP_NAME}/)
+            LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/${KS_APP_NAME})
 
     install(FILES "${KS_DIR}/${KS_APP_NAME}.desktop"
             DESTINATION ${_INSTALL_PREFIX}/share/applications/)
-- 
2.20.1

Re: [PATCH 1/5] kernel-shark: Add new dataloading method to be used by the NumPu interface

[Steven Rostedt]

On Thu, 23 May 2019 18:18:08 +0300
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> The new function loads the content of the trace data file into a
> table / matrix, made of columns / arrays of data having various integer
> types. Later those arrays will be wrapped as NumPy arrays.
> 
> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  kernel-shark/src/libkshark.c | 155 +++++++++++++++++++++++++++++++++++
>  kernel-shark/src/libkshark.h |   7 ++
>  2 files changed, 162 insertions(+)
> 
> diff --git a/kernel-shark/src/libkshark.c b/kernel-shark/src/libkshark.c
> index 175279c..ac634fd 100644
> --- a/kernel-shark/src/libkshark.c
> +++ b/kernel-shark/src/libkshark.c
> @@ -957,6 +957,161 @@ ssize_t kshark_load_data_records(struct kshark_context *kshark_ctx,
>  	return -ENOMEM;
>  }
>  
> +static bool data_matrix_alloc(size_t n_rows, uint64_t **offset_array,
> +					     uint8_t **cpu_array,
> +					     uint64_t **ts_array,
> +					     uint16_t **pid_array,
> +					     int **event_array)
> +{
> +	if (offset_array)
> +		*offset_array = NULL;
> +
> +	if (cpu_array)
> +		*cpu_array = NULL;
> +
> +	if (ts_array)
> +		*ts_array = NULL;
> +
> +	if (pid_array)
> +		*pid_array = NULL;
> +
> +	if (event_array)
> +		*event_array = NULL;
> +
> +	if (offset_array) {

The way you can do this, is remove the above and have:

> +		*offset_array = calloc(n_rows, sizeof(**offset_array));
> +		if (!*offset_array)

			return false;

> +			goto free_all;
> +	}
> +
> +	if (cpu_array) {
> +		*cpu_array = calloc(n_rows, sizeof(**cpu_array));
> +		if (!*cpu_array)

			goto free_offset;

> +			goto free_all;
> +	}
> +
> +	if (ts_array) {
> +		*ts_array = calloc(n_rows, sizeof(**ts_array));
> +		if (!*ts_array)

			goto free_cpu;

> +			goto free_all;
> +	}
> +
> +	if (pid_array) {
> +		*pid_array = calloc(n_rows, sizeof(**pid_array));
> +		if (!*pid_array)

			goto free_ts;

> +			goto free_all;
> +	}
> +
> +	if (event_array) {
> +		*event_array = calloc(n_rows, sizeof(**event_array));
> +		if (!*event_array)

			goto free_pid;

> +			goto free_all;
> +	}
> +
> +	return true;
> +

We can have a helper function:

static inline void free_ptr(void *ptr)
{
	if (ptr)
		free(*(void **)ptr);
}

free_pid:
	free_ptr(pid_array);
free_ts:
	free_ptr(ts_array);
free_cpu:
	free_ptr(cpu_array);
free_offset:
	free_ptr(offset_array);

Then have the print here.

> + free_all:
> +	fprintf(stderr, "Failed to allocate memory during data loading.\n");

	return false;

This is the way it's usually done in the Linux kernel.

> +
> +	if (offset_array)
> +		free(*offset_array);
> +
> +	if (cpu_array)
> +		free(*cpu_array);
> +
> +	if (ts_array)
> +		free(*ts_array);
> +
> +	if (pid_array)
> +		free(*pid_array);
> +
> +	if (event_array)
> +		free(*event_array);
> +
> +	return false;
> +}
> +
> +/**
> + * @brief Load the content of the trace data file into a table / matrix made
> + *	  of columns / arrays of data. The user is responsible for freeing the
> + *	  elements of the outputted array
> + *
> + * @param kshark_ctx: Input location for the session context pointer.
> + * @param offset_array: Output location for the array of record offsets.
> + * @param cpu_array: Output location for the array of CPU Ids.
> + * @param ts_array: Output location for the array of timestamps.
> + * @param pid_array: Output location for the array of Process Ids.
> + * @param event_array: Output location for the array of Event Ids.
> + *
> + * @returns The size of the outputted arrays in the case of success, or a
> + *	    negative error code on failure.
> + */
> +size_t kshark_load_data_matrix(struct kshark_context *kshark_ctx,
> +			       uint64_t **offset_array,
> +			       uint8_t **cpu_array,
> +			       uint64_t **ts_array,
> +			       uint16_t **pid_array,
> +			       int **event_array)
> +{
> +	enum rec_type type = REC_ENTRY;
> +	struct rec_list **rec_list;
> +	size_t count, total = 0;
> +	bool status;
> +	int n_cpus;
> +
> +	total = get_records(kshark_ctx, &rec_list, type);
> +	if (total < 0)
> +		goto fail;
> +
> +	n_cpus = tracecmd_cpus(kshark_ctx->handle);
> +
> +	status = data_matrix_alloc(total, offset_array,
> +					  cpu_array,
> +					  ts_array,
> +					  pid_array,
> +					  event_array);

BTW, have you looked into how much memory this takes up in a large
trace?

> +	if (!status)
> +		goto fail_free;
> +
> +	for (count = 0; count < total; count++) {
> +		int next_cpu;
> +
> +		next_cpu = pick_next_cpu(rec_list, n_cpus, type);
> +		if (next_cpu >= 0) {
> +			struct kshark_entry *e = &rec_list[next_cpu]->entry;

Hmm, this looks like we are taking an address of a field and then
freeing it down below. Looking at the definition of rec_list, this is
currently OK. But this coding style is not robust because of the tight
dependency to how rec_list is defined. If that ever changes it will be
hard to find code like this to update it.

A more robust way to do this is:

			struct rec_list *rec = rec_list[next_cpu];
			struct kshark_entry *e = &rec->entry;

> +
> +			if (offset_array)
> +				(*offset_array)[count] = e->offset;
> +
> +			if (cpu_array)
> +				(*cpu_array)[count] = e->cpu;
> +
> +			if (ts_array)
> +				(*ts_array)[count] = e->ts;
> +
> +			if (pid_array)
> +				(*pid_array)[count] = e->pid;
> +
> +			if (event_array)
> +				(*event_array)[count] = e->event_id;
> +
> +			rec_list[next_cpu] = rec_list[next_cpu]->next;

Then here:

			free(rec);

That way there's not a dependency here with the data structure layout
of rec_list and the freeing of the entry.

-- Steve


> +			free(e);
> +		}
> +	}
> +
> +	/* There should be no entries left in rec_list. */
> +	free_rec_list(rec_list, n_cpus, type);
> +	return total;
> +
> + fail_free:
> +	free_rec_list(rec_list, n_cpus, type);
> +
> + fail:
> +	fprintf(stderr, "Failed to allocate memory during data
> loading.\n");
> +	return -ENOMEM;
> +}
> +
>  static const char *kshark_get_latency(struct tep_handle *pe,
>  				      struct tep_record *record)
>  {
> diff --git a/kernel-shark/src/libkshark.h
> b/kernel-shark/src/libkshark.h index c218b61..92ade41 100644
> --- a/kernel-shark/src/libkshark.h
> +++ b/kernel-shark/src/libkshark.h
> @@ -149,6 +149,13 @@ ssize_t kshark_load_data_entries(struct
> kshark_context *kshark_ctx, ssize_t kshark_load_data_records(struct
> kshark_context *kshark_ctx, struct tep_record ***data_rows);
>  
> +size_t kshark_load_data_matrix(struct kshark_context *kshark_ctx,
> +			       uint64_t **offset_array,
> +			       uint8_t **cpu_array,
> +			       uint64_t **ts_array,
> +			       uint16_t **pid_array,
> +			       int **event_array);
> +
>  ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int
> **pids); 
>  void kshark_close(struct kshark_context *kshark_ctx);

Re: [PATCH 1/5] kernel-shark: Add new dataloading method to be used by the NumPu interface

[Yordan Karadzhov (VMware)]

On 29.05.19 г. 20:36 ч., Steven Rostedt wrote:
> On Thu, 23 May 2019 18:18:08 +0300
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
>> The new function loads the content of the trace data file into a
>> table / matrix, made of columns / arrays of data having various integer
>> types. Later those arrays will be wrapped as NumPy arrays.
>>
>> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
>> ---
>>   kernel-shark/src/libkshark.c | 155 +++++++++++++++++++++++++++++++++++
>>   kernel-shark/src/libkshark.h |   7 ++
>>   2 files changed, 162 insertions(+)
>>
>> diff --git a/kernel-shark/src/libkshark.c b/kernel-shark/src/libkshark.c
>> index 175279c..ac634fd 100644
>> --- a/kernel-shark/src/libkshark.c
>> +++ b/kernel-shark/src/libkshark.c
>> @@ -957,6 +957,161 @@ ssize_t kshark_load_data_records(struct kshark_context *kshark_ctx,
>>   	return -ENOMEM;
>>   }
>>   
>> +static bool data_matrix_alloc(size_t n_rows, uint64_t **offset_array,
>> +					     uint8_t **cpu_array,
>> +					     uint64_t **ts_array,
>> +					     uint16_t **pid_array,
>> +					     int **event_array)
>> +{
>> +	if (offset_array)
>> +		*offset_array = NULL;
>> +
>> +	if (cpu_array)
>> +		*cpu_array = NULL;
>> +
>> +	if (ts_array)
>> +		*ts_array = NULL;
>> +
>> +	if (pid_array)
>> +		*pid_array = NULL;
>> +
>> +	if (event_array)
>> +		*event_array = NULL;
>> +
>> +	if (offset_array) {
> 
> The way you can do this, is remove the above and have:
> 
>> +		*offset_array = calloc(n_rows, sizeof(**offset_array));
>> +		if (!*offset_array)
> 
> 			return false;
> 
>> +			goto free_all;
>> +	}
>> +
>> +	if (cpu_array) {
>> +		*cpu_array = calloc(n_rows, sizeof(**cpu_array));
>> +		if (!*cpu_array)
> 
> 			goto free_offset;
> 
>> +			goto free_all;
>> +	}
>> +
>> +	if (ts_array) {
>> +		*ts_array = calloc(n_rows, sizeof(**ts_array));
>> +		if (!*ts_array)
> 
> 			goto free_cpu;
> 
>> +			goto free_all;
>> +	}
>> +
>> +	if (pid_array) {
>> +		*pid_array = calloc(n_rows, sizeof(**pid_array));
>> +		if (!*pid_array)
> 
> 			goto free_ts;
> 
>> +			goto free_all;
>> +	}
>> +
>> +	if (event_array) {
>> +		*event_array = calloc(n_rows, sizeof(**event_array));
>> +		if (!*event_array)
> 
> 			goto free_pid;
> 
>> +			goto free_all;
>> +	}
>> +
>> +	return true;
>> +
> 
> We can have a helper function:
> 
> static inline void free_ptr(void *ptr)
> {
> 	if (ptr)
> 		free(*(void **)ptr);
> }
> 
> free_pid:
> 	free_ptr(pid_array);
> free_ts:
> 	free_ptr(ts_array);
> free_cpu:
> 	free_ptr(cpu_array);
> free_offset:
> 	free_ptr(offset_array);
> 
> Then have the print here.
> 
>> + free_all:
>> +	fprintf(stderr, "Failed to allocate memory during data loading.\n");
> 
> 	return false;
> 
> This is the way it's usually done in the Linux kernel.
> 

Great! Really elegant solution.


>> +
>> +	if (offset_array)
>> +		free(*offset_array);
>> +
>> +	if (cpu_array)
>> +		free(*cpu_array);
>> +
>> +	if (ts_array)
>> +		free(*ts_array);
>> +
>> +	if (pid_array)
>> +		free(*pid_array);
>> +
>> +	if (event_array)
>> +		free(*event_array);
>> +
>> +	return false;
>> +}
>> +
>> +/**
>> + * @brief Load the content of the trace data file into a table / matrix made
>> + *	  of columns / arrays of data. The user is responsible for freeing the
>> + *	  elements of the outputted array
>> + *
>> + * @param kshark_ctx: Input location for the session context pointer.
>> + * @param offset_array: Output location for the array of record offsets.
>> + * @param cpu_array: Output location for the array of CPU Ids.
>> + * @param ts_array: Output location for the array of timestamps.
>> + * @param pid_array: Output location for the array of Process Ids.
>> + * @param event_array: Output location for the array of Event Ids.
>> + *
>> + * @returns The size of the outputted arrays in the case of success, or a
>> + *	    negative error code on failure.
>> + */
>> +size_t kshark_load_data_matrix(struct kshark_context *kshark_ctx,
>> +			       uint64_t **offset_array,
>> +			       uint8_t **cpu_array,
>> +			       uint64_t **ts_array,
>> +			       uint16_t **pid_array,
>> +			       int **event_array)
>> +{
>> +	enum rec_type type = REC_ENTRY;
>> +	struct rec_list **rec_list;
>> +	size_t count, total = 0;
>> +	bool status;
>> +	int n_cpus;
>> +
>> +	total = get_records(kshark_ctx, &rec_list, type);
>> +	if (total < 0)
>> +		goto fail;
>> +
>> +	n_cpus = tracecmd_cpus(kshark_ctx->handle);
>> +
>> +	status = data_matrix_alloc(total, offset_array,
>> +					  cpu_array,
>> +					  ts_array,
>> +					  pid_array,
>> +					  event_array);
> 
> BTW, have you looked into how much memory this takes up in a large
> trace?

One record takes 24 bytes. So it will allocate total*24 bytes (or less 
if some of the input arguments is NULL).


> 
>> +	if (!status)
>> +		goto fail_free;
>> +
>> +	for (count = 0; count < total; count++) {
>> +		int next_cpu;
>> +
>> +		next_cpu = pick_next_cpu(rec_list, n_cpus, type);
>> +		if (next_cpu >= 0) {
>> +			struct kshark_entry *e = &rec_list[next_cpu]->entry;
> 
> Hmm, this looks like we are taking an address of a field and then
> freeing it down below. Looking at the definition of rec_list, this is
> currently OK. But this coding style is not robust because of the tight
> dependency to how rec_list is defined. If that ever changes it will be
> hard to find code like this to update it.
> 
> A more robust way to do this is:
> 
> 			struct rec_list *rec = rec_list[next_cpu];
> 			struct kshark_entry *e = &rec->entry;
> 
>> +
>> +			if (offset_array)
>> +				(*offset_array)[count] = e->offset;
>> +
>> +			if (cpu_array)
>> +				(*cpu_array)[count] = e->cpu;
>> +
>> +			if (ts_array)
>> +				(*ts_array)[count] = e->ts;
>> +
>> +			if (pid_array)
>> +				(*pid_array)[count] = e->pid;
>> +
>> +			if (event_array)
>> +				(*event_array)[count] = e->event_id;
>> +
>> +			rec_list[next_cpu] = rec_list[next_cpu]->next;
> 
> Then here:
> 
> 			free(rec);
> 
> That way there's not a dependency here with the data structure layout
> of rec_list and the freeing of the entry.
> 
> -- Steve
> 

Thanks a lot! Sending updated version.
Y.

> 
>> +			free(e);
>> +		}
>> +	}
>> +
>> +	/* There should be no entries left in rec_list. */
>> +	free_rec_list(rec_list, n_cpus, type);
>> +	return total;
>> +
>> + fail_free:
>> +	free_rec_list(rec_list, n_cpus, type);
>> +
>> + fail:
>> +	fprintf(stderr, "Failed to allocate memory during data
>> loading.\n");
>> +	return -ENOMEM;
>> +}
>> +
>>   static const char *kshark_get_latency(struct tep_handle *pe,
>>   				      struct tep_record *record)
>>   {
>> diff --git a/kernel-shark/src/libkshark.h
>> b/kernel-shark/src/libkshark.h index c218b61..92ade41 100644
>> --- a/kernel-shark/src/libkshark.h
>> +++ b/kernel-shark/src/libkshark.h
>> @@ -149,6 +149,13 @@ ssize_t kshark_load_data_entries(struct
>> kshark_context *kshark_ctx, ssize_t kshark_load_data_records(struct
>> kshark_context *kshark_ctx, struct tep_record ***data_rows);
>>   
>> +size_t kshark_load_data_matrix(struct kshark_context *kshark_ctx,
>> +			       uint64_t **offset_array,
>> +			       uint8_t **cpu_array,
>> +			       uint64_t **ts_array,
>> +			       uint16_t **pid_array,
>> +			       int **event_array);
>> +
>>   ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int
>> **pids);
>>   void kshark_close(struct kshark_context *kshark_ctx);
>