[PATCH v2 0/9] Introduce the basic part of the C API of KS-1.0

[PATCH v2 0/9] Introduce the basic part of the C API of KS-1.0

[Yordan Karadzhov (VMware)]

This series of patches introduces the first part of the C API used
by the Qt-based version of KernelShark. This part of the API is
responsible for loading trace data files and for filtering of this
data. Examples, demonstrationg the usage of this part of the API are
added as well.

This is the second version of this series of patches.
Major changes from v1 are:

[2/9] The Doxygen documentation shows the source code. The huge template
configuration file is replaced by a short file containing only the
parameters, having non-default values.

[6/9] This version of the patch contains a number of improvements
suggested by Steven Rostedt in his review. Thanks Steven!

[8/9] New patch introducing the Advanced filtering. This is a
sophisticated filtering, based on the content of the trace event.

[9/9] New patche adding an example how to use the Advanced filtering.


Yordan Karadzhov (VMware) (9):
  kernel-shark-qt: Add Cmake build system for the Qt based KernelShark
  kernel-shark-qt: Automatic generation of doxygen documentation
  kernel-shark-qt: Add API for loading trace.dat files
  kernel-shark-qt: Add an example showing how to load trace data
  kernel-shark-qt: Add a README file to trace-cmd/kernel-shark-qt
  kernel-shark-qt: Add filtering to the C API of KernelShark
  kernel-shark-qt: Add an example showing how to filter trace data
  kernel-shark-qt: Add Advanced filter to the session context.
  kernel-shark-qt: Add example of advanded filtering

 kernel-shark-qt/CMakeLists.txt           |  53 ++
 kernel-shark-qt/README                   |  44 ++
 kernel-shark-qt/build/FindTraceCmd.cmake |  70 +++
 kernel-shark-qt/build/cmake_clean.sh     |  11 +
 kernel-shark-qt/build/deff.h.cmake       |  20 +
 kernel-shark-qt/doc/dox_config           |  15 +
 kernel-shark-qt/examples/CMakeLists.txt  |   9 +
 kernel-shark-qt/examples/datafilter.c    | 148 +++++
 kernel-shark-qt/examples/dataload.c      |  83 +++
 kernel-shark-qt/src/CMakeLists.txt       |  13 +
 kernel-shark-qt/src/libkshark.c          | 654 +++++++++++++++++++++++
 kernel-shark-qt/src/libkshark.h          | 273 ++++++++++
 12 files changed, 1393 insertions(+)
 create mode 100644 kernel-shark-qt/CMakeLists.txt
 create mode 100644 kernel-shark-qt/README
 create mode 100644 kernel-shark-qt/build/FindTraceCmd.cmake
 create mode 100755 kernel-shark-qt/build/cmake_clean.sh
 create mode 100644 kernel-shark-qt/build/deff.h.cmake
 create mode 100644 kernel-shark-qt/doc/dox_config
 create mode 100644 kernel-shark-qt/examples/CMakeLists.txt
 create mode 100644 kernel-shark-qt/examples/datafilter.c
 create mode 100644 kernel-shark-qt/examples/dataload.c
 create mode 100644 kernel-shark-qt/src/CMakeLists.txt
 create mode 100644 kernel-shark-qt/src/libkshark.c
 create mode 100644 kernel-shark-qt/src/libkshark.h

-- 
2.17.1

[PATCH v2 1/9] kernel-shark-qt: Add Cmake build system for the Qt based KernelShark

[Yordan Karadzhov (VMware)]

This patch adds in /kernel-shark-qt a simple CMake building
infrastructure, to be used by the new, Qt-based version of
KernelShark. Currently, this does not build any objects.
It only generates a header file (src/KsDeff.h) containing some
user-specific definitions. The building infrastructure
is necessary for the coming changes.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/CMakeLists.txt           | 35 ++++++++++++
 kernel-shark-qt/build/FindTraceCmd.cmake | 70 ++++++++++++++++++++++++
 kernel-shark-qt/build/cmake_clean.sh     |  8 +++
 kernel-shark-qt/build/deff.h.cmake       | 20 +++++++
 kernel-shark-qt/src/CMakeLists.txt       |  4 ++
 5 files changed, 137 insertions(+)
 create mode 100644 kernel-shark-qt/CMakeLists.txt
 create mode 100644 kernel-shark-qt/build/FindTraceCmd.cmake
 create mode 100755 kernel-shark-qt/build/cmake_clean.sh
 create mode 100644 kernel-shark-qt/build/deff.h.cmake
 create mode 100644 kernel-shark-qt/src/CMakeLists.txt

diff --git a/kernel-shark-qt/CMakeLists.txt b/kernel-shark-qt/CMakeLists.txt
new file mode 100644
index 0000000..56fca44
--- /dev/null
+++ b/kernel-shark-qt/CMakeLists.txt
@@ -0,0 +1,35 @@
+# Check if cmake has the required version
+cmake_minimum_required(VERSION 2.8.11 FATAL_ERROR)
+
+# Set the name and version of the project
+project(kernel-shark-qt)
+
+set(KS_VERSION_MAJOR 0)
+set(KS_VERSION_MINOR 7)
+set(KS_VERSION_PATCH 0)
+set(KS_VERSION_STRING ${KS_VERSION_MAJOR}.${KS_VERSION_MINOR}.${KS_VERSION_PATCH})
+message("\n project: Kernel Shark: (version: ${KS_VERSION_STRING})\n")
+
+set(KS_DIR ${CMAKE_SOURCE_DIR})
+
+include(${KS_DIR}/build/FindTraceCmd.cmake)
+
+set(LIBRARY_OUTPUT_PATH    "${KS_DIR}/lib")
+set(EXECUTABLE_OUTPUT_PATH "${KS_DIR}/bin")
+
+set(CMAKE_C_FLAGS   "${CMAKE_C_FLAGS} -Wall -pthread")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -std=c++11 -pthread")
+
+include_directories(${KS_DIR}/src/
+                    ${KS_DIR}/build/src/
+                    ${TRACECMD_INCLUDE_DIR}
+                    ${TRACEEVENT_INCLUDE_DIR})
+
+message("")
+message(STATUS "C flags      : " ${CMAKE_C_FLAGS})
+message(STATUS "CXX flags    : " ${CMAKE_CXX_FLAGS})
+message(STATUS "Linker flags : " ${CMAKE_EXE_LINKER_FLAGS})
+
+add_subdirectory(${KS_DIR}/src)
+
+message("")
diff --git a/kernel-shark-qt/build/FindTraceCmd.cmake b/kernel-shark-qt/build/FindTraceCmd.cmake
new file mode 100644
index 0000000..fb2092a
--- /dev/null
+++ b/kernel-shark-qt/build/FindTraceCmd.cmake
@@ -0,0 +1,70 @@
+# Find traceevent and trace-cmd
+# 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.
+#
+#  TRACECMD_INCLUDE_DIR, where to find trace-cmd header.
+#  TRACECMD_LIBRARY_DIR , where to find the trace-cmd library.
+#  TRACECMD_LIBRARY, the trace-cmd library.
+#  TRACECMD_FOUND, If false, do not try to use trace-cmd.
+
+# MESSAGE(" Looking for trace-cmd ...")
+
+find_path(TRACECMD_BIN_DIR      NAMES  trace-cmd
+                                PATHS  $ENV{TRACE_CMD}/tracecmd/
+                                       ${CMAKE_SOURCE_DIR}/../tracecmd/)
+
+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_LIBRARY_DIR  NAMES  libtracecmd.a
+                                PATHS  $ENV{TRACE_CMD}/lib/trace-cmd/
+                                       ${CMAKE_SOURCE_DIR}/../lib/trace-cmd/)
+
+IF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY_DIR)
+
+  SET(TRACECMD_FOUND TRUE)
+  SET(TRACECMD_LIBRARY "${TRACECMD_LIBRARY_DIR}/libtracecmd.a")
+
+ENDIF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY_DIR)
+
+IF (TRACECMD_FOUND)
+
+  MESSAGE(STATUS "Found trace-cmd: ${TRACECMD_LIBRARY}")
+
+ELSE (TRACECMD_FOUND)
+
+  MESSAGE(FATAL_ERROR "\nCould not find trace-cmd!\n")
+
+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)
+
+  SET(TRACEEVENT_FOUND TRUE)
+  SET(TRACEEVENT_LIBRARY "${TRACEEVENT_LIBRARY_DIR}/libtraceevent.a")
+
+ENDIF (TRACEEVENT_INCLUDE_DIR AND TRACEEVENT_LIBRARY_DIR)
+
+IF (TRACEEVENT_FOUND)
+
+  MESSAGE(STATUS "Found traceevent: ${TRACEEVENT_LIBRARY}")
+
+ELSE (TRACEEVENT_FOUND)
+
+  MESSAGE(FATAL_ERROR "\nCould not find libtraceevent!\n")
+
+ENDIF (TRACEEVENT_FOUND)
diff --git a/kernel-shark-qt/build/cmake_clean.sh b/kernel-shark-qt/build/cmake_clean.sh
new file mode 100755
index 0000000..ee0abbb
--- /dev/null
+++ b/kernel-shark-qt/build/cmake_clean.sh
@@ -0,0 +1,8 @@
+make clean
+rm CMakeCache.txt
+rm cmake_install.cmake
+rm Makefile
+rm -rf CMakeFiles/
+rm -rf src/
+rm -f ../lib/*
+rm -f ../src/KsDeff.h
diff --git a/kernel-shark-qt/build/deff.h.cmake b/kernel-shark-qt/build/deff.h.cmake
new file mode 100644
index 0000000..111fcdd
--- /dev/null
+++ b/kernel-shark-qt/build/deff.h.cmake
@@ -0,0 +1,20 @@
+ /**
+ *  \file    KsDeff.h
+ *  \brief   This File is generated by CMAKE
+ */
+
+/* ! -- Do Not Hand Edit - This File is generated by CMAKE -- ! */
+
+#ifndef _KS_CONFIG_H
+#define _KS_CONFIG_H
+
+/** KernelShark Version number. */
+#cmakedefine KS_VERSION_STRING "@KS_VERSION_STRING@"
+
+/** KernelShark source code path. */
+#cmakedefine KS_DIR "@KS_DIR@"
+
+/** Location of the trace-cmd executable. */
+#cmakedefine TRACECMD_BIN_DIR "@TRACECMD_BIN_DIR@"
+
+#endif // _KS_CONFIG_H
diff --git a/kernel-shark-qt/src/CMakeLists.txt b/kernel-shark-qt/src/CMakeLists.txt
new file mode 100644
index 0000000..8c66424
--- /dev/null
+++ b/kernel-shark-qt/src/CMakeLists.txt
@@ -0,0 +1,4 @@
+message("\n src ...")
+
+configure_file( ${KS_DIR}/build/deff.h.cmake
+                ${KS_DIR}/src/KsDeff.h)
-- 
2.17.1

[PATCH v2 2/9] kernel-shark-qt: Automatic generation of doxygen documentation

[Yordan Karadzhov (VMware)]

A possibility for automatic generation of doxygen documentation
of the KernelShark code is added to the CMake build system.
In order to generate such a documentation use the "_DOXYGEN_DOC"
CMake Command-Line option.

example:
  cd build/
  cmake -D_DOXYGEN_DOC=1 ../
  make

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/CMakeLists.txt       | 17 +++++++++++++++++
 kernel-shark-qt/build/cmake_clean.sh |  2 ++
 kernel-shark-qt/doc/dox_config       | 15 +++++++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 kernel-shark-qt/doc/dox_config

diff --git a/kernel-shark-qt/CMakeLists.txt b/kernel-shark-qt/CMakeLists.txt
index 56fca44..9929937 100644
--- a/kernel-shark-qt/CMakeLists.txt
+++ b/kernel-shark-qt/CMakeLists.txt
@@ -14,6 +14,8 @@ set(KS_DIR ${CMAKE_SOURCE_DIR})
 
 include(${KS_DIR}/build/FindTraceCmd.cmake)
 
+find_package(Doxygen)
+
 set(LIBRARY_OUTPUT_PATH    "${KS_DIR}/lib")
 set(EXECUTABLE_OUTPUT_PATH "${KS_DIR}/bin")
 
@@ -32,4 +34,19 @@ message(STATUS "Linker flags : " ${CMAKE_EXE_LINKER_FLAGS})
 
 add_subdirectory(${KS_DIR}/src)
 
