[PATCH v2 0/2]kernel-shark:add new plugin for xenomai cobalt_switch_context events

[PATCH v2 0/2]kernel-shark:add new plugin for xenomai cobalt_switch_context events

[Hongzhan Chen]

1. To avoid code duplication, move some common APIs and definitions
   out to create new files and share with other plugins.
2. add new plugin for handling xenomai cobalt_switch_context events
   to visualize OOB state of RT tasks.

I tried to move common APIs and definitions to KsPlugins.cpp/hpp but
found these definitions finally depend on KsMainWindow object used
by _doubleClick of LatencyBox assigned by plugin_set_gui_ptr via
KSHARK_MENU_PLUGIN_INITIALIZER. 
I do not know how to remove this dependency so I create new files to
avoid code duplication. Please suggest if there is better way. 

Hongzhan Chen (2):
  kernel-shark: Move common APIs and definitions out to avoid
    duplication
  kernel-shark: Add plugin for handling Xenomai cobalt_context_switch

 src/libkshark-tepdata.c                    |   1 +
 src/plugins/CMakeLists.txt                 |   6 +-
 src/plugins/CobaltSwitchEvents.cpp         | 125 +++++++++++++++
 src/plugins/CommonSched.hpp                |  99 ++++++++++++
 src/plugins/SchedEvents.cpp                |  87 +----------
 src/plugins/common_sched.c                 |  37 +++++
 src/plugins/common_sched.h                 |  50 ++++++
 src/plugins/sched_events.c                 |  37 +----
 src/plugins/sched_events.h                 |  12 +-
 src/plugins/xenomai_cobalt_switch_events.c | 169 +++++++++++++++++++++
 src/plugins/xenomai_cobalt_switch_events.h |  54 +++++++
 11 files changed, 545 insertions(+), 132 deletions(-)
 create mode 100644 src/plugins/CobaltSwitchEvents.cpp
 create mode 100644 src/plugins/CommonSched.hpp
 create mode 100644 src/plugins/common_sched.c
 create mode 100644 src/plugins/common_sched.h
 create mode 100644 src/plugins/xenomai_cobalt_switch_events.c
 create mode 100644 src/plugins/xenomai_cobalt_switch_events.h

-- 
2.17.1

[PATCH v2 1/2] kernel-shark: Move common APIs and definitions out to avoid duplication

[Hongzhan Chen]

To avoid code duplication, move some common APIs and definitions
out to create new files and share with other plugins.

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
---
 src/plugins/CMakeLists.txt  |  2 +-
 src/plugins/CommonSched.hpp | 99 +++++++++++++++++++++++++++++++++++++
 src/plugins/SchedEvents.cpp | 87 +-------------------------------
 src/plugins/common_sched.c  | 37 ++++++++++++++
 src/plugins/common_sched.h  | 50 +++++++++++++++++++
 src/plugins/sched_events.c  | 37 +-------------
 src/plugins/sched_events.h  | 12 ++---
 7 files changed, 192 insertions(+), 132 deletions(-)
 create mode 100644 src/plugins/CommonSched.hpp
 create mode 100644 src/plugins/common_sched.c
 create mode 100644 src/plugins/common_sched.h

