[PATCH 0/7] Introduce the very basic part of the C API of KS-1.0

[PATCH 0/7] Introduce the very 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,
demonstration the usage of this part of the API are added as well.
The code can be found also here:
https://github.com/yordan-karadzhov/trace-cmd_qtdev.git

Yordan Karadzhov (VMware) (7):
  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/CMakeLists.txt           |   53 +
 kernel-shark-qt/README                   |   37 +
 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           | 2291 ++++++++++++++++++++++
 kernel-shark-qt/examples/CMakeLists.txt  |    9 +
 kernel-shark-qt/examples/datafilter.c    |  114 ++
 kernel-shark-qt/examples/dataload.c      |   80 +
 kernel-shark-qt/src/CMakeLists.txt       |   13 +
 kernel-shark-qt/src/libkshark.c          |  579 ++++++
 kernel-shark-qt/src/libkshark.h          |  232 +++
 12 files changed, 3509 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 1/7] 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.

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..62e6b9e
--- /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 3/7] 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 introduces
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    | 438 +++++++++++++++++++++++++++++
 kernel-shark-qt/src/libkshark.h    | 157 +++++++++++
 3 files changed, 604 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..f112a50
--- /dev/null
+++ b/kernel-shark-qt/src/libkshark.c
@@ -0,0 +1,438 @@
+// SPDX-License-Identifier: LGPL-2.1
+
+/*
+ * Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
+ */
+
+// C
+#define _GNU_SOURCE 1
+#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) {
+		free(*context);
+		*context = NULL;
+	}
+
+	if (kshark_context_handler) {
+		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 (!seq.buffer)
+		trace_seq_init(&seq);
+
+	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)
+			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;
+	}
+
+	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 = kshark_ctx->tasks->next;
+		free(task);
+	}
+
+	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;
+
+	pthread_mutex_init(&kshark_ctx->input_mutex, NULL);
+
+	kshark_ctx->handle = handle;
+	kshark_ctx->pevt = 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->pevt = 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);
+
+	free(kshark_ctx);
+
+	if (kshark_ctx == kshark_context_handler)
+		kshark_context_handler = NULL;
+
+	kshark_ctx = NULL;
+}
+
+static struct kshark_task_list *
+kshark_find_task(struct kshark_context *kshark_ctx, int pid)
+{
+	struct kshark_task_list *list = kshark_ctx->tasks;
+	while (list) {
+		if (list->pid == pid)
+			return list;
+
+		list = list->next;
+	}
+
+	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));
+	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->pevt, 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->pevt, 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;
+	size_t count, total = 0;
+	struct pevent_record *rec;
+
+	if (*data_rows)
+		free(*data_rows);
+
+	struct kshark_entry *entry, **next;
+	struct kshark_entry **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) );
+			assert(entry != NULL);
+
+			kshark_set_entry_values(kshark_ctx, rec, entry);
+			kshark_add_task(kshark_ctx, entry->pid);
+
+			entry->next = NULL;
+			next = &(entry->next);
+			free_record(rec);
+
+			++count;
+			rec = tracecmd_read_data(kshark_ctx->handle, cpu);
+		}
+
+		total += count;
+	}
+
+	struct kshark_entry **rows;
+	rows = calloc(total, sizeof(struct kshark_entry *));
+	if(!rows) {
+		fprintf(stderr, "Failed to allocate memory during data loading.\n");
+		return 0;
+	}
+
+	count = 0;
+	int next_cpu;
+	uint64_t ts;
+	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;
+}
+
+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 cpu;
+	size_t count, total = 0;
+	struct pevent_record *data;
+
+	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));
+			assert(temp_rec != NULL);
+
+			kshark_add_task(kshark_ctx,
+					pevent_data_pid(kshark_ctx->pevt, data));
+			temp_rec->rec = data;
+			temp_rec->next = NULL;
+			temp_next = &(temp_rec->next);
+
+			++count;
+			data = tracecmd_read_data(kshark_ctx->handle, cpu);
+		}
+
+		total += count;
+	}
+
+	struct pevent_record **rows;
+	rows = calloc(total, sizeof(struct pevent_record *));
+	if(!rows) {
+		fprintf(stderr, "Failed to allocate memory during data loading.\n");
+		return 0;
+	}
+
+	count = 0;
+	int next_cpu;
+	uint64_t ts;
+	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;
+}
+
+static struct pevent_record *kshark_read_at(struct kshark_context *kshark_ctx,
+					    uint64_t offset)
+{
+	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)
+{
+	if (!record || !event)
+		return NULL;
+
+	trace_seq_reset(&seq);
+	pevent_event_info(&seq, event, record);
+
+	// Remove the trailing newline from the Info string.
+	char *pos;
+	if ((pos = strchr(seq.buffer, '\n')) != NULL)
+		*pos = '\0';
+
+	return seq.buffer;
+}
+
+char* kshark_dump_entry(struct kshark_entry *entry)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	kshark_instance(&kshark_ctx);
+
+	if (!seq.buffer)
+		trace_seq_init(&seq);
+
+	trace_seq_reset(&seq);
+
+	char *tmp_str, *entry_str;
+	int size_tmp, size = 0;
+
+	struct pevent_record *data = kshark_read_at(kshark_ctx, entry->offset);
+
+	int event_id = pevent_data_type(kshark_ctx->pevt, data);
+	struct event_format *event =
+		pevent_data_event_from_type(kshark_ctx->pevt, event_id);
+
+	const char *event_name = (event)? event->name : "[UNKNOWN EVENT]";
+	const char *task = pevent_data_comm_from_pid(kshark_ctx->pevt, entry->pid);
+	const char *lat = kshark_get_latency(kshark_ctx->pevt, data);
+
+	size_tmp = asprintf(&tmp_str, "%li %s-%i; CPU %i; %s;",
+			      entry->ts,
+			      task,
+			      entry->pid,
+			      entry->cpu,
+			      lat);
+
+	const char *info = kshark_get_info(kshark_ctx->pevt, data, event);
+	if (size_tmp) {
+		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..d6e41bb
--- /dev/null
+++ b/kernel-shark-qt/src/libkshark.h
@@ -0,0 +1,157 @@
+/* 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 treace 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 treace 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		*pevt;
+
+	/** 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 4/7] 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     | 80 +++++++++++++++++++++++++
 4 files changed, 87 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..a5c0ed3
--- /dev/null
+++ b/kernel-shark-qt/examples/dataload.c
@@ -0,0 +1,80 @@
+// 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)
+{
+	// Create a new kshark session.
+	struct kshark_context *kshark_ctx = NULL;
+	kshark_instance(&kshark_ctx);
+
+	// Open a trace data file produced by trace-cmd.
+	bool status;
+	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.
+	struct kshark_entry **data = NULL;
+	size_t r, n_rows;
+	n_rows = kshark_load_data_entries(kshark_ctx, &data);
+
+	// Print to the screen the list of all tasks.
+	struct kshark_task_list* task = kshark_ctx->tasks;
+	while (task) {
+		const char *task_str = pevent_data_comm_from_pid(kshark_ctx->pevt,
+								 task->pid);
+		printf("task: %s-%i\n", task_str, task->pid);
+		task = task->next;
+	}
+
+	puts("\n\n");
+
+	// Print to the screen the first 10 entries.
+	char *entry_str;
+	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 5/7] 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 | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 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..7cae9ed
--- /dev/null
+++ b/kernel-shark-qt/README
@@ -0,0 +1,37 @@
+
+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
+
+2. In order to install the packages on Fedora, as root do the following:
+    dnf install gcc gcc-c++ git cmake -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 6/7] 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.

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

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index f112a50..c6e03a8 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -28,6 +28,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) {
 		free(*context);
@@ -112,6 +120,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->pevt = NULL;
@@ -129,6 +146,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)
@@ -171,6 +194,113 @@ kshark_add_task(struct kshark_context *kshark_ctx, int pid)
 	return list;
 }
 
+static bool kshark_show_task(struct kshark_context *kshark_ctx, int pid)
+{
+	return (!kshark_ctx->show_task_filter ||
+		!kshark_ctx->show_task_filter->count ||
+		tracecmd_filter_id_find(kshark_ctx->show_task_filter, pid)) &&
+		(!kshark_ctx->hide_task_filter ||
+		 !kshark_ctx->hide_task_filter->count ||
+		 !tracecmd_filter_id_find(kshark_ctx->hide_task_filter, pid));
+}
+
+static bool kshark_show_event(struct kshark_context *kshark_ctx, int event_id)
+{
+	return (!kshark_ctx->show_event_filter ||
+		!kshark_ctx->show_event_filter->count ||
+		tracecmd_filter_id_find(kshark_ctx->show_event_filter, event_id)) &&
+		(!kshark_ctx->hide_event_filter ||
+		 !kshark_ctx->hide_event_filter->count ||
+		 !tracecmd_filter_id_find(kshark_ctx->hide_event_filter, event_id));
+}
+
+void kshark_filter_add_id(struct kshark_context *kshark_ctx, int filter_id, int id)
+{
+	switch (filter_id) {
+		case SHOW_EVENT_FILTER:
+			tracecmd_filter_id_add(kshark_ctx->show_event_filter, id);
+			break;
+
+		case HIDE_EVENT_FILTER:
+			tracecmd_filter_id_add(kshark_ctx->hide_event_filter, id);
+			break;
+
+		case SHOW_TASK_FILTER:
+			tracecmd_filter_id_add(kshark_ctx->show_task_filter, id);
+			break;
+
+		case HIDE_TASK_FILTER:
+			tracecmd_filter_id_add(kshark_ctx->hide_task_filter, id);
+			break;
+
+		default:
+			break;
+	}
+}
+
+void kshark_filter_clear(struct kshark_context *kshark_ctx, int filter_id)
+{
+	switch (filter_id) {
+		case SHOW_EVENT_FILTER:
+			tracecmd_filter_id_clear(kshark_ctx->show_event_filter);
+			break;
+
+		case HIDE_EVENT_FILTER:
+			tracecmd_filter_id_clear(kshark_ctx->hide_event_filter);
+			break;
+
+		case SHOW_TASK_FILTER:
+			tracecmd_filter_id_clear(kshark_ctx->show_task_filter);
+			break;
+
+		case HIDE_TASK_FILTER:
+			tracecmd_filter_id_clear(kshark_ctx->hide_task_filter);
+			break;
+
+		default:
+			break;
+	}
+}
+
+static bool kshark_filter_is_set(struct kshark_context *kshark_ctx)
+{
+	if ((kshark_ctx->show_task_filter &&
+	     kshark_ctx->show_task_filter->count) ||
+	    (kshark_ctx->hide_task_filter &&
+	     kshark_ctx->hide_task_filter->count) ||
+	    (kshark_ctx->show_event_filter &&
+	     kshark_ctx->show_event_filter->count) ||
+	    (kshark_ctx->hide_event_filter &&
+	     kshark_ctx->hide_event_filter->count)) {
+		return true;
+	}
+
+	return false;
+}
+
+void kshark_filter_entries(struct kshark_context *kshark_ctx,
+			   struct kshark_entry **data,
+			   size_t n_entries)
+{
+	if (!kshark_filter_is_set(kshark_ctx))
+		return;
+
+	int i;
+	for (i = 0; i < n_entries; ++i) {
+		data[i]->visible = 0xFF;
+		if (!kshark_show_task(kshark_ctx, data[i]->pid)) {
+			data[i]->visible &= ~kshark_ctx->filter_mask;
+		}
+
+		if (!kshark_show_event(kshark_ctx, data[i]->event_id)) {
+			int mask = kshark_ctx->filter_mask;
+			mask &= ~KS_GRAPH_VIEW_FILTER_MASK;
+			mask |= KS_EVENT_VIEW_FILTER_MASK;
+			data[i]->visible &= ~mask;
+		}
+	}
+}
+
 static void kshark_set_entry_values(struct kshark_context *kshark_ctx,
 				    struct pevent_record *record,
 				    struct kshark_entry *entry)
@@ -224,6 +354,17 @@ size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 			kshark_set_entry_values(kshark_ctx, rec, entry);
 			kshark_add_task(kshark_ctx, entry->pid);
 
+			if (!kshark_show_task(kshark_ctx, entry->pid)) {
+				entry->visible &= ~kshark_ctx->filter_mask;
+			}
+
+			if (!kshark_show_event(kshark_ctx, entry->event_id)) {
+				int mask = kshark_ctx->filter_mask;
+				mask &= ~KS_GRAPH_VIEW_FILTER_MASK;
+				mask |= KS_EVENT_VIEW_FILTER_MASK;
+				entry->visible &= ~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 d6e41bb..460c0c5 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"
 
 /**
@@ -82,6 +83,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;
 };
 
 /**
@@ -150,6 +170,61 @@ 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 {
+	/** Identifier of the filter, used to specified the events to be shown. */
+	SHOW_EVENT_FILTER,
+
+	/** Identifier of the filter, used to specified the events to be filtered-out. */
+	HIDE_EVENT_FILTER,
+
+	/** Identifier of the filter, used to specified the tasks to be shown. */
+	SHOW_TASK_FILTER,
+
+	/** Identifier of the filter, used to specified the tasks to be filtered-out. */
+	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 7/7] 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   | 114 ++++++++++++++++++++++++
 2 files changed, 118 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..e824880
