All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] kernelshark: Fix the GUI plugin loading
@ 2017-11-01 11:04 Yordan Karadzhov (VMware)
  2017-11-01 11:04 ` [PATCH 2/2] kernelshark: Adding a GUI plugin for xenomai events Yordan Karadzhov (VMware)
  2017-11-01 14:22 ` [PATCH 1/2] kernelshark: Fix the GUI plugin loading Steven Rostedt
  0 siblings, 2 replies; 4+ messages in thread
From: Yordan Karadzhov (VMware) @ 2017-11-01 11:04 UTC (permalink / raw)
  To: rostedt; +Cc: jan.kiszka, linux-kernel, Yordan Karadzhov (VMware)

In add_plugin(const char *): wrong file name used when loading a plugin.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel-shark.c b/kernel-shark.c
index c952302..89723c3 100644
--- a/kernel-shark.c
+++ b/kernel-shark.c
@@ -1816,7 +1816,7 @@ static void add_plugin(const char *file)
 	struct stat st;
 	int ret;
 
-	ret = stat(default_input_file, &st);
+	ret = stat(file, &st);
 	if (ret < 0) {
 		warning("plugin %s not found", file);
 		return;
-- 
2.15.0.rc0

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH 2/2] kernelshark: Adding a GUI plugin for xenomai events
  2017-11-01 11:04 [PATCH 1/2] kernelshark: Fix the GUI plugin loading Yordan Karadzhov (VMware)