+if (_DOXYGEN_DOC AND DOXYGEN_FOUND)
+
+    message("\n doxygen documentation ...")
+    add_custom_target(doc ALL)
+    add_custom_command(TARGET doc
+                       COMMAND doxygen dox_config > dox_build.log
+                       WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/doc)
+
+    set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_MAKE_CLEAN_FILES
+                                          "${KS_DIR}/doc/dox_build.log"
+                                          "${KS_DIR}/doc/html"
+                                          "${KS_DIR}/doc/latex")
+
+endif ()
+
 message("")
diff --git a/kernel-shark-qt/build/cmake_clean.sh b/kernel-shark-qt/build/cmake_clean.sh
index ee0abbb..57c40a4 100755
--- a/kernel-shark-qt/build/cmake_clean.sh
+++ b/kernel-shark-qt/build/cmake_clean.sh
@@ -6,3 +6,5 @@ rm -rf CMakeFiles/
 rm -rf src/
 rm -f ../lib/*
 rm -f ../src/KsDeff.h
+rm -f CMakeDoxyfile.in
+rm -f CMakeDoxygenDefaults.cmake
diff --git a/kernel-shark-qt/doc/dox_config b/kernel-shark-qt/doc/dox_config
new file mode 100644
index 0000000..b982baa
--- /dev/null
+++ b/kernel-shark-qt/doc/dox_config
@@ -0,0 +1,15 @@
+# For detailed description of all Doxygen Configuration Parameters see:
+# http://www.doxygen.org/manual/config.html
+
+DOXYFILE_ENCODING      = UTF-8
+PROJECT_NAME           = "kernel-shark-qt"
+PROJECT_BRIEF = "Kernel Shark is a front-end reader of the Linux kernel tracing data."
+INPUT                  = "../src/"
+SOURCE_BROWSER         = YES
+QT_AUTOBRIEF           = YES
+TAB_SIZE               = 8
+OPTIMIZE_OUTPUT_FOR_C  = YES
+CASE_SENSE_NAMES       = YES
+SORT_MEMBER_DOCS       = NO
+STRICT_PROTO_MATCHING  = YES
+DOT_MULTI_TARGETS      = YES
-- 
2.17.1

[PATCH v2 3/9] kernel-shark-qt: Add API for loading trace.dat files

[Yordan Karadzhov (VMware)]

This patch introduces the first component of the C API used
by the new Qt-based version of KernelShark. The patch includes
only the part of the API responsible for loading data files
generated by trace-cmd. The following patch will introduce
an example, demonstrating the usage of this part of the API.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/src/CMakeLists.txt |   9 +
 kernel-shark-qt/src/libkshark.c    | 475 +++++++++++++++++++++++++++++
 kernel-shark-qt/src/libkshark.h    | 161 ++++++++++
 3 files changed, 645 insertions(+)
 create mode 100644 kernel-shark-qt/src/libkshark.c
 create mode 100644 kernel-shark-qt/src/libkshark.h

diff --git a/kernel-shark-qt/src/CMakeLists.txt b/kernel-shark-qt/src/CMakeLists.txt
index 8c66424..ed3c60e 100644
--- a/kernel-shark-qt/src/CMakeLists.txt
+++ b/kernel-shark-qt/src/CMakeLists.txt
@@ -1,4 +1,13 @@
 message("\n src ...")
 
+message(STATUS "libkshark")
+add_library(kshark SHARED libkshark.c)
+
+target_link_libraries(kshark ${CMAKE_DL_LIBS}
+                             ${TRACEEVENT_LIBRARY}
+                             ${TRACECMD_LIBRARY})
+
+set_target_properties(kshark  PROPERTIES SUFFIX	".so.${KS_VERSION_STRING}")
+
 configure_file( ${KS_DIR}/build/deff.h.cmake
                 ${KS_DIR}/src/KsDeff.h)
diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
new file mode 100644
index 0000000..8f4059a
--- /dev/null
+++ b/kernel-shark-qt/src/libkshark.c
@@ -0,0 +1,475 @@
+// SPDX-License-Identifier: LGPL-2.1
+
+/*
+ * Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
+ */
+
+ /**
+ *  @file    libkshark.c
+ *  @brief   API for processing of FTRACE (trace-cmd) data.
+ */
+
+/** Use GNU C Library. */
+#define _GNU_SOURCE 1
+
+// C
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+
+// trace-cmd
+#include "trace-filter-hash.h"
+
+// KernelShark
+#include "libkshark.h"
+
+static __thread struct trace_seq seq;
+
+static struct kshark_context *kshark_context_handler = NULL;
+
+static bool kshark_default_context(struct kshark_context **context)
+{
+	struct kshark_context *kshark_ctx;
+
+	kshark_ctx = calloc(1, sizeof(*kshark_ctx));
+	if (!kshark_ctx)
+		return false;
+
+	/* Free the existing context (if any). */
+	if (*context && *context != kshark_context_handler) {
+		kshark_free(*context);
+		*context = NULL;
+	}
+
+	if (kshark_context_handler) {
+		kshark_free(kshark_context_handler);
+		kshark_context_handler = NULL;
+	}
+
+	kshark_context_handler = kshark_ctx;
+	*context = kshark_ctx;
+
+	return true;
+}
+
+bool kshark_instance(struct kshark_context **context)
+{
+	if (*context == NULL && kshark_context_handler == NULL) {
+		/* No kshark_context exists. Create a default one. */
+		bool status = kshark_default_context(context);
+		if (status)
+			return status;
+	} else if (*context != NULL) {
+		/* Use the context provided by the user. */
+		if (kshark_context_handler)
+			kshark_free(kshark_context_handler);
+
+		kshark_context_handler = *context;
+	} else {
+		/*
+		 * No context is provided by the user, but the context handler
+		 * is already set. Use the context handler.
+		 */
+		*context = kshark_context_handler;
+	}
+
+	if (!seq.buffer)
+		trace_seq_init(&seq);
+
+	if (!seq.buffer)
+		return false;
+
+	return true;
+}
+
+static void kshark_free_task_list(struct kshark_context *kshark_ctx)
+{
+	struct kshark_task_list *task;
+
+	while (kshark_ctx->tasks) {
+		task = kshark_ctx->tasks;
+		kshark_ctx->tasks = task->next;
+		free(task);
+	}
+
+	kshark_ctx->tasks = NULL;
+}
+
+bool kshark_open(struct kshark_context *kshark_ctx, const char *file)
+{
+	kshark_free_task_list(kshark_ctx);
+	struct tracecmd_input *handle = tracecmd_open(file);
+	if (!handle)
+		return false;
+
+	if (pthread_mutex_init(&kshark_ctx->input_mutex, NULL) != 0) {
+		tracecmd_close(handle);
+		return false;
+	}
+
+	kshark_ctx->handle = handle;
+	kshark_ctx->pevent = tracecmd_get_pevent(handle);
+
+	/*
+	 * Turn off function trace indent and turn on show parent
+	 * if possible.
+	 */
+	trace_util_add_option("ftrace:parent", "1");
+	trace_util_add_option("ftrace:indent", "0");
+
+	return true;
+}
+
+void kshark_close(struct kshark_context *kshark_ctx)
+{
+	if (!kshark_ctx || !kshark_ctx->handle)
+		return;
+
+	tracecmd_close(kshark_ctx->handle);
+	kshark_ctx->handle = NULL;
+	kshark_ctx->pevent = NULL;
+
+	pthread_mutex_destroy(&kshark_ctx->input_mutex);
+}
+
+void kshark_free(struct kshark_context *kshark_ctx)
+{
+	if (kshark_ctx == NULL && kshark_context_handler == NULL)
+		return;
+
+	if (kshark_ctx == NULL) {
+		kshark_ctx = kshark_context_handler;
+		kshark_context_handler = NULL;
+	}
+
+	kshark_free_task_list(kshark_ctx);
+
+	if (seq.buffer)
+		trace_seq_destroy(&seq);
+
+	if (kshark_ctx == kshark_context_handler)
+		kshark_context_handler = NULL;
+
+	free(kshark_ctx);
+}
+
+static struct kshark_task_list *
+kshark_find_task(struct kshark_context *kshark_ctx, int pid)
+{
+	struct kshark_task_list *list;
+	for (list = kshark_ctx->tasks; list; list = list->next)
+		if (list->pid == pid)
+			return list;
+
+	return NULL;
+}
+
+static struct kshark_task_list *
+kshark_add_task(struct kshark_context *kshark_ctx, int pid)
+{
+	struct kshark_task_list *list = kshark_find_task(kshark_ctx, pid);
+	if (list)
+		return list;
+
+	list = malloc(sizeof(*list));
+	if (!list)
+		return NULL;
+
+	list->pid = pid;
+	list->next = kshark_ctx->tasks;
+	kshark_ctx->tasks = list;
+
+	return list;
+}
+
+static void kshark_set_entry_values(struct kshark_context *kshark_ctx,
+				    struct pevent_record *record,
+				    struct kshark_entry *entry)
+{
+	/* Offset of the record */
+	entry->offset = record->offset;
+
+	/* CPU Id of the record */
+	entry->cpu = record->cpu;
+
+	/* Time stamp of the record */
+	entry->ts = record->ts;
+
+	/* Event Id of the record */
+	entry->event_id = pevent_data_type(kshark_ctx->pevent, record);
+
+	/*
+	 * Is visible mask. This default value means that the entry
+	 * is visible everywhere.
+	 */
+	entry->visible = 0xFF;
+
+	/* Process Id of the record */
+	entry->pid = pevent_data_pid(kshark_ctx->pevent, record);
+}
+
+size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
+				struct kshark_entry ***data_rows)
+{
+	int n_cpus = tracecmd_cpus(kshark_ctx->handle);
+	int cpu, next_cpu;
+	size_t count, total = 0;
+	uint64_t ts;
+	
+	struct pevent_record *rec;
+	struct kshark_entry *entry, **next;
+	struct kshark_entry **cpu_list, **rows;
+	struct kshark_task_list *task;
+
+	if (*data_rows)
+		free(*data_rows);
+
+	cpu_list = calloc(n_cpus, sizeof(struct kshark_entry *));
+
+	for (cpu = 0; cpu < n_cpus; ++cpu) {
+		count = 0;
+		cpu_list[cpu] = NULL;
+		next = &cpu_list[cpu];
+
+		rec = tracecmd_read_cpu_first(kshark_ctx->handle, cpu);
+		while (rec) {
+			*next = entry = malloc(sizeof(struct kshark_entry));
+			if (!entry)
+				goto fail;
+
+			kshark_set_entry_values(kshark_ctx, rec, entry);
+			task = kshark_add_task(kshark_ctx, entry->pid);
+			if (!task)
+				goto fail;
+
+			entry->next = NULL;
+			next = &entry->next;
+			free_record(rec);
+
+			++count;
+			rec = tracecmd_read_data(kshark_ctx->handle, cpu);
+		}
+
+		total += count;
+	}
+
+	rows = calloc(total, sizeof(struct kshark_entry *));
+	if (!rows)
+		goto fail;
+
+	count = 0;
+	while (count < total) {
+		ts = 0;
+		next_cpu = -1;
+		for (cpu = 0; cpu < n_cpus; ++cpu) {
+			if (!cpu_list[cpu])
+				continue;
+
+			if (!ts || cpu_list[cpu]->ts < ts) {
+				ts = cpu_list[cpu]->ts;
+				next_cpu = cpu;
+			}
+		}
+
+		if (next_cpu >= 0) {
+			rows[count] = cpu_list[next_cpu];
+			cpu_list[next_cpu] = cpu_list[next_cpu]->next;
+		}
+		++count;
+	}
+
+	free(cpu_list);
+	*data_rows = rows;
+	return total;
+
+fail:
+	fprintf(stderr, "Failed to allocate memory during data loading.\n");
+	return 0;
+}
+
+size_t kshark_load_data_records(struct kshark_context *kshark_ctx,
+				struct pevent_record ***data_rows)
+{
+	int n_cpus = tracecmd_cpus(kshark_ctx->handle);
+	int pid, cpu, next_cpu;
+	size_t count, total = 0;
+	uint64_t ts;
+
+	struct pevent_record *data;
+	struct pevent_record **rows;
+	struct kshark_task_list *task;
+
+	struct temp {
+		struct pevent_record	*rec;
+		struct temp		*next;
+	} **cpu_list, **temp_next, *temp_rec;
+
+	cpu_list = calloc(n_cpus, sizeof(struct temp *));
+
+	for (cpu = 0; cpu < n_cpus; ++cpu) {
+		count = 0;
+		cpu_list[cpu] = NULL;
+		temp_next = &cpu_list[cpu];
+
+		data = tracecmd_read_cpu_first(kshark_ctx->handle, cpu);
+		while (data) {
+			*temp_next = temp_rec = malloc(sizeof(*temp_rec));
+			if (!temp_rec)
+				goto fail;
+
+			pid = pevent_data_pid(kshark_ctx->pevent, data);
+			task = kshark_add_task(kshark_ctx, pid);
+			if (!task)
+				goto fail;
+
+			temp_rec->rec = data;
+			temp_rec->next = NULL;
+			temp_next = &(temp_rec->next);
+
+			++count;
+			data = tracecmd_read_data(kshark_ctx->handle, cpu);
+		}
+
+		total += count;
+	}
+
+	rows = calloc(total, sizeof(struct pevent_record *));
+	if (!rows)
+		goto fail;
+
+	count = 0;
+	while (count < total) {
+		ts = 0;
+		next_cpu = -1;
+		for (cpu = 0; cpu < n_cpus; ++cpu) {
+			if (!cpu_list[cpu])
+				continue;
+
+			if (!ts || cpu_list[cpu]->rec->ts < ts) {
+				ts = cpu_list[cpu]->rec->ts;
+				next_cpu = cpu;
+			}
+		}
+
+		if (next_cpu >= 0) {
+			rows[count] = cpu_list[next_cpu]->rec;
+			temp_rec = cpu_list[next_cpu];
+			cpu_list[next_cpu] = cpu_list[next_cpu]->next;
+			free (temp_rec);
+		}
+
+		++count;
+	}
+
+	free(cpu_list);
+	*data_rows = rows;
+	return total;
+
+fail:
+	fprintf(stderr, "Failed to allocate memory during data loading.\n");
+	return 0;
+}
+
+static struct pevent_record *kshark_read_at(struct kshark_context *kshark_ctx,
+					    uint64_t offset)
+{
+	/*
+	 * It turns that tracecmd_read_at() is not thread-safe.
+	 * TODO: Understand why and see if this can be fixed.
+	 * For the time being use a mutex to protect the access.
+	 */
+	pthread_mutex_lock(&kshark_ctx->input_mutex);
+
+	struct pevent_record *data = tracecmd_read_at(kshark_ctx->handle,
+						      offset, NULL);
+
+	pthread_mutex_unlock(&kshark_ctx->input_mutex);
+
+	return data;
+}
+
+static const char *kshark_get_latency(struct pevent *pe,
+				      struct pevent_record *record)
+{
+	if (!record)
+		return NULL;
+
+	trace_seq_reset(&seq);
+	pevent_data_lat_fmt(pe, &seq, record);
+	return seq.buffer;
+}
+
+static const char *kshark_get_info(struct pevent *pe,
+				   struct pevent_record *record,
+				   struct event_format *event)
+{
+	char *pos;
+
+	if (!record || !event)
+		return NULL;
+
+	trace_seq_reset(&seq);
+	pevent_event_info(&seq, event, record);
+
+	/*
+	 * The event info string contains a trailing newline.
+	 * Remove this new line.
+	 */
+	if ((pos = strchr(seq.buffer, '\n')) != NULL)
+		*pos = '\0';
+
+	return seq.buffer;
+}
+
+char* kshark_dump_entry(struct kshark_entry *entry)
+{
+	struct pevent_record *data;
+	struct event_format *event;
+
+	const char *event_name, *task, *lat, *info;
+	char *tmp_str, *entry_str;
+	int event_id, size = 0;
+	
+	struct kshark_context *kshark_ctx = NULL;
+	kshark_instance(&kshark_ctx);
+
+	if (!seq.buffer)
+		trace_seq_init(&seq);
+
+	if (!seq.buffer)
+		return NULL;
+
+	data = kshark_read_at(kshark_ctx, entry->offset);
+
+	event_id = pevent_data_type(kshark_ctx->pevent, data);
+	event = pevent_data_event_from_type(kshark_ctx->pevent, event_id);
+
+	event_name = event? event->name : "[UNKNOWN EVENT]";
+	task = pevent_data_comm_from_pid(kshark_ctx->pevent, entry->pid);
+	lat = kshark_get_latency(kshark_ctx->pevent, data);
+
+	size = asprintf(&tmp_str, "%li %s-%i; CPU %i; %s;",
+			entry->ts,
+			task,
+			entry->pid,
+			entry->cpu,
+			lat);
+
+	info = kshark_get_info(kshark_ctx->pevent, data, event);
+	if (size > 0) {
+		size = asprintf(&entry_str, "%s %s; %s; 0x%x",
+				tmp_str,
+				event_name,
+				info,
+				entry->visible);
+
+		free(tmp_str);
+	}
+
+	free_record(data);
+
+	if (size > 0)
+		return entry_str;
+
+	return NULL;
+}
diff --git a/kernel-shark-qt/src/libkshark.h b/kernel-shark-qt/src/libkshark.h
new file mode 100644
index 0000000..da5359a
--- /dev/null
+++ b/kernel-shark-qt/src/libkshark.h
@@ -0,0 +1,161 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
+
+/*
+ * Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
+ */
+
+ /**
+ *  @file    libkshark.h
+ *  @brief   API for processing of FTRACE (trace-cmd) data.
+ */
+
+#ifndef _LIB_KSHARK_H
+#define _LIB_KSHARK_H
+
+// C
+#include <stdint.h>
+#include <pthread.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// trace-cmd
+#include "trace-cmd.h"
+#include "event-parse.h"
+
+/**
+ * Kernel Shark entry contains all information from one trace record needed
+ * in order to  visualize the time-series of trace records. The part of the
+ * data which is not directly required for the visualization (latency, record
+ * info etc.) is available on-demand via the offset into the trace file.
+ */
+struct kshark_entry {
+	/**
+	 * A bit mask controlling the visibility of the entry. A value of OxFF
+	 * would mean that the entry is visible everywhere.
+	 */
+	uint8_t		visible;
+
+	/** The CPU core of the record. */
+	uint8_t		cpu;
+
+	/** The PID of the task the record was generated. */
+	int16_t		pid;
+
+	/** Unique Id ot the trace event type. */
+	int		event_id;
+
+	/** The offset into the trace file, used to find the record. */
+	uint64_t	offset;
+
+	/**
+	 * The time of the record in nano seconds. The value is taken from
+	 * the timestamps within the trace data file, which are architecture
+	 * dependent. The time usually is the timestamp from when the system
+	 * started.
+	 */
+	uint64_t	ts;
+
+	/** Pointer to the next (in time) kshark_entry on the same CPU core. */
+	struct kshark_entry *next;
+};
+
+/** Linked list of tasks. */
+struct kshark_task_list {
+	/** Pointer to the next task's PID. */
+	struct kshark_task_list	*next;
+
+	/** PID of a task. */
+	int			 pid;
+};
+
+/** Structure representing a kshark session. */
+struct kshark_context {
+	/** Input handle for the trace data file. */
+	struct tracecmd_input	*handle;
+
+	/** Page event used to parse the page. */
+	struct pevent		*pevent;
+
+	/** List of task's PIDs. */
+	struct kshark_task_list	*tasks;
+
+	/** A mutex, used to protect the access to the input file. */
+	pthread_mutex_t input_mutex;
+};
+
+/**
+ * @brief Initialize a kshark session. This function must be called before
+ * calling any other kshark function. If the session has been initialized,
+ * this function can be used to obtain the session's context.
+ * @param kshark_ctx: Optional input/output location for context pointer.
+ * Only valid on return true.
+ * @returns true on success, or false on failure.
+
+ */
+bool kshark_instance(struct kshark_context **kshark_ctx);
+
+/**
+ * @brief Open and prepare for reading a trace data file specified by "file".
+ * If the specified file does not exist, or contains no trace data, the
+ * function returns false.
+ * @param kshark_ctx: Input location for context pointer.
+ * @param file: The file to load.
+ * @returns true on success, or false on failure.
+ */
+bool kshark_open(struct kshark_context *kshark_ctx, const char *file);
+
+/**
+ * @brief Load the content of the trace data file into an array of
+ * kshark_entries. This function provides fast loading, however the "latency"
+ * and the "info" fields can be accessed only via the offset into the file.
+ * This makes the access to these two fields much slower.
+ * @param kshark_ctx: Input location for context pointer.
+ * @param data_rows: Output location for the trace data. The user is
+ * responsible for freeing the elements of the outputted array.
+ * @returns The size of the outputted data.
+ */
+size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
+				struct kshark_entry ***data_rows);
+
+/**
+ * @brief Load the content of the trace data file into an array of
+ * pevent_records. Use this function only if you need fast access to all
+ * fields of the record.
+ * @param kshark_ctx: Input location for the session context pointer.
+ * @param data_rows: Output location for the trace data. Use free_record()
+ * to free the elements of the outputted array.
+ * @returns The size of the outputted data.
+ */
+size_t kshark_load_data_records(struct kshark_context *kshark_ctx,
+				struct pevent_record ***data_rows);
+
+/**
+ * @brief Close the trace data file and free the trace data handle.
+ * @param kshark_ctx: Input location for the session context pointer.
+ */
+void kshark_close(struct kshark_context *kshark_ctx);
+
+/**
+ * @brief Deinitialize kshark session. Should be called after closing all
+ * open trace data files and before your application terminates.
+ * @param kshark_ctx: Optional input location for session context pointer.
+ */
+void kshark_free(struct kshark_context *kshark_ctx);
+
+/**
+ * @brief Dump into a sting the content of one entry. The function allocates
+ * a null terminated string and return a pointer to this string. The user has
+ * to free the returned string.
+ * @param entry: A Kernel Shark entry to be printed.
+ * @returns The returned string contains a semicolon-separated list of data
+ * fields.
+ */
+char* kshark_dump_entry(struct kshark_entry *entry);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif // _LIB_KSHARK_H
-- 
2.17.1