--- /dev/null
+++ b/kernel-shark-qt/examples/datafilter.c
@@ -0,0 +1,114 @@
+// 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)
+{
+	// Create a new kshark session.
+	struct kshark_context *kshark_ctx = NULL;
+	kshark_instance(&kshark_ctx);
+
+	// Open a trace data file produced by trace-cmd.
+	bool status;
+	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.
+	struct kshark_entry **data = NULL;
+	size_t n_rows;
+	n_rows = kshark_load_data_entries(kshark_ctx, &data);
+
+	// Filter the trace data coming from trace-cmd.
+	struct kshark_task_list* task = kshark_ctx->tasks;
+	while (task) {
+		const char *task_str = pevent_data_comm_from_pid(kshark_ctx->pevt,
+								 task->pid);
+
+		if (strcmp(task_str, "trace-cmd") == 0)
+			kshark_filter_add_id(kshark_ctx, 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.
+	char *entry_str;
+	size_t r = 0, count = 0;
+	while (count < 10) {
+		if (data[r]->visible & KS_TEXT_VIEW_FILTER_MASK) {
+			entry_str = kshark_dump_entry(data[r]);
+			puts(entry_str);
+			free(entry_str);
+			++count;
+		}
+
+		++r;
+	}
+
+	puts("\n\n");
+
+	struct event_format *event;
+	size_t i, n_evts = kshark_ctx->pevt->nr_events;
+	for (i = 0; i < n_evts; ++i) {
+		event = kshark_ctx->pevt->events[i];
+		if (strcmp(event->system, "sched") == 0)
+			kshark_filter_add_id(kshark_ctx, SHOW_EVENT_FILTER,
+							 event->id);
+	}
+
+	kshark_filter_entries(kshark_ctx, data, n_rows);
+
+	// Print to the screen the first 10 visible entries.
+	count = 0;
+	r = 0;
+	while (count < 10) {
+		if (data[r]->visible & KS_TEXT_VIEW_FILTER_MASK) {
+			entry_str = kshark_dump_entry(data[r]);
+			puts(entry_str);
+			free(entry_str);
+			++count;
+		}
+
+		++r;
+	}
+
+	// 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

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

[Steven Rostedt]

On Mon, 25 Jun 2018 18:01:15 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> This patch adds in /kernel-shark-qt a simple CMake building
> infrastructure, to be used by the new, Qt-based version of
> KernelShark.

May want to add in the change log, something like this.

  Currently, this does not build anything. It is only to add
  the necessary infrastructure for the coming changes.

I can add it, so you don't need to send more code. Just an FYI, that if
there's something that is added for infrastructure, that doesn't really
do anything by itself, a brief note about that is usually expected.


> 
> diff --git a/kernel-shark-qt/build/deff.h.cmake b/kernel-shark-qt/build/deff.h.cmake
> new file mode 100644
> index 0000000..62e6b9e
> --- /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 --!

Is this generated? Do we need to add this in the commit, or can this
just be created during the setup?

-- Steve

> +
> +#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

Re: [PATCH 2/7] kernel-shark-qt: Automatic generation of doxygen documentation

[Steven Rostedt]

On Mon, 25 Jun 2018 18:01:16 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> 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       | 2291 ++++++++++++++++++++++++++
>  3 files changed, 2310 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..68d9ca0
> --- /dev/null
> +++ b/kernel-shark-qt/doc/dox_config
> @@ -0,0 +1,2291 @@
> +# Doxyfile 1.8.6
> +
> +# This file describes the settings to be used by the documentation system
> +# doxygen (www.doxygen.org) for a project.
> +#
> +# All text after a double hash (##) is considered a comment and is placed in
> +# front of the TAG it is preceding.
> +#
> +# All text after a single hash (#) is considered a comment and will be ignored.
> +# The format is:
> +# TAG = value [value, ...]
> +# For lists, items can also be appended using:
> +# TAG += value [value, ...]
> +# Values that contain spaces should be placed between quotes (\" \").
> +

All this text and below, was it copied from somewhere? What's the
license, and we should document where it came from.

-- Steve


> +#---------------------------------------------------------------------------
> +# Project related configuration options
> +#---------------------------------------------------------------------------
> +
> +# This tag specifies the encoding used for all characters in the config file
> +# that follow. The default is UTF-8 which is also the encoding used for all text
> +# before the first occurrence of this tag. Doxygen uses libiconv (or the iconv
> +# built into libc) for the transcoding. See http://www.gnu.org/software/libiconv
> +# for the list of possible encodings.
> +# The default value is: UTF-8.
> +
> +DOXYFILE_ENCODING      = UTF-8
> +
> +# The PROJECT_NAME tag is a single word (or a sequence of words surrounded by
> +# double-quotes, unless you are using Doxywizard) that should identify the
> +# project for which the documentation is generated. This name is used in the
> +# title of most generated pages and in a few other places.
> +# The default value is: My Project.
> +
> +PROJECT_NAME           = "kernel-shark-qt"

[..]

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

[Steven Rostedt]

On Mon, 25 Jun 2018 18:01:17 +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 introduces

	"will introduce"

> an example, demonstrating the usage of this part of the API.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>


> +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;
> +

Because in the Linux Kernel development, it is looked down on to add a
check for a ptr having non-null before free, I'll push it here too. As
free(ptr) is basically a nop if ptr == NULL, we like to eliminate if
statements checking for NULL unless they are really necessary.

To get you use to this mindset, I'll suggest here instead of:


> +	/* Free the existing context (if any). */
> +	if (*context && *context != kshark_context_handler) {
> +		free(*context);
> +		*context = NULL;
> +	}
> +
> +	if (kshark_context_handler) {
> +		free(kshark_context_handler);
> +		kshark_context_handler = NULL;
> +	}
> +
> +	kshark_context_handler = kshark_ctx;
> +	*context = kshark_ctx;

Do:

	if (*context != kshark_context_handler) {
		free(*context);
		*context = NULL;
	}

	free(kshark_context_handler);
	kshark_context_handle = kshark_ctx;


> +
> +	return true;
> +}
> +
> +bool kshark_instance(struct kshark_context **context)
> +{
> +	if (!seq.buffer)
> +		trace_seq_init(&seq);

May need to add here:

		if (!seq.buffer)
			return false;
	}

As the trace_seq_init() can fail. Hmm, I wonder if I should change that
to have a return status instead of just returning void.

> +
> +	if (*context == NULL && kshark_context_handler == NULL) {
> +		// No kshark_context exists. Create a default one.

BTW, for C files, please use the /* */ comment styles.

Only the SPDX gets to use '//'.


> +		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)

No need for the if statement.

> +			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;
> +	}
> +
> +	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 = kshark_ctx->tasks->next;

You don't really need to change this, but I'm lazy, and like to type
less. So I would have written the above as:

		kshark_ctx->tasks = task->next;


> +		free(task);
> +	}
> +
> +	task = kshark_ctx->tasks = NULL;