@ 2017-11-01 11:04 ` Yordan Karadzhov (VMware)
  2017-11-01 15:19   ` Steven Rostedt
  2017-11-01 14:22 ` [PATCH 1/2] kernelshark: Fix the GUI plugin loading Steven Rostedt
  1 sibling, 1 reply; 4+ messages in thread
From: Yordan Karadzhov (VMware) @ 2017-11-01 11:04 UTC (permalink / raw)
  To: rostedt; +Cc: jan.kiszka, linux-kernel, Yordan Karadzhov (VMware)

Makefile modified in order to support building of gui plugins.

Function handlers for processing of plugin-specific events (xenomai
events "cobalt_switch_context" and "cobalt_thread_resume" in this case)
are addet to trace_view_store and graph_info.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 Makefile             |  35 +++++--
 kernel-shark.c       |   8 +-
 kshark-plugin.c      |  65 +++++++++++++
 kshark-plugin.h      |  49 +++++++++-
 plugin_xenomai_gui.c | 257 +++++++++++++++++++++++++++++++++++++++++++++++++++
 trace-graph.c        |  48 ++++++++++
 trace-graph.h        |   7 ++
 trace-plot-task.c    |  17 +++-
 trace-view-store.c   |  24 +++++
 trace-view-store.h   |   5 +
 10 files changed, 500 insertions(+), 15 deletions(-)
 create mode 100644 kshark-plugin.c
 create mode 100644 plugin_xenomai_gui.c

diff --git a/Makefile b/Makefile
index 5c35143..96f30e2 100644
--- a/Makefile
+++ b/Makefile
@@ -293,6 +293,8 @@ else
   print_shared_lib_compile =	echo '  $(GUI)COMPILE SHARED LIB '$(GOBJ);
   print_plugin_obj_compile =	echo '  $(GUI)COMPILE PLUGIN OBJ '$(GOBJ);
   print_plugin_build =		echo '  $(GUI)BUILD PLUGIN       '$(GOBJ);
+  print_gui_plugin_obj_compile =	echo '  $(GUI)COMPILE GUI_PLUGIN OBJ '$(GOBJ);
+  print_gui_plugin_build =		echo '  $(GUI)BUILD GUI_PLUGIN       '$(GOBJ);
   print_static_lib_build =	echo '  $(GUI)BUILD STATIC LIB   '$(GOBJ);
   print_install =		echo '  $(GUI)INSTALL     '$(GSPACE)$1'	to	$(DESTDIR_SQ)$2';
 endif
@@ -317,6 +319,14 @@ do_plugin_build =				\
 	($(print_plugin_build)			\
 	$(CC) $(CFLAGS) $(LDFLAGS) -shared -nostartfiles -o $@ $<)
 
+do_compile_gui_plugin_obj =				\
+	($(print_gui_plugin_obj_compile)		\
+	$(CC) -c $(CPPFLAGS) $(CFLAGS) -fPIC -o $@ $<)
+
+do_gui_plugin_build =				\
+	($(print_gui_plugin_build)			\
+	$(CC) $(CFLAGS) $(LDFLAGS) -shared -nostartfiles -o $@ $<)
+
 do_build_static_lib =				\
 	($(print_static_lib_build)		\
 	$(RM) $@;  $(AR) rcs $@ $^)
@@ -338,17 +348,17 @@ $(obj)/%.o: $(src)/%.c
 	$(Q)$(call check_gui)
 
 TRACE_GUI_OBJS = trace-filter.o trace-compat.o trace-filter-hash.o trace-dialog.o \
-		trace-xml.o
+		trace-xml.o kshark-plugin.o
 TRACE_CMD_OBJS = trace-cmd.o trace-record.o trace-read.o trace-split.o trace-listen.o \
 	 trace-stack.o trace-hist.o trace-mem.o trace-snapshot.o trace-stat.o \
 	 trace-hash.o trace-profile.o trace-stream.o trace-record.o trace-restore.o \
 	 trace-check-events.o trace-show.o trace-list.o
-TRACE_VIEW_OBJS = trace-view.o trace-view-store.o
+TRACE_VIEW_OBJS = trace-view.o trace-view-store.o kshark-plugin.o
 TRACE_GRAPH_OBJS = trace-graph.o trace-plot.o trace-plot-cpu.o trace-plot-task.o
 TRACE_VIEW_MAIN_OBJS = trace-view-main.o $(TRACE_VIEW_OBJS) $(TRACE_GUI_OBJS)
 TRACE_GRAPH_MAIN_OBJS = trace-graph-main.o $(TRACE_GRAPH_OBJS) $(TRACE_GUI_OBJS)
 KERNEL_SHARK_OBJS = $(TRACE_VIEW_OBJS) $(TRACE_GRAPH_OBJS) $(TRACE_GUI_OBJS) \
-	trace-capture.o kernel-shark.o
+	trace-capture.o kernel-shark.o kshark-plugin.o
 
 PEVENT_LIB_OBJS = event-parse.o trace-seq.o parse-filter.o parse-utils.o str_error_r.o
 TCMD_LIB_OBJS = $(PEVENT_LIB_OBJS) trace-util.o trace-input.o trace-ftrace.o \
@@ -373,13 +383,19 @@ PLUGIN_OBJS += plugin_tlb.o
 
 PLUGINS := $(PLUGIN_OBJS:.o=.so)
 
+
+GUI_PLUGIN_OBJS =
+GUI_PLUGIN_OBJS += plugin_xenomai_gui.o
+
+GUI_PLUGINS := $(GUI_PLUGIN_OBJS:.o=.so)
+
 ALL_OBJS = $(TRACE_CMD_OBJS) $(KERNEL_SHARK_OBJS) $(TRACE_VIEW_MAIN_OBJS) \
-	$(TRACE_GRAPH_MAIN_OBJS) $(TCMD_LIB_OBJS) $(PLUGIN_OBJS)
+	$(TRACE_GRAPH_MAIN_OBJS) $(TCMD_LIB_OBJS) $(PLUGIN_OBJS) $(GUI_PLUGIN_OBJS)
 
 CMD_TARGETS = trace_plugin_dir trace_python_dir tc_version.h libparsevent.a $(LIB_FILE) \
 	trace-cmd  $(PLUGINS) $(BUILD_PYTHON)
 
-GUI_TARGETS = ks_version.h trace-graph trace-view kernelshark
+GUI_TARGETS = ks_version.h trace-graph trace-view kernelshark $(GUI_PLUGINS)
 
 TARGETS = $(CMD_TARGETS) $(GUI_TARGETS)
 
@@ -401,7 +417,7 @@ gui: $(CMD_TARGETS)
 
 all_gui: $(GUI_TARGETS) show_gui_done
 
-GUI_OBJS = $(KERNEL_SHARK_OBJS) $(TRACE_VIEW_MAIN_OBJS) $(TRACE_GRAPH_MAIN_OBJS)
+GUI_OBJS = $(KERNEL_SHARK_OBJS) $(TRACE_VIEW_MAIN_OBJS) $(TRACE_GRAPH_MAIN_OBJS) $(GUI_PLUGIN_OBJS)
 
 gui_objs := $(sort $(GUI_OBJS))
 
@@ -447,6 +463,13 @@ $(PLUGIN_OBJS): %.o : $(src)/%.c
 $(PLUGINS): %.so: %.o
 	$(Q)$(do_plugin_build)
 
+
+$(GUI_PLUGIN_OBJS): %.o : $(src)/%.c
+	$(Q)$(do_compile_gui_plugin_obj)
+
+$(GUI_PLUGINS): %.so: %.o
+	$(Q)$(do_gui_plugin_build)
+
 define make_version.h
 	(echo '/* This file is automatically generated. Do not modify. */';		\
 	echo \#define VERSION_CODE $(shell						\
diff --git a/kernel-shark.c b/kernel-shark.c
index 89723c3..9454ea0 100644
--- a/kernel-shark.c
+++ b/kernel-shark.c
@@ -1830,7 +1830,7 @@ static void add_plugin(const char *file)
 	plugin_next = &(*plugin_next)->next;
 }
 
-static void handle_plugins(struct shark_info *info)
+static void handle_plugins(struct shark_info *info, struct trace_view_store *store)
 {
 	kshark_plugin_load_func func;
 	struct plugin_list *plugin;
@@ -1852,7 +1852,7 @@ static void handle_plugins(struct shark_info *info)
 				KSHARK_PLUGIN_LOADER_NAME, plugin->file, dlerror());
 			continue;
 		}
-		func(info);
+		func(info, store);
 	}
 }
 
@@ -2495,7 +2495,9 @@ void kernel_shark(int argc, char **argv)
 
 	gtk_widget_set_size_request(window, TRACE_WIDTH, TRACE_HEIGHT);
 
-	handle_plugins(info);
+	GtkTreeModel *model = gtk_tree_view_get_model(GTK_TREE_VIEW(info->treeview));
+	handle_plugins(info, TRACE_VIEW_STORE(model));
+
 
 	gdk_threads_enter();
 
diff --git a/kshark-plugin.c b/kshark-plugin.c
new file mode 100644
index 0000000..07d1a87
--- /dev/null
+++ b/kshark-plugin.c
@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License (not later!)
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not,  see <http://www.gnu.org/licenses>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#include <stdlib.h>
+
+#include "kshark-plugin.h"
+
+struct gui_event_handler *make_gui_event_handler(int event_id, int type,
+						 kshark_plugin_event_handler_func evt_func,
+						 kshark_plugin_context_update_func ctx_func)
+{
+	struct gui_event_handler *handler = malloc(sizeof(struct gui_event_handler));
+	handler->next = NULL;
+	handler->id = event_id;
+	handler->type = type;
+	handler->event_func = evt_func;
+	handler->context_func = ctx_func;
+
+	return handler;
+}
+
+struct gui_event_handler *find_gui_event_handler(struct gui_event_handler *handlers,
+						 int event_id)
+{
+	while (handlers) {
+		if (handlers->id == event_id)
+			return handlers;
+
+		handlers = handlers->next;
+	}
+
+	return NULL;
+}
+
+void remove_gui_event_handler(struct gui_event_handler **handlers, int event_id)
+{
+	struct gui_event_handler *list = *handlers;
+	if (list == NULL)
+		return;
+
+	if (list->id == event_id) {
+		*handlers = list->next;
+		return;
+	}
+
+	remove_gui_event_handler(&list->next, event_id);
+}
+
diff --git a/kshark-plugin.h b/kshark-plugin.h
index 95ba797..65e1e93 100644
--- a/kshark-plugin.h
+++ b/kshark-plugin.h
@@ -17,6 +17,9 @@
  *
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
+
+#include "event-parse.h"
+
 #ifndef _KSHARK_PLUGIN_H
 #define _KSHARK_PLUGIN_H
 
@@ -28,8 +31,50 @@
 #define KSHARK_PLUGIN_LOADER_NAME MAKE_STR(KSHARK_PLUGIN_LOADER)
 #define KSHARK_PLUGIN_UNLOADER_NAME MAKE_STR(KSHARK_PLUGIN_UNLOADER)
 
-typedef int (*kshark_plugin_load_func)(void *info);
-typedef int (*kshark_plugin_unload_func)(void *info);
+typedef int (*kshark_plugin_load_func)(void *info, void *store);
+typedef int (*kshark_plugin_unload_func)(void *info, void *store);
+
+typedef int (*kshark_plugin_event_handler_func)(struct pevent_record *record,
+						int task_id,
+						void *val);
+
+typedef int (*kshark_plugin_context_update_func)(struct pevent *pevent, int val);
+
+enum gui_plugin_actions {
+	KSHARK_PLUGIN_GET_PID,
+	KSHARK_PLUGIN_GET_PREV_STATE,
+	KSHARK_PLUGIN_GET_COMMAND
+};
+
+enum gui_plugin_ctx_updates {
+	KSHARK_PLUGIN_UPDATE_SWITCH_EVENT = 0x1,
+	KSHARK_PLUGIN_UPDATE_WAKEUP_EVENT = 0x2,
+	KSHARK_PLUGIN_UPDATE_WAKEUP_PID = 0x4,
+	KSHARK_PLUGIN_UPDATE_SWITCH_PID = 0x8,
+	KSHARK_PLUGIN_UPDATE_PREV_STATE = 0x10,
+	KSHARK_PLUGIN_UPDATE_NEXT_NAME = 0x20
+};
+
+enum gui_event_types {
+	KSHARK_PLUGIN_SWITCH_EVENT,
+	KSHARK_PLUGIN_WAKEUP_EVENT
+};
+
+struct gui_event_handler {
+	struct gui_event_handler		*next;
+	int					id;
+	int 					type;
+	kshark_plugin_event_handler_func	event_func;
+	kshark_plugin_context_update_func       context_func;
+};
+
+struct gui_event_handler *make_gui_event_handler(int event_id, int type,
+						 kshark_plugin_event_handler_func evt_func,
+						 kshark_plugin_context_update_func ctx_func);
+
+struct gui_event_handler *find_gui_event_handler(struct gui_event_handler *handlers,
+						 int event_id);
 
+void remove_gui_event_handler(struct gui_event_handler **handlers, int event_id);
 
 #endif
diff --git a/plugin_xenomai_gui.c b/plugin_xenomai_gui.c
new file mode 100644
index 0000000..f22a7a7
--- /dev/null
+++ b/plugin_xenomai_gui.c
@@ -0,0 +1,257 @@
+/*
+ *  Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License (not later!)
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not,  see <http://www.gnu.org/licenses>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "event-parse.h"
+#include "kernel-shark.h"
+#include "kshark-plugin.h"
+
+struct xenomai_context {
+	struct shark_info	*info;
+	struct trace_view_store *store;
+	struct pevent 		*pevent;
+
+	struct event_format	*cobalt_switch_event;
+	struct format_field     *cobalt_switch_next_pid_field;
+	struct format_field     *cobalt_switch_prev_state_field;
+	struct format_field     *cobalt_switch_next_name_field;
+
+	struct event_format	*cobalt_wakeup_event;
+	struct format_field	*cobalt_wakeup_pid_field;
+};
+
+struct xenomai_context *xenomai_context_handler = NULL;
+
+static gboolean xenomai_update_context(struct pevent *pevent, int task_id)
+{
+	struct xenomai_context *ctx = xenomai_context_handler;
+	if (!ctx)
+		return FALSE;
+
+	struct event_format *event;
+
+	if (task_id & KSHARK_PLUGIN_UPDATE_SWITCH_EVENT) {
+		event = pevent_find_event_by_name(xenomai_context_handler->pevent,
+						  "cobalt_core",
+						  "cobalt_switch_context");
+		if (!event)
+			return FALSE;
+
+		ctx->cobalt_switch_event = event;
+	}
+
+	if (task_id & KSHARK_PLUGIN_UPDATE_WAKEUP_EVENT) {
+		event = pevent_find_event_by_name(xenomai_context_handler->pevent,
+						  "cobalt_core",
+						  "cobalt_thread_resume");
+		if (!event)
+			return FALSE;
+
+		ctx->cobalt_wakeup_event = event;
+	}
+
+	if (task_id & KSHARK_PLUGIN_UPDATE_SWITCH_PID) {
+		ctx->cobalt_switch_next_pid_field =
+		pevent_find_field(ctx->cobalt_switch_event, "next_pid");
+	}
+
+	if (task_id & KSHARK_PLUGIN_UPDATE_PREV_STATE) {
+		ctx->cobalt_switch_prev_state_field =
+		pevent_find_field(ctx->cobalt_switch_event, "prev_state");
+	}
+
+	if (task_id & KSHARK_PLUGIN_UPDATE_NEXT_NAME) {
+		ctx->cobalt_switch_next_name_field =
+		pevent_find_field(ctx->cobalt_switch_event, "next_name");
+	}
+
+	if (task_id & KSHARK_PLUGIN_UPDATE_WAKEUP_PID) {
+		ctx->cobalt_wakeup_pid_field =
+		pevent_find_field(ctx->cobalt_wakeup_event, "pid");
+	}
+
+	return TRUE;
+}
+
+static void xenomai_context_new(struct shark_info *info, struct trace_view_store *store)
+{
+	if (!xenomai_context_handler) {
+		xenomai_context_handler =
+		(struct xenomai_context*) malloc(sizeof(struct xenomai_context));
+	}
+
+	xenomai_context_handler->info = info;
+	xenomai_context_handler->store = store;
+	xenomai_context_handler->pevent = tracecmd_get_pevent(info->handle);
+
+	int status = xenomai_update_context(xenomai_context_handler->pevent,
+					    KSHARK_PLUGIN_UPDATE_SWITCH_EVENT |
+					    KSHARK_PLUGIN_UPDATE_WAKEUP_EVENT);
+	if (status == FALSE) {
+		free(xenomai_context_handler);
+		xenomai_context_handler = NULL;
+	}
+}
+
+static int cobalt_get_next_pid(struct xenomai_context *ctx,
+				struct pevent_record *record,
+				int *pid)
+{
+	long long unsigned int val;
+	int status = pevent_read_number_field(ctx->cobalt_switch_next_pid_field,
+					      record->data, &val);
+	if (pid)
+		*pid = val;
+
+	return status;
+}
+
+
+static int cobalt_get_prev_state(struct xenomai_context *ctx,
+				 struct pevent_record *record,
+				 int *state)
+{
+	long long unsigned int val;
+	pevent_read_number_field(ctx->cobalt_switch_prev_state_field,
+				 record->data, &val);
+
+	if (state)
+		*state = val;
+
+	return (val & 0x00000008) ? 1 : 0;
+}
+
+static void cobalt_get_command(struct xenomai_context *ctx,
+				struct pevent_record *record,
+				const char **comm)
+{
+	int offset =
+	data2host4(ctx->pevent, record->data + ctx->cobalt_switch_next_name_field->offset);
+
+	offset &= 0xffff;
+	*comm = record->data + offset;
+}
+
+static gboolean xenomai_switch_handler(struct pevent_record *record,
+					int task_id,
+					void *output)
+{
+	struct xenomai_context *ctx = xenomai_context_handler;
+	switch (task_id) {
+		case KSHARK_PLUGIN_GET_PID:
+			cobalt_get_next_pid(ctx, record, output);
+			return TRUE;
+
+		case KSHARK_PLUGIN_GET_PREV_STATE:
+			return cobalt_get_prev_state(ctx, record, output);
+
+		case KSHARK_PLUGIN_GET_COMMAND:
+			cobalt_get_command(ctx, record, (const char**) output);
+			return TRUE;
+
+		default:
+			return FALSE;
+	}
+}
+
+static int cobalt_get_wakeup_pid(struct xenomai_context *ctx,
+				 struct pevent_record *record,
+				 int *pid)
+{
+	long long unsigned int val;
+	int status =
+	pevent_read_number_field(ctx->cobalt_wakeup_pid_field, record->data, &val);
+
+	if (pid)
+		*pid = val;
+
+	return status;
+}
+
+static gboolean xenomai_wakeup_handler(struct pevent_record *record,
+					int task_id,
+					void *output)
+{
+	struct xenomai_context *ctx = xenomai_context_handler;
+	switch (task_id) {
+		case KSHARK_PLUGIN_GET_PID:
+			cobalt_get_wakeup_pid(ctx, record, output);
+			return TRUE;
+
+		default:
+			return FALSE;
+	}
+}
+
+
+void KSHARK_PLUGIN_LOADER(void *info, void *store)
+{
+	struct shark_info *ks_info = info;
+	struct trace_view_store *ks_store = store;
+
+	xenomai_context_new(ks_info, ks_store);
+	if (!xenomai_context_handler)
+		return;
+
+	struct xenomai_context *ctx = xenomai_context_handler;
+
+	struct gui_event_handler *switch_handler =
+	make_gui_event_handler(ctx->cobalt_switch_event->id,
+			       KSHARK_PLUGIN_SWITCH_EVENT,
+			       xenomai_switch_handler,
+			       xenomai_update_context);
+
+	struct gui_event_handler *wakeup_handler =
+	make_gui_event_handler(ctx->cobalt_wakeup_event->id,
+			       KSHARK_PLUGIN_WAKEUP_EVENT,
+			       xenomai_wakeup_handler,
+			       xenomai_update_context);
+
+	trace_view_store_register_gui_handler(ks_store, switch_handler);
+	trace_view_store_register_gui_handler(ks_store, wakeup_handler);
+
+	trace_graph_register_gui_handler(ks_info->ginfo, switch_handler);
+	trace_graph_register_gui_handler(ks_info->ginfo, wakeup_handler);
+}
+
+void KSHARK_PLUGIN_UNLOADER(void *info, void *store)
+{
+	struct shark_info *ks_info = info;
+	struct trace_view_store *ks_store = store;
+	struct xenomai_context *ctx = xenomai_context_handler;
+
+	remove_gui_event_handler(&ks_store->event_handlers,
+				 ctx->cobalt_switch_event->id);
+
+	remove_gui_event_handler(&ks_store->event_handlers,
+				 ctx->cobalt_wakeup_event->id);
+
+	remove_gui_event_handler(&ks_info->ginfo->event_handlers,
+				 ctx->cobalt_switch_event->id);
+
+	remove_gui_event_handler(&ks_info->ginfo->event_handlers,
+				 ctx->cobalt_wakeup_event->id);
+
+	free(ctx);
+	xenomai_context_handler = NULL;
+}
+
diff --git a/trace-graph.c b/trace-graph.c
index 1db342f..19f02c0 100644
--- a/trace-graph.c
+++ b/trace-graph.c
@@ -129,6 +129,12 @@ static void free_task_hash(struct graph_info *ginfo)
 	}
 }
 
+void trace_graph_register_gui_handler(struct graph_info *info,
+				      struct gui_event_handler *handler) {
+	handler->next = info->event_handlers;
+	info->event_handlers = handler;
+}
+
 /**
  * trace_graph_task_list - return an allocated list of all found tasks
  * @ginfo: The graph info structure
@@ -1053,6 +1059,12 @@ int trace_graph_check_sched_wakeup(struct graph_info *ginfo,
 
 	id = pevent_data_type(ginfo->pevent, record);
 
+	struct gui_event_handler *handler = find_gui_event_handler(ginfo->event_handlers, id);
+	if (handler) {
+		if (handler->type == KSHARK_PLUGIN_WAKEUP_EVENT)
+			handler->context_func(ginfo->pevent, KSHARK_PLUGIN_UPDATE_WAKEUP_PID);
+	}
+
 	if (id == ginfo->event_wakeup_id) {
 		/* We only want those that actually woke up the task */
 		if (ginfo->wakeup_success_field) {
@@ -1079,6 +1091,16 @@ int trace_graph_check_sched_wakeup(struct graph_info *ginfo,
 		return 1;
 	}
 
+	if (handler) {
+		if (handler->type == KSHARK_PLUGIN_WAKEUP_EVENT) {
+			handler->event_func(record, KSHARK_PLUGIN_GET_PID, &val);
+			if (pid)
+				*pid = val;
+
+			return 1;
+		}
+	}
+
 	return 0;
 }
 
@@ -1118,7 +1140,20 @@ int trace_graph_check_sched_switch(struct graph_info *ginfo,
 		}
 	}
 
+
 	id = pevent_data_type(ginfo->pevent, record);
+	struct gui_event_handler *handler =
+	find_gui_event_handler(ginfo->event_handlers, id);
+
+	if (handler) {
+		if (handler->type == KSHARK_PLUGIN_SWITCH_EVENT) {
+			handler->context_func(ginfo->pevent,
+					      KSHARK_PLUGIN_UPDATE_SWITCH_PID |
+					      KSHARK_PLUGIN_UPDATE_PREV_STATE |
+					      KSHARK_PLUGIN_UPDATE_NEXT_NAME);
+		}
+	}
+
 	if (id == ginfo->event_sched_switch_id) {
 		pevent_read_number_field(ginfo->event_pid_field, record->data, &val);
 		if (comm)
@@ -1139,6 +1174,17 @@ int trace_graph_check_sched_switch(struct graph_info *ginfo,
 		goto out;
 	}
 
+	if (handler) {
+		if (handler->type == KSHARK_PLUGIN_SWITCH_EVENT) {
+			handler->event_func(record, KSHARK_PLUGIN_GET_PID, &val);
+			if (pid)
+				*pid = val;
+			if(comm)
+				handler->event_func(record, KSHARK_PLUGIN_GET_COMMAND, &comm);
+			goto out;
+		}
+	}
+
 	ret = 0;
  out:
 	if (ret && comm && ginfo->read_comms) {
@@ -2781,6 +2827,8 @@ trace_graph_create_with_callbacks(struct tracecmd_input *handle,
 
 	ginfo->callbacks = cbs;
 
+	ginfo->event_handlers = NULL;
+
 	ginfo->task_filter = filter_task_hash_alloc();
 	ginfo->hide_tasks = filter_task_hash_alloc();
 
diff --git a/trace-graph.h b/trace-graph.h
index 7e66838..21e8feb 100644
--- a/trace-graph.h
+++ b/trace-graph.h
@@ -24,6 +24,7 @@
 #include "trace-cmd.h"
 #include "trace-filter-hash.h"
 #include "trace-xml.h"
+#include "kshark-plugin.h"
 
 struct graph_info;
 
@@ -258,6 +259,8 @@ struct graph_info {
 	gint			plot_data_y;
 	gint			plot_data_w;
 	gint			plot_data_h;
+
+	struct gui_event_handler *event_handlers;
 };
 
 
@@ -266,6 +269,10 @@ trace_graph_create(struct tracecmd_input *handle);
 struct graph_info *
 trace_graph_create_with_callbacks(struct tracecmd_input *handle,
 				  struct graph_callbacks *cbs);
+
+void trace_graph_register_gui_handler(struct graph_info *info,
+				      struct gui_event_handler *handler);
+
 void trace_graph_select_by_time(struct graph_info *ginfo, guint64 time);
 
 void trace_graph_event_filter_callback(gboolean accept,
diff --git a/trace-plot-task.c b/trace-plot-task.c
index 3b7e81f..56c3e96 100644
--- a/trace-plot-task.c
+++ b/trace-plot-task.c
@@ -68,11 +68,20 @@ static gboolean is_running(struct graph_info *ginfo, struct pevent_record *recor
 	int id;
 
 	id = pevent_data_type(ginfo->pevent, record);
-	if (id != ginfo->event_sched_switch_id)
-		return FALSE;
 
-	pevent_read_number_field(ginfo->event_prev_state, record->data, &val);
-	return val & ((1 << 11) - 1)? FALSE : TRUE;
+	if (id == ginfo->event_sched_switch_id) {
+		pevent_read_number_field(ginfo->event_prev_state, record->data, &val);
+		return val & ((1 << 11) - 1)? FALSE : TRUE;
+	}
+
+	struct gui_event_handler *handler =
+	find_gui_event_handler(ginfo->event_handlers, id);
+	if (handler) {
+		if (handler->type == KSHARK_PLUGIN_SWITCH_EVENT)
+			return handler->event_func(record, KSHARK_PLUGIN_GET_PREV_STATE, NULL);
+	}
+
+	return FALSE;
 }
 
 static gboolean record_matches_pid(struct graph_info *ginfo,
diff --git a/trace-view-store.c b/trace-view-store.c
index f5d0a04..d7293c3 100644
--- a/trace-view-store.c
+++ b/trace-view-store.c
@@ -70,6 +70,12 @@ static gboolean		trace_view_store_iter_parent	(GtkTreeModel	*tree_model,
 static GObjectClass *parent_class = NULL;	/* GObject stuff - nothing to worry about */
 
 
+void trace_view_store_register_gui_handler(TraceViewStore *store, struct gui_event_handler *handler)
+{
+	handler->next = store->event_handlers;
+	store->event_handlers = handler;
+}
+
 /*****************************************************************************
  *
  *	trace_view_store_get_type: here we register our new type and its interfaces
@@ -866,6 +872,7 @@ trace_view_store_new (struct tracecmd_input *handle)
 
 	g_assert( newstore != NULL );
 
+	newstore->event_handlers = NULL;
 	newstore->handle = handle;
 	newstore->cpus = tracecmd_cpus(handle);
 	tracecmd_ref(handle);
@@ -1224,6 +1231,14 @@ static gboolean show_task(TraceViewStore *store, struct pevent *pevent,
 			return TRUE;
 	}
 
+	struct gui_event_handler *handler =
+	find_gui_event_handler(store->event_handlers, event_id);
+	if (handler) {
+		handler->event_func(record, KSHARK_PLUGIN_GET_PID, &pid);
+		if (view_task(store, pid))
+			return TRUE;
+	}
+
 	return FALSE;
 }
 
@@ -1261,6 +1276,15 @@ static void update_filter_tasks(TraceViewStore *store)
 						      "pid");
 	}
 
+	struct gui_event_handler *handler = store->event_handlers;
+	while (handler) {
+		handler->context_func(	pevent,
+					KSHARK_PLUGIN_UPDATE_SWITCH_PID |
+					KSHARK_PLUGIN_UPDATE_WAKEUP_PID);
+
+		handler = handler->next;
+	}
+
 	for (cpu = 0; cpu < store->cpus; cpu++) {
 		record = tracecmd_read_cpu_first(handle, cpu);
 
diff --git a/trace-view-store.h b/trace-view-store.h
index 03141b1..333116c 100644
--- a/trace-view-store.h
+++ b/trace-view-store.h
@@ -23,6 +23,7 @@
 #include <gtk/gtk.h>
 #include "trace-cmd.h"
 #include "trace-filter-hash.h"
+#include "kshark-plugin.h"
 
 /* Some boilerplate GObject defines. 'klass' is used
  *   instead of 'class', because 'class' is a C++ keyword */
@@ -125,8 +126,12 @@ struct trace_view_store
 	guint64			*cpu_mask;  /* cpus that are enabled */
 
 	gint		stamp;	/* Random integer to check whether an iter belongs to our model */
+
+	struct gui_event_handler *event_handlers;
 };
 
+void trace_view_store_register_gui_handler(TraceViewStore *store, struct gui_event_handler *handler);
+
 gboolean trace_view_store_cpu_isset(TraceViewStore *store, gint cpu);
 
 void trace_view_store_set_all_cpus(TraceViewStore *store);
-- 
2.15.0.rc0

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] kernelshark: Fix the GUI plugin loading
  2017-11-01 11:04 [PATCH 1/2] kernelshark: Fix the GUI plugin loading Yordan Karadzhov (VMware)
  2017-11-01 11:04 ` [PATCH 2/2] kernelshark: Adding a GUI plugin for xenomai events Yordan Karadzhov (VMware)