diff --git a/src/plugins/CMakeLists.txt b/src/plugins/CMakeLists.txt
index 3e170fa..15d71d3 100644
--- a/src/plugins/CMakeLists.txt
+++ b/src/plugins/CMakeLists.txt
@@ -41,7 +41,7 @@ set(PLUGIN_LIST "")
 if (Qt5Widgets_FOUND AND TT_FONT_FILE)
 
     BUILD_GUI_PLUGIN(NAME sched_events
-                     SOURCE sched_events.c SchedEvents.cpp)
+                     SOURCE common_sched.c sched_events.c SchedEvents.cpp)
     list(APPEND PLUGIN_LIST "sched_events")
 
     BUILD_GUI_PLUGIN(NAME event_field_plot
diff --git a/src/plugins/CommonSched.hpp b/src/plugins/CommonSched.hpp
new file mode 100644
index 0000000..e1b88e4
--- /dev/null
+++ b/src/plugins/CommonSched.hpp
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
+
+/*
+ * Copyright (C) 2021 Intel Inc, Hongzhan Chen <hongzhan.chen@intel.com>
+ */
+
+/**
+ *  @file    CommonSched.hpp
+ *  @brief   Tools for plot common sched.
+ */
+
+// C++
+#include <vector>
+
+// KernelShark
+#include "libkshark.h"
+#include "libkshark-plugin.h"
+#include "KsPlotTools.hpp"
+#include "KsPlugins.hpp"
+#include "KsMainWindow.hpp"
+
+using namespace KsPlot;
+
+static KsMainWindow *ks_ptr;
+
+/**
+ * @brief Provide the plugin with a pointer to the KsMainWindow object (the GUI
+ * itself) such that the plugin can manipulate the GUI.
+ */
+__hidden void *plugin_set_gui_ptr(void *gui_ptr)
+{
+	ks_ptr = static_cast<KsMainWindow *>(gui_ptr);
+	return nullptr;
+}
+
+/**
+ * This class represents the graphical element visualizing the latency between
+ * two events such as sched_waking and sched_switch events for common Linux
+ * kernel or cobalt_switch_contexts for Xenomai.
+ */
+class LatencyBox : public Rectangle
+{
+	/** On double click do. */
+	void _doubleClick() const override
+	{
+		ks_ptr->markEntry(_data[1]->entry, DualMarkerState::B);
+		ks_ptr->markEntry(_data[0]->entry, DualMarkerState::A);
+	}
+
+public:
+	/** The trace record data that corresponds to this LatencyBox. */
+	std::vector<kshark_data_field_int64 *>	_data;
+
+	/**
+	 * @brief Distance between the click and the shape. Used to decide if
+	 *	  the double click action must be executed.
+	 *
+	 * @param x: X coordinate of the click.
+	 * @param y: Y coordinate of the click.
+	 *
+	 * @returns If the click is inside the box, the distance is zero.
+	 *	    Otherwise infinity.
+	 */
+	double distance(int x, int y) const override
+	{
+		if (x < pointX(0) || x > pointX(2))
+			return std::numeric_limits<double>::max();
+
+		if (y < pointY(0) || y > pointY(1))
+			return std::numeric_limits<double>::max();
+
+		return 0;
+	}
+};
+
+static PlotObject *makeShape(std::vector<const Graph *> graph,
+			     std::vector<int> bins,
+			     std::vector<kshark_data_field_int64 *> data,
+			     Color col, float size)
+{
+	LatencyBox *rec = new LatencyBox;
+	rec->_data = data;
+
+	Point p0 = graph[0]->bin(bins[0])._base;
+	Point p1 = graph[0]->bin(bins[1])._base;
+	int height = graph[0]->height() * .3;
+
+	rec->setFill(false);
+	rec->setPoint(0, p0.x() - 1, p0.y() - height);
+	rec->setPoint(1, p0.x() - 1, p0.y() - 1);
+
+	rec->setPoint(3, p1.x() - 1, p1.y() - height);
+	rec->setPoint(2, p1.x() - 1, p1.y() - 1);
+
+	rec->_size = size;
+	rec->_color = col;
+
+	return rec;
+}
diff --git a/src/plugins/SchedEvents.cpp b/src/plugins/SchedEvents.cpp
index b73e45f..b0fb29c 100644
--- a/src/plugins/SchedEvents.cpp
+++ b/src/plugins/SchedEvents.cpp
@@ -11,94 +11,9 @@
  *	     preempted by another task.
  */
 
-// C++
-#include <vector>
-
 // KernelShark
-#include "libkshark.h"
-#include "libkshark-plugin.h"
 #include "plugins/sched_events.h"
-#include "KsPlotTools.hpp"
-#include "KsPlugins.hpp"
-#include "KsMainWindow.hpp"
-
-using namespace KsPlot;
-
-static KsMainWindow *ks_ptr;
-
-/**
- * @brief Provide the plugin with a pointer to the KsMainWindow object (the GUI
- * itself) such that the plugin can manipulate the GUI.
- */
-__hidden void *plugin_set_gui_ptr(void *gui_ptr)
-{
-	ks_ptr = static_cast<KsMainWindow *>(gui_ptr);
-	return nullptr;
-}
-
-/**
- * This class represents the graphical element visualizing the latency between
- *  sched_waking and sched_switch events.
- */
-class LatencyBox : public Rectangle
-{
-	/** On double click do. */
-	void _doubleClick() const override
-	{
-		ks_ptr->markEntry(_data[1]->entry, DualMarkerState::B);
-		ks_ptr->markEntry(_data[0]->entry, DualMarkerState::A);
-	}
-
-public:
-	/** The trace record data that corresponds to this LatencyBox. */
-	std::vector<kshark_data_field_int64 *>	_data;
-
-	/**
-	 * @brief Distance between the click and the shape. Used to decide if
-	 *	  the double click action must be executed.
-	 *
-	 * @param x: X coordinate of the click.
-	 * @param y: Y coordinate of the click.
-	 *
-	 * @returns If the click is inside the box, the distance is zero.
-	 *	    Otherwise infinity.
-	 */
-	double distance(int x, int y) const override
-	{
-		if (x < pointX(0) || x > pointX(2))
-			return std::numeric_limits<double>::max();
-
-		if (y < pointY(0) || y > pointY(1))
-			return std::numeric_limits<double>::max();
-
-		return 0;
-	}
-};
-
-static PlotObject *makeShape(std::vector<const Graph *> graph,
-			     std::vector<int> bins,
-			     std::vector<kshark_data_field_int64 *> data,
-			     Color col, float size)
-{
-	LatencyBox *rec = new LatencyBox;
-	rec->_data = data;
-
-	Point p0 = graph[0]->bin(bins[0])._base;
-	Point p1 = graph[0]->bin(bins[1])._base;
-	int height = graph[0]->height() * .3;
-
-	rec->setFill(false);
-	rec->setPoint(0, p0.x() - 1, p0.y() - height);
-	rec->setPoint(1, p0.x() - 1, p0.y() - 1);
-
-	rec->setPoint(3, p1.x() - 1, p1.y() - height);
-	rec->setPoint(2, p1.x() - 1, p1.y() - 1);
-
-	rec->_size = size;
-	rec->_color = col;
-
-	return rec;
-};
+#include "plugins/CommonSched.hpp"
 
 /*
  * Ideally, the sched_switch has to be the last trace event recorded before the
diff --git a/src/plugins/common_sched.c b/src/plugins/common_sched.c
new file mode 100644
index 0000000..446adc8
--- /dev/null
+++ b/src/plugins/common_sched.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: LGPL-2.1
+
+/*
+ * Copyright (C) 2021 Intel Inc, Hongzhan Chen <hongzhan.chen@intel.com>
+ */
+
+/**
+ *  @file    common_sched.c
+ *  @brief
+ */
+
+// KernelShark
+#include "plugins/common_sched.h"
+
+void plugin_sched_set_pid(ks_num_field_t *field,
+				 tep_num_field_t pid)
+{
+	*field &= ~PID_MASK;
+	*field = pid & PID_MASK;
+}
+
+/**
+ * @brief Retrieve the PID value from the data field stored in the
+ *	  kshark_data_container object.
+ *
+ * @param field: Input location for the data field.
+ */
+__hidden int plugin_sched_get_pid(ks_num_field_t field)
+{
+	return field & PID_MASK;
+}
+
+/** Initialize the control interface of the plugin. */
+void *KSHARK_MENU_PLUGIN_INITIALIZER(void *gui_ptr)
+{
+	return plugin_set_gui_ptr(gui_ptr);
+}
diff --git a/src/plugins/common_sched.h b/src/plugins/common_sched.h
new file mode 100644
index 0000000..c504f3e
--- /dev/null
+++ b/src/plugins/common_sched.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
+
+/*
+ * Copyright (C) 2021 Intel Inc, Hongzhan Chen<hongzhan.chen@intel.com>
+ */
+
+/**
+ *  @file    common_sched.h
+ *  @brief   Plugin for common sched.
+ */
+
+#ifndef _KS_PLUGIN_COMMON_SCHED_H
+#define _KS_PLUGIN_COMMON_SCHED_H
+
+// KernelShark
+#include "libkshark-plugin.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+typedef unsigned long long tep_num_field_t;
+
+/** The type of the data field stored in the kshark_data_container object. */
+typedef int64_t ks_num_field_t;
+
+#define PREV_STATE_SHIFT	((int) ((sizeof(ks_num_field_t) - 1) * 8))
+
+#define PREV_STATE_MASK		(((ks_num_field_t) 1 << 8) - 1)
+
+#define PID_MASK		(((ks_num_field_t) 1 << PREV_STATE_SHIFT) - 1)
+
+void plugin_sched_set_pid(ks_num_field_t *field,
+				 tep_num_field_t pid);
+
+/**
+ * @brief Retrieve the PID value from the data field stored in the
+ *	  kshark_data_container object.
+ *
+ * @param field: Input location for the data field.
+ */
+int plugin_sched_get_pid(ks_num_field_t field);
+
+void *plugin_set_gui_ptr(void *gui_ptr);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/src/plugins/sched_events.c b/src/plugins/sched_events.c
index 198ed49..6add092 100644
--- a/src/plugins/sched_events.c
+++ b/src/plugins/sched_events.c
@@ -17,41 +17,12 @@
 #include <trace-cmd.h>
 
 // KernelShark
+#include "plugins/common_sched.h"
 #include "plugins/sched_events.h"
 #include "libkshark-tepdata.h"
 
 /** Plugin context instance. */
 
-//! @cond Doxygen_Suppress
-
-typedef unsigned long long tep_num_field_t;
-
-#define PREV_STATE_SHIFT	((int) ((sizeof(ks_num_field_t) - 1) * 8))
-
-#define PREV_STATE_MASK		(((ks_num_field_t) 1 << 8) - 1)
-
-#define PID_MASK		(((ks_num_field_t) 1 << PREV_STATE_SHIFT) - 1)
-
-//! @endcond
-
-static void plugin_sched_set_pid(ks_num_field_t *field,
-				 tep_num_field_t pid)
-{
-	*field &= ~PID_MASK;
-	*field = pid & PID_MASK;
-}
-
-/**
- * @brief Retrieve the PID value from the data field stored in the
- *	  kshark_data_container object.
- *
- * @param field: Input location for the data field.
- */
-__hidden int plugin_sched_get_pid(ks_num_field_t field)
-{
-	return field & PID_MASK;
-}
-
 /* Use the most significant byte to store the value of "prev_state". */
 static void plugin_sched_set_prev_state(ks_num_field_t *field,
 					tep_num_field_t prev_state)
@@ -230,9 +201,3 @@ int KSHARK_PLOT_PLUGIN_DEINITIALIZER(struct kshark_data_stream *stream)
 
 	return ret;
 }