Why the assignment of "task" here? It's no longer used.

> +}
> +
> +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;
> +
> +	pthread_mutex_init(&kshark_ctx->input_mutex, NULL);

pthread_mutex_init() may fail. Need to handle that case.

> +
> +	kshark_ctx->handle = handle;
> +	kshark_ctx->pevt = tracecmd_get_pevent(handle);

Let's be consistent, and use "pevent" as the field name here, instead
of "pevt". In the near future, we will be converting "pevent" to "tep",
and it would be good to do a global search and replace for that.

> +
> +	/*
> +	 * 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->pevt = 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);
> +
> +	free(kshark_ctx);
> +
> +	if (kshark_ctx == kshark_context_handler)
> +		kshark_context_handler = NULL;

Move the free(kshark_ctx) here. I understand that we are only comparing
the value of the pointer, but it's considered cleaner if you don't use
a pointer that you freed, even if you are not dereferencing it. It's
also best to keep a free(x); x = NULL; together. On separate lines, of
course.

> +
> +	kshark_ctx = NULL;
> +}
> +
> +static struct kshark_task_list *
> +kshark_find_task(struct kshark_context *kshark_ctx, int pid)
> +{
> +	struct kshark_task_list *list = kshark_ctx->tasks;
> +	while (list) {
> +		if (list->pid == pid)
> +			return list;
> +
> +		list = list->next;
> +	}

Could you convert this to a for loop?

	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));

Need to check if list == NULL 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

I know it's rather a pain, but please use /* */ comment style here.

