[PATCH 0/7] More minor modifications and fixes toward KS 2.0

[PATCH 0/7] More minor modifications and fixes toward KS 2.0

[Yordan Karadzhov (VMware)]

Yordan Karadzhov (VMware) (7):
  kernel-shark: Fix the build for 32b systems
  kernel-shark: Add "cron" job to workflows
  kernel-shark: Fix KS_DEFINE_PLUGIN_CONTEXT macro
  kernel-shark: Add cleanup of all plugin contexts
  kernel-shark: Fix memory leak in "sched events" plugin.
  kernel-shark: Disable the pop-up offset dialog
  kernel-shark: Remove kvm_combo from the list of default plugins

 .github/workflows/main.yml |  5 ++++-
 src/KsMainWindow.cpp       | 11 +----------
 src/KsSession.cpp          |  2 +-
 src/libkshark-configio.c   |  2 +-
 src/libkshark-plugin.c     | 10 ++++++++++
 src/libkshark-plugin.h     | 31 ++++++++++++++++++++++++-------
 src/libkshark-tepdata.c    |  3 +--
 src/libkshark.h            |  2 +-
 src/plugins/sched_events.c | 19 +++++++++++++++++++
 src/plugins/sched_events.h |  3 +--
 tests/libkshark-tests.cpp  |  2 +-
 11 files changed, 64 insertions(+), 26 deletions(-)

-- 
2.27.0

[PATCH 1/7] kernel-shark: Fix the build for 32b systems

[Yordan Karadzhov (VMware)]

All warnings and errors when building on 32bit Linux systems are fixed.

Reported-by: Alan Mikhak <amikhak@wirelessfabric.com>
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/KsSession.cpp        | 2 +-
 src/libkshark-configio.c | 2 +-
 src/libkshark-tepdata.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/KsSession.cpp b/src/KsSession.cpp