-
-/** Initialize the control interface of the plugin. */
-void *KSHARK_MENU_PLUGIN_INITIALIZER(void *gui_ptr)
-{
-	return plugin_set_gui_ptr(gui_ptr);
-}
diff --git a/src/plugins/sched_events.h b/src/plugins/sched_events.h
index 2c540fd..c2b0ff7 100644
--- a/src/plugins/sched_events.h
+++ b/src/plugins/sched_events.h
@@ -9,12 +9,12 @@
  *  @brief   Plugin for Sched events.
  */
 
-#ifndef _KS_PLUGIN_SHED_H
-#define _KS_PLUGIN_SHED_H
+#ifndef _KS_PLUGIN_SCHED_H
+#define _KS_PLUGIN_SCHED_H
 
 // KernelShark
 #include "libkshark.h"
-#include "libkshark-plugin.h"
+#include "plugins/common_sched.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -55,18 +55,12 @@ 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;
-
-int plugin_sched_get_pid(ks_num_field_t field);
 
 int plugin_sched_get_prev_state(ks_num_field_t field);
 
 void plugin_draw(struct kshark_cpp_argv *argv, int sd, int pid,
 		 int draw_action);
 
-void *plugin_set_gui_ptr(void *gui_ptr);
-
 #ifdef __cplusplus
 }
 #endif
-- 
2.17.1

Re: [PATCH v2 1/2] kernel-shark: Move common APIs and definitions out to avoid duplication

[Yordan Karadzhov]