> +	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->pevt, 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->pevt, 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;
> +	size_t count, total = 0;
> +	struct pevent_record *rec;

Please get into the habit of "upside-down-xmas-tree" style of arranging
of declarations. Also, let's keep to the old C style (as it is required
by the Linux kernel too and not do declarations after code.

	int n_cpus = tracecmd_cpus(kshark_ctx->handle);
	struct kshark_entry *entry, **next;
	struct kshark_entry **cpu_list;
	struct pevent_record *rec;
	size_t count, total = 0;
	int cpu;


> +
> +	if (*data_rows)
> +		free(*data_rows);
> +
> +	struct kshark_entry *entry, **next;

Move this declaration above (as noted).

> +	struct kshark_entry **cpu_list = calloc(n_cpus, sizeof(struct kshark_entry *));

Just use assignment, without the declaration.

Need to test if the calloc() passed.

> +
> +	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) {

Probably could make this a for loop:

		for (rec = tracecmd_read_cpu_first(kshark_ctx->handle, cpu),
		     rec,
		     rec = tracecmd_read_data(kshark_ctx->handle, cpu)) {

Or maybe not ;-)

> +			*next = entry = malloc( sizeof(struct kshark_entry) );
> +			assert(entry != NULL);
> +
> +			kshark_set_entry_values(kshark_ctx, rec, entry);
> +			kshark_add_task(kshark_ctx, entry->pid);
> +
> +			entry->next = NULL;
> +			next = &(entry->next);

Parenthesis are not needed: next = &entry->next;

> +			free_record(rec);
> +
> +			++count;
> +			rec = tracecmd_read_data(kshark_ctx->handle, cpu);
> +		}
> +
> +		total += count;
> +	}
> +
> +	struct kshark_entry **rows;

Move the declaration to the top of the function. I really wish that C
and C++ never allowed this "feature".

> +	rows = calloc(total, sizeof(struct kshark_entry *));
> +	if(!rows) {

Add a space between "if" and "("

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

We fail safely here on allocation, but not above with the assert?
Should be be consistent. Also, I think we need a function for errors
that can be overwritten. "warning()" I believe already does that, and
allows for nice pop-up errors.

> +		return 0;
> +	}
> +
> +	count = 0;
> +	int next_cpu;
> +	uint64_t ts;

Again, please keep all declarations at the top of the function.

> +	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;
> +}
> +
> +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 cpu;
> +	size_t count, total = 0;
> +	struct pevent_record *data;

Again, "upside-down-xmas-tree" style and move all declarations to up
here.

> +
> +	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));
> +			assert(temp_rec != NULL);

Again, should we just assert, or make it fail nicely?

BTW, "goto" is your friend in these cases ;-)

> +
> +			kshark_add_task(kshark_ctx,
> +					pevent_data_pid(kshark_ctx->pevt, data));
> +			temp_rec->rec = data;
> +			temp_rec->next = NULL;
> +			temp_next = &(temp_rec->next);