index 8d489f7..1e19a5f 100644
--- a/src/KsSession.cpp
+++ b/src/KsSession.cpp
@@ -587,7 +587,7 @@ void KsSession::saveDualMarker(KsDualMarkerSM *dm)
  */
 void KsSession::loadDualMarker(KsDualMarkerSM *dm, KsTraceGraph *graphs)
 {
-	uint64_t pos;
+	size_t pos;
 
 	dm->reset();
 	dm->setState(DualMarkerState::A);
diff --git a/src/libkshark-configio.c b/src/libkshark-configio.c
index 98098da..9a1ba60 100644
--- a/src/libkshark-configio.c
+++ b/src/libkshark-configio.c
@@ -567,7 +567,7 @@ static bool kshark_trace_file_from_json(const char **file, const char **name,
 	}
 
 	if (st.st_mtime != time) {
-		fprintf(stderr, "Timestamp mismatch! (%li!=%li)\nFile %s\n",
+		fprintf(stderr, "Timestamp mismatch! (%" PRIu64 "!=%li)\nFile %s\n",
 				time, st.st_mtime, file_str);
 		return false;
 	}
diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index 4e76ef2..bc5babb 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -1025,7 +1025,7 @@ static char *tepdata_dump_entry(struct kshark_data_stream *stream,
 			free(info);
 		} else {
 			n = asprintf(&entry_str,
-				     "%i; %li; [UNKNOWN TASK]-%i; CPU %i; ; [UNKNOWN EVENT]; [NO INFO]; 0x%x",
+				     "%i; %" PRIu64 "; [UNKNOWN TASK]-%i; CPU %i; ; [UNKNOWN EVENT]; [NO INFO]; 0x%x",
 				     entry->stream_id,
 				     entry->ts,
 				     interface->get_pid(stream, entry),
-- 
2.27.0

[PATCH 2/7] kernel-shark: Add "cron" job to workflows

[Yordan Karadzhov (VMware)]

The CI testing will execute automatically once a week on Thursday 3PM UTC.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 .github/workflows/main.yml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 4125354..925e235 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -1,6 +1,9 @@
 name: KernelShark CI (CMAKE)
 
-on: [push]
+on:
+  push:
+  schedule:
+    - cron:  '0 15 * * THU'
 
 env:
   # Customize the CMake build type here (Release, Debug, RelWithDebInfo, etc.)
-- 
2.27.0

[PATCH 3/7] kernel-shark: Fix KS_DEFINE_PLUGIN_CONTEXT macro

[Yordan Karadzhov (VMware)]

The KS_DEFINE_PLUGIN_CONTEXT macro implements methods that are used
to deal with plugin-specific context objects. However, when this macro
is used in multiple plugins and those plugins are loaded together
the symbol resolving fails, resulting in undefined behavior. Namely,
version of the function from one plugin, being called by another
plugin. Here we make sure that the methods defined in
KS_DEFINE_PLUGIN_CONTEXT are not visible outside of the corresponding
plugin.

Fixing: 15df009 (kernel-shark: Add KS_DEFINE_PLUGIN_CONTEXT macro)
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/libkshark-plugin.h     | 22 ++++++++++++++++++----
 src/plugins/sched_events.c |  3 +++
 src/plugins/sched_events.h |  3 +--
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/libkshark-plugin.h b/src/libkshark-plugin.h
index c110616..752dbeb 100644
--- a/src/libkshark-plugin.h
+++ b/src/libkshark-plugin.h
@@ -24,6 +24,8 @@ extern "C" {
 /* Quiet warnings over documenting simple structures */
 //! @cond Doxygen_Suppress
 
+#define __hidden __attribute__((visibility ("hidden")))
+
 #define _MAKE_STR(x)	#x
 
 #define MAKE_STR(x)	_MAKE_STR(x)
@@ -364,11 +366,14 @@ int kshark_handle_all_dpis(struct kshark_data_stream *stream,
 	__ok;								\
 })									\
 
-/** General purpose macro defining methods for adding plugin context. */
+/**
+ * General purpose macro defining methods for adding plugin context.
+ * Do not use this macro in header files.
+ */
 #define KS_DEFINE_PLUGIN_CONTEXT(type)					\
 static type **__context_handler;					\
 static ssize_t __n_streams = -1;					\
-static inline type *__init(int sd)					\
+__hidden type *__init(int sd)						\
 {									\
 	type *obj;							\
 	if (__n_streams < 0 && sd < KS_DEFAULT_NUM_STREAMS) {		\
@@ -388,7 +393,7 @@ static inline type *__init(int sd)					\
 	__context_handler[sd] = obj;					\
 	return obj;							\
 }									\
-static inline void __close(int sd)					\
+__hidden void __close(int sd)						\
 {									\
 	if (sd < 0) {							\
 		free(__context_handler);				\
@@ -398,13 +403,22 @@ static inline void __close(int sd)					\
 	free(__context_handler[sd]);					\
 	__context_handler[sd] = NULL;					\
 }									\
-static inline type *__get_context(int sd)				\
+__hidden type *__get_context(int sd)					\
 {									\
 	if (sd < 0 || sd >= __n_streams)				\
 		return NULL;						\
 	return __context_handler[sd];					\
 }									\
 
+/**
+ * General purpose macro declaring the methods for adding plugin context.
+ * To be used in header files.
+ */
+#define KS_DECLARE_PLUGIN_CONTEXT_METHODS(type)		\
+type *__init(int sd);					\
+void __close(int sd);					\
+type *__get_context(int sd);				\
+
 #ifdef __cplusplus
 }
 #endif // __cplusplus
diff --git a/src/plugins/sched_events.c b/src/plugins/sched_events.c
index ac4a7bf..5798322 100644
--- a/src/plugins/sched_events.c
+++ b/src/plugins/sched_events.c
@@ -73,6 +73,9 @@ int plugin_sched_get_prev_state(ks_num_field_t field)
 	return (field & mask) >> PREV_STATE_SHIFT;
 }
 
+/** A general purpose macro is used to define plugin context. */
+KS_DEFINE_PLUGIN_CONTEXT(struct plugin_sched_context);
+
 static bool plugin_sched_init_context(struct kshark_data_stream *stream,
 				      struct plugin_sched_context *plugin_ctx)
 {
diff --git a/src/plugins/sched_events.h b/src/plugins/sched_events.h
index 78cfda0..2c540fd 100644
--- a/src/plugins/sched_events.h
+++ b/src/plugins/sched_events.h
@@ -53,8 +53,7 @@ struct plugin_sched_context {
 	struct kshark_data_container	*sw_data;
 };
 
-/** A general purpose macro is used to define plugin context. */
-KS_DEFINE_PLUGIN_CONTEXT(struct plugin_sched_context);
+KS_DECLARE_PLUGIN_CONTEXT_METHODS(struct plugin_sched_context)
 
 /** The type of the data field stored in the kshark_data_container object. */
 typedef int64_t ks_num_field_t;
-- 
2.27.0

[PATCH 4/7] kernel-shark: Add cleanup of all plugin contexts

[Yordan Karadzhov (VMware)]

The when the macro KS_DEFINE_PLUGIN_CONTEXT is used by the plugin,
the final free of the memory is done by calling __close() with a
negative stream id. Althow, this was provisioned in the definition
of the macro, it was never implemented in the GUI.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/libkshark-plugin.c     | 10 ++++++++++
 src/libkshark-plugin.h     |  5 ++++-
 src/libkshark.h            |  2 +-
 src/plugins/sched_events.c |  4 ++++
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/libkshark-plugin.c b/src/libkshark-plugin.c
index ebd2579..09886ce 100644
--- a/src/libkshark-plugin.c
+++ b/src/libkshark-plugin.c
@@ -117,6 +117,9 @@ int kshark_unregister_event_handler(struct kshark_data_stream *stream,
 {
 	struct kshark_event_proc_handler **last;
 
+	if (stream->stream_id < 0)
+		return 0;
+
 	for (last = &stream->event_handlers; *last; last = &(*last)->next) {
 		if ((*last)->id == event_id &&
 		    (*last)->event_func == evt_func) {
@@ -182,6 +185,9 @@ void kshark_unregister_draw_handler(struct kshark_data_stream *stream,
 {
 	struct kshark_draw_handler **last;
 
+	if (stream->stream_id < 0)
+		return;
+
 	for (last = &stream->draw_handlers; *last; last = &(*last)->next) {
 		if ((*last)->draw_func == draw_func) {
 			struct kshark_draw_handler *this_handler;
@@ -410,12 +416,16 @@ void kshark_unregister_plugin(struct kshark_context *kshark_ctx,
  */
 void kshark_free_plugin_list(struct kshark_plugin_list *plugins)
 {
+	struct kshark_data_stream stream;
 	struct kshark_plugin_list *last;
 
+	stream.stream_id = KS_PLUGIN_CONTEXT_FREE;
 	while (plugins) {
 		last = plugins;
 		plugins = plugins->next;
 
+		if (last->process_interface)
+			last->process_interface->close(&stream);
 		free_plugin(last);
 	}
 }
diff --git a/src/libkshark-plugin.h b/src/libkshark-plugin.h
index 752dbeb..85a0a8c 100644
--- a/src/libkshark-plugin.h
+++ b/src/libkshark-plugin.h
@@ -366,6 +366,9 @@ int kshark_handle_all_dpis(struct kshark_data_stream *stream,
 	__ok;								\
 })									\
 
+/** Identifier used to free the plugin context. */
+#define KS_PLUGIN_CONTEXT_FREE	-1
+
 /**
  * General purpose macro defining methods for adding plugin context.
  * Do not use this macro in header files.
@@ -395,7 +398,7 @@ __hidden type *__init(int sd)						\
 }									\
 __hidden void __close(int sd)						\
 {									\
-	if (sd < 0) {							\
+	if (sd == KS_PLUGIN_CONTEXT_FREE) {				\
 		free(__context_handler);				\
 		__n_streams = -1;					\
 		return;							\
diff --git a/src/libkshark.h b/src/libkshark.h
index aa4b3ca..ee3a1d3 100644
--- a/src/libkshark.h
+++ b/src/libkshark.h
@@ -281,7 +281,7 @@ struct kshark_generic_stream_interface {
 /** Structure representing a stream of trace data. */
 struct kshark_data_stream {
 	/** Data stream identifier. */
-	uint16_t		stream_id;
+	int16_t			stream_id;
 
 	/** The number of CPUs presented in this data stream. */
 	int			n_cpus;
diff --git a/src/plugins/sched_events.c b/src/plugins/sched_events.c
index 5798322..5c82803 100644
--- a/src/plugins/sched_events.c
+++ b/src/plugins/sched_events.c
@@ -201,6 +201,9 @@ int KSHARK_PLOT_PLUGIN_DEINITIALIZER(struct kshark_data_stream *stream)
 	struct plugin_sched_context *plugin_ctx;
 	int sd = stream->stream_id;
 
+	if (sd == KS_PLUGIN_CONTEXT_FREE)
+		goto close;
+
 	plugin_ctx = __get_context(sd);
 	if (!plugin_ctx)
 		return 0;
@@ -215,6 +218,7 @@ int KSHARK_PLOT_PLUGIN_DEINITIALIZER(struct kshark_data_stream *stream)
 
 	kshark_unregister_draw_handler(stream, plugin_draw);
 
+ close:
 	__close(sd);
 
 	return 1;
-- 
2.27.0

[PATCH 5/7] kernel-shark: Fix memory leak in "sched events" plugin.

[Yordan Karadzhov (VMware)]

The memory used by the Data container object must be freed. The fix
includes improvement of the KS_DEFINE_PLUGIN_CONTEXT macro.

Fixing: b39499d (kernel-shark: Speed-up the sched_events plugin)
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/libkshark-plugin.h     |  4 ++--
 src/plugins/sched_events.c | 14 +++++++++++++-
 tests/libkshark-tests.cpp  |  2 +-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/libkshark-plugin.h b/src/libkshark-plugin.h
index 85a0a8c..4c5fa5a 100644
--- a/src/libkshark-plugin.h
+++ b/src/libkshark-plugin.h
@@ -373,7 +373,7 @@ int kshark_handle_all_dpis(struct kshark_data_stream *stream,
  * General purpose macro defining methods for adding plugin context.
  * Do not use this macro in header files.
  */
-#define KS_DEFINE_PLUGIN_CONTEXT(type)					\
+#define KS_DEFINE_PLUGIN_CONTEXT(type, type_free)			\
 static type **__context_handler;					\
 static ssize_t __n_streams = -1;					\
 __hidden type *__init(int sd)						\
@@ -403,7 +403,7 @@ __hidden void __close(int sd)						\
 		__n_streams = -1;					\
 		return;							\
 	}								\
-	free(__context_handler[sd]);					\
+	type_free(__context_handler[sd]);				\
 	__context_handler[sd] = NULL;					\
 }									\
 __hidden type *__get_context(int sd)					\
diff --git a/src/plugins/sched_events.c b/src/plugins/sched_events.c
index 5c82803..a66904a 100644
--- a/src/plugins/sched_events.c
+++ b/src/plugins/sched_events.c
@@ -73,8 +73,20 @@ int plugin_sched_get_prev_state(ks_num_field_t field)
 	return (field & mask) >> PREV_STATE_SHIFT;
 }
 
+static void sched_free_context(struct plugin_sched_context *plugin_ctx)
+{
+	if (!plugin_ctx)
+		return;
+
+	if (plugin_ctx->ss_data)
+		kshark_free_data_container(plugin_ctx->ss_data);
+
+	if (plugin_ctx->sw_data)
+		kshark_free_data_container(plugin_ctx->sw_data);
+}
+
 /** A general purpose macro is used to define plugin context. */
-KS_DEFINE_PLUGIN_CONTEXT(struct plugin_sched_context);
+KS_DEFINE_PLUGIN_CONTEXT(struct plugin_sched_context, sched_free_context);
 
 static bool plugin_sched_init_context(struct kshark_data_stream *stream,
 				      struct plugin_sched_context *plugin_ctx)
diff --git a/tests/libkshark-tests.cpp b/tests/libkshark-tests.cpp
index 5f7c88a..3a5b0c3 100644
--- a/tests/libkshark-tests.cpp
+++ b/tests/libkshark-tests.cpp
@@ -148,7 +148,7 @@ struct test_context {
 	char b;
 };
 
-KS_DEFINE_PLUGIN_CONTEXT(struct test_context);
+KS_DEFINE_PLUGIN_CONTEXT(struct test_context, free);
 
 BOOST_AUTO_TEST_CASE(init_close_plugin)
 {
-- 
2.27.0

[PATCH 6/7] kernel-shark: Disable the pop-up offset dialog

[Yordan Karadzhov (VMware)]

The pop-up dialog that asks for the time offset, when a trace data
file is appended, is a legacy from the time when we have been doing
the calculation of the offset by hand. It still can be useful in some
specific cases, but the user can do the same from the "Tools" menu.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/KsMainWindow.cpp | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/KsMainWindow.cpp b/src/KsMainWindow.cpp
index e830c5e..d0a434a 100644
--- a/src/KsMainWindow.cpp
+++ b/src/KsMainWindow.cpp
@@ -1248,7 +1248,7 @@ void KsMainWindow::_load(const QString& fileName, bool append)
 	QString pbLabel("Loading    ");
 	bool loadDone = false;
 	struct stat st;
-	double shift;
+	double shift(.0);
 	int ret, sd;
 
 	ret = stat(fileName.toStdString().c_str(), &st);
@@ -1264,15 +1264,6 @@ void KsMainWindow::_load(const QString& fileName, bool append)
 
 	qInfo() << "Loading " << fileName;
 
-	if (append) {
-		bool ok;
-		shift = KsTimeOffsetDialog::getValueNanoSec(fileName, &ok);
-		if (ok)
-			shift *= 1000.;
-		else
-			shift = 0.;
-	}
-
 	if (fileName.size() < 40) {
 		pbLabel += fileName;
 	} else {
-- 
2.27.0

[PATCH 7/7] kernel-shark: Remove kvm_combo from the list of default plugins

[Yordan Karadzhov (VMware)]

The "kvm_combo" plugin is still under tests and is yet to be added to
KernelShark. For the moment we remove it from the list of default
plugins for "tep" data, so that no warning message is being printed.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/libkshark-tepdata.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index bc5babb..f7cd083 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -1210,7 +1210,6 @@ static void kshark_tep_init_methods(struct kshark_generic_stream_interface *inte
 const char *tep_plugin_names[] = {
 	"sched_events",
 	"missed_events",
-	"kvm_combo",
 };
 
 /**
-- 
2.27.0