[PATCH v2 4/9] kernel-shark-qt: Add an example showing how to load trace data

[Yordan Karadzhov (VMware)]

This patch introduces a basic example, showing how to use the
C API of KernelShark and open a new session, load a trace data
file, produced by trace-cmd, print some data to the screen,
cleanup and then exit.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/CMakeLists.txt          |  1 +
 kernel-shark-qt/build/cmake_clean.sh    |  1 +
 kernel-shark-qt/examples/CMakeLists.txt |  5 ++
 kernel-shark-qt/examples/dataload.c     | 83 +++++++++++++++++++++++++
 4 files changed, 90 insertions(+)
 create mode 100644 kernel-shark-qt/examples/CMakeLists.txt
 create mode 100644 kernel-shark-qt/examples/dataload.c

diff --git a/kernel-shark-qt/CMakeLists.txt b/kernel-shark-qt/CMakeLists.txt
index 9929937..9ff12a1 100644
--- a/kernel-shark-qt/CMakeLists.txt
+++ b/kernel-shark-qt/CMakeLists.txt
@@ -33,6 +33,7 @@ message(STATUS "CXX flags    : " ${CMAKE_CXX_FLAGS})
 message(STATUS "Linker flags : " ${CMAKE_EXE_LINKER_FLAGS})
 
 add_subdirectory(${KS_DIR}/src)
+add_subdirectory(${KS_DIR}/examples)
 
 if (_DOXYGEN_DOC AND DOXYGEN_FOUND)
 
diff --git a/kernel-shark-qt/build/cmake_clean.sh b/kernel-shark-qt/build/cmake_clean.sh
index 57c40a4..acdbb43 100755
--- a/kernel-shark-qt/build/cmake_clean.sh
+++ b/kernel-shark-qt/build/cmake_clean.sh
@@ -4,6 +4,7 @@ rm cmake_install.cmake
 rm Makefile
 rm -rf CMakeFiles/
 rm -rf src/