@ 2017-11-01 14:22 ` Steven Rostedt
  1 sibling, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2017-11-01 14:22 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: jan.kiszka, linux-kernel

On Wed,  1 Nov 2017 13:04:22 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> In add_plugin(const char *): wrong file name used when loading a plugin.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>

Applied. Thanks Yordan!

-- Steve

> ---
>  kernel-shark.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel-shark.c b/kernel-shark.c
> index c952302..89723c3 100644
> --- a/kernel-shark.c
> +++ b/kernel-shark.c
> @@ -1816,7 +1816,7 @@ static void add_plugin(const char *file)
>  	struct stat st;
>  	int ret;
>  
> -	ret = stat(default_input_file, &st);
> +	ret = stat(file, &st);
>  	if (ret < 0) {
>  		warning("plugin %s not found", file);
>  		return;

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] kernelshark: Adding a GUI plugin for xenomai events
  2017-11-01 11:04 ` [PATCH 2/2] kernelshark: Adding a GUI plugin for xenomai events Yordan Karadzhov (VMware)
@ 2017-11-01 15:19   ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2017-11-01 15:19 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: jan.kiszka, linux-kernel

On Wed,  1 Nov 2017 13:04:23 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> Makefile modified in order to support building of gui plugins.
> 
> Function handlers for processing of plugin-specific events (xenomai
> events "cobalt_switch_context" and "cobalt_thread_resume" in this case)
> are addet to trace_view_store and graph_info.

Very nice.

> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  Makefile             |  35 +++++--
>  kernel-shark.c       |   8 +-
>  kshark-plugin.c      |  65 +++++++++++++
>  kshark-plugin.h      |  49 +++++++++-
>  plugin_xenomai_gui.c | 257 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  trace-graph.c        |  48 ++++++++++
>  trace-graph.h        |   7 ++
>  trace-plot-task.c    |  17 +++-
>  trace-view-store.c   |  24 +++++
>  trace-view-store.h   |   5 +
>  10 files changed, 500 insertions(+), 15 deletions(-)
>  create mode 100644 kshark-plugin.c
>  create mode 100644 plugin_xenomai_gui.c
> 
> diff --git a/Makefile b/Makefile
> index 5c35143..96f30e2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -293,6 +293,8 @@ else
>    print_shared_lib_compile =	echo '  $(GUI)COMPILE SHARED LIB '$(GOBJ);
>    print_plugin_obj_compile =	echo '  $(GUI)COMPILE PLUGIN OBJ '$(GOBJ);
>    print_plugin_build =		echo '  $(GUI)BUILD PLUGIN       '$(GOBJ);
> +  print_gui_plugin_obj_compile =	echo '  $(GUI)COMPILE GUI_PLUGIN OBJ '$(GOBJ);
> +  print_gui_plugin_build =		echo '  $(GUI)BUILD GUI_PLUGIN       '$(GOBJ);
>    print_static_lib_build =	echo '  $(GUI)BUILD STATIC LIB   '$(GOBJ);
>    print_install =		echo '  $(GUI)INSTALL     '$(GSPACE)$1'	to	$(DESTDIR_SQ)$2';
>  endif
> @@ -317,6 +319,14 @@ do_plugin_build =				\
>  	($(print_plugin_build)			\
>  	$(CC) $(CFLAGS) $(LDFLAGS) -shared -nostartfiles -o $@ $<)
>  
> +do_compile_gui_plugin_obj =				\
> +	($(print_gui_plugin_obj_compile)		\
> +	$(CC) -c $(CPPFLAGS) $(CFLAGS) -fPIC -o $@ $<)
> +
> +do_gui_plugin_build =				\
> +	($(print_gui_plugin_build)			\
> +	$(CC) $(CFLAGS) $(LDFLAGS) -shared -nostartfiles -o $@ $<)
> +
>  do_build_static_lib =				\
>  	($(print_static_lib_build)		\
>  	$(RM) $@;  $(AR) rcs $@ $^)
> @@ -338,17 +348,17 @@ $(obj)/%.o: $(src)/%.c
>  	$(Q)$(call check_gui)
>  
>  TRACE_GUI_OBJS = trace-filter.o trace-compat.o trace-filter-hash.o trace-dialog.o \
> -		trace-xml.o
> +		trace-xml.o kshark-plugin.o
>  TRACE_CMD_OBJS = trace-cmd.o trace-record.o trace-read.o trace-split.o trace-listen.o \
>  	 trace-stack.o trace-hist.o trace-mem.o trace-snapshot.o trace-stat.o \
>  	 trace-hash.o trace-profile.o trace-stream.o trace-record.o trace-restore.o \
>  	 trace-check-events.o trace-show.o trace-list.o
> -TRACE_VIEW_OBJS = trace-view.o trace-view-store.o
> +TRACE_VIEW_OBJS = trace-view.o trace-view-store.o kshark-plugin.o
>  TRACE_GRAPH_OBJS = trace-graph.o trace-plot.o trace-plot-cpu.o trace-plot-task.o
>  TRACE_VIEW_MAIN_OBJS = trace-view-main.o $(TRACE_VIEW_OBJS) $(TRACE_GUI_OBJS)
>  TRACE_GRAPH_MAIN_OBJS = trace-graph-main.o $(TRACE_GRAPH_OBJS) $(TRACE_GUI_OBJS)
>  KERNEL_SHARK_OBJS = $(TRACE_VIEW_OBJS) $(TRACE_GRAPH_OBJS) $(TRACE_GUI_OBJS) \
> -	trace-capture.o kernel-shark.o
> +	trace-capture.o kernel-shark.o kshark-plugin.o
>  
>  PEVENT_LIB_OBJS = event-parse.o trace-seq.o parse-filter.o parse-utils.o str_error_r.o
>  TCMD_LIB_OBJS = $(PEVENT_LIB_OBJS) trace-util.o trace-input.o trace-ftrace.o \
> @@ -373,13 +383,19 @@ PLUGIN_OBJS += plugin_tlb.o
>  
>  PLUGINS := $(PLUGIN_OBJS:.o=.so)
>  
> +
> +GUI_PLUGIN_OBJS =
> +GUI_PLUGIN_OBJS += plugin_xenomai_gui.o
> +
> +GUI_PLUGINS := $(GUI_PLUGIN_OBJS:.o=.so)
> +
>  ALL_OBJS = $(TRACE_CMD_OBJS) $(KERNEL_SHARK_OBJS) $(TRACE_VIEW_MAIN_OBJS) \
> -	$(TRACE_GRAPH_MAIN_OBJS) $(TCMD_LIB_OBJS) $(PLUGIN_OBJS)
> +	$(TRACE_GRAPH_MAIN_OBJS) $(TCMD_LIB_OBJS) $(PLUGIN_OBJS) $(GUI_PLUGIN_OBJS)
>  
>  CMD_TARGETS = trace_plugin_dir trace_python_dir tc_version.h libparsevent.a $(LIB_FILE) \
>  	trace-cmd  $(PLUGINS) $(BUILD_PYTHON)
>  
> -GUI_TARGETS = ks_version.h trace-graph trace-view kernelshark
> +GUI_TARGETS = ks_version.h trace-graph trace-view kernelshark $(GUI_PLUGINS)
>  
>  TARGETS = $(CMD_TARGETS) $(GUI_TARGETS)
>  
> @@ -401,7 +417,7 @@ gui: $(CMD_TARGETS)
>  
>  all_gui: $(GUI_TARGETS) show_gui_done
>  
> -GUI_OBJS = $(KERNEL_SHARK_OBJS) $(TRACE_VIEW_MAIN_OBJS) $(TRACE_GRAPH_MAIN_OBJS)
> +GUI_OBJS = $(KERNEL_SHARK_OBJS) $(TRACE_VIEW_MAIN_OBJS) $(TRACE_GRAPH_MAIN_OBJS) $(GUI_PLUGIN_OBJS)
>  
>  gui_objs := $(sort $(GUI_OBJS))
>  
> @@ -447,6 +463,13 @@ $(PLUGIN_OBJS): %.o : $(src)/%.c
>  $(PLUGINS): %.so: %.o
>  	$(Q)$(do_plugin_build)
>  
> +
> +$(GUI_PLUGIN_OBJS): %.o : $(src)/%.c
> +	$(Q)$(do_compile_gui_plugin_obj)
> +
> +$(GUI_PLUGINS): %.so: %.o
> +	$(Q)$(do_gui_plugin_build)
> +
>  define make_version.h
>  	(echo '/* This file is automatically generated. Do not modify. */';		\
>  	echo \#define VERSION_CODE $(shell						\

The above is also very nicely done. The one thing that is missing is
the install phase when "make install_gui" is performed. The gui plugins
should also be installed. But I can add a patch to do that on top of
this one. Maybe I'll send it to you for v2. (some minor nits so far).

> diff --git a/kernel-shark.c b/kernel-shark.c
> index 89723c3..9454ea0 100644
> --- a/kernel-shark.c
> +++ b/kernel-shark.c
> @@ -1830,7 +1830,7 @@ static void add_plugin(const char *file)
>  	plugin_next = &(*plugin_next)->next;
>  }
>  
> -static void handle_plugins(struct shark_info *info)
> +static void handle_plugins(struct shark_info *info, struct trace_view_store *store)
>  {
>  	kshark_plugin_load_func func;
>  	struct plugin_list *plugin;
> @@ -1852,7 +1852,7 @@ static void handle_plugins(struct shark_info *info)
>  				KSHARK_PLUGIN_LOADER_NAME, plugin->file, dlerror());
>  			continue;
>  		}
> -		func(info);
> +		func(info, store);
>  	}
>  }
>  
> @@ -2495,7 +2495,9 @@ void kernel_shark(int argc, char **argv)
>  
>  	gtk_widget_set_size_request(window, TRACE_WIDTH, TRACE_HEIGHT);
>  
> -	handle_plugins(info);
> +	GtkTreeModel *model = gtk_tree_view_get_model(GTK_TREE_VIEW(info->treeview));
> +	handle_plugins(info, TRACE_VIEW_STORE(model));
> +
>  
>  	gdk_threads_enter();
>  
> diff --git a/kshark-plugin.c b/kshark-plugin.c
> new file mode 100644
> index 0000000..07d1a87
> --- /dev/null
> +++ b/kshark-plugin.c
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License (not later!)