On 22.12.21 г. 8:40 ч., Hongzhan Chen wrote:
> To avoid code duplication, move some common APIs and definitions
> out to create new files and share with other plugins.
> 
> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
> ---
>   src/plugins/CMakeLists.txt  |  2 +-
>   src/plugins/CommonSched.hpp | 99 +++++++++++++++++++++++++++++++++++++
>   src/plugins/SchedEvents.cpp | 87 +-------------------------------
>   src/plugins/common_sched.c  | 37 ++++++++++++++
>   src/plugins/common_sched.h  | 50 +++++++++++++++++++
>   src/plugins/sched_events.c  | 37 +-------------
>   src/plugins/sched_events.h  | 12 ++---
>   7 files changed, 192 insertions(+), 132 deletions(-)
>   create mode 100644 src/plugins/CommonSched.hpp
>   create mode 100644 src/plugins/common_sched.c
>   create mode 100644 src/plugins/common_sched.h
> 
> diff --git a/src/plugins/CMakeLists.txt b/src/plugins/CMakeLists.txt
> index 3e170fa..15d71d3 100644
> --- a/src/plugins/CMakeLists.txt
> +++ b/src/plugins/CMakeLists.txt
> @@ -41,7 +41,7 @@ set(PLUGIN_LIST "")
>   if (Qt5Widgets_FOUND AND TT_FONT_FILE)
>   
>       BUILD_GUI_PLUGIN(NAME sched_events
> -                     SOURCE sched_events.c SchedEvents.cpp)
> +                     SOURCE common_sched.c sched_events.c SchedEvents.cpp)
>       list(APPEND PLUGIN_LIST "sched_events")
>   
>       BUILD_GUI_PLUGIN(NAME event_field_plot
> diff --git a/src/plugins/CommonSched.hpp b/src/plugins/CommonSched.hpp
> new file mode 100644
> index 0000000..e1b88e4
> --- /dev/null
> +++ b/src/plugins/CommonSched.hpp
> @@ -0,0 +1,99 @@
> +/* SPDX-License-Identifier: LGPL-2.1 */
> +
> +/*
> + * Copyright (C) 2021 Intel Inc, Hongzhan Chen <hongzhan.chen@intel.com>
> + */
> +
> +/**
> + *  @file    CommonSched.hpp
> + *  @brief   Tools for plot common sched.
> + */
> +
> +// C++
> +#include <vector>
> +
> +// KernelShark
> +#include "libkshark.h"
> +#include "libkshark-plugin.h"
> +#include "KsPlotTools.hpp"
> +#include "KsPlugins.hpp"
> +#include "KsMainWindow.hpp"
> +
> +using namespace KsPlot;
> +
> +static KsMainWindow *ks_ptr;
> +
> +/**
> + * @brief Provide the plugin with a pointer to the KsMainWindow object (the GUI
> + * itself) such that the plugin can manipulate the GUI.
> + */
> +__hidden void *plugin_set_gui_ptr(void *gui_ptr)
> +{
> +	ks_ptr = static_cast<KsMainWindow *>(gui_ptr);
> +	return nullptr;
> +}
> +

I would prefer the global variable above and the function to set this global variable to stay within the plugin.
Also make sure that these global variable have different names in the two plugins.


> +/**
> + * This class represents the graphical element visualizing the latency between
> + * two events such as sched_waking and sched_switch events for common Linux
> + * kernel or cobalt_switch_contexts for Xenomai.
> + */
> +class LatencyBox : public Rectangle
> +{
> +	/** On double click do. */
> +	void _doubleClick() const override
> +	{
> +		ks_ptr->markEntry(_data[1]->entry, DualMarkerState::B);
> +		ks_ptr->markEntry(_data[0]->entry, DualMarkerState::A);
> +	}
> +
So, here comes the problem of depending on some KS GUI definitions that are not available in KsPlugins.cpp/hpp.
This is a C++ code so let's do it the C++(OOP) way. In KsPlugins you will provide a generic (dummy) version of the 
double-click function

void _doubleClick() const override {}

all the rest is the same.

> +public:
> +	/** The trace record data that corresponds to this LatencyBox. */
> +	std::vector<kshark_data_field_int64 *>	_data;
> +
> +	/**
> +	 * @brief Distance between the click and the shape. Used to decide if
> +	 *	  the double click action must be executed.
> +	 *
> +	 * @param x: X coordinate of the click.
> +	 * @param y: Y coordinate of the click.
> +	 *
> +	 * @returns If the click is inside the box, the distance is zero.
> +	 *	    Otherwise infinity.
> +	 */
> +	double distance(int x, int y) const override
> +	{
> +		if (x < pointX(0) || x > pointX(2))
> +			return std::numeric_limits<double>::max();
> +
> +		if (y < pointY(0) || y > pointY(1))
> +			return std::numeric_limits<double>::max();
> +
> +		return 0;
> +	}
> +};
> +
> +static PlotObject *makeShape(std::vector<const Graph *> graph,
> +			     std::vector<int> bins,
> +			     std::vector<kshark_data_field_int64 *> data,
> +			     Color col, float size)
> +{
> +	LatencyBox *rec = new LatencyBox;

This function can also be in KsPlugins.hpp, if you change the definition to something like this

template<class T> KsPlot::PlotObject *
makeLatencyBox(std::vector<const KsPlot::Graph *> graph,
	       std::vector<int> bins,
	       std::vector<kshark_data_field_int64 *> data,
	       KsPlot::Color col, float size)
{
	LatencyBox *rec = new T;

> +	rec->_data = data;
> +
> +	Point p0 = graph[0]->bin(bins[0])._base;
> +	Point p1 = graph[0]->bin(bins[1])._base;
> +	int height = graph[0]->height() * .3;
> +
> +	rec->setFill(false);
> +	rec->setPoint(0, p0.x() - 1, p0.y() - height);
> +	rec->setPoint(1, p0.x() - 1, p0.y() - 1);
> +
> +	rec->setPoint(3, p1.x() - 1, p1.y() - height);
> +	rec->setPoint(2, p1.x() - 1, p1.y() - 1);
> +
> +	rec->_size = size;
> +	rec->_color = col;
> +
> +	return rec;
> +}
> diff --git a/src/plugins/SchedEvents.cpp b/src/plugins/SchedEvents.cpp
> index b73e45f..b0fb29c 100644
> --- a/src/plugins/SchedEvents.cpp
> +++ b/src/plugins/SchedEvents.cpp
> @@ -11,94 +11,9 @@
>    *	     preempted by another task.
>    */
>   
> -// C++
> -#include <vector>
> -
>   // KernelShark
> -#include "libkshark.h"
> -#include "libkshark-plugin.h"
>   #include "plugins/sched_events.h"
> -#include "KsPlotTools.hpp"
> -#include "KsPlugins.hpp"
> -#include "KsMainWindow.hpp"
> -
> -using namespace KsPlot;
> -
> -static KsMainWindow *ks_ptr;
> -
> -/**
> - * @brief Provide the plugin with a pointer to the KsMainWindow object (the GUI
> - * itself) such that the plugin can manipulate the GUI.
> - */
> -__hidden void *plugin_set_gui_ptr(void *gui_ptr)
> -{
> -	ks_ptr = static_cast<KsMainWindow *>(gui_ptr);
> -	return nullptr;
> -}
> -
> -/**
> - * This class represents the graphical element visualizing the latency between
> - *  sched_waking and sched_switch events.
> - */
> -class LatencyBox : public Rectangle
> -{
> -	/** On double click do. */
> -	void _doubleClick() const override
> -	{
> -		ks_ptr->markEntry(_data[1]->entry, DualMarkerState::B);
> -		ks_ptr->markEntry(_data[0]->entry, DualMarkerState::A);
> -	}
> -
> -public:
> -	/** The trace record data that corresponds to this LatencyBox. */
> -	std::vector<kshark_data_field_int64 *>	_data;
> -
> -	/**
> -	 * @brief Distance between the click and the shape. Used to decide if
> -	 *	  the double click action must be executed.
> -	 *
> -	 * @param x: X coordinate of the click.
> -	 * @param y: Y coordinate of the click.
> -	 *
> -	 * @returns If the click is inside the box, the distance is zero.
> -	 *	    Otherwise infinity.
> -	 */
> -	double distance(int x, int y) const override
> -	{
> -		if (x < pointX(0) || x > pointX(2))
> -			return std::numeric_limits<double>::max();
> -
> -		if (y < pointY(0) || y > pointY(1))
> -			return std::numeric_limits<double>::max();
> -
> -		return 0;
> -	}
> -};
> -

Here you will have a child class that implements only the double-click function

class SchedLatencyBox : public LatencyBox
{
	/** On double click do. */
	void _doubleClick() const override
	{
		ks_ptr->markEntry(_data[1]->entry, DualMarkerState::B);
		ks_ptr->markEntry(_data[0]->entry, DualMarkerState::A);
	}
};

Again, make sure that the child classes have different names in the two plugins. This will prevent any funny things that 
can potentially happen during linking.

> -static PlotObject *makeShape(std::vector<const Graph *> graph,
> -			     std::vector<int> bins,
> -			     std::vector<kshark_data_field_int64 *> data,
> -			     Color col, float size)
> -{
> -	LatencyBox *rec = new LatencyBox;
> -	rec->_data = data;
> -
> -	Point p0 = graph[0]->bin(bins[0])._base;
> -	Point p1 = graph[0]->bin(bins[1])._base;
> -	int height = graph[0]->height() * .3;
> -
> -	rec->setFill(false);
> -	rec->setPoint(0, p0.x() - 1, p0.y() - height);
> -	rec->setPoint(1, p0.x() - 1, p0.y() - 1);
> -
> -	rec->setPoint(3, p1.x() - 1, p1.y() - height);
> -	rec->setPoint(2, p1.x() - 1, p1.y() - 1);
> -
> -	rec->_size = size;
> -	rec->_color = col;
> -
> -	return rec;
> -};

and you don't need a local definition of makeShape. You can use makeLatencyBox<SchedLatencyBox>

Note that I haven't tested this at all, so do not copy-paste my code blindly.


> +#include "plugins/CommonSched.hpp"
>   
>   /*
>    * Ideally, the sched_switch has to be the last trace event recorded before the
> diff --git a/src/plugins/common_sched.c b/src/plugins/common_sched.c
> new file mode 100644
> index 0000000..446adc8
> --- /dev/null
> +++ b/src/plugins/common_sched.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +
> +/*
> + * Copyright (C) 2021 Intel Inc, Hongzhan Chen <hongzhan.chen@intel.com>
> + */
> +
> +/**
> + *  @file    common_sched.c
> + *  @brief
> + */
> +
> +// KernelShark
> +#include "plugins/common_sched.h"
> +
> +void plugin_sched_set_pid(ks_num_field_t *field,
> +				 tep_num_field_t pid)
> +{
> +	*field &= ~PID_MASK;
> +	*field = pid & PID_MASK;
> +}
> +
> +/**
> + * @brief Retrieve the PID value from the data field stored in the
> + *	  kshark_data_container object.
> + *
> + * @param field: Input location for the data field.
> + */
> +__hidden int plugin_sched_get_pid(ks_num_field_t field)
> +{
> +	return field & PID_MASK;
> +}
> +

No need to have this file. All functions above can stay in the header as 'static inline'
the one below must stay within the plugin.

cheers,
Yordan

> +/** Initialize the control interface of the plugin. */
> +void *KSHARK_MENU_PLUGIN_INITIALIZER(void *gui_ptr)
> +{
> +	return plugin_set_gui_ptr(gui_ptr);
> +}
> diff --git a/src/plugins/common_sched.h b/src/plugins/common_sched.h
> new file mode 100644
> index 0000000..c504f3e
> --- /dev/null
> +++ b/src/plugins/common_sched.h
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: LGPL-2.1 */
> +
> +/*
> + * Copyright (C) 2021 Intel Inc, Hongzhan Chen<hongzhan.chen@intel.com>
> + */
> +
> +/**
> + *  @file    common_sched.h
> + *  @brief   Plugin for common sched.
> + */
> +
> +#ifndef _KS_PLUGIN_COMMON_SCHED_H
> +#define _KS_PLUGIN_COMMON_SCHED_H
> +
> +// KernelShark
> +#include "libkshark-plugin.h"
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +typedef unsigned long long tep_num_field_t;
> +
> +/** The type of the data field stored in the kshark_data_container object. */
> +typedef int64_t ks_num_field_t;
> +
> +#define PREV_STATE_SHIFT	((int) ((sizeof(ks_num_field_t) - 1) * 8))
> +
> +#define PREV_STATE_MASK		(((ks_num_field_t) 1 << 8) - 1)
> +
> +#define PID_MASK		(((ks_num_field_t) 1 << PREV_STATE_SHIFT) - 1)
> +
> +void plugin_sched_set_pid(ks_num_field_t *field,
> +				 tep_num_field_t pid);
> +
> +/**
> + * @brief Retrieve the PID value from the data field stored in the
> + *	  kshark_data_container object.
> + *
> + * @param field: Input location for the data field.
> + */
> +int plugin_sched_get_pid(ks_num_field_t field);
> +
> +void *plugin_set_gui_ptr(void *gui_ptr);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/src/plugins/sched_events.c b/src/plugins/sched_events.c
> index 198ed49..6add092 100644
> --- a/src/plugins/sched_events.c
> +++ b/src/plugins/sched_events.c
> @@ -17,41 +17,12 @@
>   #include <trace-cmd.h>
>   
>   // KernelShark
> +#include "plugins/common_sched.h"
>   #include "plugins/sched_events.h"
>   #include "libkshark-tepdata.h"
>   
>   /** Plugin context instance. */
>   
> -//! @cond Doxygen_Suppress
> -
> -typedef unsigned long long tep_num_field_t;
> -
> -#define PREV_STATE_SHIFT	((int) ((sizeof(ks_num_field_t) - 1) * 8))
> -
> -#define PREV_STATE_MASK		(((ks_num_field_t) 1 << 8) - 1)
> -
> -#define PID_MASK		(((ks_num_field_t) 1 << PREV_STATE_SHIFT) - 1)
> -
> -//! @endcond
> -
> -static void plugin_sched_set_pid(ks_num_field_t *field,
> -				 tep_num_field_t pid)
> -{
> -	*field &= ~PID_MASK;
> -	*field = pid & PID_MASK;
> -}
> -
> -/**
> - * @brief Retrieve the PID value from the data field stored in the
> - *	  kshark_data_container object.
> - *
> - * @param field: Input location for the data field.
> - */
> -__hidden int plugin_sched_get_pid(ks_num_field_t field)
> -{
> -	return field & PID_MASK;
> -}
> -
>   /* Use the most significant byte to store the value of "prev_state". */
>   static void plugin_sched_set_prev_state(ks_num_field_t *field,
>   					tep_num_field_t prev_state)
> @@ -230,9 +201,3 @@ int KSHARK_PLOT_PLUGIN_DEINITIALIZER(struct kshark_data_stream *stream)
>   
>   	return ret;
>   }
> -
> -/** Initialize the control interface of the plugin. */
> -void *KSHARK_MENU_PLUGIN_INITIALIZER(void *gui_ptr)
> -{
> -	return plugin_set_gui_ptr(gui_ptr);
> -}
> diff --git a/src/plugins/sched_events.h b/src/plugins/sched_events.h
> index 2c540fd..c2b0ff7 100644
> --- a/src/plugins/sched_events.h
> +++ b/src/plugins/sched_events.h
> @@ -9,12 +9,12 @@
>    *  @brief   Plugin for Sched events.
>    */
>   
> -#ifndef _KS_PLUGIN_SHED_H
> -#define _KS_PLUGIN_SHED_H
> +#ifndef _KS_PLUGIN_SCHED_H
> +#define _KS_PLUGIN_SCHED_H
>   
>   // KernelShark
>   #include "libkshark.h"
> -#include "libkshark-plugin.h"
> +#include "plugins/common_sched.h"
>   
>   #ifdef __cplusplus
>   extern "C" {
> @@ -55,18 +55,12 @@ 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;
> -
> -int plugin_sched_get_pid(ks_num_field_t field);
>   
>   int plugin_sched_get_prev_state(ks_num_field_t field);
>   
>   void plugin_draw(struct kshark_cpp_argv *argv, int sd, int pid,
>   		 int draw_action);
>   
> -void *plugin_set_gui_ptr(void *gui_ptr);
> -
>   #ifdef __cplusplus
>   }
>   #endif
> 

[PATCH v2 2/2] kernel-shark: Add plugin for handling Xenomai cobalt_context_switch

[Hongzhan Chen]

For Xenomai-cobalt enabled system, cobalt_switch_context means
that there is schedule and context switch in companion core(realtime
core), which we may need to do special treatment and take correct
action as main kernel sched_switch to visualize out-of-band state
of realtime tasks running in cobalt core. To achive our target,
we implement following:

  1. store corresponding cobalt_switch_context events into
     container data.
  2. modify pid stored in entry to be equal to next_pid to
     show correct color in cpu bar when cobalt_switch_context
     event happen.
  3. show blue hollow box to mark out-of-band state according to
     cobalt_switch_context events.
  4. clickable cobalt_switch_context plugin shapes.

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
---
 src/libkshark-tepdata.c                    |   1 +
 src/plugins/CMakeLists.txt                 |   4 +
 src/plugins/CobaltSwitchEvents.cpp         | 125 +++++++++++++++
 src/plugins/xenomai_cobalt_switch_events.c | 169 +++++++++++++++++++++
 src/plugins/xenomai_cobalt_switch_events.h |  54 +++++++
 5 files changed, 353 insertions(+)
 create mode 100644 src/plugins/CobaltSwitchEvents.cpp
 create mode 100644 src/plugins/xenomai_cobalt_switch_events.c
 create mode 100644 src/plugins/xenomai_cobalt_switch_events.h

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index 9740ed9..e933b39 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -1208,6 +1208,7 @@ static void kshark_tep_init_methods(struct kshark_generic_stream_interface *inte
 /** A list of built in default plugins for FTRACE (trace-cmd) data. */
 const char *tep_plugin_names[] = {
 	"sched_events",
+	"xenomai_cobalt_switch_events",
 	"missed_events",
 	"kvm_combo",
 };
diff --git a/src/plugins/CMakeLists.txt b/src/plugins/CMakeLists.txt
index 15d71d3..c3badd8 100644
--- a/src/plugins/CMakeLists.txt
+++ b/src/plugins/CMakeLists.txt
@@ -44,6 +44,10 @@ if (Qt5Widgets_FOUND AND TT_FONT_FILE)
                      SOURCE common_sched.c sched_events.c SchedEvents.cpp)
     list(APPEND PLUGIN_LIST "sched_events")
 
+    BUILD_GUI_PLUGIN(NAME xenomai_cobalt_switch_events
+                     SOURCE common_sched.c xenomai_cobalt_switch_events.c CobaltSwitchEvents.cpp)
+    list(APPEND PLUGIN_LIST "xenomai_cobalt_switch_events")
+
     BUILD_GUI_PLUGIN(NAME event_field_plot
                      MOC EventFieldDialog.hpp
                      SOURCE event_field_plot.c EventFieldDialog.cpp EventFieldPlot.cpp)
diff --git a/src/plugins/CobaltSwitchEvents.cpp b/src/plugins/CobaltSwitchEvents.cpp
new file mode 100644
index 0000000..6b01c48
--- /dev/null
+++ b/src/plugins/CobaltSwitchEvents.cpp
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: LGPL-2.1
+
+/*
+ * Copyright (C) 2021 Intel Inc, Hongzhan Chen <hongzhan.chen@intel.com>
+ */
+
+/**
+ *  @file    CobaltSwitchEvents.cpp
+ *  @brief   Defines a callback function for Xenomai Cobalt context switch
+ *           events used to plot in blue the out-of-band state of the task
+ */
+
+// KernelShark
+#include "plugins/xenomai_cobalt_switch_events.h"
+#include "plugins/CommonSched.hpp"
+
+/*
+ * Ideally, the cobalt_switch_context has to be the last trace event recorded before
+ * the task is preempted. Because of this, when the data is loaded (the first pass),
+ * the "pid" field of the cobalt_switch_context entries gets edited by this plugin
+ * to be equal to the "next pid" of the cobalt_switch_context event. However, in
+ * reality the cobalt_switch_context event may be followed by some trailing events
+ * from the same task (printk events for example). This has the effect of extending
+ * the graph of the task outside of the actual duration of the task. The "second
+ * pass" over the data is used to fix this problem. It takes advantage of the
+ * "next" field of the entry (this field is set during the first pass) to search
+ * for trailing events after the "cobalt_switch_context". In addition, when
+ * we find that it try to switch in-band because next-pid is zero, we prefer to
+ * skip this event because it try to leave oob not enterring.
+ */
+static void secondPass(plugin_cobalt_context *plugin_ctx)
+{
+	kshark_data_container *cSS;
+	kshark_entry *e;
+	int pid_rec, switch_inband;
+
+	cSS = plugin_ctx->cs_data;
+	for (ssize_t i = 0; i < cSS->size; ++i) {
+		switch_inband = plugin_cobalt_check_switch_inband(
+				cSS->data[i]->field);
+
+		/* we skip cobalt_switch_context that try to
+		 * switch into in-band state because we just handle
+		 * out-of-band
+		 */
+		if (switch_inband)
+			continue;
+		pid_rec = plugin_sched_get_pid(cSS->data[i]->field);
+		e = cSS->data[i]->entry;
+		if (!e->next || e->pid == 0 ||
+		    e->event_id == e->next->event_id ||
+		    pid_rec != e->next->pid)
+			continue;
+
+		e = e->next;
+		/* Find all trailing events. */
+		for (; e->next; e = e->next) {
+			if (e->pid == plugin_sched_get_pid(
+						cSS->data[i]->field)) {
+				/*
+				 * Change the "pid" to be equal to the "next
+				 * pid" of the cobalt_switch_context event
+				 * and leave a sign that you edited this
+				 * entry.
+				 */
+				e->pid = cSS->data[i]->entry->pid;
+				e->visible &= ~KS_PLUGIN_UNTOUCHED_MASK;
+
+				/*  This is the last trailing event, we finish
+				 *  this round.
+				 */
+				if (e->next->pid != plugin_sched_get_pid(
+						cSS->data[i]->field))
+					break;
+			}
+		}
+	}
+}
+
+/**
+ * @brief Plugin's draw function.
+ *
+ * @param argv_c: A C pointer to be converted to KsCppArgV (C++ struct).
+ * @param sd: Data stream identifier.
+ * @param pid: Process Id.
+ * @param draw_action: Draw action identifier.
+ */
+__hidden void plugin_cobalt_draw(kshark_cpp_argv *argv_c,
+			  int sd, int pid, int draw_action)
+{
+	plugin_cobalt_context *plugin_ctx;
+
+	if (!(draw_action & KSHARK_TASK_DRAW) || pid == 0)
+		return;
+
+	plugin_ctx = __get_context(sd);
+	if (!plugin_ctx)
+		return;
+
+	KsCppArgV *argvCpp = KS_ARGV_TO_CPP(argv_c);
+
+	if (!plugin_ctx->second_pass_done) {
+		/* The second pass is not done yet. */
+		secondPass(plugin_ctx);
+		plugin_ctx->second_pass_done = true;
+	}
+
+	IsApplicableFunc checkFieldCS = [=] (kshark_data_container *d,
+					     ssize_t i) {
+		return !(plugin_cobalt_check_switch_inband(d->data[i]->field)) &&
+			d->data[i]->entry->pid == pid;
+	};
+
+	IsApplicableFunc checkEntryPid = [=] (kshark_data_container *d,
+					      ssize_t i) {
+		return plugin_sched_get_pid(d->data[i]->field) == pid;
+	};
+
+	eventFieldIntervalPlot(argvCpp,
+			       plugin_ctx->cs_data, checkFieldCS,
+			       plugin_ctx->cs_data, checkEntryPid,
+			       makeShape,
+			       {0, 0, 255}, // Blue
+			       -1);         // Default size
+}
diff --git a/src/plugins/xenomai_cobalt_switch_events.c b/src/plugins/xenomai_cobalt_switch_events.c
new file mode 100644
index 0000000..67df82f
--- /dev/null
+++ b/src/plugins/xenomai_cobalt_switch_events.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: LGPL-2.1
+
+/*
+ * Copyright (C) 2021 Intel Inc, Hongzhan Chen <hongzhan.chen@intel.com>
+ */
+
+/**
+ *  @file    xenomai_cobalt_switch_events.c
+ *  @brief   handle xenomai cobalt switch context event
+ */
+
+// C
+#include <stdlib.h>
+#include <stdio.h>
+
+// trace-cmd
+#include <trace-cmd.h>
+
+// KernelShark
+#include "plugins/common_sched.h"
+#include "plugins/xenomai_cobalt_switch_events.h"
+#include "libkshark-tepdata.h"
+
+/** Plugin context instance. */
+
+//! @cond Doxygen_Suppress
+
+#define SWITCH_INBAND_SHIFT	PREV_STATE_SHIFT
+
+#define SWITCH_INBAND_MASK	PREV_STATE_MASK
+
+//! @endcond
+
+static void cobalt_free_context(struct plugin_cobalt_context *plugin_ctx)
+{
+	if (!plugin_ctx)
+		return;
+
+	kshark_free_data_container(plugin_ctx->cs_data);
+}
+
+/* Use the most significant byte to store state marking switch in-band. */
+static void plugin_cobalt_set_switch_inband_state(ks_num_field_t *field,
+					ks_num_field_t inband_state)
+{
+	unsigned long long mask = SWITCH_INBAND_MASK << SWITCH_INBAND_SHIFT;
+	*field &= ~mask;
+	*field |= (inband_state & SWITCH_INBAND_MASK) << SWITCH_INBAND_SHIFT;
+}
+
+/**
+ * @brief Retrieve the state of switch-in-band from the data field stored in
+ *        the kshark_data_container object.
+ *
+ * @param field: Input location for the data field.
+ */
+__hidden int plugin_cobalt_check_switch_inband(ks_num_field_t field)
+{
+	unsigned long long mask = SWITCH_INBAND_MASK << SWITCH_INBAND_SHIFT;
+
+	return (field & mask) >> SWITCH_INBAND_SHIFT;
+}
+
+/** A general purpose macro is used to define plugin context. */
+KS_DEFINE_PLUGIN_CONTEXT(struct plugin_cobalt_context, cobalt_free_context);
+
+static bool plugin_cobalt_init_context(struct kshark_data_stream *stream,
+				      struct plugin_cobalt_context *plugin_ctx)
+{
+	struct tep_event *event;
+
+	if (!kshark_is_tep(stream))
+		return false;
+
+	plugin_ctx->tep = kshark_get_tep(stream);
+
+	event = tep_find_event_by_name(plugin_ctx->tep,
+					"cobalt_core", "cobalt_switch_context");
+	if (!event)
+		return false;
+
+	plugin_ctx->cobalt_switch_event = event;
+	plugin_ctx->cobalt_switch_next_field =
+		tep_find_any_field(event, "next_pid");
+
+	plugin_ctx->second_pass_done = false;
+
+	plugin_ctx->cs_data = kshark_init_data_container();
+	if (!plugin_ctx->cs_data)
+		return false;
+
+	return true;
+}
+
+static void plugin_cobalt_switch_action(struct kshark_data_stream *stream,
+				      void *rec, struct kshark_entry *entry)
+{
+	struct tep_record *record = (struct tep_record *) rec;
+	struct plugin_cobalt_context *plugin_ctx;
+	unsigned long long next_pid;
+	ks_num_field_t ks_field;
+	int ret;
+
+	plugin_ctx = __get_context(stream->stream_id);
+	if (!plugin_ctx)
+		return;
+
+	ret = tep_read_number_field(plugin_ctx->cobalt_switch_next_field,
+				    record->data, &next_pid);
+
+	if (ret == 0 && next_pid >= 0) {
+		plugin_sched_set_pid(&ks_field, entry->pid);
+		if (next_pid == 0) {
+			plugin_cobalt_set_switch_inband_state(&ks_field,
+					SWITCH_INBAND_STATE);
+		}
+
+		kshark_data_container_append(plugin_ctx->cs_data,
+				entry, ks_field);
+
+		if (next_pid > 0)
+			entry->pid = next_pid;
+	}
+}
+
+/** Load this plugin. */
+int KSHARK_PLOT_PLUGIN_INITIALIZER(struct kshark_data_stream *stream)
+{
+	struct plugin_cobalt_context *plugin_ctx;
+
+	plugin_ctx = __init(stream->stream_id);
+	if (!plugin_ctx || !plugin_cobalt_init_context(stream, plugin_ctx)) {
+		__close(stream->stream_id);
+		return 0;
+	}
+
+	if (plugin_ctx->cobalt_switch_event) {
+		kshark_register_event_handler(stream,
+						plugin_ctx->cobalt_switch_event->id,
+						plugin_cobalt_switch_action);
+	}
+
+	kshark_register_draw_handler(stream, plugin_cobalt_draw);
+
+	return 1;
+}
+
+/** Unload this plugin. */
+int KSHARK_PLOT_PLUGIN_DEINITIALIZER(struct kshark_data_stream *stream)
+{
+	struct plugin_cobalt_context *plugin_ctx = __get_context(stream->stream_id);
+	int ret = 0;
+
+	if (plugin_ctx) {
+		if (plugin_ctx->cobalt_switch_event) {
+			kshark_unregister_event_handler(stream,
+							plugin_ctx->cobalt_switch_event->id,
+							plugin_cobalt_switch_action);
+		}
+
+		kshark_unregister_draw_handler(stream, plugin_cobalt_draw);
+
+		ret = 1;
+	}
+
+	__close(stream->stream_id);
+
+	return ret;
+}
diff --git a/src/plugins/xenomai_cobalt_switch_events.h b/src/plugins/xenomai_cobalt_switch_events.h
new file mode 100644
index 0000000..7528101
--- /dev/null
+++ b/src/plugins/xenomai_cobalt_switch_events.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
+
+/*
+ * Copyright (C) 2021 Intel Inc, Hongzhan Chen<hongzhan.chen@intel.com>
+ */
+
+/**
+ *  @file    xenomai_cobalt_switch_events.h
+ *  @brief   Plugin for xenomai cobalt switch context event
+ */
+
+#ifndef _KS_PLUGIN_SHED_H
+#define _KS_PLUGIN_SHED_H
+
+// KernelShark
+#include "libkshark.h"
+#include "plugins/common_sched.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define SWITCH_INBAND_STATE	1
+/** Structure representing a plugin-specific context. */
+struct plugin_cobalt_context {
+	/** Page event used to parse the page. */
+	struct tep_handle	*tep;
+
+	/** Pointer to the cobalt_switch_event object. */
+	struct tep_event	*cobalt_switch_event;
+
+	 /** Pointer to the cobalt_switch_next_field format descriptor. */
+	struct tep_format_field *cobalt_switch_next_field;
+
+	/** True if the second pass is already done. */
+	bool	second_pass_done;
+
+	/** Data container for cobalt_switch_context data. */
+	struct kshark_data_container	*cs_data;
+
+};
+
+KS_DECLARE_PLUGIN_CONTEXT_METHODS(struct plugin_cobalt_context)
+
+int plugin_cobalt_check_switch_inband(ks_num_field_t field);
+
+void plugin_cobalt_draw(struct kshark_cpp_argv *argv, int sd, int pid,
+		 int draw_action);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
-- 
2.17.1

Re: [PATCH v2 0/2]kernel-shark:add new plugin for xenomai cobalt_switch_context events

[Yordan Karadzhov]

On 22.12.21 г. 8:40 ч., Hongzhan Chen wrote:
> 1. To avoid code duplication, move some common APIs and definitions
>     out to create new files and share with other plugins.
> 2. add new plugin for handling xenomai cobalt_switch_context events
>     to visualize OOB state of RT tasks.
> 
> I tried to move common APIs and definitions to KsPlugins.cpp/hpp but
> found these definitions finally depend on KsMainWindow object used
> by _doubleClick of LatencyBox assigned by plugin_set_gui_ptr via
> KSHARK_MENU_PLUGIN_INITIALIZER.
> I do not know how to remove this dependency so I create new files to
> avoid code duplication. Please suggest if there is better way.

Hi Hongzhan,

Indeed, this turns to be a bit trickier than I expected, but it is possible.
I will comment in the patch.

thanks,
Yordan

> 
> Hongzhan Chen (2):
>    kernel-shark: Move common APIs and definitions out to avoid
>      duplication
>    kernel-shark: Add plugin for handling Xenomai cobalt_context_switch
> 
>   src/libkshark-tepdata.c                    |   1 +
>   src/plugins/CMakeLists.txt                 |   6 +-
>   src/plugins/CobaltSwitchEvents.cpp         | 125 +++++++++++++++
>   src/plugins/CommonSched.hpp                |  99 ++++++++++++
>   src/plugins/SchedEvents.cpp                |  87 +----------
>   src/plugins/common_sched.c                 |  37 +++++
>   src/plugins/common_sched.h                 |  50 ++++++
>   src/plugins/sched_events.c                 |  37 +----
>   src/plugins/sched_events.h                 |  12 +-
>   src/plugins/xenomai_cobalt_switch_events.c | 169 +++++++++++++++++++++
>   src/plugins/xenomai_cobalt_switch_events.h |  54 +++++++
>   11 files changed, 545 insertions(+), 132 deletions(-)
>   create mode 100644 src/plugins/CobaltSwitchEvents.cpp
>   create mode 100644 src/plugins/CommonSched.hpp
>   create mode 100644 src/plugins/common_sched.c
>   create mode 100644 src/plugins/common_sched.h
>   create mode 100644 src/plugins/xenomai_cobalt_switch_events.c
>   create mode 100644 src/plugins/xenomai_cobalt_switch_events.h
> 

Re: [PATCH v2 0/2]kernel-shark:add new plugin for xenomai cobalt_switch_context events

[Steven Rostedt]

This was sent during the Christmas vacation. I don't want it to be
forgotten about.

Yordan, what's your thoughts on this?

-- Steve


On Wed, 22 Dec 2021 01:40:12 -0500
Hongzhan Chen <hongzhan.chen@intel.com> wrote:

> 1. To avoid code duplication, move some common APIs and definitions
>    out to create new files and share with other plugins.
> 2. add new plugin for handling xenomai cobalt_switch_context events
>    to visualize OOB state of RT tasks.
> 
> I tried to move common APIs and definitions to KsPlugins.cpp/hpp but
> found these definitions finally depend on KsMainWindow object used
> by _doubleClick of LatencyBox assigned by plugin_set_gui_ptr via
> KSHARK_MENU_PLUGIN_INITIALIZER. 
> I do not know how to remove this dependency so I create new files to
> avoid code duplication. Please suggest if there is better way. 
> 
> Hongzhan Chen (2):
>   kernel-shark: Move common APIs and definitions out to avoid
>     duplication
>   kernel-shark: Add plugin for handling Xenomai cobalt_context_switch
> 
>  src/libkshark-tepdata.c                    |   1 +
>  src/plugins/CMakeLists.txt                 |   6 +-
>  src/plugins/CobaltSwitchEvents.cpp         | 125 +++++++++++++++
>  src/plugins/CommonSched.hpp                |  99 ++++++++++++
>  src/plugins/SchedEvents.cpp                |  87 +----------
>  src/plugins/common_sched.c                 |  37 +++++
>  src/plugins/common_sched.h                 |  50 ++++++
>  src/plugins/sched_events.c                 |  37 +----
>  src/plugins/sched_events.h                 |  12 +-
>  src/plugins/xenomai_cobalt_switch_events.c | 169 +++++++++++++++++++++
>  src/plugins/xenomai_cobalt_switch_events.h |  54 +++++++
>  11 files changed, 545 insertions(+), 132 deletions(-)
>  create mode 100644 src/plugins/CobaltSwitchEvents.cpp
>  create mode 100644 src/plugins/CommonSched.hpp
>  create mode 100644 src/plugins/common_sched.c
>  create mode 100644 src/plugins/common_sched.h
>  create mode 100644 src/plugins/xenomai_cobalt_switch_events.c
>  create mode 100644 src/plugins/xenomai_cobalt_switch_events.h
> 

Re: [PATCH v2 0/2]kernel-shark:add new plugin for xenomai cobalt_switch_context events

[Yordan Karadzhov]

Hi Steven,
Happy New Year!

I already replayed, asking for some minor corrections.
https://lore.kernel.org/all/bb1741d5-600a-82a9-d8bc-91222400e129@gmail.com/
I think the plugin is almost ready.

BTW have you finished the renovation of your office? If yes, I will send you a review for your trace-cruncher patch.

cheers,
Yordan


On 6.01.22 г. 19:18 ч., Steven Rostedt wrote:
> 
> This was sent during the Christmas vacation. I don't want it to be
> forgotten about.
> 
> Yordan, what's your thoughts on this?
> 
> -- Steve
> 
> 
> On Wed, 22 Dec 2021 01:40:12 -0500
> Hongzhan Chen <hongzhan.chen@intel.com> wrote:
> 
>> 1. To avoid code duplication, move some common APIs and definitions
>>     out to create new files and share with other plugins.
>> 2. add new plugin for handling xenomai cobalt_switch_context events
>>     to visualize OOB state of RT tasks.
>>
>> I tried to move common APIs and definitions to KsPlugins.cpp/hpp but
>> found these definitions finally depend on KsMainWindow object used
>> by _doubleClick of LatencyBox assigned by plugin_set_gui_ptr via
>> KSHARK_MENU_PLUGIN_INITIALIZER.
>> I do not know how to remove this dependency so I create new files to
>> avoid code duplication. Please suggest if there is better way.
>>
>> Hongzhan Chen (2):
>>    kernel-shark: Move common APIs and definitions out to avoid
>>      duplication
>>    kernel-shark: Add plugin for handling Xenomai cobalt_context_switch
>>
>>   src/libkshark-tepdata.c                    |   1 +
>>   src/plugins/CMakeLists.txt                 |   6 +-
>>   src/plugins/CobaltSwitchEvents.cpp         | 125 +++++++++++++++
>>   src/plugins/CommonSched.hpp                |  99 ++++++++++++
>>   src/plugins/SchedEvents.cpp                |  87 +----------
>>   src/plugins/common_sched.c                 |  37 +++++
>>   src/plugins/common_sched.h                 |  50 ++++++
>>   src/plugins/sched_events.c                 |  37 +----
>>   src/plugins/sched_events.h                 |  12 +-
>>   src/plugins/xenomai_cobalt_switch_events.c | 169 +++++++++++++++++++++
>>   src/plugins/xenomai_cobalt_switch_events.h |  54 +++++++
>>   11 files changed, 545 insertions(+), 132 deletions(-)
>>   create mode 100644 src/plugins/CobaltSwitchEvents.cpp
>>   create mode 100644 src/plugins/CommonSched.hpp
>>   create mode 100644 src/plugins/common_sched.c
>>   create mode 100644 src/plugins/common_sched.h
>>   create mode 100644 src/plugins/xenomai_cobalt_switch_events.c
>>   create mode 100644 src/plugins/xenomai_cobalt_switch_events.h
>>
> 

Re: [PATCH v2 0/2]kernel-shark:add new plugin for xenomai cobalt_switch_context events

[Steven Rostedt]

On Thu, 6 Jan 2022 19:32:56 +0200
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> Hi Steven,
> Happy New Year!

Happy New Year to you too Yordan!

> 
> I already replayed, asking for some minor corrections.
> https://lore.kernel.org/all/bb1741d5-600a-82a9-d8bc-91222400e129@gmail.com/
> I think the plugin is almost ready.

Strange, I didn't receive it. I wonder if I got booted from the email lists
because I had my mail server offline for too long (/me goes check).

> 
> BTW have you finished the renovation of your office? If yes, I will send you a review for your trace-cruncher patch.
>

Yes, I just finished. Now I'm trying to catch up on everything and also
work on some things that I may not be able to work on when I start working
again.

Cheers,

-- Steve