+rm -rf examples/
 rm -f ../lib/*
 rm -f ../src/KsDeff.h
 rm -f CMakeDoxyfile.in
diff --git a/kernel-shark-qt/examples/CMakeLists.txt b/kernel-shark-qt/examples/CMakeLists.txt
new file mode 100644
index 0000000..98df9d8
--- /dev/null
+++ b/kernel-shark-qt/examples/CMakeLists.txt
@@ -0,0 +1,5 @@
+message("\n examples ...")
+
+message(STATUS "dataload")
+add_executable(dload          dataload.c)
+target_link_libraries(dload   kshark)
diff --git a/kernel-shark-qt/examples/dataload.c b/kernel-shark-qt/examples/dataload.c
new file mode 100644
index 0000000..5a66765
--- /dev/null
+++ b/kernel-shark-qt/examples/dataload.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2018 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
+ */
+
+// C
+#include <stdio.h>
+#include <stdlib.h>
+
+// KernelShark
+#include "libkshark.h"
+
+const char *default_file = "trace.dat";
+
+int main(int argc, char **argv)
+{
+	char *entry_str;
+	struct kshark_entry **data = NULL;
+	size_t r, n_rows;
+	bool status;
+	struct kshark_task_list* task;
+
+	/* Create a new kshark session. */
+	struct kshark_context *kshark_ctx = NULL;
+	if(!kshark_instance(&kshark_ctx))
+		return 1;
+
+	/* Open a trace data file produced by trace-cmd. */
+	if (argc > 1)
+		status = kshark_open(kshark_ctx, argv[1]);
+	else
+		status = kshark_open(kshark_ctx, default_file);
+
+	if (!status) {
+		kshark_free(kshark_ctx);
+		return 1;
+	}
+
+	/* Load the content of the file into an array of entries. */
+	n_rows = kshark_load_data_entries(kshark_ctx, &data);
+
+	/* Print to the screen the list of all tasks. */
+	for (task = kshark_ctx->tasks; task; task = task->next) {
+		const char *task_str =
+			pevent_data_comm_from_pid(kshark_ctx->pevent,
+						  task->pid);
+
+		printf("task: %s-%i\n", task_str, task->pid);
+	}
+
+	puts("\n\n");
+
+	/* Print to the screen the first 10 entries. */
+	for (r = 0; r < 10; ++r) {
+		entry_str = kshark_dump_entry(data[r]);
+		puts(entry_str);
+		free(entry_str);
+	}
+
+	puts("\n...\n");
+
+	/* Print the last 10 entries. */
+	for (r = n_rows - 10; r < n_rows; ++r) {
+		entry_str = kshark_dump_entry(data[r]);
+		puts(entry_str);
+		free(entry_str);
+	}
+
+	/* Free the memory. */
+	for (r = 0; r < n_rows; ++r)
+		free(data[r]);
+
+	free(data);
+
+	/* Close the file. */
+	kshark_close(kshark_ctx);
+
+	/* Close the session. */
+	kshark_free(kshark_ctx);
+
+	return 0;
+}
-- 
2.17.1

[PATCH v2 5/9] kernel-shark-qt: Add a README file to trace-cmd/kernel-shark-qt

[Yordan Karadzhov (VMware)]

The README file contains instructions for building and using the Qt-based
version of KernelShark.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/README | 44 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 kernel-shark-qt/README

diff --git a/kernel-shark-qt/README b/kernel-shark-qt/README
new file mode 100644
index 0000000..6ff77a5
--- /dev/null
+++ b/kernel-shark-qt/README
@@ -0,0 +1,44 @@
+
+This directory contains the new Qt-based version of the KernelShark GUI.
+
+
+Third Party Software:
+------------------------------------------------------------
+The external dependencies:
+1. In order to install the packages on Ubuntu do the following:
+    sudo apt-get install build-essential git cmake -y
+
+1.1 I you want to be able to generate Doxygen documentation:
+    sudo apt-get install graphviz doxygen-gui -y
+
+
+2. In order to install the packages on Fedora, as root do the following:
+    dnf install gcc gcc-c++ git cmake -y
+
+2.1 I you want to be able to generate Doxygen documentation:
+    dnf install graphviz doxygen -y
+
+
+Building:
+------------------------------------------------------------
+1. Follow the instructions given in trace-cmd/README and build
+the original trace-cmd end traceevent libraries.
+
+2. Do:
+    cd kernel-shark-qt/build
+    cmake ../
+    make
+
+2.1 In order to create a Doxygen documentation add -D_DOXYGEN_DOC=1
+as a CMake Command-Line option.
+    cd kernel-shark-qt/build
+    cmake -D_DOXYGEN_DOC=1 ../
+    make
+
+2.2.1 Use "make clean" if you want to delete all already compiled object.
+
+2.2.2 Use the script "cmake_clean.sh" if you want to delete all already
+compiled object and all files generated by CMake.
+
+3. After building the code "kernel-shark-qt/lib" will contain all libraries
+and "kernel-shark-qt/bin" will contain all executables.
-- 
2.17.1

[PATCH v2 6/9] kernel-shark-qt: Add filtering to the C API of KernelShark

[Yordan Karadzhov (VMware)]

Instruments for trace data filtering are added to the C API of
the Qt-based version of KernelShark. The following patch will
introduces an example, demonstrating the usage of this instruments.

This version of the patch contains a number of improvements suggested
by Steven Rostedt. Thanks Steven!

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/src/libkshark.c | 150 ++++++++++++++++++++++++++++++++
 kernel-shark-qt/src/libkshark.h | 103 ++++++++++++++++++++++
 2 files changed, 253 insertions(+)

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 8f4059a..22f82a9 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -35,6 +35,14 @@ static bool kshark_default_context(struct kshark_context **context)
 	if (!kshark_ctx)
 		return false;
 
+	kshark_ctx->show_task_filter = tracecmd_filter_id_hash_alloc();
+	kshark_ctx->hide_task_filter = tracecmd_filter_id_hash_alloc();
+
+	kshark_ctx->show_event_filter = tracecmd_filter_id_hash_alloc();
+	kshark_ctx->hide_event_filter = tracecmd_filter_id_hash_alloc();
+
+	kshark_ctx->filter_mask = 0x0;
+
 	/* Free the existing context (if any). */
 	if (*context && *context != kshark_context_handler) {
 		kshark_free(*context);
@@ -125,6 +133,15 @@ void kshark_close(struct kshark_context *kshark_ctx)
 	if (!kshark_ctx || !kshark_ctx->handle)
 		return;
 
+	/*
+	 * All Id filters are file specific. Make sure that the Pids and Event Ids
+	 * from this file are not going to be used with another file.
+	 */
+	tracecmd_filter_id_clear(kshark_ctx->show_task_filter);
+	tracecmd_filter_id_clear(kshark_ctx->hide_task_filter);
+	tracecmd_filter_id_clear(kshark_ctx->show_event_filter);
+	tracecmd_filter_id_clear(kshark_ctx->hide_event_filter);
+
 	tracecmd_close(kshark_ctx->handle);
 	kshark_ctx->handle = NULL;
 	kshark_ctx->pevent = NULL;
@@ -142,6 +159,12 @@ void kshark_free(struct kshark_context *kshark_ctx)
 		kshark_context_handler = NULL;
 	}
 
+	tracecmd_filter_id_hash_free(kshark_ctx->show_task_filter);
+	tracecmd_filter_id_hash_free(kshark_ctx->hide_task_filter);
+
+	tracecmd_filter_id_hash_free(kshark_ctx->show_event_filter);
+	tracecmd_filter_id_hash_free(kshark_ctx->hide_event_filter);
+
 	kshark_free_task_list(kshark_ctx);
 
 	if (seq.buffer)
@@ -182,6 +205,124 @@ kshark_add_task(struct kshark_context *kshark_ctx, int pid)
 	return list;
 }
 
+static bool filter_find(struct tracecmd_filter_id *filter, int pid,
+			bool test)
+{
+	return !filter || !filter->count ||
+		!!(unsigned long)tracecmd_filter_id_find(filter, pid) == test;
+}
+
+static bool kshark_show_task(struct kshark_context *kshark_ctx, int pid)
+{
+	return filter_find(kshark_ctx->show_task_filter, pid, true) &&
+	       filter_find(kshark_ctx->hide_task_filter, pid, false);
+}
+
+static bool kshark_show_event(struct kshark_context *kshark_ctx, int pid)
+{
+	return filter_find(kshark_ctx->show_event_filter, pid, true) &&
+	       filter_find(kshark_ctx->hide_event_filter, pid, false);
+}
+
+void kshark_filter_add_id(struct kshark_context *kshark_ctx,
+			  int filter_id, int id)
+{
+	struct tracecmd_filter_id *filter;
+
+	switch (filter_id) {
+		case KS_SHOW_EVENT_FILTER:
+			filter = kshark_ctx->show_event_filter;
+			break;
+		case KS_HIDE_EVENT_FILTER:
+			filter = kshark_ctx->hide_event_filter;
+			break;
+		case KS_SHOW_TASK_FILTER:
+			filter = kshark_ctx->show_task_filter;
+			break;
+		case KS_HIDE_TASK_FILTER:
+			filter = kshark_ctx->hide_task_filter;
+			break;
+		default:
+			return;
+	}
+
+	tracecmd_filter_id_add(filter, id);
+}
+
+void kshark_filter_clear(struct kshark_context *kshark_ctx, int filter_id)
+{
+	struct tracecmd_filter_id *filter;
+
+	switch (filter_id) {
+		case KS_SHOW_EVENT_FILTER:
+			filter = kshark_ctx->show_event_filter;
+			break;
+		case KS_HIDE_EVENT_FILTER:
+			filter = kshark_ctx->hide_event_filter;
+			break;
+		case KS_SHOW_TASK_FILTER:
+			filter = kshark_ctx->show_task_filter;
+			break;
+		case KS_HIDE_TASK_FILTER:
+			filter = kshark_ctx->hide_task_filter;
+			break;
+		default:
+			return;
+	}
+
+	tracecmd_filter_id_clear(filter);
+}
+
+static bool filter_is_set(struct tracecmd_filter_id *filter)
+{
+	return filter && filter->count;
+}
+
+static bool kshark_filter_is_set(struct kshark_context *kshark_ctx)
+{
+	return filter_is_set(kshark_ctx->show_task_filter) ||
+	       filter_is_set(kshark_ctx->hide_task_filter) ||
+	       filter_is_set(kshark_ctx->show_event_filter) ||
+	       filter_is_set(kshark_ctx->hide_event_filter);
+}
+
+static void unset_event_filter_flag(struct kshark_context *kshark_ctx,
+				    struct kshark_entry *e)
+{
+	/*
+	 * All entries, filtered-out by the event filters, will be treated
+	 * differently, when visualized. Because of this, ignore the value
+	 * of the GRAPH_VIEW flag provided by the user via
+	 * kshark_ctx->filter_mask and unset the EVENT_VIEW flag.
+	 */
+	int event_mask = kshark_ctx->filter_mask;
+	event_mask &= ~KS_GRAPH_VIEW_FILTER_MASK;
+	event_mask |= KS_EVENT_VIEW_FILTER_MASK;
+	e->visible &= ~event_mask;
+}
+
+void kshark_filter_entries(struct kshark_context *kshark_ctx,
+			   struct kshark_entry **data,
+			   size_t n_entries)
+{
+	int i;
+	if (!kshark_filter_is_set(kshark_ctx))
+		return;
+
+	for (i = 0; i < n_entries; ++i) {
+		/* Start with and entry which is visible everywhere. */
+		data[i]->visible = 0xFF;
+
+		/* Apply event filtering. */
+		if (!kshark_show_event(kshark_ctx, data[i]->event_id))
+			unset_event_filter_flag(kshark_ctx, data[i]);
+
+		/* Apply task filtering. */
+		if (!kshark_show_task(kshark_ctx, data[i]->pid))
+			data[i]->visible &= ~kshark_ctx->filter_mask;
+	}
+}
+
 static void kshark_set_entry_values(struct kshark_context *kshark_ctx,
 				    struct pevent_record *record,
 				    struct kshark_entry *entry)
@@ -242,6 +383,15 @@ size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 			if (!task)
 				goto fail;
 
+			/* Apply event filtering. */
+			if (!kshark_show_event(kshark_ctx, entry->event_id))
+				unset_event_filter_flag(kshark_ctx, entry);
+
+			/* Apply task filtering. */
+			if (!kshark_show_task(kshark_ctx, entry->pid)) {
+				entry->visible &= ~kshark_ctx->filter_mask;
+			}
+
 			entry->next = NULL;
 			next = &entry->next;
 			free_record(rec);