I wonder if we could make this part of a library, and then license this
under LGPL.

> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not,  see <http://www.gnu.org/licenses>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +#include <stdlib.h>
> +
> +#include "kshark-plugin.h"
> +
> +struct gui_event_handler *make_gui_event_handler(int event_id, int type,

If this does become a library (maybe not, not sure yet), we probably
need to prefix all the functions with "kshark_".

> +						 kshark_plugin_event_handler_func evt_func,
> +						 kshark_plugin_context_update_func ctx_func)
> +{
> +	struct gui_event_handler *handler = malloc(sizeof(struct gui_event_handler));

Add a empty line between the declarations and the code.

Also, you need to check if handler failed to allocate.

	if (!handler)
		return NULL;

Hopefully all callers of this check the return value too.

> +	handler->next = NULL;
> +	handler->id = event_id;
> +	handler->type = type;
> +	handler->event_func = evt_func;
> +	handler->context_func = ctx_func;
> +
> +	return handler;
> +}
> +
> +struct gui_event_handler *find_gui_event_handler(struct gui_event_handler *handlers,
> +						 int event_id)
> +{
> +	while (handlers) {
> +		if (handlers->id == event_id)
> +			return handlers;
> +
> +		handlers = handlers->next;
> +	}
> +
> +	return NULL;

This is fine for now. But if there is a bunch of handlers created, we
are going to have to optimize it more than searching a list.

> +}
> +
> +void remove_gui_event_handler(struct gui_event_handler **handlers, int event_id)
> +{
> +	struct gui_event_handler *list = *handlers;

Add an empty line here.

> +	if (list == NULL)
> +		return;
> +
> +	if (list->id == event_id) {
> +		*handlers = list->next;

Hmm, where do we free list?

> +		return;
> +	}
> +
> +	remove_gui_event_handler(&list->next, event_id);

Just because I'm a kernel programmer, I hate recursive logic like this.
In the kernel, there's a limited stack, and recursive functions can
cause a kernel crash. Although, this works fine as is, I rather have a
loop to handle this. The "kernel" way to perform this is like so:

	struct gui_event_handler **last = handlers;
	struct gui_event_handler *list;

	for (list = *handlers; list; list = list->next) {
		if (list->id == event_id) {
			*last = list->next;
			free(list);
			break;
		}
		last = &list->next;
	}



> +}
> +
> diff --git a/kshark-plugin.h b/kshark-plugin.h
> index 95ba797..65e1e93 100644
> --- a/kshark-plugin.h
> +++ b/kshark-plugin.h
> @@ -17,6 +17,9 @@
>   *
>   * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   */
> +
> +#include "event-parse.h"
> +
>  #ifndef _KSHARK_PLUGIN_H
>  #define _KSHARK_PLUGIN_H
>  
> @@ -28,8 +31,50 @@
>  #define KSHARK_PLUGIN_LOADER_NAME MAKE_STR(KSHARK_PLUGIN_LOADER)
>  #define KSHARK_PLUGIN_UNLOADER_NAME MAKE_STR(KSHARK_PLUGIN_UNLOADER)
>  
> -typedef int (*kshark_plugin_load_func)(void *info);
> -typedef int (*kshark_plugin_unload_func)(void *info);
> +typedef int (*kshark_plugin_load_func)(void *info, void *store);
> +typedef int (*kshark_plugin_unload_func)(void *info, void *store);
> +
> +typedef int (*kshark_plugin_event_handler_func)(struct pevent_record *record,
> +						int task_id,
> +						void *val);
> +
> +typedef int (*kshark_plugin_context_update_func)(struct pevent *pevent, int val);
> +
> +enum gui_plugin_actions {
> +	KSHARK_PLUGIN_GET_PID,
> +	KSHARK_PLUGIN_GET_PREV_STATE,
> +	KSHARK_PLUGIN_GET_COMMAND
> +};
> +
> +enum gui_plugin_ctx_updates {
> +	KSHARK_PLUGIN_UPDATE_SWITCH_EVENT = 0x1,
> +	KSHARK_PLUGIN_UPDATE_WAKEUP_EVENT = 0x2,
> +	KSHARK_PLUGIN_UPDATE_WAKEUP_PID = 0x4,
> +	KSHARK_PLUGIN_UPDATE_SWITCH_PID = 0x8,
> +	KSHARK_PLUGIN_UPDATE_PREV_STATE = 0x10,
> +	KSHARK_PLUGIN_UPDATE_NEXT_NAME = 0x20

You want to use (1<<x) notation and also tab align the assignments:

	KSHARK_PLUGIN_UPDATE_SWITCH_EVENT	= (1 << 0),
	KSHARK_PLUGIN_UPDATE_WAKEUP_EVENT	= (1 << 1),
	KSHARK_PLUGIN_UPDATE_WAKEUP_PID		= (1 << 2),
	KSHARK_PLUGIN_UPDATE_SWITCH_PID		= (1 << 3),
	KSHARK_PLUGIN_UPDATE_PREV_STATE		= (1 << 4),
	KSHARK_PLUGIN_UPDATE_NEXT_NAME		= (1 << 5),

Notice I ended with a comma. enums are fine with the last element
ending with a comma. We do that because it makes it easier to add new
elements later.

> +};
> +
> +enum gui_event_types {
> +	KSHARK_PLUGIN_SWITCH_EVENT,
> +	KSHARK_PLUGIN_WAKEUP_EVENT
> +};
> +
> +struct gui_event_handler {
> +	struct gui_event_handler		*next;
> +	int					id;
> +	int 					type;
> +	kshark_plugin_event_handler_func	event_func;
> +	kshark_plugin_context_update_func       context_func;
> +};
> +
> +struct gui_event_handler *make_gui_event_handler(int event_id, int type,
> +						 kshark_plugin_event_handler_func evt_func,
> +						 kshark_plugin_context_update_func ctx_func);
> +
> +struct gui_event_handler *find_gui_event_handler(struct gui_event_handler *handlers,
> +						 int event_id);
>  
> +void remove_gui_event_handler(struct gui_event_handler **handlers, int event_id);
>  
>  #endif
> diff --git a/plugin_xenomai_gui.c b/plugin_xenomai_gui.c
> new file mode 100644
> index 0000000..f22a7a7
> --- /dev/null
> +++ b/plugin_xenomai_gui.c
> @@ -0,0 +1,257 @@
> +/*
> + *  Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation;
> + * version 2.1 of the License (not later!)
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this program; if not,  see <http://www.gnu.org/licenses>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "event-parse.h"
> +#include "kernel-shark.h"
> +#include "kshark-plugin.h"
> +
> +struct xenomai_context {
> +	struct shark_info	*info;
> +	struct trace_view_store *store;
> +	struct pevent 		*pevent;
> +
> +	struct event_format	*cobalt_switch_event;
> +	struct format_field     *cobalt_switch_next_pid_field;
> +	struct format_field     *cobalt_switch_prev_state_field;
> +	struct format_field     *cobalt_switch_next_name_field;
> +
> +	struct event_format	*cobalt_wakeup_event;
> +	struct format_field	*cobalt_wakeup_pid_field;
> +};
> +
> +struct xenomai_context *xenomai_context_handler = NULL;
> +
> +static gboolean xenomai_update_context(struct pevent *pevent, int task_id)
> +{
> +	struct xenomai_context *ctx = xenomai_context_handler;

Add empty line here.

> +	if (!ctx)
> +		return FALSE;
> +
> +	struct event_format *event;

Also, declare all variables at the start of the function, as old C style
would require. I find it easier to understand code this way than having
variables declared in the middle of functions, and having to search for
what types they are.

> +
> +	if (task_id & KSHARK_PLUGIN_UPDATE_SWITCH_EVENT) {
> +		event = pevent_find_event_by_name(xenomai_context_handler->pevent,
> +						  "cobalt_core",
> +						  "cobalt_switch_context");
> +		if (!event)
> +			return FALSE;
> +
> +		ctx->cobalt_switch_event = event;
> +	}
> +
> +	if (task_id & KSHARK_PLUGIN_UPDATE_WAKEUP_EVENT) {
> +		event = pevent_find_event_by_name(xenomai_context_handler->pevent,
> +						  "cobalt_core",
> +						  "cobalt_thread_resume");
> +		if (!event)
> +			return FALSE;
> +
> +		ctx->cobalt_wakeup_event = event;
> +	}
> +
> +	if (task_id & KSHARK_PLUGIN_UPDATE_SWITCH_PID) {
> +		ctx->cobalt_switch_next_pid_field =
> +		pevent_find_field(ctx->cobalt_switch_event, "next_pid");

Indent more "pevent_find_field()" as it is a continuation of the
previous assignment.

> +	}
> +
> +	if (task_id & KSHARK_PLUGIN_UPDATE_PREV_STATE) {
> +		ctx->cobalt_switch_prev_state_field =
> +		pevent_find_field(ctx->cobalt_switch_event, "prev_state");

Do it for all of them too.

> +	}
> +
> +	if (task_id & KSHARK_PLUGIN_UPDATE_NEXT_NAME) {
> +		ctx->cobalt_switch_next_name_field =
> +		pevent_find_field(ctx->cobalt_switch_event, "next_name");
> +	}
> +
> +	if (task_id & KSHARK_PLUGIN_UPDATE_WAKEUP_PID) {
> +		ctx->cobalt_wakeup_pid_field =
> +		pevent_find_field(ctx->cobalt_wakeup_event, "pid");
> +	}
> +
> +	return TRUE;
> +}
> +
> +static void xenomai_context_new(struct shark_info *info, struct trace_view_store *store)
> +{
> +	if (!xenomai_context_handler) {
> +		xenomai_context_handler =
> +		(struct xenomai_context*) malloc(sizeof(struct xenomai_context));
> +	}
> +
> +	xenomai_context_handler->info = info;
> +	xenomai_context_handler->store = store;
> +	xenomai_context_handler->pevent = tracecmd_get_pevent(info->handle);
> +
> +	int status = xenomai_update_context(xenomai_context_handler->pevent,
> +					    KSHARK_PLUGIN_UPDATE_SWITCH_EVENT |
> +					    KSHARK_PLUGIN_UPDATE_WAKEUP_EVENT);
> +	if (status == FALSE) {
> +		free(xenomai_context_handler);
> +		xenomai_context_handler = NULL;
> +	}
> +}
> +
> +static int cobalt_get_next_pid(struct xenomai_context *ctx,
> +				struct pevent_record *record,
> +				int *pid)
> +{
> +	long long unsigned int val;
> +	int status = pevent_read_number_field(ctx->cobalt_switch_next_pid_field,
> +					      record->data, &val);
> +	if (pid)
> +		*pid = val;
> +
> +	return status;
> +}
> +
> +
> +static int cobalt_get_prev_state(struct xenomai_context *ctx,
> +				 struct pevent_record *record,
> +				 int *state)
> +{
> +	long long unsigned int val;
> +	pevent_read_number_field(ctx->cobalt_switch_prev_state_field,
> +				 record->data, &val);
> +
> +	if (state)
> +		*state = val;
> +
> +	return (val & 0x00000008) ? 1 : 0;
> +}
> +
> +static void cobalt_get_command(struct xenomai_context *ctx,
> +				struct pevent_record *record,
> +				const char **comm)
> +{
> +	int offset =
> +	data2host4(ctx->pevent, record->data + ctx->cobalt_switch_next_name_field->offset);
> +
> +	offset &= 0xffff;
> +	*comm = record->data + offset;
> +}
> +
> +static gboolean xenomai_switch_handler(struct pevent_record *record,
> +					int task_id,
> +					void *output)
> +{
> +	struct xenomai_context *ctx = xenomai_context_handler;
> +	switch (task_id) {
> +		case KSHARK_PLUGIN_GET_PID:
> +			cobalt_get_next_pid(ctx, record, output);
> +			return TRUE;
> +
> +		case KSHARK_PLUGIN_GET_PREV_STATE:
> +			return cobalt_get_prev_state(ctx, record, output);
> +
> +		case KSHARK_PLUGIN_GET_COMMAND:
> +			cobalt_get_command(ctx, record, (const char**) output);
> +			return TRUE;
> +
> +		default:
> +			return FALSE;
> +	}
> +}
> +
> +static int cobalt_get_wakeup_pid(struct xenomai_context *ctx,
> +				 struct pevent_record *record,
> +				 int *pid)
> +{
> +	long long unsigned int val;
> +	int status =
> +	pevent_read_number_field(ctx->cobalt_wakeup_pid_field, record->data, &val);
> +
> +	if (pid)
> +		*pid = val;
> +
> +	return status;
> +}
> +
> +static gboolean xenomai_wakeup_handler(struct pevent_record *record,
> +					int task_id,
> +					void *output)
> +{
> +	struct xenomai_context *ctx = xenomai_context_handler;
> +	switch (task_id) {
> +		case KSHARK_PLUGIN_GET_PID:
> +			cobalt_get_wakeup_pid(ctx, record, output);
> +			return TRUE;
> +
> +		default:
> +			return FALSE;
> +	}
> +}
> +
> +
> +void KSHARK_PLUGIN_LOADER(void *info, void *store)
> +{
> +	struct shark_info *ks_info = info;
> +	struct trace_view_store *ks_store = store;
> +
> +	xenomai_context_new(ks_info, ks_store);
> +	if (!xenomai_context_handler)
> +		return;
> +
> +	struct xenomai_context *ctx = xenomai_context_handler;
> +
> +	struct gui_event_handler *switch_handler =
> +	make_gui_event_handler(ctx->cobalt_switch_event->id,
> +			       KSHARK_PLUGIN_SWITCH_EVENT,
> +			       xenomai_switch_handler,
> +			       xenomai_update_context);
> +
> +	struct gui_event_handler *wakeup_handler =
> +	make_gui_event_handler(ctx->cobalt_wakeup_event->id,
> +			       KSHARK_PLUGIN_WAKEUP_EVENT,
> +			       xenomai_wakeup_handler,
> +			       xenomai_update_context);
> +
> +	trace_view_store_register_gui_handler(ks_store, switch_handler);
> +	trace_view_store_register_gui_handler(ks_store, wakeup_handler);
> +
> +	trace_graph_register_gui_handler(ks_info->ginfo, switch_handler);
> +	trace_graph_register_gui_handler(ks_info->ginfo, wakeup_handler);
> +}
> +
> +void KSHARK_PLUGIN_UNLOADER(void *info, void *store)
> +{
> +	struct shark_info *ks_info = info;
> +	struct trace_view_store *ks_store = store;
> +	struct xenomai_context *ctx = xenomai_context_handler;
> +
> +	remove_gui_event_handler(&ks_store->event_handlers,
> +				 ctx->cobalt_switch_event->id);
> +
> +	remove_gui_event_handler(&ks_store->event_handlers,
> +				 ctx->cobalt_wakeup_event->id);
> +
> +	remove_gui_event_handler(&ks_info->ginfo->event_handlers,
> +				 ctx->cobalt_switch_event->id);
> +
> +	remove_gui_event_handler(&ks_info->ginfo->event_handlers,
> +				 ctx->cobalt_wakeup_event->id);
> +
> +	free(ctx);
> +	xenomai_context_handler = NULL;
> +}
> +
> diff --git a/trace-graph.c b/trace-graph.c
> index 1db342f..19f02c0 100644
> --- a/trace-graph.c
> +++ b/trace-graph.c
> @@ -129,6 +129,12 @@ static void free_task_hash(struct graph_info *ginfo)
>  	}
>  }
>  
> +void trace_graph_register_gui_handler(struct graph_info *info,
> +				      struct gui_event_handler *handler) {
> +	handler->next = info->event_handlers;
> +	info->event_handlers = handler;
> +}
> +
>  /**
>   * trace_graph_task_list - return an allocated list of all found tasks
>   * @ginfo: The graph info structure
> @@ -1053,6 +1059,12 @@ int trace_graph_check_sched_wakeup(struct graph_info *ginfo,
>  
>  	id = pevent_data_type(ginfo->pevent, record);
>  
> +	struct gui_event_handler *handler = find_gui_event_handler(ginfo->event_handlers, id);

The handler needs to be declared at the beginning of the function. (Old
C style). And yes, we need to optimize this, because this is in the
fast path.

> +	if (handler) {
> +		if (handler->type == KSHARK_PLUGIN_WAKEUP_EVENT)
> +			handler->context_func(ginfo->pevent, KSHARK_PLUGIN_UPDATE_WAKEUP_PID);
> +	}

Hmm. The context_func() is used to search if the required event exists.
We don't want to do that for every wake up event. Just once. Perhaps
the handler should have a "seen" mask that can skip calling the
context_func.

	if (handler && !(handler->seen & KSHARK_PLUGIN_WAKEUP_EVENT) {
		handler->seen |= KSHARK_PLUGIN_WAKEUP_EVENT;
		if (handler->type == KSHARK_PLUGIN_WAKEUP_EVENT)
			handler->context_func(...);
	}

Something like that. The event then needs to be a bitmask.

> +
>  	if (id == ginfo->event_wakeup_id) {
>  		/* We only want those that actually woke up the task */
>  		if (ginfo->wakeup_success_field) {
> @@ -1079,6 +1091,16 @@ int trace_graph_check_sched_wakeup(struct graph_info *ginfo,
>  		return 1;
>  	}
>  
> +	if (handler) {
> +		if (handler->type == KSHARK_PLUGIN_WAKEUP_EVENT) {
> +			handler->event_func(record, KSHARK_PLUGIN_GET_PID, &val);
> +			if (pid)
> +				*pid = val;
> +
> +			return 1;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1118,7 +1140,20 @@ int trace_graph_check_sched_switch(struct graph_info *ginfo,
>  		}
>  	}
>  
> +
>  	id = pevent_data_type(ginfo->pevent, record);
> +	struct gui_event_handler *handler =
> +	find_gui_event_handler(ginfo->event_handlers, id);

Indent find_gui_event_handler() and all declarations must be at the
begging of the function.

And didn't we already find the handler?

> +
> +	if (handler) {
> +		if (handler->type == KSHARK_PLUGIN_SWITCH_EVENT) {
> +			handler->context_func(ginfo->pevent,
> +					      KSHARK_PLUGIN_UPDATE_SWITCH_PID |
> +					      KSHARK_PLUGIN_UPDATE_PREV_STATE |
> +					      KSHARK_PLUGIN_UPDATE_NEXT_NAME);

We should be able to consolidate the updating of context_func.

> +		}
> +	}
> +
>  	if (id == ginfo->event_sched_switch_id) {
>  		pevent_read_number_field(ginfo->event_pid_field, record->data, &val);
>  		if (comm)
> @@ -1139,6 +1174,17 @@ int trace_graph_check_sched_switch(struct graph_info *ginfo,
>  		goto out;
>  	}
>  
> +	if (handler) {
> +		if (handler->type == KSHARK_PLUGIN_SWITCH_EVENT) {
> +			handler->event_func(record, KSHARK_PLUGIN_GET_PID, &val);
> +			if (pid)
> +				*pid = val;
> +			if(comm)
> +				handler->event_func(record, KSHARK_PLUGIN_GET_COMMAND, &comm);
> +			goto out;
> +		}
> +	}
> +
>  	ret = 0;
>   out:
>  	if (ret && comm && ginfo->read_comms) {
> @@ -2781,6 +2827,8 @@ trace_graph_create_with_callbacks(struct tracecmd_input *handle,
>  
>  	ginfo->callbacks = cbs;
>  
> +	ginfo->event_handlers = NULL;
> +
>  	ginfo->task_filter = filter_task_hash_alloc();
>  	ginfo->hide_tasks = filter_task_hash_alloc();
>  
> diff --git a/trace-graph.h b/trace-graph.h
> index 7e66838..21e8feb 100644
> --- a/trace-graph.h
> +++ b/trace-graph.h
> @@ -24,6 +24,7 @@
>  #include "trace-cmd.h"
>  #include "trace-filter-hash.h"
>  #include "trace-xml.h"
> +#include "kshark-plugin.h"
>  
>  struct graph_info;
>  
> @@ -258,6 +259,8 @@ struct graph_info {
>  	gint			plot_data_y;
>  	gint			plot_data_w;
>  	gint			plot_data_h;
> +
> +	struct gui_event_handler *event_handlers;
>  };
>  
>  
> @@ -266,6 +269,10 @@ trace_graph_create(struct tracecmd_input *handle);
>  struct graph_info *
>  trace_graph_create_with_callbacks(struct tracecmd_input *handle,
>  				  struct graph_callbacks *cbs);
> +
> +void trace_graph_register_gui_handler(struct graph_info *info,
> +				      struct gui_event_handler *handler);
> +
>  void trace_graph_select_by_time(struct graph_info *ginfo, guint64 time);
>  
>  void trace_graph_event_filter_callback(gboolean accept,
> diff --git a/trace-plot-task.c b/trace-plot-task.c
> index 3b7e81f..56c3e96 100644
> --- a/trace-plot-task.c
> +++ b/trace-plot-task.c
> @@ -68,11 +68,20 @@ static gboolean is_running(struct graph_info *ginfo, struct pevent_record *recor
>  	int id;
>  
>  	id = pevent_data_type(ginfo->pevent, record);
> -	if (id != ginfo->event_sched_switch_id)
> -		return FALSE;
>  
> -	pevent_read_number_field(ginfo->event_prev_state, record->data, &val);
> -	return val & ((1 << 11) - 1)? FALSE : TRUE;

Ug. Not your issues, but I need to remove those hard coded numbers.

> +	if (id == ginfo->event_sched_switch_id) {
> +		pevent_read_number_field(ginfo->event_prev_state, record->data, &val);
> +		return val & ((1 << 11) - 1)? FALSE : TRUE;
> +	}
> +
> +	struct gui_event_handler *handler =
> +	find_gui_event_handler(ginfo->event_handlers, id);
> +	if (handler) {
> +		if (handler->type == KSHARK_PLUGIN_SWITCH_EVENT)
> +			return handler->event_func(record, KSHARK_PLUGIN_GET_PREV_STATE, NULL);
> +	}
> +
> +	return FALSE;
>  }
>  
>  static gboolean record_matches_pid(struct graph_info *ginfo,
> diff --git a/trace-view-store.c b/trace-view-store.c
> index f5d0a04..d7293c3 100644
> --- a/trace-view-store.c
> +++ b/trace-view-store.c
> @@ -70,6 +70,12 @@ static gboolean		trace_view_store_iter_parent	(GtkTreeModel	*tree_model,
>  static GObjectClass *parent_class = NULL;	/* GObject stuff - nothing to worry about */
>  
>  
> +void trace_view_store_register_gui_handler(TraceViewStore *store, struct gui_event_handler *handler)
> +{
> +	handler->next = store->event_handlers;
> +	store->event_handlers = handler;
> +}
> +
>  /*****************************************************************************
>   *
>   *	trace_view_store_get_type: here we register our new type and its interfaces
> @@ -866,6 +872,7 @@ trace_view_store_new (struct tracecmd_input *handle)
>  
>  	g_assert( newstore != NULL );
>  
> +	newstore->event_handlers = NULL;
>  	newstore->handle = handle;
>  	newstore->cpus = tracecmd_cpus(handle);
>  	tracecmd_ref(handle);
> @@ -1224,6 +1231,14 @@ static gboolean show_task(TraceViewStore *store, struct pevent *pevent,
>  			return TRUE;
>  	}
>  
> +	struct gui_event_handler *handler =
> +	find_gui_event_handler(store->event_handlers, event_id);
> +	if (handler) {
> +		handler->event_func(record, KSHARK_PLUGIN_GET_PID, &pid);
> +		if (view_task(store, pid))
> +			return TRUE;
> +	}
> +
>  	return FALSE;
>  }
>  
> @@ -1261,6 +1276,15 @@ static void update_filter_tasks(TraceViewStore *store)
>  						      "pid");
>  	}
>  
> +	struct gui_event_handler *handler = store->event_handlers;

Also, declare handler at the beginning of the function. It's fine to do
assignments here.

> +	while (handler) {

Can you make this a for loop.

> +		handler->context_func(	pevent,
> +					KSHARK_PLUGIN_UPDATE_SWITCH_PID |
> +					KSHARK_PLUGIN_UPDATE_WAKEUP_PID);

We could add the "seen" here too.

Thanks!

-- Steve

> +
> +		handler = handler->next;
> +	}
> +
>  	for (cpu = 0; cpu < store->cpus; cpu++) {
>  		record = tracecmd_read_cpu_first(handle, cpu);
>  
> diff --git a/trace-view-store.h b/trace-view-store.h
> index 03141b1..333116c 100644
> --- a/trace-view-store.h
> +++ b/trace-view-store.h
> @@ -23,6 +23,7 @@
>  #include <gtk/gtk.h>
>  #include "trace-cmd.h"
>  #include "trace-filter-hash.h"
> +#include "kshark-plugin.h"
>  
>  /* Some boilerplate GObject defines. 'klass' is used
>   *   instead of 'class', because 'class' is a C++ keyword */
> @@ -125,8 +126,12 @@ struct trace_view_store
>  	guint64			*cpu_mask;  /* cpus that are enabled */
>  
>  	gint		stamp;	/* Random integer to check whether an iter belongs to our model */
> +
> +	struct gui_event_handler *event_handlers;
>  };
>  
> +void trace_view_store_register_gui_handler(TraceViewStore *store, struct gui_event_handler *handler);
> +
>  gboolean trace_view_store_cpu_isset(TraceViewStore *store, gint cpu);
>  
>  void trace_view_store_set_all_cpus(TraceViewStore *store);

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-01 15:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 11:04 [PATCH 1/2] kernelshark: Fix the GUI plugin loading Yordan Karadzhov (VMware)
2017-11-01 11:04 ` [PATCH 2/2] kernelshark: Adding a GUI plugin for xenomai events Yordan Karadzhov (VMware)
2017-11-01 15:19   ` Steven Rostedt
2017-11-01 14:22 ` [PATCH 1/2] kernelshark: Fix the GUI plugin loading Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.