Again, no need for the parenthesis. The &temp_rec->next is common
nomenclature for this type of construct.

> +
> +			++count;
> +			data = tracecmd_read_data(kshark_ctx->handle, cpu);
> +		}
> +
> +		total += count;
> +	}
> +
> +	struct pevent_record **rows;
> +	rows = calloc(total, sizeof(struct pevent_record *));
> +	if(!rows) {
> +		fprintf(stderr, "Failed to allocate memory during data loading.\n");
> +		return 0;
> +	}
> +

The below is awfully similar to the end of kshark_load_data_entries(),
I wonder if we could use the same code.

> +	count = 0;
> +	int next_cpu;
> +	uint64_t ts;
> +	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);

We could move the freeing of cpu_list content out of this code (do it
separately), and then we can make a helper function to do the row
assignments that both functions use.

> +		}
> +
> +		++count;
> +	}
> +
> +	free(cpu_list);
> +	*data_rows = rows;
> +	return total;
> +}
> +
> +static struct pevent_record *kshark_read_at(struct kshark_context *kshark_ctx,
> +					    uint64_t offset)
> +{
> +	pthread_mutex_lock(&kshark_ctx->input_mutex);

I'm curious, what is this lock protecting?

> +
> +	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)
> +{
> +	if (!record || !event)
> +		return NULL;
> +
> +	trace_seq_reset(&seq);
> +	pevent_event_info(&seq, event, record);
> +
> +	// Remove the trailing newline from the Info string.
> +	char *pos;
> +	if ((pos = strchr(seq.buffer, '\n')) != NULL)

This does not actually do what the comment says. It removes the first
'\n' from the Info string, not the last one.

> +		*pos = '\0';
> +
> +	return seq.buffer;
> +}
> +
> +char* kshark_dump_entry(struct kshark_entry *entry)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	kshark_instance(&kshark_ctx);
> +
> +	if (!seq.buffer)
> +		trace_seq_init(&seq);

Need to check the result of trace_seq_init().

> +
> +	trace_seq_reset(&seq);

Why reset here? You also reset it within kshark_get_latency().

> +
> +	char *tmp_str, *entry_str;
> +	int size_tmp, size = 0;

Please move these to the top of the function. Also, there's no reason
to make two "size" variables. You can use size for the size_tmp and it
won't affect the code.

> +
> +	struct pevent_record *data = kshark_read_at(kshark_ctx, entry->offset);
> +
> +	int event_id = pevent_data_type(kshark_ctx->pevt, data);
> +	struct event_format *event =
> +		pevent_data_event_from_type(kshark_ctx->pevt, event_id);
> +
> +	const char *event_name = (event)? event->name : "[UNKNOWN EVENT]";

No need for the parenthesis around "event".

> +	const char *task = pevent_data_comm_from_pid(kshark_ctx->pevt, entry->pid);
> +	const char *lat = kshark_get_latency(kshark_ctx->pevt, data);

And separate these as declarations and assignments.

> +
> +	size_tmp = asprintf(&tmp_str, "%li %s-%i; CPU %i; %s;",
> +			      entry->ts,
> +			      task,
> +			      entry->pid,
> +			      entry->cpu,
> +			      lat);
> +
> +	const char *info = kshark_get_info(kshark_ctx->pevt, data, event);

Where is "info" used?

> +	if (size_tmp) {
> +		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..d6e41bb
> --- /dev/null
> +++ b/kernel-shark-qt/src/libkshark.h
> @@ -0,0 +1,157 @@
> +/* 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

								       ^
                                                          Remove the comma.

> + * 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 treace file.
> + */

Also, keep descriptions like the above <= 80 characters wide.

> +struct kshark_entry {
> +	/**

You don't need the double star within here. If you do this, (at least
for the recommended way in the Linux kernel), then you should add all
the comments about the variables above the struct.

> +	 * A bit mask controlling the visibility of the entry. A value of OxFF would mean
> +	 * that the entry is visible everywhere.

And here too (80 chars)

I'm not too strict on 80 char max width, but the Linux kernel folks are
somewhat. These comments definitely should fit within an 80 character
screen.

As for the bitmask, what other values are acceptable?

> +	 */
> +	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 treace file, used to find the record. */

Typo "treace".

> +	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. */

Note, the Linux kernel doc way of doing these types of comments is like
this:

/**
 * kshark_task_list - Linked list of tasks
 * @next:	Pointer to the text task's PID
 * @pid:	PID of a task.
 */

> +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		*pevt;
> +
> +	/** 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.

Looks like you have a different set of commands for the doxygen
comments than the kernel uses. Of course, the kernel may be using
"kernel doc" format. I'm not an expert on this. But I would prefer to
keep it more like what the kernel does.

How hard would it be to switch over?

-- Steve

> +
> + */
> +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

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

[Steven Rostedt]

On Mon, 25 Jun 2018 18:01:18 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:


> --- /dev/null
> +++ b/kernel-shark-qt/examples/dataload.c
> @@ -0,0 +1,80 @@
> +// 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)
> +{
> +	// Create a new kshark session.

Again, for C files, please use the /* */ comment format.

> +	struct kshark_context *kshark_ctx = NULL;
> +	kshark_instance(&kshark_ctx);


Shouldn't you check the return value here?


> +
> +	// Open a trace data file produced by trace-cmd.
> +	bool status;

Even though this is an example, please put declarations at the top.

Other than that, this looks fine.

-- Steve

> +	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.
> +	struct kshark_entry **data = NULL;
> +	size_t r, n_rows;
> +	n_rows = kshark_load_data_entries(kshark_ctx, &data);
> +
> +	// Print to the screen the list of all tasks.
> +	struct kshark_task_list* task = kshark_ctx->tasks;
> +	while (task) {
> +		const char *task_str = pevent_data_comm_from_pid(kshark_ctx->pevt,
> +								 task->pid);
> +		printf("task: %s-%i\n", task_str, task->pid);
> +		task = task->next;
> +	}
> +
> +	puts("\n\n");
> +
> +	// Print to the screen the first 10 entries.
> +	char *entry_str;
> +	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;
> +}

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

[Steven Rostedt]

On Mon, 25 Jun 2018 18:01:19 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> +++ b/kernel-shark-qt/README
> @@ -0,0 +1,37 @@
> +
> +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
> +
> +2. In order to install the packages on Fedora, as root do the following:
> +    dnf install gcc gcc-c++ git cmake -y

I'm not so sure how useful this "third party software" section is. I
think it's pretty obvious that gcc, c++ and cmake will be needed.


> +
> +
> +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.

Note, after all the code is in the repo, I expect to have the top level
Makefile, do all the cmake work for the user.

-- Steve

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

[Steven Rostedt]

On Mon, 25 Jun 2018 18:01:20 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:


> +static bool kshark_show_task(struct kshark_context *kshark_ctx, int pid)
> +{
> +	return (!kshark_ctx->show_task_filter ||
> +		!kshark_ctx->show_task_filter->count ||
> +		tracecmd_filter_id_find(kshark_ctx->show_task_filter, pid)) &&
> +		(!kshark_ctx->hide_task_filter ||
> +		 !kshark_ctx->hide_task_filter->count ||
> +		 !tracecmd_filter_id_find(kshark_ctx->hide_task_filter, pid));
> +}
> +
> +static bool kshark_show_event(struct kshark_context *kshark_ctx, int event_id)
> +{
> +	return (!kshark_ctx->show_event_filter ||
> +		!kshark_ctx->show_event_filter->count ||
> +		tracecmd_filter_id_find(kshark_ctx->show_event_filter, event_id)) &&
> +		(!kshark_ctx->hide_event_filter ||
> +		 !kshark_ctx->hide_event_filter->count ||
> +		 !tracecmd_filter_id_find(kshark_ctx->hide_event_filter, event_id));
> +}

To make the above cleaner, can we add some helper functions:

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->show_task_filter, pid, true) ||
	       filter_find(kshark->hid_task_filter, pid, false);
}

static bool kshark_show_event(struct kshark_context *kshark_ctx, int pid)
{
	return filter_find(kshark->show_event_filter, pid, true) ||
	       filter_find(kshark->hide_event_filter, pid, false);
}
		

> +
> +void kshark_filter_add_id(struct kshark_context *kshark_ctx, int filter_id, int id)
> +{
> +	switch (filter_id) {
> +		case SHOW_EVENT_FILTER:
> +			tracecmd_filter_id_add(kshark_ctx->show_event_filter, id);
> +			break;
> +
> +		case HIDE_EVENT_FILTER:
> +			tracecmd_filter_id_add(kshark_ctx->hide_event_filter, id);
> +			break;
> +
> +		case SHOW_TASK_FILTER:
> +			tracecmd_filter_id_add(kshark_ctx->show_task_filter, id);
> +			break;
> +
> +		case HIDE_TASK_FILTER:
> +			tracecmd_filter_id_add(kshark_ctx->hide_task_filter, id);
> +			break;
> +
> +		default:
> +			break;
> +	}
> +}

And these could be simplified a little with:

	struct tracecmd_filter_id *filter;

	switch (filter_id) {
		case SHOW_EVENT_FILTER:
			filter = kshark_ctx->show_event_filter;
			break;
		case HIDE_EVENT_FILTER:
			filter = kshark_ctx->hide_event_filter;
			break;
		case SHOW_TASK_FILTER:
			filter = kshark_ctx->show_task_filter;
			break;
		case HIDE_TASK_FILTER:
			filter = kshark_ctx->hide_task_filter;
			break;
		default:
			return;
	}
	tracecmd_filter_id_add(filter, id);

Hmm, I noticed that the tracecmd_filter_id_add() doesn't return if it
allocated, just asserts. We may want to change that and test for
returns :-/

> +
> +void kshark_filter_clear(struct kshark_context *kshark_ctx, int filter_id)
> +{
> +	switch (filter_id) {
> +		case SHOW_EVENT_FILTER:
> +			tracecmd_filter_id_clear(kshark_ctx->show_event_filter);
> +			break;
> +
> +		case HIDE_EVENT_FILTER:
> +			tracecmd_filter_id_clear(kshark_ctx->hide_event_filter);
> +			break;
> +
> +		case SHOW_TASK_FILTER:
> +			tracecmd_filter_id_clear(kshark_ctx->show_task_filter);
> +			break;
> +
> +		case HIDE_TASK_FILTER:
> +			tracecmd_filter_id_clear(kshark_ctx->hide_task_filter);
> +			break;
> +
> +		default:
> +			break;
> +	}

Smae here.

> +}
> +
> +static bool kshark_filter_is_set(struct kshark_context *kshark_ctx)
> +{
> +	if ((kshark_ctx->show_task_filter &&
> +	     kshark_ctx->show_task_filter->count) ||
> +	    (kshark_ctx->hide_task_filter &&
> +	     kshark_ctx->hide_task_filter->count) ||
> +	    (kshark_ctx->show_event_filter &&
> +	     kshark_ctx->show_event_filter->count) ||
> +	    (kshark_ctx->hide_event_filter &&
> +	     kshark_ctx->hide_event_filter->count)) {
> +		return true;

And above, a little helper that makes it easier to read.

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);
}


> +	}
> +
> +	return false;
> +}
> +

Global functions should have a kernel doc comment.