diff --git a/kernel-shark-qt/src/libkshark.h b/kernel-shark-qt/src/libkshark.h
index da5359a..2b79bc8 100644
--- a/kernel-shark-qt/src/libkshark.h
+++ b/kernel-shark-qt/src/libkshark.h
@@ -22,6 +22,7 @@ extern "C" {
 
 // trace-cmd
 #include "trace-cmd.h"
+// #include "trace-filter-hash.h"
 #include "event-parse.h"
 
 /**
@@ -83,6 +84,25 @@ struct kshark_context {
 
 	/** A mutex, used to protect the access to the input file. */
 	pthread_mutex_t input_mutex;
+
+	/** Hash of tasks to filter on. */
+	struct tracecmd_filter_id	*show_task_filter;
+
+	/** Hash of tasks to not display. */
+	struct tracecmd_filter_id	*hide_task_filter;
+
+	/** Hash of events to filter on. */
+	struct tracecmd_filter_id	*show_event_filter;
+
+	/** Hash of events to not display. */
+	struct tracecmd_filter_id	*hide_event_filter;
+
+	/**
+	 * Bit mask, controlling the visibility of the entries after filtering.
+	 * If given bit is set here, all entries which are filtered-out will
+	 * have this bit unset in their "visible" fields.
+	 */
+	uint8_t				filter_mask;
 };
 
 /**
@@ -111,6 +131,10 @@ bool kshark_open(struct kshark_context *kshark_ctx, const char *file);
  * kshark_entries. This function provides fast loading, however the "latency"
  * and the "info" fields can be accessed only via the offset into the file.
  * This makes the access to these two fields much slower.
+ * If some of the filters is set, the "visible" fields of each entry is
+ * updated according to the criteria provided by the filters. The field
+ * "filter_mask" of the session's context is used to control the level of
+ * visibility/invisibility of the filtered entries.
  * @param kshark_ctx: Input location for context pointer.
  * @param data_rows: Output location for the trace data. The user is
  * responsible for freeing the elements of the outputted array.
@@ -154,6 +178,85 @@ void kshark_free(struct kshark_context *kshark_ctx);
  */
 char* kshark_dump_entry(struct kshark_entry *entry);
 
+/** Bit masks used to control the visibility of the entry after filtering. */
+enum kshark_filter_masks {
+	/**
+	 * Use this mask to check the visibility of the entry in the text
+	 * view.
+	 */
+	KS_TEXT_VIEW_FILTER_MASK	= 1 << 0,
+
+	/**
+	 * Use this mask to check the visibility of the entry in the graph
+	 * view.
+	 */
+	KS_GRAPH_VIEW_FILTER_MASK	= 1 << 1,
+
+	/** Special mask used whene filtering events. */
+	KS_EVENT_VIEW_FILTER_MASK	= 1 << 2,
+};
+
+/** Filter type identifier. */
+enum kshark_filter_type {
+	/** Dummy filter identifier reserved for future use. */
+	KS_NO_FILTER,
+
+	/**
+	 * Identifier of the filter, used to specified the events to be shown.
+	 */
+	KS_SHOW_EVENT_FILTER,
+
+	/**
+	 * Identifier of the filter, used to specified the events to be
+	 * filtered-out.
+	 */
+	KS_HIDE_EVENT_FILTER,
+
+	/**
+	 * Identifier of the filter, used to specified the tasks to be shown.
+	 */
+	KS_SHOW_TASK_FILTER,
+
+	/**
+	 * Identifier of the filter, used to specified the tasks to be
+	 * filtered-out.
+	 */
+	KS_HIDE_TASK_FILTER,
+};
+
+/**
+ * @brief Add an Id value to the filster specified by "filter_id".
+ * @param kshark_ctx: kshark_ctx: Input location for the session context
+ * pointer.
+ * @param filter_id: Identifier of the filter.
+ * @param id: Id value to be added to the filter.
+ */
+void kshark_filter_add_id(struct kshark_context *kshark_ctx,
+			  int filter_id, int id);
+
+/**
+ * @brief Clear (reset) the filster specified by "filter_id".
+ * @param kshark_ctx: kshark_ctx: Input location for the session context
+ * pointer.
+ * @param filter_id: Identifier of the filter.
+ */
+void kshark_filter_clear(struct kshark_context *kshark_ctx, int filter_id);
+
+/**
+ * @brief This function loops over the array of entries specified by "data"
+ * and "n_entries" and sets the "visible" fields of each entry according to
+ * the criteria provided by the filters of the session's context. The field
+ * "filter_mask" of the session's context is used to control the level of
+ * visibility/invisibility of the entries which are filtered-out.
+ * @param kshark_ctx: kshark_ctx: Input location for the session context
+ * pointer.
+ * @param data: Input location for the trace data to be filtered.
+ * @param n_entries: The size of the inputted data.
+ */
+void kshark_filter_entries(struct kshark_context *kshark_ctx,
+			   struct kshark_entry **data,
+			   size_t n_entries);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.17.1

[PATCH v2 7/9] kernel-shark-qt: Add an example showing how to filter trace data

[Yordan Karadzhov (VMware)]

This patch introduces a basic example, showing how to use the
C API of KernelShark in order to filter trace data.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/examples/CMakeLists.txt |   4 +
 kernel-shark-qt/examples/datafilter.c   | 123 ++++++++++++++++++++++++
 2 files changed, 127 insertions(+)
 create mode 100644 kernel-shark-qt/examples/datafilter.c

diff --git a/kernel-shark-qt/examples/CMakeLists.txt b/kernel-shark-qt/examples/CMakeLists.txt
index 98df9d8..009fd1e 100644
--- a/kernel-shark-qt/examples/CMakeLists.txt
+++ b/kernel-shark-qt/examples/CMakeLists.txt
@@ -3,3 +3,7 @@ message("\n examples ...")
 message(STATUS "dataload")
 add_executable(dload          dataload.c)
 target_link_libraries(dload   kshark)
+
+message(STATUS "datafilter")
+add_executable(dfilter          datafilter.c)
+target_link_libraries(dfilter   kshark)
diff --git a/kernel-shark-qt/examples/datafilter.c b/kernel-shark-qt/examples/datafilter.c
new file mode 100644
index 0000000..e766f3d
--- /dev/null
+++ b/kernel-shark-qt/examples/datafilter.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2018 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
+ */
+
+// C
+#include <stdio.h>
+#include <stdlib.h>
+
+// KernelShark
+#include "libkshark.h"
+
+const char *default_file = "trace.dat";
+
+int main(int argc, char **argv)
+{
+	char *entry_str;
+	struct kshark_entry **data = NULL;
+	size_t i, n_rows, count, n_evts;
+	bool status;
+	struct kshark_task_list* task;
+
+	/* Create a new kshark session. */
+	struct kshark_context *kshark_ctx = NULL;
+	if(!kshark_instance(&kshark_ctx))
+		return 1;
+
+	/* Open a trace data file produced by trace-cmd. */
+	if (argc > 1)
+		status = kshark_open(kshark_ctx, argv[1]);
+	else
+		status = kshark_open(kshark_ctx, default_file);
+
+	if (!status) {
+		kshark_free(kshark_ctx);
+		return 1;
+	}
+
+	/* Load the content of the file into an array of entries. */
+	n_rows = kshark_load_data_entries(kshark_ctx, &data);
+
+	/* Filter the trace data coming from trace-cmd. */
+	task = kshark_ctx->tasks;
+	while (task) {
+		const char *task_str =
+			pevent_data_comm_from_pid(kshark_ctx->pevent,
+						  task->pid);
+
+		if (strcmp(task_str, "trace-cmd") == 0)
+			kshark_filter_add_id(kshark_ctx, KS_HIDE_TASK_FILTER,
+							 task->pid);
+
+		task = task->next;
+	}
+
+	/*
+	 * Set the Filter Mask. In this case we want to avoid showing the filterd
+	 * entris in text format.
+	 */
+	kshark_ctx->filter_mask = KS_TEXT_VIEW_FILTER_MASK;
+	kshark_filter_entries(kshark_ctx, data, n_rows);
+
+	/* Print to the screen the first 10 visible entries. */
+	count = 0;
+	i = 0;
+	for (i = 0; i < n_rows; ++i) {
+		if (data[i]->visible & KS_TEXT_VIEW_FILTER_MASK) {
+			entry_str = kshark_dump_entry(data[i]);
+			puts(entry_str);
+			free(entry_str);
+
+			if (++count > 10)
+				break;
+		}
+
+		++i;
+	}
+
+	puts("\n\n");
+
+	/* Show only "sched" events. */
+	struct event_format *event;
+	n_evts = kshark_ctx->pevent->nr_events;
+	for (i = 0; i < n_evts; ++i) {
+		event = kshark_ctx->pevent->events[i];
+		if (strcmp(event->system, "sched") == 0)
+			kshark_filter_add_id(kshark_ctx, KS_SHOW_EVENT_FILTER,
+							 event->id);
+	}
+
+	kshark_filter_entries(kshark_ctx, data, n_rows);
+
+	/* Print to the screen the first 10 visible entries. */
+	count = 0;
+	i = 0;
+	for (i = 0; i < n_rows; ++i) {
+		if (data[i]->visible & KS_TEXT_VIEW_FILTER_MASK) {
+			entry_str = kshark_dump_entry(data[i]);
+			puts(entry_str);
+			free(entry_str);
+
+			if (++count > 10)
+				break;
+		}
+
+		++i;
+	}
+
+	/* Free the memory. */
+	for (i = 0; i < n_rows; ++i)
+		free(data[i]);
+
+	free(data);
+
+	/* Close the file. */
+	kshark_close(kshark_ctx);
+
+	/* Close the session. */
+	kshark_free(kshark_ctx);
+
+	return 0;
+}
-- 
2.17.1

[PATCH v2 8/9] kernel-shark-qt: Add Advanced filter to the session context.

[Yordan Karadzhov (VMware)]

This patch adds to the KernelShark session's context instrumentation
for sophisticated filtering based on the content of the trace event.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/src/libkshark.c | 36 ++++++++++++++++++++++++++++++---
 kernel-shark-qt/src/libkshark.h | 11 +++++++++-
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 22f82a9..7d70142 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -118,6 +118,9 @@ bool kshark_open(struct kshark_context *kshark_ctx, const char *file)
 	kshark_ctx->handle = handle;
 	kshark_ctx->pevent = tracecmd_get_pevent(handle);
 
+	kshark_ctx->advanced_event_filter =
+		pevent_filter_alloc(kshark_ctx->pevent);
+
 	/*
 	 * Turn off function trace indent and turn on show parent
 	 * if possible.
@@ -134,7 +137,7 @@ void kshark_close(struct kshark_context *kshark_ctx)
 		return;
 
 	/*
-	 * All Id filters are file specific. Make sure that the Pids and Event Ids
+	 * All filters are file specific. Make sure that the Pids and Event Ids
 	 * from this file are not going to be used with another file.
 	 */
 	tracecmd_filter_id_clear(kshark_ctx->show_task_filter);
@@ -142,6 +145,12 @@ void kshark_close(struct kshark_context *kshark_ctx)
 	tracecmd_filter_id_clear(kshark_ctx->show_event_filter);
 	tracecmd_filter_id_clear(kshark_ctx->hide_event_filter);
 
+	if (kshark_ctx->advanced_event_filter) {
+		pevent_filter_reset(kshark_ctx->advanced_event_filter);
+		pevent_filter_free(kshark_ctx->advanced_event_filter);
+		kshark_ctx->advanced_event_filter = NULL;
+	}
+
 	tracecmd_close(kshark_ctx->handle);
 	kshark_ctx->handle = NULL;
 	kshark_ctx->pevent = NULL;
