tools lib traceevent: bitmask handling and plugin updates

From: "Steven Rostedt (Red Hat)" <[email protected]>

The traceevent plugins allows developers to have their events print out information that is more advanced than what can be achieved by the trace event format files.

As these plugins are used on the userspace side of the tracing tools, it is only logical that the tools should be able to produce different types of output for the events. The types of events still need to be defined by the plugins thus we need a way to pass information from the tool to the plugin to specify what type of information to be shown.

Not only does the information need to be passed by the tool to plugin, but the plugin also requires a way to notify the tool of what options it can provide.

This builds the plugin option infrastructure that is taken from trace-cmd that is used to allow plugins to produce different output based on the options specified by the tool.

Signed-off-by Steven Rostedt <[email protected]>

---
 tools/lib/traceevent/event-parse.h  |  16 ++-
 tools/lib/traceevent/event-plugin.c | 197 ++++++++++++++++++++++++++++++++++++
 2 files changed, 209 insertions(+), 4 deletions(-)

tools/lib/traceevent/event-parse.h +0 -0

index a68ec3d8289f..d6c610a66006 100644

tools/lib/traceevent/event-parse.h +1 -1

 	void				*handle;
 	char				*file;
 	char				*name;

@@ -135,7 +135,7 @@ struct plugin_option {
  * PEVENT_PLUGIN_OPTIONS:  (optional)
  *   Plugin options that can be set before loading
  *
- *   struct plugin_option PEVENT_PLUGIN_OPTIONS[] = {
+ *   struct pevent_plugin_option PEVENT_PLUGIN_OPTIONS[] = {
  *	{
  *		.name = "option-name",
  *		.plugin_alias = "overide-file-name", (optional)

tools/lib/traceevent/event-parse.h +8 -0

 };
 
 #define PEVENT_ERRORS 							      \

@@ -415,6 +415,14 @@ struct plugin_list;
 struct plugin_list *traceevent_load_plugins(struct pevent *pevent);
 void traceevent_unload_plugins(struct plugin_list *plugin_list,
 			       struct pevent *pevent);
+char **traceevent_plugin_list_options(void);
+void traceevent_plugin_free_options_list(char **list);
+int traceevent_plugin_add_options(const char *name,
+				  struct pevent_plugin_option *options);
+void traceevent_plugin_remove_options(struct pevent_plugin_option *options);
+void traceevent_print_plugins(struct trace_seq *s,
+			      const char *prefix, const char *suffix,
+			      const struct plugin_list *list);
 
 struct cmdline;
 struct cmdline_list;

tools/lib/traceevent/event-plugin.c +13 -0

index 317466bd1a37..a24479426d58 100644
@@ -18,6 +18,7 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 
+#include <stdio.h>
 #include <string.h>
 #include <dlfcn.h>
 #include <stdlib.h>

@@ -30,12 +31,208 @@
 
 #define LOCAL_PLUGIN_DIR ".traceevent/plugins"
 
+static struct registered_plugin_options {
+	struct registered_plugin_options	*next;
+	struct pevent_plugin_option		*options;
+} *registered_options;
+
+static struct trace_plugin_options {
+	struct trace_plugin_options	*next;
+	char				*plugin;
+	char				*option;
+	char				*value;
+} *trace_plugin_options;
+
 struct plugin_list {
 	struct plugin_list	*next;
 	char			*name;
 	void			*handle;
 };

tools/lib/traceevent/event-plugin.c +12 -0

+ *
+ * Must be freed with traceevent_plugin_free_options_list().
+ */
+char **traceevent_plugin_list_options(void)
+{
+	struct registered_plugin_options *reg;
+	struct pevent_plugin_option *op;
+	char **list = NULL;
+	char *name;
+	int count = 0;
+
+	for (reg = registered_options; reg; reg = reg->next) {

tools/lib/traceevent/event-plugin.c +5 -0

+		free(list[count]);
+	free(list);
+
+	return (char **)((unsigned long)-1);
+}

Hi Steve,

2014-05-16 (금), 10:02 -0400, Steven Rostedt:

Quoted 34 lines

From: "Steven Rostedt (Red Hat)" <[email protected]>

The traceevent plugins allows developers to have their events print out information that is more advanced than what can be achieved by the trace event format files.

As these plugins are used on the userspace side of the tracing tools, it is only logical that the tools should be able to produce different types of output for the events. The types of events still need to be defined by the plugins thus we need a way to pass information from the tool to the plugin to specify what type of information to be shown.

Not only does the information need to be passed by the tool to plugin, but the plugin also requires a way to notify the tool of what options it can provide.

This builds the plugin option infrastructure that is taken from trace-cmd that is used to allow plugins to produce different output based on the options specified by the tool.

Signed-off-by Steven Rostedt <[email protected]>

---
 tools/lib/traceevent/event-parse.h  |  16 ++-
 tools/lib/traceevent/event-plugin.c | 197 ++++++++++++++++++++++++++++++++++++
 2 files changed, 209 insertions(+), 4 deletions(-)

tools/lib/traceevent/event-parse.h +0 -0

index a68ec3d8289f..d6c610a66006 100644
@@ -107,8 +107,8 @@ typedef int (*pevent_event_handler_func)(struct trace_seq *s,
 typedef int (*pevent_plugin_load_func)(struct pevent *pevent);
 typedef int (*pevent_plugin_unload_func)(struct pevent *pevent);
 

tools/lib/traceevent/event-parse.h typedef int (*pevent_event_handler_func)(struct trace_seq *s, · line 111

Quoted 4 lines

tools/lib/traceevent/event-parse.h +2 -2

-struct plugin_option {
-	struct plugin_option		*next;
+struct pevent_plugin_option {
+	struct pevent_plugin_option	*next;

Hmm.. this name change reminds me that it might be better if this plugin and option list belong to a pevent..

Quoted 14 lines

tools/lib/traceevent/event-parse.h +1 -1

 	void				*handle;
 	char				*file;
 	char				*name;

@@ -135,7 +135,7 @@ struct plugin_option {
  * PEVENT_PLUGIN_OPTIONS:  (optional)
  *   Plugin options that can be set before loading
  *
- *   struct plugin_option PEVENT_PLUGIN_OPTIONS[] = {
+ *   struct pevent_plugin_option PEVENT_PLUGIN_OPTIONS[] = {
  *	{
  *		.name = "option-name",
  *		.plugin_alias = "overide-file-name", (optional)

tools/lib/traceevent/event-parse.h enum pevent_func_arg_type { · line 358

Quoted 6 lines

tools/lib/traceevent/event-parse.h +1 -1

@@ -355,7 +355,7 @@ enum pevent_func_arg_type {
 enum pevent_flag {
 	PEVENT_NSEC_OUTPUT		= 1,	/* output in NSECS */
 	PEVENT_DISABLE_SYS_PLUGINS	= 1 << 1,
-	PEVENT_DISABLE_PLUGINS		= 1 << 2,
+	PEVENT_DISABLE_PLUGINS		= 1 << 2

Unnecessary change?

Quoted 54 lines

tools/lib/traceevent/event-parse.h +8 -0

 };
 
 #define PEVENT_ERRORS 							      \

@@ -415,6 +415,14 @@ struct plugin_list;
 struct plugin_list *traceevent_load_plugins(struct pevent *pevent);
 void traceevent_unload_plugins(struct plugin_list *plugin_list,
 			       struct pevent *pevent);
+char **traceevent_plugin_list_options(void);
+void traceevent_plugin_free_options_list(char **list);
+int traceevent_plugin_add_options(const char *name,
+				  struct pevent_plugin_option *options);
+void traceevent_plugin_remove_options(struct pevent_plugin_option *options);
+void traceevent_print_plugins(struct trace_seq *s,
+			      const char *prefix, const char *suffix,
+			      const struct plugin_list *list);
 
 struct cmdline;
 struct cmdline_list;

tools/lib/traceevent/event-plugin.c +13 -0

index 317466bd1a37..a24479426d58 100644
@@ -18,6 +18,7 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 
+#include <stdio.h>
 #include <string.h>
 #include <dlfcn.h>
 #include <stdlib.h>

@@ -30,12 +31,208 @@
 
 #define LOCAL_PLUGIN_DIR ".traceevent/plugins"
 
+static struct registered_plugin_options {
+	struct registered_plugin_options	*next;
+	struct pevent_plugin_option		*options;
+} *registered_options;
+
+static struct trace_plugin_options {
+	struct trace_plugin_options	*next;
+	char				*plugin;
+	char				*option;
+	char				*value;
+} *trace_plugin_options;
+
 struct plugin_list {
 	struct plugin_list	*next;
 	char			*name;
 	void			*handle;
 };
 

tools/lib/traceevent/event-plugin.c line 60

Quoted 9 lines

tools/lib/traceevent/event-plugin.c +9 -0

+/**
+ * traceevent_plugin_list_options - get list of plugin options
+ *
+ * Returns an array of char strings that list the currently registered
+ * plugin options in the format of <plugin>:<option>. This list can be
+ * used by toggling the option.
+ *
+ * Returns NULL if there's no options registered. On error it returns
+ * an (char **)-1 (must check for that)

What about making it a macro like INVALID_OPTION_LIST?

Quoted 17 lines

tools/lib/traceevent/event-plugin.c +16 -0

+ *
+ * Must be freed with traceevent_plugin_free_options_list().
+ */
+char **traceevent_plugin_list_options(void)
+{
+	struct registered_plugin_options *reg;
+	struct pevent_plugin_option *op;
+	char **list = NULL;
+	char *name;
+	int count = 0;
+
+	for (reg = registered_options; reg; reg = reg->next) {
+		for (op = reg->options; op->name; op++) {
+			char *alias = op->plugin_alias ? op->plugin_alias : op->file;
+
+			name = malloc(strlen(op->name) + strlen(alias) + 2);

tools/lib/traceevent/event-plugin.c line 82

Quoted 6 lines

tools/lib/traceevent/event-plugin.c +6 -0

+			if (!name)
+				goto err;
+
+			sprintf(name, "%s:%s", alias, op->name);
+			list = realloc(list, count + 2);
+			if (!list) {

This will lost the original list pointer. Please use a temp variable.

tools/lib/traceevent/event-plugin.c line 92

Quoted 12 lines

tools/lib/traceevent/event-plugin.c +10 -0

+				free(name);
+				goto err;
+			}
+			list[count++] = name;
+			list[count] = NULL;
+		}
+	}
+	if (!count)
+		return NULL;
+	return list;

Isn't it enough to simply return the list?

tools/lib/traceevent/event-plugin.c line 95

Quoted 4 lines

tools/lib/traceevent/event-plugin.c +3 -0

+
+ err:
+	while (--count > 0)

Shouldn't it be >= instead of > ?

Thanks,
Namhyung

Quoted 7 lines

tools/lib/traceevent/event-plugin.c +5 -0

+		free(list[count]);
+	free(list);
+
+	return (char **)((unsigned long)-1);
+}

Quoted 9 lines · Namhyung Kim

2014-05-16 (금), 10:02 -0400, Steven Rostedt:

tools/lib/traceevent/event-parse.h typedef int (*pevent_event_handler_func)(struct trace_seq *s, · line 111

Quoted 4 lines

tools/lib/traceevent/event-parse.h +2 -2

-struct plugin_option {
-   struct plugin_option            *next;
+struct pevent_plugin_option {
+   struct pevent_plugin_option     *next;

Hmm.. this name change reminds me that it might be better if this plugin and option list belong to a pevent..

Yeah, eventually, but for now I want to try to sync what I have in trace-cmd with what is here in libtraceevent. At least to save history. Then we can modify it to be per pevent. For now, there's no users that need it per pevent. But that should definitely be changed before we push this out to distros.

Quoted 11 lines

Quoted 10 lines

tools/lib/traceevent/event-plugin.c +10 -0

+                           free(name);
+                           goto err;
+                   }
+                   list[count++] = name;
+                   list[count] = NULL;
+           }
+   }
+   if (!count)
+           return NULL;
+   return list;

Quoted 1 line

Isn't it enough to simply return the list?

Confused. Do you mean "Is it enough to simply return the list?"

BTW, Are you at LinuxCon Japan?

Quoted 5 lines

- * struct plugin_option PEVENT_PLUGIN_OPTIONS[] = {
+ * struct pevent_plugin_option PEVENT_PLUGIN_OPTIONS[] = {
* {
* .name = "option-name",
* .plugin_alias = "overide-file-name", (optional)

tools/lib/traceevent/event-parse.h enum pevent_func_arg_type { · line 358

Quoted 6 lines

tools/lib/traceevent/event-parse.h +1 -1

@@ -355,7 +355,7 @@ enum pevent_func_arg_type {
 enum pevent_flag {
 	PEVENT_NSEC_OUTPUT		= 1,	/* output in NSECS */
 	PEVENT_DISABLE_SYS_PLUGINS	= 1 << 1,
-	PEVENT_DISABLE_PLUGINS		= 1 << 2,
+	PEVENT_DISABLE_PLUGINS		= 1 << 2

Unnecessary change?

tools/lib/traceevent/event-plugin.c line 60

Quoted 9 lines

tools/lib/traceevent/event-plugin.c +9 -0

+/**
+ * traceevent_plugin_list_options - get list of plugin options
+ *
+ * Returns an array of char strings that list the currently registered
+ * plugin options in the format of <plugin>:<option>. This list can be
+ * used by toggling the option.
+ *
+ * Returns NULL if there's no options registered. On error it returns
+ * an (char **)-1 (must check for that)

What about making it a macro like INVALID_OPTION_LIST?

Quoted 17 lines

+ *
+ * Must be freed with traceevent_plugin_free_options_list().
+ */

tools/lib/traceevent/event-plugin.c +13 -0

+char **traceevent_plugin_list_options(void)
+{
+	struct registered_plugin_options *reg;
+	struct pevent_plugin_option *op;
+	char **list = NULL;
+	char *name;
+	int count = 0;
+
+	for (reg = registered_options; reg; reg = reg->next) {
+		for (op = reg->options; op->name; op++) {
+			char *alias = op->plugin_alias ? op->plugin_alias : op->file;
+
+			name = malloc(strlen(op->name) + strlen(alias) + 2);

tools/lib/traceevent/event-plugin.c line 82

Quoted 6 lines

tools/lib/traceevent/event-plugin.c +6 -0

+			if (!name)
+				goto err;
+
+			sprintf(name, "%s:%s", alias, op->name);
+			list = realloc(list, count + 2);
+			if (!list) {

This will lost the original list pointer. Please use a temp variable.

tools/lib/traceevent/event-plugin.c line 92

Quoted 12 lines

tools/lib/traceevent/event-plugin.c +10 -0

+				free(name);
+				goto err;
+			}
+			list[count++] = name;
+			list[count] = NULL;
+		}
+	}
+	if (!count)
+		return NULL;
+	return list;

Isn't it enough to simply return the list?

tools/lib/traceevent/event-plugin.c line 95

Quoted 4 lines

tools/lib/traceevent/event-plugin.c +3 -0

+
+ err:
+	while (--count > 0)

Shouldn't it be >= instead of > ?

Thanks,
Namhyung

Quoted 5 lines

tools/lib/traceevent/event-plugin.c +5 -0

+		free(list[count]);
+	free(list);
+
+	return (char **)((unsigned long)-1);
+}

tools/lib/traceevent/event-parse.c +1 -0

@@ -3691,6 +3784,23 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 	case PRINT_BSTRING:
 		print_str_to_seq(s, format, len_arg, arg->string.string);
 		break;
+	case PRINT_BITMASK: {

tools/lib/traceevent/event-parse.c +6 -0

+		bitmask_size = bitmask_offset >> 16;
+		bitmask_offset &= 0xffff;
+		print_bitmask_to_seq(pevent, s, format, len_arg,
+				     data + bitmask_offset, bitmask_size);
+		break;
+	}

Hi Steve,

Quoted 9 lines · Steven Rostedt

tools/lib/traceevent/event-parse.c +5 -0

@@ -3691,6 +3784,23 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 	case PRINT_BSTRING:
 		print_str_to_seq(s, format, len_arg, arg->string.string);
 		break;
+	case PRINT_BITMASK: {
+		int bitmask_offset;
+		int bitmask_size;
+
+		if (arg->bitmask.offset == -1) {

tools/lib/traceevent/event-parse.c static void print_str_arg(struct trace_seq *s, void *data, int size, · line 3797

Quoted 6 lines

tools/lib/traceevent/event-parse.c +6 -0

+			struct format_field *f;
+
+			f = pevent_find_any_field(event, arg->bitmask.bitmask);
+			arg->bitmask.offset = f->offset;
+		}
+		bitmask_offset = data2host4(pevent, data + arg->string.offset);

s/string.offset/bitmask.offset/

Thanks,
Namhyung

Quoted 6 lines

tools/lib/traceevent/event-parse.c +6 -0

+		bitmask_size = bitmask_offset >> 16;
+		bitmask_offset &= 0xffff;
+		print_bitmask_to_seq(pevent, s, format, len_arg,
+				     data + bitmask_offset, bitmask_size);
+		break;
+	}