> +void kshark_filter_entries(struct kshark_context *kshark_ctx,
> +			   struct kshark_entry **data,
> +			   size_t n_entries)
> +{
> +	if (!kshark_filter_is_set(kshark_ctx))
> +		return;
> +
> +	int i;

Move the int declaration to the top.

> +	for (i = 0; i < n_entries; ++i) {
> +		data[i]->visible = 0xFF;
> +		if (!kshark_show_task(kshark_ctx, data[i]->pid)) {
> +			data[i]->visible &= ~kshark_ctx->filter_mask;
> +		}
> +
> +		if (!kshark_show_event(kshark_ctx, data[i]->event_id)) {
> +			int mask = kshark_ctx->filter_mask;

BTW, this declaration is fine. It's still at a top of a block.

> +			mask &= ~KS_GRAPH_VIEW_FILTER_MASK;
> +			mask |= KS_EVENT_VIEW_FILTER_MASK;
> +			data[i]->visible &= ~mask;

Why is this function only filtering event_view and not graph view?

> +		}
> +	}
> +}
> +
>  static void kshark_set_entry_values(struct kshark_context *kshark_ctx,
>  				    struct pevent_record *record,
>  				    struct kshark_entry *entry)
> @@ -224,6 +354,17 @@ size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
>  			kshark_set_entry_values(kshark_ctx, rec, entry);
>  			kshark_add_task(kshark_ctx, entry->pid);
>  
> +			if (!kshark_show_task(kshark_ctx, entry->pid)) {
> +				entry->visible &= ~kshark_ctx->filter_mask;
> +			}
> +
> +			if (!kshark_show_event(kshark_ctx, entry->event_id)) {
> +				int mask = kshark_ctx->filter_mask;
> +				mask &= ~KS_GRAPH_VIEW_FILTER_MASK;
> +				mask |= KS_EVENT_VIEW_FILTER_MASK;
> +				entry->visible &= ~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 d6e41bb..460c0c5 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"
>  
>  /**
> @@ -82,6 +83,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;
>  };
>  
>  /**
> @@ -150,6 +170,61 @@ 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),

Parenthesis are not needed.

> +};
> +
> +/** Filter type identifier. */
> +enum kshark_filter_type {

I wonder if we should have:

	NO_FILTER,

So that all types are > 0?

> +	/** Identifier of the filter, used to specified the events to be shown. */
> +	SHOW_EVENT_FILTER,
> +
> +	/** Identifier of the filter, used to specified the events to be filtered-out. */
> +	HIDE_EVENT_FILTER,
> +
> +	/** Identifier of the filter, used to specified the tasks to be shown. */
> +	SHOW_TASK_FILTER,
> +
> +	/** Identifier of the filter, used to specified the tasks to be filtered-out. */
> +	HIDE_TASK_FILTER,

Also, if you ever expect this to be used outside of KernelShark, we
should have "KS_" prefixed to avoid namespace collisions.

-- Steve

> +};
> +
> +/**
> + * @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

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

[Yordan Karadzhov (VMware)]

On 25.06.2018 19:06, Steven Rostedt wrote:
> On Mon, 25 Jun 2018 18:01:15 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> This patch adds in /kernel-shark-qt a simple CMake building
>> infrastructure, to be used by the new, Qt-based version of
>> KernelShark.
> 
> May want to add in the change log, something like this.
> 
>    Currently, this does not build anything. It is only to add
>    the necessary infrastructure for the coming changes.
> 
> I can add it, so you don't need to send more code. Just an FYI, that if
> there's something that is added for infrastructure, that doesn't really
> do anything by itself, a brief note about that is usually expected.
> 
> 
>>
>> diff --git a/kernel-shark-qt/build/deff.h.cmake b/kernel-shark-qt/build/deff.h.cmake
>> new file mode 100644
>> index 0000000..62e6b9e
>> --- /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 --!
> 
> Is this generated? Do we need to add this in the commit, or can this
> just be created during the setup?
> 

This is a configuration file used to tell CMake how to generate a 
special header file. Based on the content of this configuration file, 
CMake will create src/KsDeff.h

Indeed, it isn't true that the build system is doing nothing in the case 
of this particular patch. The build system will generate src/KsDeff.h

I will explain this in the change log and send the patch again.

Thanks!
Yordan

> -- Steve
> 
>> +
>> +#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

Re: [PATCH 2/7] kernel-shark-qt: Automatic generation of doxygen documentation

[Yordan Karadzhov (VMware)]

On 25.06.2018 19:09, Steven Rostedt wrote:
> On Mon, 25 Jun 2018 18:01:16 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> 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       | 2291 ++++++++++++++++++++++++++
>>   3 files changed, 2310 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..68d9ca0
>> --- /dev/null
>> +++ b/kernel-shark-qt/doc/dox_config
>> @@ -0,0 +1,2291 @@
>> +# Doxyfile 1.8.6
>> +
>> +# This file describes the settings to be used by the documentation system
>> +# doxygen (www.doxygen.org) for a project.
>> +#
>> +# All text after a double hash (##) is considered a comment and is placed in
>> +# front of the TAG it is preceding.
>> +#
>> +# All text after a single hash (#) is considered a comment and will be ignored.
>> +# The format is:
>> +# TAG = value [value, ...]
>> +# For lists, items can also be appended using:
>> +# TAG += value [value, ...]
>> +# Values that contain spaces should be placed between quotes (\" \").
>> +
> 
> All this text and below, was it copied from somewhere? What's the
> license, and we should document where it came from.
> 

A template Config file for Doxygen, like the one added in this patch, 
can be generated automatically (doxygen -g <Config File Name>).

However you are right that, adding to the project this huge file may 
cause confusion. Maybe it will be better if we use a simple file 
containing only the parameters, having non-default values and a link to 
the official Doxigen documentation page.

What do you think?

Thanks!
Yordan

> -- Steve
> 
> 
>> +#---------------------------------------------------------------------------
>> +# Project related configuration options
>> +#---------------------------------------------------------------------------
>> +
>> +# This tag specifies the encoding used for all characters in the config file
>> +# that follow. The default is UTF-8 which is also the encoding used for all text
>> +# before the first occurrence of this tag. Doxygen uses libiconv (or the iconv
>> +# built into libc) for the transcoding. See http://www.gnu.org/software/libiconv
>> +# for the list of possible encodings.
>> +# The default value is: UTF-8.
>> +
>> +DOXYFILE_ENCODING      = UTF-8
>> +
>> +# The PROJECT_NAME tag is a single word (or a sequence of words surrounded by
>> +# double-quotes, unless you are using Doxywizard) that should identify the
>> +# project for which the documentation is generated. This name is used in the
>> +# title of most generated pages and in a few other places.
>> +# The default value is: My Project.
>> +
>> +PROJECT_NAME           = "kernel-shark-qt"
> 
> [..]
> 

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

[Yordan Karadzhov (VMware)]

On 25.06.2018 21:30, Steven Rostedt wrote:
>> +/** Linked list of tasks. */
> Note, the Linux kernel doc way of doing these types of comments is like
> this:
> 
> /**
>   * kshark_task_list - Linked list of tasks
>   * @next:	Pointer to the text task's PID
>   * @pid:	PID of a task.
>   */
> 

If we want to be able to produce Doxygen documentation for the code we 
have to stick to the Doxygen syntax for comments.

>> +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		*pevt;
>> +
>> +	/** 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.
> Looks like you have a different set of commands for the doxygen
> comments than the kernel uses. Of course, the kernel may be using
> "kernel doc" format. I'm not an expert on this. But I would prefer to
> keep it more like what the kernel does.
> 
> How hard would it be to switch over?

I have no experience with "kernel doc", so I have no idea if it will be 
easy or hard. I can try to investigate, but this will take some time.
I this the way to go?

Thanks!
Yordan

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

[Yordan Karadzhov (VMware)]

On 25.06.2018 21:38, Steven Rostedt wrote:
> On Mon, 25 Jun 2018 18:01:19 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> +++ b/kernel-shark-qt/README
>> @@ -0,0 +1,37 @@
>> +
>> +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
>> +
>> +2. In order to install the packages on Fedora, as root do the following:
>> +    dnf install gcc gcc-c++ git cmake -y
> 
> I'm not so sure how useful this "third party software" section is. I
> think it's pretty obvious that gcc, c++ and cmake will be needed.
> 

I would like to have here instructions how to build the code on a 
default out-of-the-box installation of Ubuntu or Fedora. I agree that it 
doesn't make sense as it is right now, but it will start making sense 
when we start adding more and more dependencies.

> 
>> +
>> +
>> +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.
> 
> Note, after all the code is in the repo, I expect to have the top level
> Makefile, do all the cmake work for the user.
> 

Do you mean, you want to completely remove this README file?

Thanks!
Yordan


> -- Steve
> 

Re: [PATCH 2/7] kernel-shark-qt: Automatic generation of doxygen documentation

[Steven Rostedt]

On Tue, 26 Jun 2018 17:29:09 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> On 25.06.2018 19:09, Steven Rostedt wrote:
> > On Mon, 25 Jun 2018 18:01:16 +0300
> > "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> >   
> >> 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       | 2291 ++++++++++++++++++++++++++
> >>   3 files changed, 2310 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..68d9ca0
> >> --- /dev/null
> >> +++ b/kernel-shark-qt/doc/dox_config
> >> @@ -0,0 +1,2291 @@
> >> +# Doxyfile 1.8.6
> >> +
> >> +# This file describes the settings to be used by the documentation system
> >> +# doxygen (www.doxygen.org) for a project.
> >> +#
> >> +# All text after a double hash (##) is considered a comment and is placed in
> >> +# front of the TAG it is preceding.
> >> +#
> >> +# All text after a single hash (#) is considered a comment and will be ignored.
> >> +# The format is:
> >> +# TAG = value [value, ...]
> >> +# For lists, items can also be appended using:
> >> +# TAG += value [value, ...]
> >> +# Values that contain spaces should be placed between quotes (\" \").
> >> +  
> > 
> > All this text and below, was it copied from somewhere? What's the
> > license, and we should document where it came from.
> >   
> 
> A template Config file for Doxygen, like the one added in this patch, 
> can be generated automatically (doxygen -g <Config File Name>).
> 
> However you are right that, adding to the project this huge file may 
> cause confusion. Maybe it will be better if we use a simple file 
> containing only the parameters, having non-default values and a link to 
> the official Doxigen documentation page.
> 
> What do you think?
> 

It's probably fine to keep the text, as long as we document where it
came from.

-- Steve

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

[Steven Rostedt]

On Tue, 26 Jun 2018 17:47:13 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> > How hard would it be to switch over?  
> 
> I have no experience with "kernel doc", so I have no idea if it will be 
> easy or hard. I can try to investigate, but this will take some time.
> I this the way to go?

Doing a search, I found this link:

https://groups.google.com/forum/#!topic/jailhouse-dev/mkfNiU3epaY

It's from Jan Kiszka (you may have met him in Prague. He gave one of
the keynotes, and is the author of Jailhouse). Seems he finds
kernel-doc limiting, and prefers doxygen. I respect Jan's opinion in
this (oh, and he was on the Alps with me too). Let's follow the
jailhouse use of doxygen. Would that do?

-- Steve

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

[Steven Rostedt]

On Tue, 26 Jun 2018 17:51:07 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:


> > I'm not so sure how useful this "third party software" section is. I
> > think it's pretty obvious that gcc, c++ and cmake will be needed.
> >   
> 
> I would like to have here instructions how to build the code on a 
> default out-of-the-box installation of Ubuntu or Fedora. I agree that it 
> doesn't make sense as it is right now, but it will start making sense 
> when we start adding more and more dependencies.

OK, I'm just worried about maintaining it.

> > Note, after all the code is in the repo, I expect to have the top level
> > Makefile, do all the cmake work for the user.
> >   
> 
> Do you mean, you want to completely remove this README file?
> 

We may not need to, if it can still be built separately. But ideally, I
want people to simply type "make gui" at the top level and it does
everything (cmake and all). I don't think that will be to difficult to
implement. I was actually planning on doing the implementation myself.

-- Steve

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

[Yordan Karadzhov (VMware)]

On 26.06.2018 18:16, Steven Rostedt wrote:
> On Tue, 26 Jun 2018 17:47:13 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>>> How hard would it be to switch over?
>>
>> I have no experience with "kernel doc", so I have no idea if it will be
>> easy or hard. I can try to investigate, but this will take some time.
>> I this the way to go?
> 
> Doing a search, I found this link:
> 
> https://groups.google.com/forum/#!topic/jailhouse-dev/mkfNiU3epaY
> 
> It's from Jan Kiszka (you may have met him in Prague. He gave one of
> the keynotes, and is the author of Jailhouse). Seems he finds
> kernel-doc limiting, and prefers doxygen. I respect Jan's opinion in
> this (oh, and he was on the Alps with me too). Let's follow the
> jailhouse use of doxygen. Would that do?
> 

OK, great!
I have been using Doxygen documentation as a user (to learn new 
libraries) so I feel a bit attached to it. It is really useful. That why 
I wanted to use Doxygen to document our code.
Thanks!
Yordan


> -- Steve
>