@@ -306,9 +315,23 @@ void kshark_filter_entries(struct kshark_context *kshark_ctx,
 			   size_t n_entries)
 {
 	int i;
+	if (kshark_ctx->advanced_event_filter->filters) {
+		/*
+		 * The advanced filter is set but applying it requires
+		 * access to prevent_record, hence the data has to be
+		 * reloaded.
+		 */
+		fprintf(stderr,
+			"Failed to filter!\n");
+		fprintf(stderr,
+			"Reset the Advanced filter or reload the data.\n");
+		return;
+	}
+
 	if (!kshark_filter_is_set(kshark_ctx))
 		return;
 
+	/* No need to reload the data. Apply the Id filters. */
 	for (i = 0; i < n_entries; ++i) {
 		/* Start with and entry which is visible everywhere. */
 		data[i]->visible = 0xFF;
@@ -353,7 +376,7 @@ size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 				struct kshark_entry ***data_rows)
 {
 	int n_cpus = tracecmd_cpus(kshark_ctx->handle);
-	int cpu, next_cpu;
+	int cpu, next_cpu, ret;
 	size_t count, total = 0;
 	uint64_t ts;
 	
@@ -361,6 +384,7 @@ size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 	struct kshark_entry *entry, **next;
 	struct kshark_entry **cpu_list, **rows;
 	struct kshark_task_list *task;
+	struct event_filter *adv_filter = kshark_ctx->advanced_event_filter;
 
 	if (*data_rows)
 		free(*data_rows);
@@ -384,8 +408,14 @@ size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 				goto fail;
 
 			/* Apply event filtering. */
-			if (!kshark_show_event(kshark_ctx, entry->event_id))
+			ret = FILTER_NONE;
+			if (adv_filter->filters)
+				ret = pevent_filter_match(adv_filter, rec);
+
+			if (!kshark_show_event(kshark_ctx, entry->event_id) ||
+			    ret != FILTER_MATCH) {
 				unset_event_filter_flag(kshark_ctx, entry);
+			}
 
 			/* Apply task filtering. */
 			if (!kshark_show_task(kshark_ctx, entry->pid)) {
diff --git a/kernel-shark-qt/src/libkshark.h b/kernel-shark-qt/src/libkshark.h
index 2b79bc8..2ec8450 100644
--- a/kernel-shark-qt/src/libkshark.h
+++ b/kernel-shark-qt/src/libkshark.h
@@ -103,6 +103,12 @@ struct kshark_context {
 	 * have this bit unset in their "visible" fields.
 	 */
 	uint8_t				filter_mask;
+
+	/**
+	 * Filter allowing sophisticated filtering based on the content of
+	 * the event.
+	 */
+	struct event_filter		*advanced_event_filter;
 };
 
 /**
@@ -245,9 +251,12 @@ void kshark_filter_clear(struct kshark_context *kshark_ctx, int filter_id);
 /**
  * @brief This function loops over the array of entries specified by "data"
  * and "n_entries" and sets the "visible" fields of each entry according to
- * the criteria provided by the filters of the session's context. The field
+ * the criteria provided by the Id filters of the session's context. The field
  * "filter_mask" of the session's context is used to control the level of
  * visibility/invisibility of the entries which are filtered-out.
+ * WARNING: Do not use this function if the advanced filter is set. Applying
+ * the advanced filter requires access to prevent_record, hence the data has
+ * to be reloaded using kshark_load_data_entries().
  * @param kshark_ctx: kshark_ctx: Input location for the session context
  * pointer.
  * @param data: Input location for the trace data to be filtered.
-- 
2.17.1

[PATCH v2 9/9] kernel-shark-qt: Add example of advanded filtering

[Yordan Karadzhov (VMware)]

The filtering example is extended and now demonstrates filtering
based on the content of the event.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/examples/datafilter.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/kernel-shark-qt/examples/datafilter.c b/kernel-shark-qt/examples/datafilter.c
index e766f3d..cbb6ec9 100644
--- a/kernel-shark-qt/examples/datafilter.c
+++ b/kernel-shark-qt/examples/datafilter.c
@@ -107,6 +107,31 @@ int main(int argc, char **argv)
 		++i;
 	}
 
+	puts("\n\n");
+
+	/* Clear all filters. */
+	kshark_filter_clear(kshark_ctx, KS_HIDE_TASK_FILTER);
+	kshark_filter_clear(kshark_ctx, KS_SHOW_EVENT_FILTER);
+
+	/* Use the Advanced filter to do event content based filtering. */
+	struct event_filter *adv_filter = kshark_ctx->advanced_event_filter;
+	pevent_filter_add_filter_str(adv_filter, "sched/sched_wakeup:target_cpu==1");
+
+	/* The Advanced filter requires reloading the data. */
+	n_rows = kshark_load_data_entries(kshark_ctx, &data);
+
+	count = 0;
+	for (i = 0; i < n_rows; ++i) {
+		if (data[i]->visible & KS_EVENT_VIEW_FILTER_MASK) {
+			entry_str = kshark_dump_entry(data[i]);
+			puts(entry_str);
+			free(entry_str);
+
+			if(++count > 10)
+				break;
+		}
+	}
+
 	/* Free the memory. */
 	for (i = 0; i < n_rows; ++i)
 		free(data[i]);
-- 
2.17.1

Re: [PATCH v2 3/9] kernel-shark-qt: Add API for loading trace.dat files

[Steven Rostedt]

On Thu, 28 Jun 2018 19:30:06 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:


> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
> new file mode 100644
> index 0000000..8f4059a
> --- /dev/null
> +++ b/kernel-shark-qt/src/libkshark.c
> @@ -0,0 +1,475 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +

[..]
> +}
> +
> +bool kshark_instance(struct kshark_context **context)
> +{
> +	if (*context == NULL && kshark_context_handler == NULL) {
> +		/* No kshark_context exists. Create a default one. */
> +		bool status = kshark_default_context(context);
> +		if (status)
> +			return status;
> +	} else if (*context != NULL) {
> +		/* Use the context provided by the user. */
> +		if (kshark_context_handler)
> +			kshark_free(kshark_context_handler);
> +
> +		kshark_context_handler = *context;
> +	} else {
> +		/*
> +		 * No context is provided by the user, but the context handler
> +		 * is already set. Use the context handler.
> +		 */
> +		*context = kshark_context_handler;
> +	}
> +
> +	if (!seq.buffer)
> +		trace_seq_init(&seq);
> +
> +	if (!seq.buffer)
> +		return false;
> +
> +	return true;
> +}
> +
[..]

> diff --git a/kernel-shark-qt/src/libkshark.h b/kernel-shark-qt/src/libkshark.h
> new file mode 100644
> index 0000000..da5359a
> 

> +
> +/**
> + * @brief Initialize a kshark session. This function must be called before
> + * calling any other kshark function. If the session has been initialized,
> + * this function can be used to obtain the session's context.
> + * @param kshark_ctx: Optional input/output location for context pointer.
> + * Only valid on return true.
> + * @returns true on success, or false on failure.
> +
> + */
> +bool kshark_instance(struct kshark_context **kshark_ctx);
> +

Keeping with the Linux styling, it is preferred to have the descriptive
comments above the C function and not the prototype.

I understand the rational for having it above the prototype, because
you would think that's where people may look if they don't have the
source (just the headers). But realistically, people who use etags and
such, when they want to see a function, the vim and emacs commands will
bring the user to the C code and not the prototype. This is why Linux
enforces the comments above the C code.

Please keep the comments above the C function (move them to the .c file)
If you are using the headers for documentation, it would probably be
easier to read about them from the output of the doxygen documentation.

-- Steve

Re: [PATCH v2 3/9] kernel-shark-qt: Add API for loading trace.dat files

[Steven Rostedt]

On Thu, 28 Jun 2018 16:32:35 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> I understand the rational for having it above the prototype, because
> you would think that's where people may look if they don't have the
> source (just the headers). But realistically, people who use etags and
> such, when they want to see a function, the vim and emacs commands will
> bring the user to the C code and not the prototype. This is why Linux
> enforces the comments above the C code.

One more reason for having the comments above the code and not the
prototype (which I'm struggling with right now), is that when reviewing
a function, I have no idea what it's suppose to be doing. I need to
search the header to find the comments associated with it. Makes
reviewing much more time consuming.

-- Steve

Re: [PATCH v2 3/9] kernel-shark-qt: Add API for loading trace.dat files

[Steven Rostedt]

On Thu, 28 Jun 2018 19:30:06 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> This patch introduces the first component of the C API used
> by the new Qt-based version of KernelShark. The patch includes
> only the part of the API responsible for loading data files
> generated by trace-cmd. The following patch will introduce
> an example, demonstrating the usage of this part of the API.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  kernel-shark-qt/src/CMakeLists.txt |   9 +
>  kernel-shark-qt/src/libkshark.c    | 475 +++++++++++++++++++++++++++++
>  kernel-shark-qt/src/libkshark.h    | 161 ++++++++++
>  3 files changed, 645 insertions(+)
>  create mode 100644 kernel-shark-qt/src/libkshark.c
>  create mode 100644 kernel-shark-qt/src/libkshark.h
> 
> diff --git a/kernel-shark-qt/src/CMakeLists.txt b/kernel-shark-qt/src/CMakeLists.txt
> index 8c66424..ed3c60e 100644
> --- a/kernel-shark-qt/src/CMakeLists.txt
> +++ b/kernel-shark-qt/src/CMakeLists.txt
> @@ -1,4 +1,13 @@
>  message("\n src ...")
>  
> +message(STATUS "libkshark")
> +add_library(kshark SHARED libkshark.c)
> +
> +target_link_libraries(kshark ${CMAKE_DL_LIBS}
> +                             ${TRACEEVENT_LIBRARY}
> +                             ${TRACECMD_LIBRARY})
> +
> +set_target_properties(kshark  PROPERTIES SUFFIX	".so.${KS_VERSION_STRING}")
> +
>  configure_file( ${KS_DIR}/build/deff.h.cmake
>                  ${KS_DIR}/src/KsDeff.h)
> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
> new file mode 100644
> index 0000000..8f4059a
> --- /dev/null
> +++ b/kernel-shark-qt/src/libkshark.c
> @@ -0,0 +1,475 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +
> +/*
> + * Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
> + */
> +
> + /**
> + *  @file    libkshark.c
> + *  @brief   API for processing of FTRACE (trace-cmd) data.
> + */
> +
> +/** Use GNU C Library. */
> +#define _GNU_SOURCE 1
> +
> +// C
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <assert.h>
> +
> +// trace-cmd
> +#include "trace-filter-hash.h"
> +
> +// KernelShark
> +#include "libkshark.h"
> +
> +static __thread struct trace_seq seq;
> +
> +static struct kshark_context *kshark_context_handler = NULL;
> +
> +static bool kshark_default_context(struct kshark_context **context)
> +{
> +	struct kshark_context *kshark_ctx;
> +
> +	kshark_ctx = calloc(1, sizeof(*kshark_ctx));
> +	if (!kshark_ctx)
> +		return false;
> +
> +	/* Free the existing context (if any). */
> +	if (*context && *context != kshark_context_handler) {
> +		kshark_free(*context);
> +		*context = NULL;
> +	}
> +
> +	if (kshark_context_handler) {
> +		kshark_free(kshark_context_handler);
> +		kshark_context_handler = NULL;

If kshark_context_handler is passed into kshark_free(), then
kshark_free() already sets kshark_context_handler to NULL, so this
isn't needed.

> +	}
> +
> +	kshark_context_handler = kshark_ctx;
> +	*context = kshark_ctx;

In fact, I think we can simplify this a bit by doing:

{
	struct kshark_context *kshark_ctx;

	kshark_ctx = calloc(1, sizeof(*kshark_ctx);
	if (!kshark_ctx)
		return false;

	/* Will free kshark_context_handler */
	kshark_free(NULL);

	/* Will do nothing if *context is NULL */
	kshark_free(*context);

	*context = kshark_context_handler = kshark_ctx;


> +
> +	return true;
> +}
> +
> +bool kshark_instance(struct kshark_context **context)
> +{
> +	if (*context == NULL && kshark_context_handler == NULL) {
> +		/* No kshark_context exists. Create a default one. */
> +		bool status = kshark_default_context(context);
> +		if (status)
> +			return status;
> +	} else if (*context != NULL) {
> +		/* Use the context provided by the user. */
> +		if (kshark_context_handler)
> +			kshark_free(kshark_context_handler);
> +
> +		kshark_context_handler = *context;
> +	} else {
> +		/*
> +		 * No context is provided by the user, but the context handler
> +		 * is already set. Use the context handler.
> +		 */
> +		*context = kshark_context_handler;
> +	}
> +
> +	if (!seq.buffer)
> +		trace_seq_init(&seq);
> +
> +	if (!seq.buffer)
> +		return false;
> +
> +	return true;

I think the above can be simplified as:

	bool status = true;

	if (*context != NULL) {
		/* Will free kshark_context_handler */
		kshark_free(NULL);
		kshark_context_handler = *context;
	} else {
		if (kshark_context_handler)
			*context = kshark_context_handler;
		else
			status = kshark_default_context(context);
	}

	if (!seq.buffer)
		trace_seq_init(&seq);

	if (!seq.buffer)
		return false;

	return status;

> +}
> +
> +static void kshark_free_task_list(struct kshark_context *kshark_ctx)
> +{
> +	struct kshark_task_list *task;
> +

As a fail-safe, might want to add:

	if (!kshark_ctx)
		return;

To allow this to be called with NULL.

> +	while (kshark_ctx->tasks) {
> +		task = kshark_ctx->tasks;
> +		kshark_ctx->tasks = task->next;
> +		free(task);
> +	}
> +
> +	kshark_ctx->tasks = NULL;
> +}
> +
> +bool kshark_open(struct kshark_context *kshark_ctx, const char *file)
> +{

Please move the declaration of handle to here:

	struct tracecmd_input *handle;

> +	kshark_free_task_list(kshark_ctx);
> +	struct tracecmd_input *handle = tracecmd_open(file);

	handle = tracecmd_open(file);

> +	if (!handle)
> +		return false;
> +
> +	if (pthread_mutex_init(&kshark_ctx->input_mutex, NULL) != 0) {
> +		tracecmd_close(handle);
> +		return false;
> +	}
> +
> +	kshark_ctx->handle = handle;
> +	kshark_ctx->pevent = tracecmd_get_pevent(handle);
> +
> +	/*
> +	 * Turn off function trace indent and turn on show parent
> +	 * if possible.
> +	 */
> +	trace_util_add_option("ftrace:parent", "1");
> +	trace_util_add_option("ftrace:indent", "0");
> +
> +	return true;
> +}
> +
> +void kshark_close(struct kshark_context *kshark_ctx)
> +{
> +	if (!kshark_ctx || !kshark_ctx->handle)
> +		return;
> +
> +	tracecmd_close(kshark_ctx->handle);
> +	kshark_ctx->handle = NULL;
> +	kshark_ctx->pevent = NULL;
> +
> +	pthread_mutex_destroy(&kshark_ctx->input_mutex);
> +}
> +
> +void kshark_free(struct kshark_context *kshark_ctx)
> +{
> +	if (kshark_ctx == NULL && kshark_context_handler == NULL)
> +		return;
> +
> +	if (kshark_ctx == NULL) {
> +		kshark_ctx = kshark_context_handler;
> +		kshark_context_handler = NULL;
> +	}

To understand the above better, consolidate it to:

	if (kshark_ctx == NULL) {
		if (kshark_context_handler == NULL)
			return;
		kshark_ctx = kshark_context_handler;

		/* kshark_ctx_handler will be set to NULL below */
	}


> +
> +	kshark_free_task_list(kshark_ctx);
> +
> +	if (seq.buffer)
> +		trace_seq_destroy(&seq);
> +
> +	if (kshark_ctx == kshark_context_handler)
> +		kshark_context_handler = NULL;
> +
> +	free(kshark_ctx);
> +}
> +
> +static struct kshark_task_list *
> +kshark_find_task(struct kshark_context *kshark_ctx, int pid)
> +{
> +	struct kshark_task_list *list;
> +	for (list = kshark_ctx->tasks; list; list = list->next)
> +		if (list->pid == pid)
> +			return list;

I wonder if we should make the above into a sorted array, and use
sort/bsearch to find it. After hitting my own issue with a linear
search algorithm, I'm a bit weary of this type of look up.

> +
> +	return NULL;
> +}
> +
> +static struct kshark_task_list *
> +kshark_add_task(struct kshark_context *kshark_ctx, int pid)
> +{
> +	struct kshark_task_list *list = kshark_find_task(kshark_ctx, pid);
> +	if (list)
> +		return list;
> +
> +	list = malloc(sizeof(*list));
> +	if (!list)
> +		return NULL;

Yeah, let's make the kshark_ctx->tasks into an array and just use
realloc() to increase it here.

> +
> +	list->pid = pid;
> +	list->next = kshark_ctx->tasks;
> +	kshark_ctx->tasks = list;
> +
> +	return list;
> +}
> +
> +static void kshark_set_entry_values(struct kshark_context *kshark_ctx,
> +				    struct pevent_record *record,
> +				    struct kshark_entry *entry)
> +{
> +	/* Offset of the record */
> +	entry->offset = record->offset;
> +
> +	/* CPU Id of the record */
> +	entry->cpu = record->cpu;
> +
> +	/* Time stamp of the record */
> +	entry->ts = record->ts;
> +
> +	/* Event Id of the record */
> +	entry->event_id = pevent_data_type(kshark_ctx->pevent, record);
> +
> +	/*
> +	 * Is visible mask. This default value means that the entry
> +	 * is visible everywhere.
> +	 */
> +	entry->visible = 0xFF;
> +
> +	/* Process Id of the record */
> +	entry->pid = pevent_data_pid(kshark_ctx->pevent, record);
> +}
> +
> +size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
> +				struct kshark_entry ***data_rows)
> +{
> +	int n_cpus = tracecmd_cpus(kshark_ctx->handle);
> +	int cpu, next_cpu;
> +	size_t count, total = 0;
> +	uint64_t ts;
> +	
> +	struct pevent_record *rec;
> +	struct kshark_entry *entry, **next;
> +	struct kshark_entry **cpu_list, **rows;
> +	struct kshark_task_list *task;
> +
> +	if (*data_rows)
> +		free(*data_rows);
> +
> +	cpu_list = calloc(n_cpus, sizeof(struct kshark_entry *));
> +
> +	for (cpu = 0; cpu < n_cpus; ++cpu) {
> +		count = 0;
> +		cpu_list[cpu] = NULL;
> +		next = &cpu_list[cpu];
> +
> +		rec = tracecmd_read_cpu_first(kshark_ctx->handle, cpu);
> +		while (rec) {
> +			*next = entry = malloc(sizeof(struct kshark_entry));
> +			if (!entry)
> +				goto fail;
> +
> +			kshark_set_entry_values(kshark_ctx, rec, entry);
> +			task = kshark_add_task(kshark_ctx, entry->pid);
> +			if (!task)
> +				goto fail;
> +
> +			entry->next = NULL;
> +			next = &entry->next;
> +			free_record(rec);
> +
> +			++count;
> +			rec = tracecmd_read_data(kshark_ctx->handle, cpu);
> +		}
> +
> +		total += count;
> +	}
> +
> +	rows = calloc(total, sizeof(struct kshark_entry *));
> +	if (!rows)
> +		goto fail;
> +
> +	count = 0;
> +	while (count < total) {
> +		ts = 0;
> +		next_cpu = -1;
> +		for (cpu = 0; cpu < n_cpus; ++cpu) {
> +			if (!cpu_list[cpu])
> +				continue;
> +
> +			if (!ts || cpu_list[cpu]->ts < ts) {
> +				ts = cpu_list[cpu]->ts;
> +				next_cpu = cpu;
> +			}
> +		}
> +
> +		if (next_cpu >= 0) {
> +			rows[count] = cpu_list[next_cpu];
> +			cpu_list[next_cpu] = cpu_list[next_cpu]->next;
> +		}
> +		++count;
> +	}
> +
> +	free(cpu_list);
> +	*data_rows = rows;
> +	return total;
> +
> +fail:
> +	fprintf(stderr, "Failed to allocate memory during data loading.\n");
> +	return 0;
> +}
> +
> +size_t kshark_load_data_records(struct kshark_context *kshark_ctx,
> +				struct pevent_record ***data_rows)
> +{
> +	int n_cpus = tracecmd_cpus(kshark_ctx->handle);
> +	int pid, cpu, next_cpu;
> +	size_t count, total = 0;
> +	uint64_t ts;
> +
> +	struct pevent_record *data;
> +	struct pevent_record **rows;
> +	struct kshark_task_list *task;
> +
> +	struct temp {
> +		struct pevent_record	*rec;
> +		struct temp		*next;
> +	} **cpu_list, **temp_next, *temp_rec;
> +
> +	cpu_list = calloc(n_cpus, sizeof(struct temp *));
> +
> +	for (cpu = 0; cpu < n_cpus; ++cpu) {
> +		count = 0;
> +		cpu_list[cpu] = NULL;
> +		temp_next = &cpu_list[cpu];
> +
> +		data = tracecmd_read_cpu_first(kshark_ctx->handle, cpu);
> +		while (data) {
> +			*temp_next = temp_rec = malloc(sizeof(*temp_rec));
> +			if (!temp_rec)
> +				goto fail;
> +
> +			pid = pevent_data_pid(kshark_ctx->pevent, data);
> +			task = kshark_add_task(kshark_ctx, pid);
> +			if (!task)
> +				goto fail;
> +
> +			temp_rec->rec = data;
> +			temp_rec->next = NULL;
> +			temp_next = &(temp_rec->next);
> +
> +			++count;
> +			data = tracecmd_read_data(kshark_ctx->handle, cpu);
> +		}
> +
> +		total += count;
> +	}
> +
> +	rows = calloc(total, sizeof(struct pevent_record *));
> +	if (!rows)
> +		goto fail;
> +
> +	count = 0;
> +	while (count < total) {
> +		ts = 0;
> +		next_cpu = -1;
> +		for (cpu = 0; cpu < n_cpus; ++cpu) {
> +			if (!cpu_list[cpu])
> +				continue;
> +
> +			if (!ts || cpu_list[cpu]->rec->ts < ts) {
> +				ts = cpu_list[cpu]->rec->ts;
> +				next_cpu = cpu;
> +			}
> +		}
> +
> +		if (next_cpu >= 0) {
> +			rows[count] = cpu_list[next_cpu]->rec;
> +			temp_rec = cpu_list[next_cpu];
> +			cpu_list[next_cpu] = cpu_list[next_cpu]->next;
> +			free (temp_rec);
> +		}
> +
> +		++count;
> +	}
> +
> +	free(cpu_list);
> +	*data_rows = rows;
> +	return total;
> +
> +fail:
> +	fprintf(stderr, "Failed to allocate memory during data loading.\n");
> +	return 0;
> +}
> +
> +static struct pevent_record *kshark_read_at(struct kshark_context *kshark_ctx,
> +					    uint64_t offset)
> +{
> +	/*
> +	 * It turns that tracecmd_read_at() is not thread-safe.
> +	 * TODO: Understand why and see if this can be fixed.
> +	 * For the time being use a mutex to protect the access.
> +	 */

Doing a quick look, it's because it does things in states. That is, it
calls one function that will set up state in the handle (peek_event),
which saves a record, and then tracecmd_read_data() which then uses
that record stored in the handle to retrieve the data.

All access to the handle, should be protected by the input mutex.

> +	pthread_mutex_lock(&kshark_ctx->input_mutex);
> +
> +	struct pevent_record *data = tracecmd_read_at(kshark_ctx->handle,
> +						      offset, NULL);
> +
> +	pthread_mutex_unlock(&kshark_ctx->input_mutex);
> +
> +	return data;
> +}
> +
> +static const char *kshark_get_latency(struct pevent *pe,
> +				      struct pevent_record *record)
> +{
> +	if (!record)
> +		return NULL;
> +
> +	trace_seq_reset(&seq);
> +	pevent_data_lat_fmt(pe, &seq, record);
> +	return seq.buffer;
> +}
> +
> +static const char *kshark_get_info(struct pevent *pe,
> +				   struct pevent_record *record,
> +				   struct event_format *event)
> +{
> +	char *pos;
> +
> +	if (!record || !event)
> +		return NULL;
> +
> +	trace_seq_reset(&seq);
> +	pevent_event_info(&seq, event, record);
> +
> +	/*
> +	 * The event info string contains a trailing newline.
> +	 * Remove this new line.
> +	 */
> +	if ((pos = strchr(seq.buffer, '\n')) != NULL)
> +		*pos = '\0';
> +
> +	return seq.buffer;
> +}
> +
> +char* kshark_dump_entry(struct kshark_entry *entry)
> +{
> +	struct pevent_record *data;
> +	struct event_format *event;
> +
> +	const char *event_name, *task, *lat, *info;
> +	char *tmp_str, *entry_str;
> +	int event_id, size = 0;
> +	
> +	struct kshark_context *kshark_ctx = NULL;
> +	kshark_instance(&kshark_ctx);
> +
> +	if (!seq.buffer)
> +		trace_seq_init(&seq);
> +
> +	if (!seq.buffer)
> +		return NULL;

Since the above double test is done a few times, probably best to make helper function:

bool init_thread_seq(void)
{
	if (!seq.buffer)
		trace_seq_init(&seq);
	return seq.buffer != NULL;
}

Then you can just do:

	if (!init_thread_seq())
		return NULL;

> +
> +	data = kshark_read_at(kshark_ctx, entry->offset);

Does this access need to be protected by the mutex?

> +
> +	event_id = pevent_data_type(kshark_ctx->pevent, data);
> +	event = pevent_data_event_from_type(kshark_ctx->pevent, event_id);
> +
> +	event_name = event? event->name : "[UNKNOWN EVENT]";
> +	task = pevent_data_comm_from_pid(kshark_ctx->pevent, entry->pid);
> +	lat = kshark_get_latency(kshark_ctx->pevent, data);
> +
> +	size = asprintf(&tmp_str, "%li %s-%i; CPU %i; %s;",
> +			entry->ts,
> +			task,
> +			entry->pid,
> +			entry->cpu,
> +			lat);
> +
> +	info = kshark_get_info(kshark_ctx->pevent, data, event);
> +	if (size > 0) {
> +		size = asprintf(&entry_str, "%s %s; %s; 0x%x",
> +				tmp_str,
> +				event_name,
> +				info,
> +				entry->visible);
> +
> +		free(tmp_str);
> +	}
> +
> +	free_record(data);
> +
> +	if (size > 0)
> +		return entry_str;
> +
> +	return NULL;
> +}
> diff --git a/kernel-shark-qt/src/libkshark.h
> b/kernel-shark-qt/src/libkshark.h new file mode 100644
> index 0000000..da5359a
> --- /dev/null
> +++ b/kernel-shark-qt/src/libkshark.h
> @@ -0,0 +1,161 @@
> +/* SPDX-License-Identifier: LGPL-2.1 */
> +
> +/*
> + * Copyright (C) 2017 VMware Inc, Yordan Karadzhov
> <y.karadz@gmail.com>
> + */
> +
> + /**
> + *  @file    libkshark.h
> + *  @brief   API for processing of FTRACE (trace-cmd) data.
> + */
> +
> +#ifndef _LIB_KSHARK_H
> +#define _LIB_KSHARK_H
> +
> +// C
> +#include <stdint.h>
> +#include <pthread.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +// trace-cmd
> +#include "trace-cmd.h"
> +#include "event-parse.h"
> +
> +/**
> + * Kernel Shark entry contains all information from one trace record needed
> + * in order to  visualize the time-series of trace records. The part of the
> + * data which is not directly required for the visualization (latency, record
> + * info etc.) is available on-demand via the offset into the trace file.
> + */
> +struct kshark_entry {
> +	/**
> +	 * A bit mask controlling the visibility of the entry. A value of OxFF
> +	 * would mean that the entry is visible everywhere.

Is there a comment somewhere describing what other values mean?

> +	 */
> +	uint8_t		visible;
> +
> +	/** The CPU core of the record. */
> +	uint8_t		cpu;
> +
> +	/** The PID of the task the record was generated. */
> +	int16_t		pid;
> +
> +	/** Unique Id ot the trace event type. */
> +	int		event_id;
> +
> +	/** The offset into the trace file, used to find the record. */
> +	uint64_t	offset;
> +
> +	/**
> +	 * The time of the record in nano seconds. The value is taken from
> +	 * the timestamps within the trace data file, which are architecture
> +	 * dependent. The time usually is the timestamp from when the system
> +	 * started.

You don't need to change the comment, but FYI, the default of a
trace-cmd record is usually the time from when the system started, but
it will be a different default when we get things like the virtserver
working.

> +	 */
> +	uint64_t	ts;
> +
> +	/** Pointer to the next (in time) kshark_entry on the same CPU core. */
> +	struct kshark_entry *next;
> +};
> +
> +/** Linked list of tasks. */
> +struct kshark_task_list {
> +	/** Pointer to the next task's PID. */
> +	struct kshark_task_list	*next;
> +
> +	/** PID of a task. */
> +	int			 pid;
> +};
> +
> +/** Structure representing a kshark session. */
> +struct kshark_context {
> +	/** Input handle for the trace data file. */
> +	struct tracecmd_input	*handle;
> +
> +	/** Page event used to parse the page. */
> +	struct pevent		*pevent;
> +
> +	/** List of task's PIDs. */
> +	struct kshark_task_list	*tasks;
> +
> +	/** A mutex, used to protect the access to the input file. */
> +	pthread_mutex_t input_mutex;
> +};
> +
> +/**
> + * @brief Initialize a kshark session. This function must be called before
> + * calling any other kshark function. If the session has been initialized,
> + * this function can be used to obtain the session's context.
> + * @param kshark_ctx: Optional input/output location for context pointer.
> + * Only valid on return true.
> + * @returns true on success, or false on failure.

BTW, to make the above easier to read, can we add indentation? Like
this:

 * @brief Initialize a kshark session. This function must be called before
 *        calling any other kshark function. If the session has been initialized,
 *        this function can be used to obtain the session's context.
 * @param kshark_ctx: Optional input/output location for context pointer.
 *                    Only valid on return true.
 * @returns true on success, or false on failure.

?

Also, probably want to specify exactly how the session is obtained. That is:

 * @param kshark_ctx: Optional input/output location for context pointer.
 *		      If it points to a context, that context will become
 *		      the new session. If it points to NULL, it will obtain
 *		      the current (or new) session. The result is only
 *		      valid on return of true.

Finally, another reason to keep the comments with the code. They are
less likely to become stale. I'm finding that I'm jumping back and
forth from the C file to here to see if the comments match the code.
>From my experience, the farther the comments are from the code they
describe, the more likely they never get updated when the code does.


> +
> + */
> +bool kshark_instance(struct kshark_context **kshark_ctx);
> +
> +/**
> + * @brief Open and prepare for reading a trace data file specified by "file".
> + * If the specified file does not exist, or contains no trace data, the
> + * function returns false.
> + * @param kshark_ctx: Input location for context pointer.
> + * @param file: The file to load.
> + * @returns true on success, or false on failure.
> + */
> +bool kshark_open(struct kshark_context *kshark_ctx, const char *file);
> +
> +/**
> + * @brief Load the content of the trace data file into an array of
> + * kshark_entries. This function provides fast loading, however the "latency"
> + * and the "info" fields can be accessed only via the offset into the file.
> + * This makes the access to these two fields much slower.
> + * @param kshark_ctx: Input location for context pointer.
> + * @param data_rows: Output location for the trace data. The user is
> + * responsible for freeing the elements of the outputted array.
> + * @returns The size of the outputted data.

I wonder if we should have it return negative on error, as zero may be
a valid return value, if the kshark_ctx does not have any rows.

> + */
> +size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
> +				struct kshark_entry ***data_rows);
> +
> +/**
> + * @brief Load the content of the trace data file into an array of
> + * pevent_records. Use this function only if you need fast access to all
> + * fields of the record.
> + * @param kshark_ctx: Input location for the session context pointer.
> + * @param data_rows: Output location for the trace data. Use free_record()
> + * to free the elements of the outputted array.
> + * @returns The size of the outputted data.

Same here.

> + */
> +size_t kshark_load_data_records(struct kshark_context *kshark_ctx,
> +				struct pevent_record ***data_rows);
> +
> +/**
> + * @brief Close the trace data file and free the trace data handle.
> + * @param kshark_ctx: Input location for the session context pointer.
> + */
> +void kshark_close(struct kshark_context *kshark_ctx);
> +
> +/**
> + * @brief Deinitialize kshark session. Should be called after closing all
> + * open trace data files and before your application terminates.
> + * @param kshark_ctx: Optional input location for session context pointer.
> + */

This does not describe what it does if kshark_ctx is NULL.

> +void kshark_free(struct kshark_context *kshark_ctx);
> +
> +/**
> + * @brief Dump into a sting the content of one entry. The function allocates

Does the shark dump into a sting ray?  (I think you meant string ;-)

> + * a null terminated string and return a pointer to this string. The user has

		"and returns a pointer"

-- Steve

> + * to free the returned string.
> + * @param entry: A Kernel Shark entry to be printed.
> + * @returns The returned string contains a semicolon-separated list of data
> + * fields.
> + */
> +char* kshark_dump_entry(struct kshark_entry *entry);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif // _LIB_KSHARK_H

Re: [PATCH v2 3/9] kernel-shark-qt: Add API for loading trace.dat files

[Steven Rostedt]

On Thu, 28 Jun 2018 19:30:06 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> +size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
> +				struct kshark_entry ***data_rows)
> +{
> +	int n_cpus = tracecmd_cpus(kshark_ctx->handle);
> +	int cpu, next_cpu;
> +	size_t count, total = 0;
> +	uint64_t ts;
> +	

Git warned me that the above has whitespace (tab) at end of line.

> +	struct pevent_record *rec;
> +	struct kshark_entry *entry, **next;
> +	struct kshark_entry **cpu_list, **rows;
> +	struct kshark_task_list *task;

[..]

> +char* kshark_dump_entry(struct kshark_entry *entry)
> +{
> +	struct pevent_record *data;
> +	struct event_format *event;
> +
> +	const char *event_name, *task, *lat, *info;
> +	char *tmp_str, *entry_str;
> +	int event_id, size = 0;
> +	

And here too.

-- Steve

> +	struct kshark_context *kshark_ctx = NULL;
> +	kshark_instance(&kshark_ctx);
> +
> +	if (!seq.buffer)
> +		trace_seq_init(&seq);
> +
> +	if (!seq.buffer)
> +		return NULL;
> +

Re: [PATCH v2 6/9] kernel-shark-qt: Add filtering to the C API of KernelShark

[Steven Rostedt]

On Thu, 28 Jun 2018 19:30:09 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> +static void unset_event_filter_flag(struct kshark_context *kshark_ctx,
> +				    struct kshark_entry *e)
> +{
> +	/*
> +	 * All entries, filtered-out by the event filters, will be treated
> +	 * differently, when visualized. Because of this, ignore the value
> +	 * of the GRAPH_VIEW flag provided by the user via
> +	 * kshark_ctx->filter_mask and unset the EVENT_VIEW flag.
> +	 */
> +	int event_mask = kshark_ctx->filter_mask;

Just to get you use to Linux kernel coding style. Always add a space
between the declaration and the code. Even when the declaration has
code.

There's a few places where this requirement is not strictly forced, but
we'll discuss that later.

> +	event_mask &= ~KS_GRAPH_VIEW_FILTER_MASK;
> +	event_mask |= KS_EVENT_VIEW_FILTER_MASK;
> +	e->visible &= ~event_mask;
> +}
> +
> +void kshark_filter_entries(struct kshark_context *kshark_ctx,
> +			   struct kshark_entry **data,
> +			   size_t n_entries)
> +{
> +	int i;

Here too, and any other places that has declarations.

-- Steve

> +	if (!kshark_filter_is_set(kshark_ctx))
> +		return;
> +

Re: [PATCH v2 8/9] kernel-shark-qt: Add Advanced filter to the session context.

[Steven Rostedt]

On Thu, 28 Jun 2018 19:30:11 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> @@ -306,9 +315,23 @@ void kshark_filter_entries(struct kshark_context *kshark_ctx,
>  			   size_t n_entries)
>  {
>  	int i;

Also add a space here.

> +	if (kshark_ctx->advanced_event_filter->filters) {

Can this be called with kshark_ctx->advanced_event_filter == NULL?

> +		/*
> +		 * The advanced filter is set but applying it requires
> +		 * access to prevent_record, hence the data has to be
> +		 * reloaded.
> +		 */

I'm a little confused by the comment above. So if this is called with
advanced filters, then it is an error?

> +		fprintf(stderr,
> +			"Failed to filter!\n");
> +		fprintf(stderr,
> +			"Reset the Advanced filter or reload the data.\n");
> +		return;
> +	}
> +
>  	if (!kshark_filter_is_set(kshark_ctx))
>  		return;
>  
> +	/* No need to reload the data. Apply the Id filters. */

Why is this comment added here? Would it make sense to add it in the
original patch? Does advanced filters make this different?

-- Steve

>  	for (i = 0; i < n_entries; ++i) {
>  		/* Start with and entry which is visible everywhere. */
>  		data[i]->visible = 0xFF;

Re: [PATCH v2 6/9] kernel-shark-qt: Add filtering to the C API of KernelShark

[Steven Rostedt]

On Thu, 28 Jun 2018 19:30:09 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> +/**
> + * @brief This function loops over the array of entries specified by "data"
> + * and "n_entries" and sets the "visible" fields of each entry according to
> + * the criteria provided by the filters of the session's context. The field
> + * "filter_mask" of the session's context is used to control the level of
> + * visibility/invisibility of the entries which are filtered-out.
> + * @param kshark_ctx: kshark_ctx: Input location for the session context

Double "kshark_ctx:"

-- Steve


> + * pointer.
> + * @param data: Input location for the trace data to be filtered.
> + * @param n_entries: The size of the inputted data.
> + */
> +void kshark_filter_entries(struct kshark_context *kshark_ctx,
> +			   struct kshark_entry **data,
> +			   size_t n_entries);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --