linux-rt-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API
@ 2020-01-24 22:56 Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 01/12] tracing: Add trace_array_find/_get() to find instance trace arrays Tom Zanussi
                   ` (12 more replies)
  0 siblings, 13 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Hi,

This is v3 of 'tracing: Add support for in-kernel dynamic event API',
incorporating changes based on suggestions from Masami and Steven.

  - Rebased to trace/for-next

  - Regularized the entire API to use synth_event_*, kprobe_event_*,
    dynevent_*, and added some new macros and functions to make things
    more consistent

  - Introduced trace_array_find_get() and used it in
    trace_array_get_file() as suggested
  
  - Removed exports from dynevent_cmd functions that didn't need to be
    exported

  - Switched the param order of __kprobe_event_gen_cmd_start() to fix
    a problem found building with clang.  Apparently varargs and
    implicit promotion of types like bool don't mix.  Thanks to Nick
    Desaulniers for pointing that out.

  - Updated the documentation for all of the above

Text from the v2 posting:

This is v2 of the previous 'tracing: Add support for in-kernel
synthetic event API', and is largely a rewrite based on suggestions
from Masami to expand the scope to include other types of dynamic
events such as kprobe events, in addition to the original sythetic
event focus.

The functionality of the original API remains available, though it's
in a slightly different form due to the use of the new dynevent_cmd
API it's now based on.  The dynevent_cmd API provides a common dynamic
event command generation capability that both the synthetic event API
and the kprobe event API are now based on, and there are now test
modules demonstrating both the synthetic event and kprobes APIs.

A couple of the patches are snippets from Masami's 'tracing:
bootconfig: Boot-time tracing and Extra boot config' series, and the
patch implementing the dynevent_cmd API includes some of the
spnprintf() generating code from that patchset.

Because I used Masami's gen_*_cmd() naming suggestion for generating
the commands, the previous patchset's generate_*() functions were
renamed to trace_*() to avoid confusion, and probably is better naming
anyway.

An overview of the user-visible changes in comparison to v1:

  - create_synth_event() using an array of synth_desc_fields remains
    unchanged and works the same way as previously

  - gen_synth_cmd() takes a variable-length number of args which
    represent 'type field;' pairs.  Using this with no field args
    basically replaces the previous 'create_empty_synth_event()'

  - The 'add_synth_field()' and 'add_synth_fields()' function from v1
    are essentially the same except that they now take a dynevent_cmd
    instead of a synth_event pointer

  - The finalize_synth_event() from v1 is replaced by
    create_dynevent() in the new version.
  
  - The new v2 API includes some additional functions to initialize
    the dynevent_cmd - synth_dynevent_cmd() is used to do that.  While
    it's an extra step, it makes it easier to do error handling.

  - There's a new trace_synth_event() function that traces a synthetic
    event using a variable-arg list of values.

  - The original generate_synth_event() using an array of values is
    now called trace_synth_event_array().

  - For piecewise event tracing, the original
    generate_synth_event_start() and generate_synth_event_end() have
    now been renamed to trace_synth_event_end().

  - add_next_synth_val() and add_synth_val() remain the same.

  - A similar API and test module demonstrating the API has been added
    for kprobe events

  - Both the synthetic events and kprobe events API is based on the
    dynevent_cmd API, newly added

  - The Documentation for all of the above has been updated

Text from the orginal v1 posting:

I've recently had several requests and suggestions from users to add
support for the creation and generation of synthetic events from
kernel code such as modules, and not just from the available command
line commands.

This patchset adds support for that.  The first three patches add some
minor preliminary setup, followed by the two main patches that add the
ability to create and generate synthetic events from the kernel.  The
next patch adds a test module that demonstrates actual use of the API
and verifies that it works as intended, followed by Documentation.

Special thanks to Artem Bityutskiy, who worked with me over several
iterations of the API, and who had many great suggestions on the
details of the interface, and pointed out several problems with the
code itself.

The following changes since commit 659ded30272d67a04b3692f0bfa12263be20d790:

  trace/kprobe: Remove unused MAX_KPROBE_CMDLINE_SIZE (2020-01-22 07:07:38 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.git ftrace/synth-event-gen-v3

Tom Zanussi (12):
  tracing: Add trace_array_find/_get() to find instance trace arrays
  tracing: Add trace_get/put_event_file()
  tracing: Add synth_event_delete()
  tracing: Add dynamic event command creation interface
  tracing: Add synthetic event command generation functions
  tracing: Change trace_boot to use synth_event interface
  tracing: Add synth_event_trace() and related functions
  tracing: Add synth event generation test module
  tracing: Add kprobe event command generation functions
  tracing: Change trace_boot to use kprobe_event interface
  tracing: Add kprobe event command generation test module
  tracing: Documentation for in-kernel synthetic event API

 Documentation/trace/events.rst       | 515 ++++++++++++++++++++
 include/linux/trace_events.h         | 124 +++++
 kernel/trace/Kconfig                 |  25 +
 kernel/trace/Makefile                |   2 +
 kernel/trace/kprobe_event_gen_test.c | 225 +++++++++
 kernel/trace/synth_event_gen_test.c  | 523 ++++++++++++++++++++
 kernel/trace/trace.c                 |  43 +-
 kernel/trace/trace.h                 |  36 ++
 kernel/trace/trace_boot.c            |  66 ++-
 kernel/trace/trace_events.c          | 325 +++++++++++++
 kernel/trace/trace_events_hist.c     | 896 ++++++++++++++++++++++++++++++++++-
 kernel/trace/trace_kprobe.c          | 160 ++++++-
 12 files changed, 2868 insertions(+), 72 deletions(-)
 create mode 100644 kernel/trace/kprobe_event_gen_test.c
 create mode 100644 kernel/trace/synth_event_gen_test.c

-- 
2.14.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 01/12] tracing: Add trace_array_find/_get() to find instance trace arrays
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 02/12] tracing: Add trace_get/put_event_file() Tom Zanussi
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add a new trace_array_find() function that can be used to find a trace
array given the instance name, and replace existing code that does the
same thing with it.  Also add trace_array_find_get() which does the
same but returns the trace array after upping its refcount.

Also make both available for use outside of trace.c.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 kernel/trace/trace.c | 43 +++++++++++++++++++++++++++++++++----------
 kernel/trace/trace.h |  2 ++
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d1410b4462ac..2d4b70bd85a2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8501,6 +8501,34 @@ static void update_tracer_options(struct trace_array *tr)
 	mutex_unlock(&trace_types_lock);
 }
 
+/* Must have trace_types_lock held */
+struct trace_array *trace_array_find(const char *instance)
+{
+	struct trace_array *tr, *found = NULL;
+
+	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
+		if (tr->name && strcmp(tr->name, instance) == 0) {
+			found = tr;
+			break;
+		}
+	}
+
+	return found;
+}
+
+struct trace_array *trace_array_find_get(const char *instance)
+{
+	struct trace_array *tr;
+
+	mutex_lock(&trace_types_lock);
+	tr = trace_array_find(instance);
+	if (tr)
+		tr->ref++;
+	mutex_unlock(&trace_types_lock);
+
+	return tr;
+}
+
 static struct trace_array *trace_array_create(const char *name)
 {
 	struct trace_array *tr;
@@ -8577,10 +8605,8 @@ static int instance_mkdir(const char *name)
 	mutex_lock(&trace_types_lock);
 
 	ret = -EEXIST;
-	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
-		if (tr->name && strcmp(tr->name, name) == 0)
-			goto out_unlock;
-	}
+	if (trace_array_find(name))
+		goto out_unlock;
 
 	tr = trace_array_create(name);
 
@@ -8704,12 +8730,9 @@ static int instance_rmdir(const char *name)
 	mutex_lock(&trace_types_lock);
 
 	ret = -ENODEV;
-	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
-		if (tr->name && strcmp(tr->name, name) == 0) {
-			ret = __remove_instance(tr);
-			break;
-		}
-	}
+	tr = trace_array_find(name);
+	if (tr)
+		ret = __remove_instance(tr);
 
 	mutex_unlock(&trace_types_lock);
 	mutex_unlock(&event_mutex);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 4812a36affac..8fe3b16c8cec 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -346,6 +346,8 @@ extern struct mutex trace_types_lock;
 
 extern int trace_array_get(struct trace_array *tr);
 extern int tracing_check_open_get_tr(struct trace_array *tr);
+extern struct trace_array *trace_array_find(const char *instance);
+extern struct trace_array *trace_array_find_get(const char *instance);
 
 extern int tracing_set_time_stamp_abs(struct trace_array *tr, bool abs);
 extern int tracing_set_clock(struct trace_array *tr, const char *clockstr);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 02/12] tracing: Add trace_get/put_event_file()
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 01/12] tracing: Add trace_array_find/_get() to find instance trace arrays Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-29 17:52   ` Steven Rostedt
  2020-01-24 22:56 ` [PATCH v3 03/12] tracing: Add synth_event_delete() Tom Zanussi
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add a function to get an event file and prevent it from going away on
module or instance removal.

trace_get_event_file() will find an event file in a given instance (if
instance is NULL, it assumes the top trace array) and return it,
pinning the instance's trace array as well as the event's module, if
applicable, so they won't go away while in use.

trace_put_event_file() does the matching release.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 include/linux/trace_events.h |  5 +++
 kernel/trace/trace_events.c  | 85 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 20948ee56f8c..8d621a73c97e 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -349,6 +349,11 @@ enum {
 	EVENT_FILE_FL_WAS_ENABLED_BIT,
 };
 
+extern struct trace_event_file *trace_get_event_file(const char *instance,
+						     const char *system,
+						     const char *event);
+extern void trace_put_event_file(struct trace_event_file *file);
+
 /*
  * Event file flags:
  *  ENABLED	  - The event is enabled
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index dfb736a964d6..402426a82cfb 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2536,6 +2536,91 @@ find_event_file(struct trace_array *tr, const char *system, const char *event)
 	return file;
 }
 
+/**
+ * trace_get_event_file - Find and return a trace event file
+ * @instance: The name of the trace instance containing the event
+ * @system: The name of the system containing the event
+ * @event: The name of the event
+ *
+ * Return a trace event file given the trace instance name, trace
+ * system, and trace event name.  If the instance name is NULL, it
+ * refers to the top-level trace array.
+ *
+ * This function will look it up and return it if found, after calling
+ * trace_array_get() to prevent the instance from going away, and
+ * increment the event's module refcount to prevent it from being
+ * removed.
+ *
+ * To release the file, call trace_put_event_file(), which will call
+ * trace_array_put() and decrement the event's module refcount.
+ *
+ * Return: The trace event on success, ERR_PTR otherwise.
+ */
+struct trace_event_file *trace_get_event_file(const char *instance,
+					      const char *system,
+					      const char *event)
+{
+	struct trace_array *tr = top_trace_array();
+	struct trace_event_file *file = NULL;
+	int ret = -EINVAL;
+
+	if (instance) {
+		tr = trace_array_find_get(instance);
+		if (!tr)
+			return ERR_PTR(-ENOENT);
+	} else {
+		ret = trace_array_get(tr);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
+	mutex_lock(&event_mutex);
+
+	file = find_event_file(tr, system, event);
+	if (!file) {
+		trace_array_put(tr);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Don't let event modules unload while in use */
+	ret = try_module_get(file->event_call->mod);
+	if (!ret) {
+		trace_array_put(tr);
+		ret = -EBUSY;
+		goto out;
+	}
+
+	ret = 0;
+ out:
+	mutex_unlock(&event_mutex);
+
+	if (ret)
+		file = ERR_PTR(ret);
+
+	return file;
+}
+EXPORT_SYMBOL_GPL(trace_get_event_file);
+
+/**
+ * trace_put_event_file - Release a file from trace_get_event_file()
+ * @file: The trace event file
+ *
+ * If a file was retrieved using trace_get_event_file(), this should
+ * be called when it's no longer needed.  It will cancel the previous
+ * trace_array_get() called by that function, and decrement the
+ * event's module refcount.
+ */
+void trace_put_event_file(struct trace_event_file *file)
+{
+	trace_array_put(file->tr);
+
+	mutex_lock(&event_mutex);
+	module_put(file->event_call->mod);
+	mutex_unlock(&event_mutex);
+}
+EXPORT_SYMBOL_GPL(trace_put_event_file);
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 /* Avoid typos */
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 03/12] tracing: Add synth_event_delete()
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 01/12] tracing: Add trace_array_find/_get() to find instance trace arrays Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 02/12] tracing: Add trace_get/put_event_file() Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 04/12] tracing: Add dynamic event command creation interface Tom Zanussi
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

create_or_delete_synth_event() contains code to delete a synthetic
event, which would be useful on its own - specifically, it would be
useful to allow event-creating modules to call it separately.

Separate out the delete code from that function and create an exported
function named synth_event_delete().

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 include/linux/trace_events.h     |  2 ++
 kernel/trace/trace_events_hist.c | 57 +++++++++++++++++++++++++++++-----------
 2 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 8d621a73c97e..25fe743bcbaf 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -354,6 +354,8 @@ extern struct trace_event_file *trace_get_event_file(const char *instance,
 						     const char *event);
 extern void trace_put_event_file(struct trace_event_file *file);
 
+extern int synth_event_delete(const char *name);
+
 /*
  * Event file flags:
  *  ENABLED	  - The event is enabled
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 117a1202a6b9..94ee1e576a1e 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1354,29 +1354,54 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
 	goto out;
 }
 
+static int destroy_synth_event(struct synth_event *se)
+{
+	int ret;
+
+	if (se->ref)
+		ret = -EBUSY;
+	else {
+		ret = unregister_synth_event(se);
+		if (!ret) {
+			dyn_event_remove(&se->devent);
+			free_synth_event(se);
+		}
+	}
+
+	return ret;
+}
+
+/**
+ * synth_event_delete - Delete a synthetic event
+ * @event_name: The name of the new sythetic event
+ *
+ * Delete a synthetic event that was created with synth_event_create().
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int synth_event_delete(const char *event_name)
+{
+	struct synth_event *se = NULL;
+	int ret = -ENOENT;
+
+	mutex_lock(&event_mutex);
+	se = find_synth_event(event_name);
+	if (se)
+		ret = destroy_synth_event(se);
+	mutex_unlock(&event_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_delete);
+
 static int create_or_delete_synth_event(int argc, char **argv)
 {
 	const char *name = argv[0];
-	struct synth_event *event = NULL;
 	int ret;
 
 	/* trace_run_command() ensures argc != 0 */
 	if (name[0] == '!') {
-		mutex_lock(&event_mutex);
-		event = find_synth_event(name + 1);
-		if (event) {
-			if (event->ref)
-				ret = -EBUSY;
-			else {
-				ret = unregister_synth_event(event);
-				if (!ret) {
-					dyn_event_remove(&event->devent);
-					free_synth_event(event);
-				}
-			}
-		} else
-			ret = -ENOENT;
-		mutex_unlock(&event_mutex);
+		ret = synth_event_delete(name + 1);
 		return ret;
 	}
 
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 04/12] tracing: Add dynamic event command creation interface
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (2 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 03/12] tracing: Add synth_event_delete() Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-27 13:44   ` Masami Hiramatsu
  2020-01-29 18:00   ` Steven Rostedt
  2020-01-24 22:56 ` [PATCH v3 05/12] tracing: Add synthetic event command generation functions Tom Zanussi
                   ` (8 subsequent siblings)
  12 siblings, 2 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add an interface used to build up dynamic event creation commands,
such as synthetic and kprobe events.  Interfaces specific to those
particular types of events and others can be built on top of this
interface.

Command creation is started by first using the dynevent_cmd_init()
function to initialize the dynevent_cmd object.  Following that, args
are appended and optionally checked by the dynevent_arg_add() and
dynevent_arg_pair_add() functions, which use objects representing
arguments and pairs of arguments, initialized respectively by
dynevent_arg_init() and dynevent_arg_pair_init().  Finally, once all
args have been successfully added, the command is finalized and
actually created using dynevent_create().

The code here for actually printing into the dyn_event->cmd buffer
using snprintf() etc was adapted from v4 of Masami's 'tracing/boot:
Add synthetic event support' patch.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/trace_events.h |  23 +++++
 kernel/trace/trace.h         |  34 ++++++
 kernel/trace/trace_events.c  | 240 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 297 insertions(+)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 25fe743bcbaf..651b03d5e272 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -354,6 +354,29 @@ extern struct trace_event_file *trace_get_event_file(const char *instance,
 						     const char *event);
 extern void trace_put_event_file(struct trace_event_file *file);
 
+#define MAX_DYNEVENT_CMD_LEN	(2048)
+
+enum dynevent_type {
+	DYNEVENT_TYPE_NONE,
+};
+
+struct dynevent_cmd;
+
+typedef int (*dynevent_create_fn_t)(struct dynevent_cmd *cmd);
+
+struct dynevent_cmd {
+	char			*buf;
+	const char		*event_name;
+	int			maxlen;
+	int			remaining;
+	unsigned int		n_fields;
+	enum dynevent_type	type;
+	dynevent_create_fn_t	run_command;
+	void			*private_data;
+};
+
+extern int dynevent_create(struct dynevent_cmd *cmd);
+
 extern int synth_event_delete(const char *name);
 
 /*
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 8fe3b16c8cec..0e8afc32d972 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1877,6 +1877,40 @@ static inline bool event_command_needs_rec(struct event_command *cmd_ops)
 
 extern int trace_event_enable_disable(struct trace_event_file *file,
 				      int enable, int soft_disable);
+
+extern void dynevent_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen,
+			      enum dynevent_type type,
+			      dynevent_create_fn_t run_command);
+
+typedef int (*dynevent_check_arg_fn_t)(void *data);
+
+struct dynevent_arg {
+	const char		*str;
+	char			separator; /* e.g. ';', ',', or nothing */
+	dynevent_check_arg_fn_t	check_arg;
+};
+
+extern void dynevent_arg_init(struct dynevent_arg *arg,
+			      dynevent_check_arg_fn_t check_arg,
+			      char separator);
+extern int dynevent_arg_add(struct dynevent_cmd *cmd,
+			    struct dynevent_arg *arg);
+
+struct dynevent_arg_pair {
+	const char		*lhs;
+	const char		*rhs;
+	char			operator; /* e.g. '=' or nothing */
+	char			separator; /* e.g. ';', ',', or nothing */
+	dynevent_check_arg_fn_t	check_arg;
+};
+
+extern void dynevent_arg_pair_init(struct dynevent_arg_pair *arg_pair,
+				   dynevent_check_arg_fn_t check_arg,
+				   char operator, char separator);
+extern int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
+				 struct dynevent_arg_pair *arg_pair);
+extern int dynevent_str_add(struct dynevent_cmd *cmd, const char *str);
+
 extern int tracing_alloc_snapshot(void);
 extern void tracing_snapshot_cond(struct trace_array *tr, void *cond_data);
 extern int tracing_snapshot_cond_enable(struct trace_array *tr, void *cond_data, cond_update_fn_t update);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 402426a82cfb..aa56bb6ff6ca 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -3300,6 +3300,246 @@ void __init trace_event_init(void)
 	event_trace_enable();
 }
 
+/**
+ * dynevent_arg_add - Add an arg to a dynevent_cmd
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd
+ * @arg: The argument to append to the current cmd
+ *
+ * Append an argument to a dynevent_cmd.  The argument string will be
+ * appended to the current cmd string, followed by a separator, if
+ * applicable.  Before the argument is added, the check_arg()
+ * function, if defined, is called.
+ *
+ * The cmd string, separator, and check_arg() function should be set
+ * using the dynevent_arg_init() before any arguments are added using
+ * this function.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int dynevent_arg_add(struct dynevent_cmd *cmd,
+		     struct dynevent_arg *arg)
+{
+	int ret = 0;
+	int delta;
+	char *q;
+
+	if (arg->check_arg) {
+		ret = arg->check_arg(arg);
+		if (ret)
+			return ret;
+	}
+
+	q = cmd->buf + (cmd->maxlen - cmd->remaining);
+
+	delta = snprintf(q, cmd->remaining, " %s%c", arg->str, arg->separator);
+	if (delta >= cmd->remaining) {
+		pr_err("String is too long: %s\n", arg->str);
+		return -E2BIG;
+	}
+	cmd->remaining -= delta;
+
+	return ret;
+}
+
+/**
+ * dynevent_arg_pair_add - Add an arg pair to a dynevent_cmd
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd
+ * @arg_pair: The argument pair to append to the current cmd
+ *
+ * Append an argument pair to a dynevent_cmd.  An argument pair
+ * consists of a left-hand-side argument and a right-hand-side
+ * argument separated by an operator, which can be whitespace, all
+ * followed by a separator, if applicable.  This can be used to add
+ * arguments of the form 'type variable_name;' or 'x+y'.
+ *
+ * The lhs argument string will be appended to the current cmd string,
+ * followed by an operator, if applicable, followd by the rhs string,
+ * followed finally by a separator, if applicable.  Before anything is
+ * added, the check_arg() function, if defined, is called.
+ *
+ * The cmd strings, operator, separator, and check_arg() function
+ * should be set using the dynevent_arg_pair_init() before any arguments
+ * are added using this function.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
+			  struct dynevent_arg_pair *arg_pair)
+{
+	int ret = 0;
+	int delta;
+	char *q;
+
+	if (arg_pair->check_arg) {
+		ret = arg_pair->check_arg(arg_pair);
+		if (ret)
+			return ret;
+	}
+
+	q = cmd->buf + (cmd->maxlen - cmd->remaining);
+
+	delta = snprintf(q, cmd->remaining, " %s%c", arg_pair->lhs,
+			 arg_pair->operator);
+	if (delta >= cmd->remaining) {
+		pr_err("field string is too long: %s\n", arg_pair->lhs);
+		return -E2BIG;
+	}
+	cmd->remaining -= delta; q += delta;
+
+	delta = snprintf(q, cmd->remaining, "%s%c", arg_pair->rhs,
+			 arg_pair->separator);
+	if (delta >= cmd->remaining) {
+		pr_err("field string is too long: %s\n", arg_pair->rhs);
+		return -E2BIG;
+	}
+	cmd->remaining -= delta; q += delta;
+
+	return ret;
+}
+
+/**
+ * dynevent_str_add - Add a string to a dynevent_cmd
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd
+ * @str: The string to append to the current cmd
+ *
+ * Append a string to a dynevent_cmd.  The string will be appended to
+ * the current cmd string as-is, with nothing prepended or appended.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int dynevent_str_add(struct dynevent_cmd *cmd, const char *str)
+{
+	int ret = 0;
+	int delta;
+	char *q;
+
+	q = cmd->buf + (cmd->maxlen - cmd->remaining);
+
+	delta = snprintf(q, cmd->remaining, "%s", str);
+	if (delta >= cmd->remaining) {
+		pr_err("String is too long: %s\n", str);
+		return -E2BIG;
+	}
+	cmd->remaining -= delta;
+
+	return ret;
+}
+
+/**
+ * dynevent_cmd_init - Initialize a dynevent_cmd object
+ * @cmd: A pointer to the dynevent_cmd struct representing the cmd
+ * @buf: A pointer to the buffer to generate the command into
+ * @maxlen: The length of the buffer the command will be generated into
+ * @type: The type of the cmd, checked against further operations
+ * @run_command: The type-specific function that will actually run the command
+ *
+ * Initialize a dynevent_cmd.  A dynevent_cmd is used to build up and
+ * run dynamic event creation commands, such as commands for creating
+ * synthetic and kprobe events.  Before calling any of the functions
+ * used to build the command, a dynevent_cmd object should be
+ * instantiated and initialized using this function.
+ *
+ * The initialization sets things up by saving a pointer to the
+ * user-supplied buffer and its length via the @buf and @maxlen
+ * params, and by saving the cmd-specific @type and @run_command
+ * params which are used to check subsequent dynevent_cmd operations
+ * and actually run the command when complete.
+ */
+void dynevent_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen,
+		       enum dynevent_type type,
+		       dynevent_create_fn_t run_command)
+{
+	memset(cmd, '\0', sizeof(*cmd));
+
+	cmd->buf = buf;
+	cmd->maxlen = maxlen;
+	cmd->remaining = cmd->maxlen;
+	cmd->type = type;
+	cmd->run_command = run_command;
+}
+
+/**
+ * dynevent_arg_init - Initialize a dynevent_arg object
+ * @arg: A pointer to the dynevent_arg struct representing the arg
+ * @check_arg: An (optional) pointer to a function checking arg sanity
+ * @separator: An (optional) separator, appended after adding the arg
+ *
+ * Initialize a dynevent_arg object.  A dynevent_arg represents an
+ * object used to append single arguments to the current command
+ * string.  The @check_arg function, if present, will be used to check
+ * the sanity of the current arg string (which is directly set by the
+ * caller).  After the arg string is successfully appended to the
+ * command string, the optional @separator is appended.  If no
+ * separator was specified when initializing the arg, a space will be
+ * appended.
+ */
+void dynevent_arg_init(struct dynevent_arg *arg,
+		       dynevent_check_arg_fn_t check_arg,
+		       char separator)
+{
+	memset(arg, '\0', sizeof(*arg));
+
+	if (!separator)
+		separator = ' ';
+	arg->separator = separator;
+
+	arg->check_arg = check_arg;
+}
+
+/**
+ * dynevent_arg_pair_init - Initialize a dynevent_arg_pair object
+ * @arg_pair: A pointer to the dynevent_arg_pair struct representing the arg
+ * @check_arg: An (optional) pointer to a function checking arg sanity
+ * @operator: An (optional) operator, appended after adding the first arg
+ * @separator: An (optional) separator, appended after adding the second arg
+ *
+ * Initialize a dynevent_arg_pair object.  A dynevent_arg_pair
+ * represents an object used to append argument pairs such as 'type
+ * variable_name;' or 'x+y' to the current command string.  An
+ * argument pair consists of a left-hand-side argument and a
+ * right-hand-side argument separated by an operator, which can be
+ * whitespace, all followed by a separator, if applicable. The
+ * @check_arg function, if present, will be used to check the sanity
+ * of the current arg strings (which is directly set by the caller).
+ * After the first arg string is successfully appended to the command
+ * string, the optional @operator is appended, followed by the second
+ * arg and and optional @separator.  If no separator was specified
+ * when initializing the arg, a space will be appended.
+ */
+void dynevent_arg_pair_init(struct dynevent_arg_pair *arg_pair,
+			    dynevent_check_arg_fn_t check_arg,
+			    char operator, char separator)
+{
+	memset(arg_pair, '\0', sizeof(*arg_pair));
+
+	if (!operator)
+		operator = ' ';
+	arg_pair->operator = operator;
+
+	if (!separator)
+		separator = ' ';
+	arg_pair->separator = separator;
+
+	arg_pair->check_arg = check_arg;
+}
+
+/**
+ * dynevent_create - Create the dynamic event contained in dynevent_cmd
+ * @cmd: The dynevent_cmd object containing the dynamic event creation command
+ *
+ * Once a dynevent_cmd object has been successfully built up via the
+ * dynevent_cmd_init(), dynevent_arg_add() and dynevent_arg_pair_add()
+ * functions, this function runs the final command to actually create
+ * the event.
+ *
+ * Return: 0 if the event was successfully created, error otherwise.
+ */
+int dynevent_create(struct dynevent_cmd *cmd)
+{
+	return cmd->run_command(cmd);
+}
+EXPORT_SYMBOL_GPL(dynevent_create);
+
 #ifdef CONFIG_EVENT_TRACE_STARTUP_TEST
 
 static DEFINE_SPINLOCK(test_spinlock);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 05/12] tracing: Add synthetic event command generation functions
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (3 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 04/12] tracing: Add dynamic event command creation interface Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 06/12] tracing: Change trace_boot to use synth_event interface Tom Zanussi
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add functions used to generate synthetic event commands, built on top
of the dynevent_cmd interface.

synth_event_gen_cmd_start() is used to create a synthetic event
command using a variable arg list and
synth_event_gen_cmd_array_start() does the same thing but using an
array of field descriptors.  synth_event_add_field(),
synth_event_add_field_str() and synth_event_add_fields() can be used
to add single fields one by one or as a group.  Once all desired
fields are added, synth_event_gen_cmd_end() is used to actually
execute the command and create the event.

synth_event_create() does everything, including creating the event, in
a single call.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 include/linux/trace_events.h     |  37 ++++
 kernel/trace/trace_events_hist.c | 379 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 412 insertions(+), 4 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 651b03d5e272..07b83532a3c6 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -357,6 +357,7 @@ extern void trace_put_event_file(struct trace_event_file *file);
 #define MAX_DYNEVENT_CMD_LEN	(2048)
 
 enum dynevent_type {
+	DYNEVENT_TYPE_SYNTH = 1,
 	DYNEVENT_TYPE_NONE,
 };
 
@@ -379,6 +380,42 @@ extern int dynevent_create(struct dynevent_cmd *cmd);
 
 extern int synth_event_delete(const char *name);
 
+extern void synth_event_cmd_init(struct dynevent_cmd *cmd,
+				 char *buf, int maxlen);
+
+extern int __synth_event_gen_cmd_start(struct dynevent_cmd *cmd,
+				       const char *name,
+				       struct module *mod, ...);
+
+#define synth_event_gen_cmd_start(cmd, name, mod, ...)	\
+	__synth_event_gen_cmd_start(cmd, name, mod, ## __VA_ARGS__, NULL)
+
+struct synth_field_desc {
+	const char *type;
+	const char *name;
+};
+
+extern int synth_event_gen_cmd_array_start(struct dynevent_cmd *cmd,
+					   const char *name,
+					   struct module *mod,
+					   struct synth_field_desc *fields,
+					   unsigned int n_fields);
+extern int synth_event_create(const char *name,
+			      struct synth_field_desc *fields,
+			      unsigned int n_fields, struct module *mod);
+
+extern int synth_event_add_field(struct dynevent_cmd *cmd,
+				 const char *type,
+				 const char *name);
+extern int synth_event_add_field_str(struct dynevent_cmd *cmd,
+				     const char *type_name);
+extern int synth_event_add_fields(struct dynevent_cmd *cmd,
+				  struct synth_field_desc *fields,
+				  unsigned int n_fields);
+
+#define synth_event_gen_cmd_end(cmd)	\
+	dynevent_create(cmd)
+
 /*
  * Event file flags:
  *  ENABLED	  - The event is enabled
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 94ee1e576a1e..2e7181ce51aa 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -374,7 +374,7 @@ struct hist_trigger_data {
 	unsigned int			n_save_var_str;
 };
 
-static int synth_event_create(int argc, const char **argv);
+static int create_synth_event(int argc, const char **argv);
 static int synth_event_show(struct seq_file *m, struct dyn_event *ev);
 static int synth_event_release(struct dyn_event *ev);
 static bool synth_event_is_busy(struct dyn_event *ev);
@@ -382,7 +382,7 @@ static bool synth_event_match(const char *system, const char *event,
 			int argc, const char **argv, struct dyn_event *ev);
 
 static struct dyn_event_operations synth_event_ops = {
-	.create = synth_event_create,
+	.create = create_synth_event,
 	.show = synth_event_show,
 	.is_busy = synth_event_is_busy,
 	.free = synth_event_release,
@@ -407,6 +407,7 @@ struct synth_event {
 	struct trace_event_class		class;
 	struct trace_event_call			call;
 	struct tracepoint			*tp;
+	struct module				*mod;
 };
 
 static bool is_synth_event(struct dyn_event *ev)
@@ -1286,6 +1287,273 @@ struct hist_var_data {
 	struct hist_trigger_data *hist_data;
 };
 
+static int synth_event_check_arg_fn(void *data)
+{
+	struct dynevent_arg_pair *arg_pair = data;
+	int size;
+
+	size = synth_field_size((char *)arg_pair->lhs);
+
+	return size ? 0 : -EINVAL;
+}
+
+/**
+ * synth_event_add_field - Add a new field to a synthetic event cmd
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @type: The type of the new field to add
+ * @name: The name of the new field to add
+ *
+ * Add a new field to a synthetic event cmd object.  Field ordering is in
+ * the same order the fields are added.
+ *
+ * See synth_field_size() for available types. If field_name contains
+ * [n] the field is considered to be an array.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int synth_event_add_field(struct dynevent_cmd *cmd, const char *type,
+			  const char *name)
+{
+	struct dynevent_arg_pair arg_pair;
+	int ret;
+
+	if (cmd->type != DYNEVENT_TYPE_SYNTH)
+		return -EINVAL;
+
+	if (!type || !name)
+		return -EINVAL;
+
+	dynevent_arg_pair_init(&arg_pair, synth_event_check_arg_fn, 0, ';');
+
+	arg_pair.lhs = type;
+	arg_pair.rhs = name;
+
+	ret = dynevent_arg_pair_add(cmd, &arg_pair);
+	if (ret)
+		return ret;
+
+	if (++cmd->n_fields > SYNTH_FIELDS_MAX)
+		ret = -EINVAL;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_add_field);
+
+/**
+ * synth_event_add_field_str - Add a new field to a synthetic event cmd
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @type_name: The type and name of the new field to add, as a single string
+ *
+ * Add a new field to a synthetic event cmd object, as a single
+ * string.  The @type_name string is expected to be of the form 'type
+ * name', which will be appended by ';'.  No sanity checking is done -
+ * what's passed in is assumed to already be well-formed.  Field
+ * ordering is in the same order the fields are added.
+ *
+ * See synth_field_size() for available types. If field_name contains
+ * [n] the field is considered to be an array.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int synth_event_add_field_str(struct dynevent_cmd *cmd, const char *type_name)
+{
+	struct dynevent_arg arg;
+	int ret;
+
+	if (cmd->type != DYNEVENT_TYPE_SYNTH)
+		return -EINVAL;
+
+	if (!type_name)
+		return -EINVAL;
+
+	dynevent_arg_init(&arg, NULL, ';');
+
+	arg.str = type_name;
+
+	ret = dynevent_arg_add(cmd, &arg);
+	if (ret)
+		return ret;
+
+	if (++cmd->n_fields > SYNTH_FIELDS_MAX)
+		ret = -EINVAL;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_add_field_str);
+
+/**
+ * synth_event_add_fields - Add multiple fields to a synthetic event cmd
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @fields: An array of type/name field descriptions
+ * @n_fields: The number of field descriptions contained in the fields array
+ *
+ * Add a new set of fields to a synthetic event cmd object.  The event
+ * fields that will be defined for the event should be passed in as an
+ * array of struct synth_field_desc, and the number of elements in the
+ * array passed in as n_fields.  Field ordering will retain the
+ * ordering given in the fields array.
+ *
+ * See synth_field_size() for available types. If field_name contains
+ * [n] the field is considered to be an array.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int synth_event_add_fields(struct dynevent_cmd *cmd,
+			   struct synth_field_desc *fields,
+			   unsigned int n_fields)
+{
+	unsigned int i;
+	int ret = 0;
+
+	for (i = 0; i < n_fields; i++) {
+		if (fields[i].type == NULL || fields[i].name == NULL) {
+			ret = -EINVAL;
+			break;
+		}
+
+		ret = synth_event_add_field(cmd, fields[i].type, fields[i].name);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_add_fields);
+
+/**
+ * __synth_event_gen_cmd_start - Start a synthetic event command from arg list
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @name: The name of the synthetic event
+ * @mod: The module creating the event, NULL if not created from a module
+ * @args: Variable number of arg (pairs), one pair for each field
+ *
+ * NOTE: Users normally won't want to call this function directly, but
+ * rather use the synth_event_gen_cmd_start() wrapper, which
+ * automatically adds a NULL to the end of the arg list.  If this
+ * function is used directly, make sure the last arg in the variable
+ * arg list is NULL.
+ *
+ * Generate a synthetic event command to be executed by
+ * synth_event_gen_cmd_end().  This function can be used to generate
+ * the complete command or only the first part of it; in the latter
+ * case, synth_event_add_field(), synth_event_add_field_str(), or
+ * synth_event_add_fields() can be used to add more fields following
+ * this.
+ *
+ * There should be an even number variable args, each pair consisting
+ * of a type followed by a field name.
+ *
+ * See synth_field_size() for available types. If field_name contains
+ * [n] the field is considered to be an array.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int __synth_event_gen_cmd_start(struct dynevent_cmd *cmd, const char *name,
+				struct module *mod, ...)
+{
+	struct dynevent_arg arg;
+	va_list args;
+	int ret;
+
+	cmd->event_name = name;
+	cmd->private_data = mod;
+
+	if (cmd->type != DYNEVENT_TYPE_SYNTH)
+		return -EINVAL;
+
+	dynevent_arg_init(&arg, NULL, 0);
+	arg.str = name;
+	ret = dynevent_arg_add(cmd, &arg);
+	if (ret)
+		return ret;
+
+	va_start(args, mod);
+	for (;;) {
+		const char *type, *name;
+
+		type = va_arg(args, const char *);
+		if (!type)
+			break;
+		name = va_arg(args, const char *);
+		if (!name)
+			break;
+
+		if (++cmd->n_fields > SYNTH_FIELDS_MAX) {
+			ret = -EINVAL;
+			break;
+		}
+
+		ret = synth_event_add_field(cmd, type, name);
+		if (ret)
+			break;
+	}
+	va_end(args);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__synth_event_gen_cmd_start);
+
+/**
+ * synth_event_gen_cmd_array_start - Start synthetic event command from an array
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @name: The name of the synthetic event
+ * @fields: An array of type/name field descriptions
+ * @n_fields: The number of field descriptions contained in the fields array
+ *
+ * Generate a synthetic event command to be executed by
+ * synth_event_gen_cmd_end().  This function can be used to generate
+ * the complete command or only the first part of it; in the latter
+ * case, synth_event_add_field(), synth_event_add_field_str(), or
+ * synth_event_add_fields() can be used to add more fields following
+ * this.
+ *
+ * The event fields that will be defined for the event should be
+ * passed in as an array of struct synth_field_desc, and the number of
+ * elements in the array passed in as n_fields.  Field ordering will
+ * retain the ordering given in the fields array.
+ *
+ * See synth_field_size() for available types. If field_name contains
+ * [n] the field is considered to be an array.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int synth_event_gen_cmd_array_start(struct dynevent_cmd *cmd, const char *name,
+				    struct module *mod,
+				    struct synth_field_desc *fields,
+				    unsigned int n_fields)
+{
+	struct dynevent_arg arg;
+	unsigned int i;
+	int ret = 0;
+
+	cmd->event_name = name;
+	cmd->private_data = mod;
+
+	if (cmd->type != DYNEVENT_TYPE_SYNTH)
+		return -EINVAL;
+
+	if (n_fields > SYNTH_FIELDS_MAX)
+		return -EINVAL;
+
+	dynevent_arg_init(&arg, NULL, 0);
+	arg.str = name;
+	ret = dynevent_arg_add(cmd, &arg);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < n_fields; i++) {
+		if (fields[i].type == NULL || fields[i].name == NULL)
+			return -EINVAL;
+
+		ret = synth_event_add_field(cmd, fields[i].type, fields[i].name);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_gen_cmd_array_start);
+
 static int __create_synth_event(int argc, const char *name, const char **argv)
 {
 	struct synth_field *field, *fields[SYNTH_FIELDS_MAX];
@@ -1354,6 +1622,56 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
 	goto out;
 }
 
+/**
+ * synth_event_create - Create a new synthetic event
+ * @name: The name of the new sythetic event
+ * @fields: An array of type/name field descriptions
+ * @n_fields: The number of field descriptions contained in the fields array
+ * @mod: The module creating the event, NULL if not created from a module
+ *
+ * Create a new synthetic event with the given name under the
+ * trace/events/synthetic/ directory.  The event fields that will be
+ * defined for the event should be passed in as an array of struct
+ * synth_field_desc, and the number elements in the array passed in as
+ * n_fields. Field ordering will retain the ordering given in the
+ * fields array.
+ *
+ * If the new synthetic event is being created from a module, the mod
+ * param must be non-NULL.  This will ensure that the trace buffer
+ * won't contain unreadable events.
+ *
+ * The new synth event should be deleted using synth_event_delete()
+ * function.  The new synthetic event can be generated from modules or
+ * other kernel code using trace_synth_event() and related functions.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int synth_event_create(const char *name, struct synth_field_desc *fields,
+		       unsigned int n_fields, struct module *mod)
+{
+	struct dynevent_cmd cmd;
+	char *buf;
+	int ret;
+
+	buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	synth_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+	ret = synth_event_gen_cmd_array_start(&cmd, name, mod,
+					      fields, n_fields);
+	if (ret)
+		goto out;
+
+	ret = synth_event_gen_cmd_end(&cmd);
+ out:
+	kfree(buf);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_create);
+
 static int destroy_synth_event(struct synth_event *se)
 {
 	int ret;
@@ -1382,14 +1700,33 @@ static int destroy_synth_event(struct synth_event *se)
 int synth_event_delete(const char *event_name)
 {
 	struct synth_event *se = NULL;
+	struct module *mod = NULL;
 	int ret = -ENOENT;
 
 	mutex_lock(&event_mutex);
 	se = find_synth_event(event_name);
-	if (se)
+	if (se) {
+		mod = se->mod;
 		ret = destroy_synth_event(se);
+	}
 	mutex_unlock(&event_mutex);
 
+	if (mod) {
+		mutex_lock(&trace_types_lock);
+		/*
+		 * It is safest to reset the ring buffer if the module
+		 * being unloaded registered any events that were
+		 * used. The only worry is if a new module gets
+		 * loaded, and takes on the same id as the events of
+		 * this module. When printing out the buffer, traced
+		 * events left over from this module may be passed to
+		 * the new module events and unexpected results may
+		 * occur.
+		 */
+		tracing_reset_all_online_cpus();
+		mutex_unlock(&trace_types_lock);
+	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(synth_event_delete);
@@ -1414,7 +1751,41 @@ int synth_event_run_command(const char *command)
 	return trace_run_command(command, create_or_delete_synth_event);
 }
 
-static int synth_event_create(int argc, const char **argv)
+static int synth_event_run_cmd(struct dynevent_cmd *cmd)
+{
+	struct synth_event *se;
+	int ret;
+
+	ret = trace_run_command(cmd->buf, create_or_delete_synth_event);
+	if (ret)
+		return ret;
+
+	se = find_synth_event(cmd->event_name);
+	if (WARN_ON(!se))
+		return -ENOENT;
+
+	se->mod = cmd->private_data;
+
+	return ret;
+}
+
+/**
+ * synth_event_cmd_init - Initialize a synthetic event command object
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @buf: A pointer to the buffer used to build the command
+ * @maxlen: The length of the buffer passed in @buf
+ *
+ * Initialize a synthetic event command object.  Use this before
+ * calling any of the other dyenvent_cmd functions.
+ */
+void synth_event_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen)
+{
+	dynevent_cmd_init(cmd, buf, maxlen, DYNEVENT_TYPE_SYNTH,
+			  synth_event_run_cmd);
+}
+EXPORT_SYMBOL_GPL(synth_event_cmd_init);
+
+static int create_synth_event(int argc, const char **argv)
 {
 	const char *name = argv[0];
 	int len;
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 06/12] tracing: Change trace_boot to use synth_event interface
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (4 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 05/12] tracing: Add synthetic event command generation functions Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 07/12] tracing: Add synth_event_trace() and related functions Tom Zanussi
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Have trace_boot_add_synth_event() use the synth_event interface.

Also, rename synth_event_run_cmd() to synth_event_run_command() now
that trace_boot's version is gone.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 kernel/trace/trace_boot.c        | 31 ++++++++++++-------------------
 kernel/trace/trace_events_hist.c |  9 ++-------
 2 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index cd541ac1cbc1..33665718008c 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -133,38 +133,31 @@ trace_boot_add_kprobe_event(struct xbc_node *node, const char *event)
 #endif
 
 #ifdef CONFIG_HIST_TRIGGERS
-extern int synth_event_run_command(const char *command);
-
 static int __init
 trace_boot_add_synth_event(struct xbc_node *node, const char *event)
 {
+	struct dynevent_cmd cmd;
 	struct xbc_node *anode;
-	char buf[MAX_BUF_LEN], *q;
+	char buf[MAX_BUF_LEN];
 	const char *p;
-	int len, delta, ret;
+	int ret;
 
-	len = ARRAY_SIZE(buf);
-	delta = snprintf(buf, len, "%s", event);
-	if (delta >= len) {
-		pr_err("Event name is too long: %s\n", event);
-		return -E2BIG;
-	}
-	len -= delta; q = buf + delta;
+	synth_event_cmd_init(&cmd, buf, MAX_BUF_LEN);
+
+	ret = synth_event_gen_cmd_start(&cmd, event, NULL);
+	if (ret)
+		return ret;
 
 	xbc_node_for_each_array_value(node, "fields", anode, p) {
-		delta = snprintf(q, len, " %s;", p);
-		if (delta >= len) {
-			pr_err("fields string is too long: %s\n", p);
-			return -E2BIG;
-		}
-		len -= delta; q += delta;
+		ret = synth_event_add_field_str(&cmd, p);
+		if (ret)
+			return ret;
 	}
 
-	ret = synth_event_run_command(buf);
+	ret = synth_event_gen_cmd_end(&cmd);
 	if (ret < 0)
 		pr_err("Failed to add synthetic event: %s\n", buf);
 
-
 	return ret;
 }
 #else
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 2e7181ce51aa..41a82f5c5be5 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1746,12 +1746,7 @@ static int create_or_delete_synth_event(int argc, char **argv)
 	return ret == -ECANCELED ? -EINVAL : ret;
 }
 
-int synth_event_run_command(const char *command)
-{
-	return trace_run_command(command, create_or_delete_synth_event);
-}
-
-static int synth_event_run_cmd(struct dynevent_cmd *cmd)
+static int synth_event_run_command(struct dynevent_cmd *cmd)
 {
 	struct synth_event *se;
 	int ret;
@@ -1781,7 +1776,7 @@ static int synth_event_run_cmd(struct dynevent_cmd *cmd)
 void synth_event_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen)
 {
 	dynevent_cmd_init(cmd, buf, maxlen, DYNEVENT_TYPE_SYNTH,
-			  synth_event_run_cmd);
+			  synth_event_run_command);
 }
 EXPORT_SYMBOL_GPL(synth_event_cmd_init);
 
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 07/12] tracing: Add synth_event_trace() and related functions
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (5 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 06/12] tracing: Change trace_boot to use synth_event interface Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 08/12] tracing: Add synth event generation test module Tom Zanussi
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add an exported function named synth_event_trace(), allowing modules
or other kernel code to trace synthetic events.

Also added are several functions that allow the same functionality to
be broken out in a piecewise fashion, which are useful in situations
where tracing an event from a full array of values would be
cumbersome.  Those functions are synth_event_trace_start/end() and
synth_event_add_(next)_val().

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 include/linux/trace_events.h     |  26 +++
 kernel/trace/trace_events_hist.c | 463 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 489 insertions(+)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 07b83532a3c6..bf03d12efb28 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -416,6 +416,32 @@ extern int synth_event_add_fields(struct dynevent_cmd *cmd,
 #define synth_event_gen_cmd_end(cmd)	\
 	dynevent_create(cmd)
 
+struct synth_event;
+
+struct synth_event_trace_state {
+	struct trace_event_buffer fbuffer;
+	struct synth_trace_event *entry;
+	struct trace_buffer *buffer;
+	struct synth_event *event;
+	unsigned int cur_field;
+	unsigned int n_u64;
+	bool enabled;
+	bool add_next;
+	bool add_name;
+};
+
+extern int synth_event_trace(struct trace_event_file *file,
+			     unsigned int n_vals, ...);
+extern int synth_event_trace_array(struct trace_event_file *file, u64 *vals,
+				   unsigned int n_vals);
+extern int synth_event_trace_start(struct trace_event_file *file,
+				   struct synth_event_trace_state *trace_state);
+extern int synth_event_add_next_val(u64 val,
+				    struct synth_event_trace_state *trace_state);
+extern int synth_event_add_val(const char *field_name, u64 val,
+			       struct synth_event_trace_state *trace_state);
+extern int synth_event_trace_end(struct synth_event_trace_state *trace_state);
+
 /*
  * Event file flags:
  *  ENABLED	  - The event is enabled
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 41a82f5c5be5..b1b42ca1cf13 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -393,6 +393,7 @@ struct synth_field {
 	char *type;
 	char *name;
 	size_t size;
+	unsigned int offset;
 	bool is_signed;
 	bool is_string;
 };
@@ -662,6 +663,8 @@ static int synth_event_define_fields(struct trace_event_call *call)
 		if (ret)
 			break;
 
+		event->fields[i]->offset = n_u64;
+
 		if (event->fields[i]->is_string) {
 			offset += STR_VAR_LEN_MAX;
 			n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
@@ -1780,6 +1783,466 @@ void synth_event_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen)
 }
 EXPORT_SYMBOL_GPL(synth_event_cmd_init);
 
+/**
+ * synth_event_trace - Trace a synthetic event
+ * @file: The trace_event_file representing the synthetic event
+ * @n_vals: The number of values in vals
+ * @args: Variable number of args containing the event values
+ *
+ * Trace a synthetic event using the values passed in the variable
+ * argument list.
+ *
+ * The argument list should be a list 'n_vals' u64 values.  The number
+ * of vals must match the number of field in the synthetic event, and
+ * must be in the same order as the synthetic event fields.
+ *
+ * All vals should be cast to u64, and string vals are just pointers
+ * to strings, cast to u64.  Strings will be copied into space
+ * reserved in the event for the string, using these pointers.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int synth_event_trace(struct trace_event_file *file, unsigned int n_vals, ...)
+{
+	struct trace_event_buffer fbuffer;
+	struct synth_trace_event *entry;
+	struct trace_buffer *buffer;
+	struct synth_event *event;
+	unsigned int i, n_u64;
+	int fields_size = 0;
+	va_list args;
+	int ret = 0;
+
+	/*
+	 * Normal event generation doesn't get called at all unless
+	 * the ENABLED bit is set (which attaches the probe thus
+	 * allowing this code to be called, etc).  Because this is
+	 * called directly by the user, we don't have that but we
+	 * still need to honor not logging when disabled.
+	 */
+	if (!(file->flags & EVENT_FILE_FL_ENABLED))
+		return 0;
+
+	event = file->event_call->data;
+
+	if (n_vals != event->n_fields)
+		return -EINVAL;
+
+	if (trace_trigger_soft_disabled(file))
+		return -EINVAL;
+
+	fields_size = event->n_u64 * sizeof(u64);
+
+	/*
+	 * Avoid ring buffer recursion detection, as this event
+	 * is being performed within another event.
+	 */
+	buffer = file->tr->array_buffer.buffer;
+	ring_buffer_nest_start(buffer);
+
+	entry = trace_event_buffer_reserve(&fbuffer, file,
+					   sizeof(*entry) + fields_size);
+	if (!entry) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	va_start(args, n_vals);
+	for (i = 0, n_u64 = 0; i < event->n_fields; i++) {
+		u64 val;
+
+		val = va_arg(args, u64);
+
+		if (event->fields[i]->is_string) {
+			char *str_val = (char *)(long)val;
+			char *str_field = (char *)&entry->fields[n_u64];
+
+			strscpy(str_field, str_val, STR_VAR_LEN_MAX);
+			n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
+		} else {
+			entry->fields[n_u64] = val;
+			n_u64++;
+		}
+	}
+	va_end(args);
+
+	trace_event_buffer_commit(&fbuffer);
+out:
+	ring_buffer_nest_end(buffer);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_trace);
+
+/**
+ * synth_event_trace_array - Trace a synthetic event from an array
+ * @file: The trace_event_file representing the synthetic event
+ * @vals: Array of values
+ * @n_vals: The number of values in vals
+ *
+ * Trace a synthetic event using the values passed in as 'vals'.
+ *
+ * The 'vals' array is just an array of 'n_vals' u64.  The number of
+ * vals must match the number of field in the synthetic event, and
+ * must be in the same order as the synthetic event fields.
+ *
+ * All vals should be cast to u64, and string vals are just pointers
+ * to strings, cast to u64.  Strings will be copied into space
+ * reserved in the event for the string, using these pointers.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int synth_event_trace_array(struct trace_event_file *file, u64 *vals,
+			    unsigned int n_vals)
+{
+	struct trace_event_buffer fbuffer;
+	struct synth_trace_event *entry;
+	struct trace_buffer *buffer;
+	struct synth_event *event;
+	unsigned int i, n_u64;
+	int fields_size = 0;
+	int ret = 0;
+
+	/*
+	 * Normal event generation doesn't get called at all unless
+	 * the ENABLED bit is set (which attaches the probe thus
+	 * allowing this code to be called, etc).  Because this is
+	 * called directly by the user, we don't have that but we
+	 * still need to honor not logging when disabled.
+	 */
+	if (!(file->flags & EVENT_FILE_FL_ENABLED))
+		return 0;
+
+	event = file->event_call->data;
+
+	if (n_vals != event->n_fields)
+		return -EINVAL;
+
+	if (trace_trigger_soft_disabled(file))
+		return -EINVAL;
+
+	fields_size = event->n_u64 * sizeof(u64);
+
+	/*
+	 * Avoid ring buffer recursion detection, as this event
+	 * is being performed within another event.
+	 */
+	buffer = file->tr->array_buffer.buffer;
+	ring_buffer_nest_start(buffer);
+
+	entry = trace_event_buffer_reserve(&fbuffer, file,
+					   sizeof(*entry) + fields_size);
+	if (!entry) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	for (i = 0, n_u64 = 0; i < event->n_fields; i++) {
+		if (event->fields[i]->is_string) {
+			char *str_val = (char *)(long)vals[i];
+			char *str_field = (char *)&entry->fields[n_u64];
+
+			strscpy(str_field, str_val, STR_VAR_LEN_MAX);
+			n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
+		} else {
+			entry->fields[n_u64] = vals[i];
+			n_u64++;
+		}
+	}
+
+	trace_event_buffer_commit(&fbuffer);
+out:
+	ring_buffer_nest_end(buffer);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_trace_array);
+
+/**
+ * synth_event_trace_start - Start piecewise synthetic event trace
+ * @file: The trace_event_file representing the synthetic event
+ * @trace_state: A pointer to object tracking the piecewise trace state
+ *
+ * Start the trace of a synthetic event field-by-field rather than all
+ * at once.
+ *
+ * This function 'opens' an event trace, which means space is reserved
+ * for the event in the trace buffer, after which the event's
+ * individual field values can be set through either
+ * synth_event_add_next_val() or synth_event_add_val().
+ *
+ * A pointer to a trace_state object is passed in, which will keep
+ * track of the current event trace state until the event trace is
+ * closed (and the event finally traced) using
+ * synth_event_trace_end().
+ *
+ * Note that synth_event_trace_end() must be called after all values
+ * have been added for each event trace, regardless of whether adding
+ * all field values succeeded or not.
+ *
+ * Note also that for a given event trace, all fields must be added
+ * using either synth_event_add_next_val() or synth_event_add_val()
+ * but not both together or interleaved.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int synth_event_trace_start(struct trace_event_file *file,
+			    struct synth_event_trace_state *trace_state)
+{
+	struct synth_trace_event *entry;
+	int fields_size = 0;
+	int ret = 0;
+
+	if (!trace_state) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	memset(trace_state, '\0', sizeof(*trace_state));
+
+	/*
+	 * Normal event tracing doesn't get called at all unless the
+	 * ENABLED bit is set (which attaches the probe thus allowing
+	 * this code to be called, etc).  Because this is called
+	 * directly by the user, we don't have that but we still need
+	 * to honor not logging when disabled.  For the the iterated
+	 * trace case, we save the enabed state upon start and just
+	 * ignore the following data calls.
+	 */
+	if (!(file->flags & EVENT_FILE_FL_ENABLED)) {
+		trace_state->enabled = false;
+		goto out;
+	}
+
+	trace_state->enabled = true;
+
+	trace_state->event = file->event_call->data;
+
+	if (trace_trigger_soft_disabled(file)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	fields_size = trace_state->event->n_u64 * sizeof(u64);
+
+	/*
+	 * Avoid ring buffer recursion detection, as this event
+	 * is being performed within another event.
+	 */
+	trace_state->buffer = file->tr->array_buffer.buffer;
+	ring_buffer_nest_start(trace_state->buffer);
+
+	entry = trace_event_buffer_reserve(&trace_state->fbuffer, file,
+					   sizeof(*entry) + fields_size);
+	if (!entry) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	trace_state->entry = entry;
+out:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_trace_start);
+
+static int save_synth_val(struct synth_field *field, u64 val,
+			  struct synth_event_trace_state *trace_state)
+{
+	struct synth_trace_event *entry = trace_state->entry;
+
+	if (field->is_string) {
+		char *str_val = (char *)(long)val;
+		char *str_field;
+
+		if (!str_val)
+			return -EINVAL;
+
+		str_field = (char *)&entry->fields[field->offset];
+		strscpy(str_field, str_val, STR_VAR_LEN_MAX);
+	} else
+		entry->fields[field->offset] = val;
+
+	return 0;
+}
+
+/**
+ * synth_event_add_next_val - Add the next field's value to an open synth trace
+ * @val: The value to set the next field to
+ * @trace_state: A pointer to object tracking the piecewise trace state
+ *
+ * Set the value of the next field in an event that's been opened by
+ * synth_event_trace_start().
+ *
+ * The val param should be the value cast to u64.  If the value points
+ * to a string, the val param should be a char * cast to u64.
+ *
+ * This function assumes all the fields in an event are to be set one
+ * after another - successive calls to this function are made, one for
+ * each field, in the order of the fields in the event, until all
+ * fields have been set.  If you'd rather set each field individually
+ * without regard to ordering, synth_event_add_val() can be used
+ * instead.
+ *
+ * Note however that synth_event_add_next_val() and
+ * synth_event_add_val() can't be intermixed for a given event trace -
+ * one or the other but not both can be used at the same time.
+ *
+ * Note also that synth_event_trace_end() must be called after all
+ * values have been added for each event trace, regardless of whether
+ * adding all field values succeeded or not.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int synth_event_add_next_val(u64 val,
+			     struct synth_event_trace_state *trace_state)
+{
+	struct synth_field *field;
+	struct synth_event *event;
+	int ret = 0;
+
+	if (!trace_state) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* can't mix add_next_synth_val() with add_synth_val() */
+	if (trace_state->add_name) {
+		ret = -EINVAL;
+		goto out;
+	}
+	trace_state->add_next = true;
+
+	if (!trace_state->enabled)
+		goto out;
+
+	event = trace_state->event;
+
+	if (trace_state->cur_field >= event->n_fields) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	field = event->fields[trace_state->cur_field++];
+	ret = save_synth_val(field, val, trace_state);
+ out:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_add_next_val);
+
+static struct synth_field *find_synth_field(struct synth_event *event,
+					    const char *field_name)
+{
+	struct synth_field *field = NULL;
+	unsigned int i;
+
+	for (i = 0; i < event->n_fields; i++) {
+		field = event->fields[i];
+		if (strcmp(field->name, field_name) == 0)
+			return field;
+	}
+
+	return NULL;
+}
+
+/**
+ * synth_event_add_val - Add a named field's value to an open synth trace
+ * @field_name: The name of the synthetic event field value to set
+ * @val: The value to set the next field to
+ * @trace_state: A pointer to object tracking the piecewise trace state
+ *
+ * Set the value of the named field in an event that's been opened by
+ * synth_event_trace_start().
+ *
+ * The val param should be the value cast to u64.  If the value points
+ * to a string, the val param should be a char * cast to u64.
+ *
+ * This function looks up the field name, and if found, sets the field
+ * to the specified value.  This lookup makes this function more
+ * expensive than synth_event_add_next_val(), so use that or the
+ * none-piecewise synth_event_trace() instead if efficiency is more
+ * important.
+ *
+ * Note however that synth_event_add_next_val() and
+ * synth_event_add_val() can't be intermixed for a given event trace -
+ * one or the other but not both can be used at the same time.
+ *
+ * Note also that synth_event_trace_end() must be called after all
+ * values have been added for each event trace, regardless of whether
+ * adding all field values succeeded or not.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int synth_event_add_val(const char *field_name, u64 val,
+			struct synth_event_trace_state *trace_state)
+{
+	struct synth_trace_event *entry;
+	struct synth_event *event;
+	struct synth_field *field;
+	int ret = 0;
+
+	if (!trace_state) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* can't mix add_next_synth_val() with add_synth_val() */
+	if (trace_state->add_next) {
+		ret = -EINVAL;
+		goto out;
+	}
+	trace_state->add_name = true;
+
+	if (!trace_state->enabled)
+		goto out;
+
+	event = trace_state->event;
+	entry = trace_state->entry;
+
+	field = find_synth_field(event, field_name);
+	if (!field) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = save_synth_val(field, val, trace_state);
+ out:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(synth_event_add_val);
+
+/**
+ * synth_event_trace_end - End piecewise synthetic event trace
+ * @trace_state: A pointer to object tracking the piecewise trace state
+ *
+ * End the trace of a synthetic event opened by
+ * synth_event_trace__start().
+ *
+ * This function 'closes' an event trace, which basically means that
+ * it commits the reserved event and cleans up other loose ends.
+ *
+ * A pointer to a trace_state object is passed in, which will keep
+ * track of the current event trace state opened with
+ * synth_event_trace_start().
+ *
+ * Note that this function must be called after all values have been
+ * added for each event trace, regardless of whether adding all field
+ * values succeeded or not.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int synth_event_trace_end(struct synth_event_trace_state *trace_state)
+{
+	if (!trace_state)
+		return -EINVAL;
+
+	trace_event_buffer_commit(&trace_state->fbuffer);
+
+	ring_buffer_nest_end(trace_state->buffer);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(synth_event_trace_end);
+
 static int create_synth_event(int argc, const char **argv)
 {
 	const char *name = argv[0];
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 08/12] tracing: Add synth event generation test module
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (6 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 07/12] tracing: Add synth_event_trace() and related functions Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 09/12] tracing: Add kprobe event command generation functions Tom Zanussi
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add a test module that checks the basic functionality of the in-kernel
synthetic event generation API by generating and tracing synthetic
events from a module.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 kernel/trace/Kconfig                |  13 +
 kernel/trace/Makefile               |   1 +
 kernel/trace/synth_event_gen_test.c | 523 ++++++++++++++++++++++++++++++++++++
 3 files changed, 537 insertions(+)
 create mode 100644 kernel/trace/synth_event_gen_test.c

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 75326d8ab1af..4f2041166a2f 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -774,6 +774,19 @@ config PREEMPTIRQ_DELAY_TEST
 
 	  If unsure, say N
 
+config SYNTH_EVENT_GEN_TEST
+	tristate "Test module for in-kernel synthetic event generation"
+	depends on HIST_TRIGGERS
+	help
+          This option creates a test module to check the base
+          functionality of in-kernel synthetic event definition and
+          generation.
+
+          To test, insert the module, and then check the trace buffer
+	  for the generated sample events.
+
+	  If unsure, say N.
+
 config TRACE_EVAL_MAP_FILE
        bool "Show eval mappings for trace events"
        depends on TRACING
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 395e2db9c742..32012f50fb79 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_TRACING) += trace_stat.o
 obj-$(CONFIG_TRACING) += trace_printk.o
 obj-$(CONFIG_TRACING_MAP) += tracing_map.o
 obj-$(CONFIG_PREEMPTIRQ_DELAY_TEST) += preemptirq_delay_test.o
+obj-$(CONFIG_SYNTH_EVENT_GEN_TEST) += synth_event_gen_test.o
 obj-$(CONFIG_CONTEXT_SWITCH_TRACER) += trace_sched_switch.o
 obj-$(CONFIG_FUNCTION_TRACER) += trace_functions.o
 obj-$(CONFIG_PREEMPTIRQ_TRACEPOINTS) += trace_preemptirq.o
diff --git a/kernel/trace/synth_event_gen_test.c b/kernel/trace/synth_event_gen_test.c
new file mode 100644
index 000000000000..4aefe003cb7c
--- /dev/null
+++ b/kernel/trace/synth_event_gen_test.c
@@ -0,0 +1,523 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test module for in-kernel sythetic event creation and generation.
+ *
+ * Copyright (C) 2019 Tom Zanussi <zanussi@kernel.org>
+ */
+
+#include <linux/module.h>
+#include <linux/trace_events.h>
+
+/*
+ * This module is a simple test of basic functionality for in-kernel
+ * synthetic event creation and generation, the first and second tests
+ * using synth_event_gen_cmd_start() and synth_event_add_field(), the
+ * third uses synth_event_create() to do it all at once with a static
+ * field array.
+ *
+ * Following that are a few examples using the created events to test
+ * various ways of tracing a synthetic event.
+ *
+ * To test, select CONFIG_SYNTH_EVENT_GEN_TEST and build the module.
+ * Then:
+ *
+ * # insmod kernel/trace/synth_event_gen_test.ko
+ * # cat /sys/kernel/debug/tracing/trace
+ *
+ * You should see several events in the trace buffer -
+ * "create_synth_test", "empty_synth_test", and several instances of
+ * "gen_synth_test".
+ *
+ * To remove the events, remove the module:
+ *
+ * # rmmod synth_event_gen_test
+ *
+ */
+
+static struct trace_event_file *create_synth_test;
+static struct trace_event_file *empty_synth_test;
+static struct trace_event_file *gen_synth_test;
+
+/*
+ * Test to make sure we can create a synthetic event, then add more
+ * fields.
+ */
+static int __init test_gen_synth_cmd(void)
+{
+	struct dynevent_cmd cmd;
+	u64 vals[7];
+	char *buf;
+	int ret;
+
+	/* Create a buffer to hold the generated command */
+	buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/* Before generating the command, initialize the cmd object */
+	synth_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+	/*
+	 * Create the empty gen_synth_test synthetic event with the
+	 * first 4 fields.
+	 */
+	ret = synth_event_gen_cmd_start(&cmd, "gen_synth_test", THIS_MODULE,
+					"pid_t", "next_pid_field",
+					"char[16]", "next_comm_field",
+					"u64", "ts_ns",
+					"u64", "ts_ms");
+	if (ret)
+		goto free;
+
+	/* Use synth_event_add_field to add the rest of the fields */
+
+	ret = synth_event_add_field(&cmd, "unsigned int", "cpu");
+	if (ret)
+		goto free;
+
+	ret = synth_event_add_field(&cmd, "char[64]", "my_string_field");
+	if (ret)
+		goto free;
+
+	ret = synth_event_add_field(&cmd, "int", "my_int_field");
+	if (ret)
+		goto free;
+
+	ret = synth_event_gen_cmd_end(&cmd);
+	if (ret)
+		goto free;
+
+	/*
+	 * Now get the gen_synth_test event file.  We need to prevent
+	 * the instance and event from disappearing from underneath
+	 * us, which trace_get_event_file() does (though in this case
+	 * we're using the top-level instance which never goes away).
+	 */
+	gen_synth_test = trace_get_event_file(NULL, "synthetic",
+					      "gen_synth_test");
+	if (IS_ERR(gen_synth_test)) {
+		ret = PTR_ERR(gen_synth_test);
+		goto delete;
+	}
+
+	/* Enable the event or you won't see anything */
+	ret = trace_array_set_clr_event(gen_synth_test->tr,
+					"synthetic", "gen_synth_test", true);
+	if (ret) {
+		trace_put_event_file(gen_synth_test);
+		goto delete;
+	}
+
+	/* Create some bogus values just for testing */
+
+	vals[0] = 777;			/* next_pid_field */
+	vals[1] = (u64)"hula hoops";	/* next_comm_field */
+	vals[2] = 1000000;		/* ts_ns */
+	vals[3] = 1000;			/* ts_ms */
+	vals[4] = smp_processor_id();	/* cpu */
+	vals[5] = (u64)"thneed";	/* my_string_field */
+	vals[6] = 598;			/* my_int_field */
+
+	/* Now generate a gen_synth_test event */
+	ret = synth_event_trace_array(gen_synth_test, vals, ARRAY_SIZE(vals));
+ out:
+	return ret;
+ delete:
+	/* We got an error after creating the event, delete it */
+	synth_event_delete("gen_synth_test");
+ free:
+	kfree(buf);
+
+	goto out;
+}
+
+/*
+ * Test to make sure we can create an initially empty synthetic event,
+ * then add all the fields.
+ */
+static int __init test_empty_synth_event(void)
+{
+	struct dynevent_cmd cmd;
+	u64 vals[7];
+	char *buf;
+	int ret;
+
+	/* Create a buffer to hold the generated command */
+	buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/* Before generating the command, initialize the cmd object */
+	synth_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+	/*
+	 * Create the empty_synth_test synthetic event with no fields.
+	 */
+	ret = synth_event_gen_cmd_start(&cmd, "empty_synth_test", THIS_MODULE);
+	if (ret)
+		goto free;
+
+	/* Use synth_event_add_field to add all of the fields */
+
+	ret = synth_event_add_field(&cmd, "pid_t", "next_pid_field");
+	if (ret)
+		goto free;
+
+	ret = synth_event_add_field(&cmd, "char[16]", "next_comm_field");
+	if (ret)
+		goto free;
+
+	ret = synth_event_add_field(&cmd, "u64", "ts_ns");
+	if (ret)
+		goto free;
+
+	ret = synth_event_add_field(&cmd, "u64", "ts_ms");
+	if (ret)
+		goto free;
+
+	ret = synth_event_add_field(&cmd, "unsigned int", "cpu");
+	if (ret)
+		goto free;
+
+	ret = synth_event_add_field(&cmd, "char[64]", "my_string_field");
+	if (ret)
+		goto free;
+
+	ret = synth_event_add_field(&cmd, "int", "my_int_field");
+	if (ret)
+		goto free;
+
+	/* All fields have been added, close and register the synth event */
+
+	ret = synth_event_gen_cmd_end(&cmd);
+	if (ret)
+		goto free;
+
+	/*
+	 * Now get the empty_synth_test event file.  We need to
+	 * prevent the instance and event from disappearing from
+	 * underneath us, which trace_get_event_file() does (though in
+	 * this case we're using the top-level instance which never
+	 * goes away).
+	 */
+	empty_synth_test = trace_get_event_file(NULL, "synthetic",
+						"empty_synth_test");
+	if (IS_ERR(empty_synth_test)) {
+		ret = PTR_ERR(empty_synth_test);
+		goto delete;
+	}
+
+	/* Enable the event or you won't see anything */
+	ret = trace_array_set_clr_event(empty_synth_test->tr,
+					"synthetic", "empty_synth_test", true);
+	if (ret) {
+		trace_put_event_file(empty_synth_test);
+		goto delete;
+	}
+
+	/* Create some bogus values just for testing */
+
+	vals[0] = 777;			/* next_pid_field */
+	vals[1] = (u64)"tiddlywinks";	/* next_comm_field */
+	vals[2] = 1000000;		/* ts_ns */
+	vals[3] = 1000;			/* ts_ms */
+	vals[4] = smp_processor_id();	/* cpu */
+	vals[5] = (u64)"thneed_2.0";	/* my_string_field */
+	vals[6] = 399;			/* my_int_field */
+
+	/* Now trace an empty_synth_test event */
+	ret = synth_event_trace_array(empty_synth_test, vals, ARRAY_SIZE(vals));
+ out:
+	return ret;
+ delete:
+	/* We got an error after creating the event, delete it */
+	synth_event_delete("empty_synth_test");
+ free:
+	kfree(buf);
+
+	goto out;
+}
+
+static struct synth_field_desc create_synth_test_fields[] = {
+	{ .type = "pid_t",		.name = "next_pid_field" },
+	{ .type = "char[16]",		.name = "next_comm_field" },
+	{ .type = "u64",		.name = "ts_ns" },
+	{ .type = "u64",		.name = "ts_ms" },
+	{ .type = "unsigned int",	.name = "cpu" },
+	{ .type = "char[64]",		.name = "my_string_field" },
+	{ .type = "int",		.name = "my_int_field" },
+};
+
+/*
+ * Test synthetic event creation all at once from array of field
+ * descriptors.
+ */
+static int __init test_create_synth_event(void)
+{
+	u64 vals[7];
+	int ret;
+
+	/* Create the create_synth_test event with the fields above */
+	ret = synth_event_create("create_synth_test",
+				 create_synth_test_fields,
+				 ARRAY_SIZE(create_synth_test_fields),
+				 THIS_MODULE);
+	if (ret)
+		goto out;
+
+	/*
+	 * Now get the create_synth_test event file.  We need to
+	 * prevent the instance and event from disappearing from
+	 * underneath us, which trace_get_event_file() does (though in
+	 * this case we're using the top-level instance which never
+	 * goes away).
+	 */
+	create_synth_test = trace_get_event_file(NULL, "synthetic",
+						 "create_synth_test");
+	if (IS_ERR(create_synth_test)) {
+		ret = PTR_ERR(create_synth_test);
+		goto delete;
+	}
+
+	/* Enable the event or you won't see anything */
+	ret = trace_array_set_clr_event(create_synth_test->tr,
+					"synthetic", "create_synth_test", true);
+	if (ret) {
+		trace_put_event_file(create_synth_test);
+		goto delete;
+	}
+
+	/* Create some bogus values just for testing */
+
+	vals[0] = 777;			/* next_pid_field */
+	vals[1] = (u64)"tiddlywinks";	/* next_comm_field */
+	vals[2] = 1000000;		/* ts_ns */
+	vals[3] = 1000;			/* ts_ms */
+	vals[4] = smp_processor_id();	/* cpu */
+	vals[5] = (u64)"thneed";	/* my_string_field */
+	vals[6] = 398;			/* my_int_field */
+
+	/* Now generate a create_synth_test event */
+	ret = synth_event_trace_array(create_synth_test, vals, ARRAY_SIZE(vals));
+ out:
+	return ret;
+ delete:
+	/* We got an error after creating the event, delete it */
+	ret = synth_event_delete("create_synth_test");
+
+	goto out;
+}
+
+/*
+ * Test tracing a synthetic event by reserving trace buffer space,
+ * then filling in fields one after another.
+ */
+static int __init test_add_next_synth_val(void)
+{
+	struct synth_event_trace_state trace_state;
+	int ret;
+
+	/* Start by reserving space in the trace buffer */
+	ret = synth_event_trace_start(gen_synth_test, &trace_state);
+	if (ret)
+		return ret;
+
+	/* Write some bogus values into the trace buffer, one after another */
+
+	/* next_pid_field */
+	ret = synth_event_add_next_val(777, &trace_state);
+	if (ret)
+		goto out;
+
+	/* next_comm_field */
+	ret = synth_event_add_next_val((u64)"slinky", &trace_state);
+	if (ret)
+		goto out;
+
+	/* ts_ns */
+	ret = synth_event_add_next_val(1000000, &trace_state);
+	if (ret)
+		goto out;
+
+	/* ts_ms */
+	ret = synth_event_add_next_val(1000, &trace_state);
+	if (ret)
+		goto out;
+
+	/* cpu */
+	ret = synth_event_add_next_val(smp_processor_id(), &trace_state);
+	if (ret)
+		goto out;
+
+	/* my_string_field */
+	ret = synth_event_add_next_val((u64)"thneed_2.01", &trace_state);
+	if (ret)
+		goto out;
+
+	/* my_int_field */
+	ret = synth_event_add_next_val(395, &trace_state);
+ out:
+	/* Finally, commit the event */
+	ret = synth_event_trace_end(&trace_state);
+
+	return ret;
+}
+
+/*
+ * Test tracing a synthetic event by reserving trace buffer space,
+ * then filling in fields using field names, which can be done in any
+ * order.
+ */
+static int __init test_add_synth_val(void)
+{
+	struct synth_event_trace_state trace_state;
+	int ret;
+
+	/* Start by reserving space in the trace buffer */
+	ret = synth_event_trace_start(gen_synth_test, &trace_state);
+	if (ret)
+		return ret;
+
+	/* Write some bogus values into the trace buffer, using field names */
+
+	ret = synth_event_add_val("ts_ns", 1000000, &trace_state);
+	if (ret)
+		goto out;
+
+	ret = synth_event_add_val("ts_ms", 1000, &trace_state);
+	if (ret)
+		goto out;
+
+	ret = synth_event_add_val("cpu", smp_processor_id(), &trace_state);
+	if (ret)
+		goto out;
+
+	ret = synth_event_add_val("next_pid_field", 777, &trace_state);
+	if (ret)
+		goto out;
+
+	ret = synth_event_add_val("next_comm_field", (u64)"silly putty",
+				  &trace_state);
+	if (ret)
+		goto out;
+
+	ret = synth_event_add_val("my_string_field", (u64)"thneed_9",
+				  &trace_state);
+	if (ret)
+		goto out;
+
+	ret = synth_event_add_val("my_int_field", 3999, &trace_state);
+ out:
+	/* Finally, commit the event */
+	ret = synth_event_trace_end(&trace_state);
+
+	return ret;
+}
+
+/*
+ * Test tracing a synthetic event all at once from array of values.
+ */
+static int __init test_trace_synth_event(void)
+{
+	int ret;
+
+	/* Trace some bogus values just for testing */
+	ret = synth_event_trace(create_synth_test, 7,	/* number of values */
+				444,			/* next_pid_field */
+				(u64)"clackers",	/* next_comm_field */
+				1000000,		/* ts_ns */
+				1000,			/* ts_ms */
+				smp_processor_id(),	/* cpu */
+				(u64)"Thneed",		/* my_string_field */
+				999);			/* my_int_field */
+	return ret;
+}
+
+static int __init synth_event_gen_test_init(void)
+{
+	int ret;
+
+	ret = test_gen_synth_cmd();
+	if (ret)
+		return ret;
+
+	ret = test_empty_synth_event();
+	if (ret) {
+		WARN_ON(trace_array_set_clr_event(gen_synth_test->tr,
+						  "synthetic",
+						  "gen_synth_test", false));
+		trace_put_event_file(gen_synth_test);
+		WARN_ON(synth_event_delete("gen_synth_test"));
+		goto out;
+	}
+
+	ret = test_create_synth_event();
+	if (ret) {
+		WARN_ON(trace_array_set_clr_event(gen_synth_test->tr,
+						  "synthetic",
+						  "gen_synth_test", false));
+		trace_put_event_file(gen_synth_test);
+		WARN_ON(synth_event_delete("gen_synth_test"));
+
+		WARN_ON(trace_array_set_clr_event(empty_synth_test->tr,
+						  "synthetic",
+						  "empty_synth_test", false));
+		trace_put_event_file(empty_synth_test);
+		WARN_ON(synth_event_delete("empty_synth_test"));
+		goto out;
+	}
+
+	ret = test_add_next_synth_val();
+	WARN_ON(ret);
+
+	ret = test_add_synth_val();
+	WARN_ON(ret);
+
+	ret = test_trace_synth_event();
+	WARN_ON(ret);
+ out:
+	return ret;
+}
+
+static void __exit synth_event_gen_test_exit(void)
+{
+	/* Disable the event or you can't remove it */
+	WARN_ON(trace_array_set_clr_event(gen_synth_test->tr,
+					  "synthetic",
+					  "gen_synth_test", false));
+
+	/* Now give the file and instance back */
+	trace_put_event_file(gen_synth_test);
+
+	/* Now unregister and free the synthetic event */
+	WARN_ON(synth_event_delete("gen_synth_test"));
+
+	/* Disable the event or you can't remove it */
+	WARN_ON(trace_array_set_clr_event(empty_synth_test->tr,
+					  "synthetic",
+					  "empty_synth_test", false));
+
+	/* Now give the file and instance back */
+	trace_put_event_file(empty_synth_test);
+
+	/* Now unregister and free the synthetic event */
+	WARN_ON(synth_event_delete("empty_synth_test"));
+
+	/* Disable the event or you can't remove it */
+	WARN_ON(trace_array_set_clr_event(create_synth_test->tr,
+					  "synthetic",
+					  "create_synth_test", false));
+
+	/* Now give the file and instance back */
+	trace_put_event_file(create_synth_test);
+
+	/* Now unregister and free the synthetic event */
+	WARN_ON(synth_event_delete("create_synth_test"));
+}
+
+module_init(synth_event_gen_test_init)
+module_exit(synth_event_gen_test_exit)
+
+MODULE_AUTHOR("Tom Zanussi");
+MODULE_DESCRIPTION("synthetic event generation test");
+MODULE_LICENSE("GPL v2");
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 09/12] tracing: Add kprobe event command generation functions
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (7 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 08/12] tracing: Add synth event generation test module Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 10/12] tracing: Change trace_boot to use kprobe_event interface Tom Zanussi
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add functions used to generate kprobe event commands, built on top of
the dynevent_cmd interface.

kprobe_event_gen_cmd_start() is used to create a kprobe event command
using a variable arg list, and kretprobe_event_gen_cmd_start() does
the same for kretprobe event commands.  kprobe_event_add_fields() can
be used to add single fields one by one or as a group.  Once all
desired fields are added, kprobe_event_gen_cmd_end() or
kretprobe_event_gen_cmd_end() respectively are used to actually
execute the command and create the event.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 include/linux/trace_events.h |  31 +++++++++
 kernel/trace/trace_kprobe.c  | 161 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 192 insertions(+)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index bf03d12efb28..7c307a7c9c6a 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -358,6 +358,7 @@ extern void trace_put_event_file(struct trace_event_file *file);
 
 enum dynevent_type {
 	DYNEVENT_TYPE_SYNTH = 1,
+	DYNEVENT_TYPE_KPROBE,
 	DYNEVENT_TYPE_NONE,
 };
 
@@ -442,6 +443,36 @@ extern int synth_event_add_val(const char *field_name, u64 val,
 			       struct synth_event_trace_state *trace_state);
 extern int synth_event_trace_end(struct synth_event_trace_state *trace_state);
 
+extern int kprobe_event_delete(const char *name);
+
+extern void kprobe_event_cmd_init(struct dynevent_cmd *cmd,
+				  char *buf, int maxlen);
+
+#define kprobe_event_gen_cmd_start(cmd, name, loc, ...)			\
+	__kprobe_event_gen_cmd_start(cmd, false, name, loc, ## __VA_ARGS__, NULL)
+
+#define kretprobe_event_gen_cmd_start(cmd, name, loc, ...)		\
+	__kprobe_event_gen_cmd_start(cmd, true, name, loc, ## __VA_ARGS__, NULL)
+
+extern int __kprobe_event_gen_cmd_start(struct dynevent_cmd *cmd,
+					bool kretprobe,
+					const char *name,
+					const char *loc, ...);
+
+#define kprobe_event_add_fields(cmd, ...)	\
+	__kprobe_event_add_fields(cmd, ## __VA_ARGS__, NULL)
+
+#define kprobe_event_add_field(cmd, field)	\
+	__kprobe_event_add_fields(cmd, field, NULL)
+
+extern int __kprobe_event_add_fields(struct dynevent_cmd *cmd, ...);
+
+#define kprobe_event_gen_cmd_end(cmd)		\
+	dynevent_create(cmd)
+
+#define kretprobe_event_gen_cmd_end(cmd)	\
+	dynevent_create(cmd)
+
 /*
  * Event file flags:
  *  ENABLED	  - The event is enabled
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index bf20cd7f2666..f43548b466d0 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -906,6 +906,167 @@ int trace_kprobe_run_command(const char *command)
 	return trace_run_command(command, create_or_delete_trace_kprobe);
 }
 
+static int trace_kprobe_run_cmd(struct dynevent_cmd *cmd)
+{
+	return trace_run_command(cmd->buf, create_or_delete_trace_kprobe);
+}
+
+/**
+ * kprobe_event_cmd_init - Initialize a kprobe event command object
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @buf: A pointer to the buffer used to build the command
+ * @maxlen: The length of the buffer passed in @buf
+ *
+ * Initialize a synthetic event command object.  Use this before
+ * calling any of the other kprobe_event functions.
+ */
+void kprobe_event_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen)
+{
+	dynevent_cmd_init(cmd, buf, maxlen, DYNEVENT_TYPE_KPROBE,
+			  trace_kprobe_run_cmd);
+}
+EXPORT_SYMBOL_GPL(kprobe_event_cmd_init);
+
+/**
+ * __kprobe_event_gen_cmd_start - Generate a kprobe event command from arg list
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @name: The name of the kprobe event
+ * @loc: The location of the kprobe event
+ * @kretprobe: Is this a return probe?
+ * @args: Variable number of arg (pairs), one pair for each field
+ *
+ * NOTE: Users normally won't want to call this function directly, but
+ * rather use the kprobe_event_gen_cmd_start() wrapper, which automatically
+ * adds a NULL to the end of the arg list.  If this function is used
+ * directly, make sure the last arg in the variable arg list is NULL.
+ *
+ * Generate a kprobe event command to be executed by
+ * kprobe_event_gen_cmd_end().  This function can be used to generate the
+ * complete command or only the first part of it; in the latter case,
+ * kprobe_event_add_fields() can be used to add more fields following this.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int __kprobe_event_gen_cmd_start(struct dynevent_cmd *cmd, bool kretprobe,
+				 const char *name, const char *loc, ...)
+{
+	char buf[MAX_EVENT_NAME_LEN];
+	struct dynevent_arg arg;
+	va_list args;
+	int ret;
+
+	if (cmd->type != DYNEVENT_TYPE_KPROBE)
+		return -EINVAL;
+
+	if (kretprobe)
+		snprintf(buf, MAX_EVENT_NAME_LEN, "r:kprobes/%s", name);
+	else
+		snprintf(buf, MAX_EVENT_NAME_LEN, "p:kprobes/%s", name);
+
+	ret = dynevent_str_add(cmd, buf);
+	if (ret)
+		return ret;
+
+	dynevent_arg_init(&arg, NULL, 0);
+	arg.str = loc;
+	ret = dynevent_arg_add(cmd, &arg);
+	if (ret)
+		return ret;
+
+	va_start(args, loc);
+	for (;;) {
+		const char *field;
+
+		field = va_arg(args, const char *);
+		if (!field)
+			break;
+
+		if (++cmd->n_fields > MAX_TRACE_ARGS) {
+			ret = -EINVAL;
+			break;
+		}
+
+		arg.str = field;
+		ret = dynevent_arg_add(cmd, &arg);
+		if (ret)
+			break;
+	}
+	va_end(args);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__kprobe_event_gen_cmd_start);
+
+/**
+ * __kprobe_event_add_fields - Add probe fields to a kprobe command from arg list
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event
+ * @args: Variable number of arg (pairs), one pair for each field
+ *
+ * NOTE: Users normally won't want to call this function directly, but
+ * rather use the kprobe_event_add_fields() wrapper, which
+ * automatically adds a NULL to the end of the arg list.  If this
+ * function is used directly, make sure the last arg in the variable
+ * arg list is NULL.
+ *
+ * Add probe fields to an existing kprobe command using a variable
+ * list of args.  Fields are added in the same order they're listed.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int __kprobe_event_add_fields(struct dynevent_cmd *cmd, ...)
+{
+	struct dynevent_arg arg;
+	va_list args;
+	int ret;
+
+	if (cmd->type != DYNEVENT_TYPE_KPROBE)
+		return -EINVAL;
+
+	dynevent_arg_init(&arg, NULL, 0);
+
+	va_start(args, cmd);
+	for (;;) {
+		const char *field;
+
+		field = va_arg(args, const char *);
+		if (!field)
+			break;
+
+		if (++cmd->n_fields > MAX_TRACE_ARGS) {
+			ret = -EINVAL;
+			break;
+		}
+
+		arg.str = field;
+		ret = dynevent_arg_add(cmd, &arg);
+		if (ret)
+			break;
+	}
+	va_end(args);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__kprobe_event_add_fields);
+
+/**
+ * kprobe_event_delete - Delete a kprobe event
+ * @name: The name of the kprobe event to delete
+ *
+ * Delete a kprobe event with the give @name from kernel code rather
+ * than directly from the command line.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int kprobe_event_delete(const char *name)
+{
+	char buf[MAX_EVENT_NAME_LEN];
+
+	snprintf(buf, MAX_EVENT_NAME_LEN, "-:%s", name);
+
+	return trace_run_command(buf, create_or_delete_trace_kprobe);
+}
+EXPORT_SYMBOL_GPL(kprobe_event_delete);
+
 static int trace_kprobe_release(struct dyn_event *ev)
 {
 	struct trace_kprobe *tk = to_trace_kprobe(ev);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 10/12] tracing: Change trace_boot to use kprobe_event interface
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (8 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 09/12] tracing: Add kprobe event command generation functions Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 11/12] tracing: Add kprobe event command generation test module Tom Zanussi
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Have trace_boot_add_kprobe_event() use the kprobe_event interface.

Also, rename kprobe_event_run_cmd() to kprobe_event_run_command() now
that trace_boot's version is gone.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 kernel/trace/trace_boot.c   | 35 +++++++++++++++--------------------
 kernel/trace/trace_kprobe.c |  9 ++-------
 2 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index 33665718008c..76ffa8e198ff 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -91,37 +91,32 @@ trace_boot_enable_events(struct trace_array *tr, struct xbc_node *node)
 }
 
 #ifdef CONFIG_KPROBE_EVENTS
-extern int trace_kprobe_run_command(const char *command);
-
 static int __init
 trace_boot_add_kprobe_event(struct xbc_node *node, const char *event)
 {
+	struct dynevent_cmd cmd;
 	struct xbc_node *anode;
 	char buf[MAX_BUF_LEN];
 	const char *val;
-	char *p;
-	int len;
+	int ret;
 
-	len = snprintf(buf, ARRAY_SIZE(buf) - 1, "p:kprobes/%s ", event);
-	if (len >= ARRAY_SIZE(buf)) {
-		pr_err("Event name is too long: %s\n", event);
-		return -E2BIG;
-	}
-	p = buf + len;
-	len = ARRAY_SIZE(buf) - len;
+	kprobe_event_cmd_init(&cmd, buf, MAX_BUF_LEN);
+
+	ret = kprobe_event_gen_cmd_start(&cmd, event, NULL);
+	if (ret)
+		return ret;
 
 	xbc_node_for_each_array_value(node, "probes", anode, val) {
-		if (strlcpy(p, val, len) >= len) {
-			pr_err("Probe definition is too long: %s\n", val);
-			return -E2BIG;
-		}
-		if (trace_kprobe_run_command(buf) < 0) {
-			pr_err("Failed to add probe: %s\n", buf);
-			return -EINVAL;
-		}
+		ret = kprobe_event_add_field(&cmd, val);
+		if (ret)
+			return ret;
 	}
 
-	return 0;
+	ret = kprobe_event_gen_cmd_end(&cmd);
+	if (ret)
+		pr_err("Failed to add probe: %s\n", buf);
+
+	return ret;
 }
 #else
 static inline int __init
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index f43548b466d0..307abb724a71 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -901,12 +901,7 @@ static int create_or_delete_trace_kprobe(int argc, char **argv)
 	return ret == -ECANCELED ? -EINVAL : ret;
 }
 
-int trace_kprobe_run_command(const char *command)
-{
-	return trace_run_command(command, create_or_delete_trace_kprobe);
-}
-
-static int trace_kprobe_run_cmd(struct dynevent_cmd *cmd)
+static int trace_kprobe_run_command(struct dynevent_cmd *cmd)
 {
 	return trace_run_command(cmd->buf, create_or_delete_trace_kprobe);
 }
@@ -923,7 +918,7 @@ static int trace_kprobe_run_cmd(struct dynevent_cmd *cmd)
 void kprobe_event_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen)
 {
 	dynevent_cmd_init(cmd, buf, maxlen, DYNEVENT_TYPE_KPROBE,
-			  trace_kprobe_run_cmd);
+			  trace_kprobe_run_command);
 }
 EXPORT_SYMBOL_GPL(kprobe_event_cmd_init);
 
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 11/12] tracing: Add kprobe event command generation test module
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (9 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 10/12] tracing: Change trace_boot to use kprobe_event interface Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-24 22:56 ` [PATCH v3 12/12] tracing: Documentation for in-kernel synthetic event API Tom Zanussi
  2020-01-27 13:48 ` [PATCH v3 00/12] tracing: Add support for in-kernel dynamic " Masami Hiramatsu
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add a test module that checks the basic functionality of the in-kernel
kprobe event command generation API by creating kprobe events from a
module.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 kernel/trace/Kconfig                 |  12 ++
 kernel/trace/Makefile                |   1 +
 kernel/trace/kprobe_event_gen_test.c | 225 +++++++++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+)
 create mode 100644 kernel/trace/kprobe_event_gen_test.c

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 4f2041166a2f..4484e783f68d 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -787,6 +787,18 @@ config SYNTH_EVENT_GEN_TEST
 
 	  If unsure, say N.
 
+config KPROBE_EVENT_GEN_TEST
+	tristate "Test module for in-kernel kprobe event generation"
+	depends on KPROBE_EVENTS
+	help
+          This option creates a test module to check the base
+          functionality of in-kernel kprobe event definition.
+
+          To test, insert the module, and then check the trace buffer
+	  for the generated kprobe events.
+
+	  If unsure, say N.
+
 config TRACE_EVAL_MAP_FILE
        bool "Show eval mappings for trace events"
        depends on TRACING
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 32012f50fb79..f9dcd19165fa 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_TRACING) += trace_printk.o
 obj-$(CONFIG_TRACING_MAP) += tracing_map.o
 obj-$(CONFIG_PREEMPTIRQ_DELAY_TEST) += preemptirq_delay_test.o
 obj-$(CONFIG_SYNTH_EVENT_GEN_TEST) += synth_event_gen_test.o
+obj-$(CONFIG_KPROBE_EVENT_GEN_TEST) += kprobe_event_gen_test.o
 obj-$(CONFIG_CONTEXT_SWITCH_TRACER) += trace_sched_switch.o
 obj-$(CONFIG_FUNCTION_TRACER) += trace_functions.o
 obj-$(CONFIG_PREEMPTIRQ_TRACEPOINTS) += trace_preemptirq.o
diff --git a/kernel/trace/kprobe_event_gen_test.c b/kernel/trace/kprobe_event_gen_test.c
new file mode 100644
index 000000000000..18b0f1cbb947
--- /dev/null
+++ b/kernel/trace/kprobe_event_gen_test.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test module for in-kernel kprobe event creation and generation.
+ *
+ * Copyright (C) 2019 Tom Zanussi <zanussi@kernel.org>
+ */
+
+#include <linux/module.h>
+#include <linux/trace_events.h>
+
+/*
+ * This module is a simple test of basic functionality for in-kernel
+ * kprobe/kretprobe event creation.  The first test uses
+ * kprobe_event_gen_cmd_start(), kprobe_event_add_fields() and
+ * kprobe_event_gen_cmd_end() to create a kprobe event, which is then
+ * enabled in order to generate trace output.  The second creates a
+ * kretprobe event using kretprobe_event_gen_cmd_start() and
+ * kretprobe_event_gen_cmd_end(), and is also then enabled.
+ *
+ * To test, select CONFIG_KPROBE_EVENT_GEN_TEST and build the module.
+ * Then:
+ *
+ * # insmod kernel/trace/kprobe_event_gen_test.ko
+ * # cat /sys/kernel/debug/tracing/trace
+ *
+ * You should see many instances of the "gen_kprobe_test" and
+ * "gen_kretprobe_test" events in the trace buffer.
+ *
+ * To remove the events, remove the module:
+ *
+ * # rmmod kprobe_event_gen_test
+ *
+ */
+
+static struct trace_event_file *gen_kprobe_test;
+static struct trace_event_file *gen_kretprobe_test;
+
+/*
+ * Test to make sure we can create a kprobe event, then add more
+ * fields.
+ */
+static int __init test_gen_kprobe_cmd(void)
+{
+	struct dynevent_cmd cmd;
+	char *buf;
+	int ret;
+
+	/* Create a buffer to hold the generated command */
+	buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/* Before generating the command, initialize the cmd object */
+	kprobe_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+	/*
+	 * Define the gen_kprobe_test event with the first 2 kprobe
+	 * fields.
+	 */
+	ret = kprobe_event_gen_cmd_start(&cmd, "gen_kprobe_test",
+					 "do_sys_open",
+					 "dfd=%ax", "filename=%dx");
+	if (ret)
+		goto free;
+
+	/* Use kprobe_event_add_fields to add the rest of the fields */
+
+	ret = kprobe_event_add_fields(&cmd, "flags=%cx", "mode=+4($stack)");
+	if (ret)
+		goto free;
+
+	/*
+	 * This actually creates the event.
+	 */
+	ret = kprobe_event_gen_cmd_end(&cmd);
+	if (ret)
+		goto free;
+
+	/*
+	 * Now get the gen_kprobe_test event file.  We need to prevent
+	 * the instance and event from disappearing from underneath
+	 * us, which trace_get_event_file() does (though in this case
+	 * we're using the top-level instance which never goes away).
+	 */
+	gen_kprobe_test = trace_get_event_file(NULL, "kprobes",
+					       "gen_kprobe_test");
+	if (IS_ERR(gen_kprobe_test)) {
+		ret = PTR_ERR(gen_kprobe_test);
+		goto delete;
+	}
+
+	/* Enable the event or you won't see anything */
+	ret = trace_array_set_clr_event(gen_kprobe_test->tr,
+					"kprobes", "gen_kprobe_test", true);
+	if (ret) {
+		trace_put_event_file(gen_kprobe_test);
+		goto delete;
+	}
+ out:
+	return ret;
+ delete:
+	/* We got an error after creating the event, delete it */
+	ret = kprobe_event_delete("gen_kprobe_test");
+ free:
+	kfree(buf);
+
+	goto out;
+}
+
+/*
+ * Test to make sure we can create a kretprobe event.
+ */
+static int __init test_gen_kretprobe_cmd(void)
+{
+	struct dynevent_cmd cmd;
+	char *buf;
+	int ret;
+
+	/* Create a buffer to hold the generated command */
+	buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/* Before generating the command, initialize the cmd object */
+	kprobe_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+	/*
+	 * Define the kretprobe event.
+	 */
+	ret = kretprobe_event_gen_cmd_start(&cmd, "gen_kretprobe_test",
+					    "do_sys_open",
+					    "$retval");
+	if (ret)
+		goto free;
+
+	/*
+	 * This actually creates the event.
+	 */
+	ret = kretprobe_event_gen_cmd_end(&cmd);
+	if (ret)
+		goto free;
+
+	/*
+	 * Now get the gen_kretprobe_test event file.  We need to
+	 * prevent the instance and event from disappearing from
+	 * underneath us, which trace_get_event_file() does (though in
+	 * this case we're using the top-level instance which never
+	 * goes away).
+	 */
+	gen_kretprobe_test = trace_get_event_file(NULL, "kprobes",
+						  "gen_kretprobe_test");
+	if (IS_ERR(gen_kretprobe_test)) {
+		ret = PTR_ERR(gen_kretprobe_test);
+		goto delete;
+	}
+
+	/* Enable the event or you won't see anything */
+	ret = trace_array_set_clr_event(gen_kretprobe_test->tr,
+					"kprobes", "gen_kretprobe_test", true);
+	if (ret) {
+		trace_put_event_file(gen_kretprobe_test);
+		goto delete;
+	}
+ out:
+	return ret;
+ delete:
+	/* We got an error after creating the event, delete it */
+	ret = kprobe_event_delete("gen_kretprobe_test");
+ free:
+	kfree(buf);
+
+	goto out;
+}
+
+static int __init kprobe_event_gen_test_init(void)
+{
+	int ret;
+
+	ret = test_gen_kprobe_cmd();
+	if (ret)
+		return ret;
+
+	ret = test_gen_kretprobe_cmd();
+	if (ret) {
+		WARN_ON(trace_array_set_clr_event(gen_kretprobe_test->tr,
+						  "kprobes",
+						  "gen_kretprobe_test", false));
+		trace_put_event_file(gen_kretprobe_test);
+		WARN_ON(kprobe_event_delete("gen_kretprobe_test"));
+	}
+
+	return ret;
+}
+
+static void __exit kprobe_event_gen_test_exit(void)
+{
+	/* Disable the event or you can't remove it */
+	WARN_ON(trace_array_set_clr_event(gen_kprobe_test->tr,
+					  "kprobes",
+					  "gen_kprobe_test", false));
+
+	/* Now give the file and instance back */
+	trace_put_event_file(gen_kprobe_test);
+
+	/* Now unregister and free the event */
+	WARN_ON(kprobe_event_delete("gen_kprobe_test"));
+
+	/* Disable the event or you can't remove it */
+	WARN_ON(trace_array_set_clr_event(gen_kprobe_test->tr,
+					  "kprobes",
+					  "gen_kretprobe_test", false));
+
+	/* Now give the file and instance back */
+	trace_put_event_file(gen_kretprobe_test);
+
+	/* Now unregister and free the event */
+	WARN_ON(kprobe_event_delete("gen_kretprobe_test"));
+}
+
+module_init(kprobe_event_gen_test_init)
+module_exit(kprobe_event_gen_test_exit)
+
+MODULE_AUTHOR("Tom Zanussi");
+MODULE_DESCRIPTION("kprobe event generation test");
+MODULE_LICENSE("GPL v2");
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 12/12] tracing: Documentation for in-kernel synthetic event API
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (10 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 11/12] tracing: Add kprobe event command generation test module Tom Zanussi
@ 2020-01-24 22:56 ` Tom Zanussi
  2020-01-27 13:48 ` [PATCH v3 00/12] tracing: Add support for in-kernel dynamic " Masami Hiramatsu
  12 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-24 22:56 UTC (permalink / raw)
  To: rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Add Documentation for creating and generating synthetic events from
modules.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 Documentation/trace/events.rst | 515 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 515 insertions(+)

diff --git a/Documentation/trace/events.rst b/Documentation/trace/events.rst
index f7e1fcc0953c..ed79b220bd07 100644
--- a/Documentation/trace/events.rst
+++ b/Documentation/trace/events.rst
@@ -525,3 +525,518 @@ The following commands are supported:
   event counts (hitcount).
 
   See Documentation/trace/histogram.rst for details and examples.
+
+6.3 In-kernel trace event API
+-----------------------------
+
+In most cases, the command-line interface to trace events is more than
+sufficient.  Sometimes, however, applications might find the need for
+more complex relationships than can be expressed through a simple
+series of linked command-line expressions, or putting together sets of
+commands may be simply too cumbersome.  An example might be an
+application that needs to 'listen' to the trace stream in order to
+maintain an in-kernel state machine detecting, for instance, when an
+illegal kernel state occurs in the scheduler.
+
+The trace event subsystem provides an in-kernel API allowing modules
+or other kernel code to generate user-defined 'synthetic' events at
+will, which can be used to either augment the existing trace stream
+and/or signal that a particular important state has occurred.
+
+A similar in-kernel API is also available for creating kprobe and
+kretprobe events.
+
+Both the synthetic event and k/ret/probe event APIs are built on top
+of a lower-level "dynevent_cmd" event command API, which is also
+available for more specialized applications, or as the basis of other
+higher-level trace event APIs.
+
+The API provided for these purposes is describe below and allows the
+following:
+
+  - dynamically creating synthetic event definitions
+  - dynamically creating kprobe and kretprobe event definitions
+  - tracing synthetic events from in-kernel code
+  - the low-level "dynevent_cmd" API
+
+6.3.1 Dyamically creating synthetic event definitions
+-----------------------------------------------------
+
+There are a couple ways to create a new synthetic event from a kernel
+module or other kernel code.
+
+The first creates the event in one step, using synth_event_create().
+In this method, the name of the event to create and an array defining
+the fields is supplied to synth_event_create().  If successful, a
+synthetic event with that name and fields will exist following that
+call.  For example, to create a new "schedtest" synthetic event:
+
+  ret = synth_event_create("schedtest", sched_fields,
+                           ARRAY_SIZE(sched_fields), THIS_MODULE);
+
+The sched_fields param in this example points to an array of struct
+synth_field_desc, each of which describes an event field by type and
+name:
+
+  static struct synth_field_desc sched_fields[] = {
+        { .type = "pid_t",              .name = "next_pid_field" },
+        { .type = "char[16]",           .name = "next_comm_field" },
+        { .type = "u64",                .name = "ts_ns" },
+        { .type = "u64",                .name = "ts_ms" },
+        { .type = "unsigned int",       .name = "cpu" },
+        { .type = "char[64]",           .name = "my_string_field" },
+        { .type = "int",                .name = "my_int_field" },
+  };
+
+See synth_field_size() for available types. If field_name contains [n]
+the field is considered to be an array.
+
+If the event is created from within a module, a pointer to the module
+must be passed to synth_event_create().  This will ensure that the
+trace buffer won't contain unreadable events when the module is
+removed.
+
+At this point, the event object is ready to be used for generating new
+events.
+
+In the second method, the event is created in several steps.  This
+allows events to be created dynamically and without the need to create
+and populate an array of fields beforehand.
+
+To use this method, an empty or partially empty synthetic event should
+first be created using synth_event_gen_cmd_start() or
+synth_event_gen_cmd_array_start().  For synth_event_gen_cmd_start(),
+the name of the event along with one or more pairs of args each pair
+representing a 'type field_name;' field specification should be
+supplied.  For synth_event_gen_cmd_array_start(), the name of the
+event along with an array of struct synth_field_desc should be
+supplied. Before calling synth_event_gen_cmd_start() or
+synth_event_gen_cmd_array_start(), the user should create and
+initialize a dynevent_cmd object using synth_event_cmd_init().
+
+For example, to create a new "schedtest" synthetic event with two
+fields:
+
+  struct dynevent_cmd cmd;
+  char *buf;
+
+  /* Create a buffer to hold the generated command */
+  buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+
+  /* Before generating the command, initialize the cmd object */
+  synth_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+  ret = synth_event_gen_cmd_start(&cmd, "schedtest", THIS_MODULE,
+                                  "pid_t", "next_pid_field",
+                                  "u64", "ts_ns");
+
+Alternatively, using an array of struct synth_field_desc fields
+containing the same information:
+
+  ret = synth_event_gen_cmd_array_start(&cmd, "schedtest", THIS_MODULE,
+                                        fields, n_fields);
+
+Once the synthetic event object has been created, it can then be
+populated with more fields.  Fields are added one by one using
+synth_event_add_field(), supplying the dynevent_cmd object, a field
+type, and a field name.  For example, to add a new int field named
+"intfield", the following call should be made:
+
+  ret = synth_event_add_field(&cmd, "int", "intfield");
+
+See synth_field_size() for available types. If field_name contains [n]
+the field is considered to be an array.
+
+A group of fields can also be added all at once using an array of
+synth_field_desc with add_synth_fields().  For example, this would add
+just the first four sched_fields:
+
+  ret = synth_event_add_fields(&cmd, sched_fields, 4);
+
+If you already have a string of the form 'type field_name',
+synth_event_add_field_str() can be used to add it as-is; it will
+also automatically append a ';' to the string.
+
+Once all the fields have been added, the event should be finalized and
+registered by calling the synth_event_gen_cmd_end() function:
+
+  ret = synth_event_gen_cmd_end(&cmd);
+
+At this point, the event object is ready to be used for tracing new
+events.
+
+6.3.3 Tracing synthetic events from in-kernel code
+--------------------------------------------------
+
+To trace a synthetic event, there are several options.  The first
+option is to trace the event in one call, using synth_event_trace()
+with a variable number of values, or synth_event_trace_array() with an
+array of values to be set.  A second option can be used to avoid the
+need for a pre-formed array of values or list of arguments, via
+synth_event_trace_start() and synth_event_trace_end() along with
+synth_event_add_next_val() or synth_event_add_val() to add the values
+piecewise.
+
+6.3.3.1 Tracing a synthetic event all at once
+---------------------------------------------
+
+To trace a synthetic event all at once, the synth_event_trace() or
+synth_event_trace_array() functions can be used.
+
+The synth_event_trace() function is passed the trace_event_file
+representing the synthetic event (which can be retrieved using
+trace_get_event_file() using the synthetic event name, "synthetic" as
+the system name, and the trace instance name (NULL if using the global
+trace array)), along with an variable number of u64 args, one for each
+synthetic event field, and the number of values being passed.
+
+So, to trace an event corresponding to the synthetic event definition
+above, code like the following could be used:
+
+  ret = synth_event_trace(create_synth_test, 7, /* number of values */
+                          444,             /* next_pid_field */
+                          (u64)"clackers", /* next_comm_field */
+                          1000000,         /* ts_ns */
+                          1000,            /* ts_ms */
+                          smp_processor_id(),/* cpu */
+                          (u64)"Thneed",   /* my_string_field */
+                          999);            /* my_int_field */
+
+All vals should be cast to u64, and string vals are just pointers to
+strings, cast to u64.  Strings will be copied into space reserved in
+the event for the string, using these pointers.
+
+Alternatively, the synth_event_trace_array() function can be used to
+accomplish the same thing.  It is passed the trace_event_file
+representing the synthetic event (which can be retrieved using
+trace_get_event_file() using the synthetic event name, "synthetic" as
+the system name, and the trace instance name (NULL if using the global
+trace array)), along with an array of u64, one for each synthetic
+event field.
+
+To trace an event corresponding to the synthetic event definition
+above, code like the following could be used:
+
+  u64 vals[7];
+
+  vals[0] = 777;                  /* next_pid_field */
+  vals[1] = (u64)"tiddlywinks";   /* next_comm_field */
+  vals[2] = 1000000;              /* ts_ns */
+  vals[3] = 1000;                 /* ts_ms */
+  vals[4] = smp_processor_id();   /* cpu */
+  vals[5] = (u64)"thneed";        /* my_string_field */
+  vals[6] = 398;                  /* my_int_field */
+
+The 'vals' array is just an array of u64, the number of which must
+match the number of field in the synthetic event, and which must be in
+the same order as the synthetic event fields.
+
+All vals should be cast to u64, and string vals are just pointers to
+strings, cast to u64.  Strings will be copied into space reserved in
+the event for the string, using these pointers.
+
+In order to trace a synthetic event, a pointer to the trace event file
+is needed.  The trace_get_event_file() function can be used to get
+it - it will find the file in the given trace instance (in this case
+NULL since the top trace array is being used) while at the same time
+preventing the instance containing it from going away:
+
+       schedtest_event_file = trace_get_event_file(NULL, "synthetic",
+                                                   "schedtest");
+
+Before tracing the event, it should be enabled in some way, otherwise
+the synthetic event won't actually show up in the trace buffer.
+
+To enable a synthetic event from the kernel, trace_array_set_clr_event()
+can be used (which is not specific to synthetic events, so does need
+the "synthetic" system name to be specified explicitly).
+
+To enable the event, pass 'true' to it:
+
+       trace_array_set_clr_event(schedtest_event_file->tr,
+                                 "synthetic", "schedtest", true);
+
+To disable it pass false:
+
+       trace_array_set_clr_event(schedtest_event_file->tr,
+                                 "synthetic", "schedtest", false);
+
+Finally, synth_event_trace_array() can be used to actually trace the
+event, which should be visible in the trace buffer afterwards:
+
+       ret = synth_event_trace_array(schedtest_event_file, vals,
+                                     ARRAY_SIZE(vals));
+
+To remove the synthetic event, the event should be disabled, and the
+trace instance should be 'put' back using trace_put_event_file():
+
+       trace_array_set_clr_event(schedtest_event_file->tr,
+                                 "synthetic", "schedtest", false);
+       trace_put_event_file(schedtest_event_file);
+
+If those have been successful, synth_event_delete() can be called to
+remove the event:
+
+       ret = synth_event_delete("schedtest");
+
+6.3.3.1 Tracing a synthetic event piecewise
+-------------------------------------------
+
+To trace a synthetic using the piecewise method described above, the
+synth_event_trace_start() function is used to 'open' the synthetic
+event trace:
+
+       struct synth_trace_state trace_state;
+
+       ret = synth_event_trace_start(schedtest_event_file, &trace_state);
+
+It's passed the trace_event_file representing the synthetic event
+using the same methods as described above, along with a pointer to a
+struct synth_trace_state object, which will be zeroed before use and
+used to maintain state between this and following calls.
+
+Once the event has been opened, which means space for it has been
+reserved in the trace buffer, the individual fields can be set.  There
+are two ways to do that, either one after another for each field in
+the event, which requires no lookups, or by name, which does.  The
+tradeoff is flexibility in doing the assignments vs the cost of a
+lookup per field.
+
+To assign the values one after the other without lookups,
+synth_event_add_next_val() should be used.  Each call is passed the
+same synth_trace_state object used in the synth_event_trace_start(),
+along with the value to set the next field in the event.  After each
+field is set, the 'cursor' points to the next field, which will be set
+by the subsequent call, continuing until all the fields have been set
+in order.  The same sequence of calls as in the above examples using
+this method would be (without error-handling code):
+
+       /* next_pid_field */
+       ret = synth_event_add_next_val(777, &trace_state);
+
+       /* next_comm_field */
+       ret = synth_event_add_next_val((u64)"slinky", &trace_state);
+
+       /* ts_ns */
+       ret = synth_event_add_next_val(1000000, &trace_state);
+
+       /* ts_ms */
+       ret = synth_event_add_next_val(1000, &trace_state);
+
+       /* cpu */
+       ret = synth_event_add_next_val(smp_processor_id(), &trace_state);
+
+       /* my_string_field */
+       ret = synth_event_add_next_val((u64)"thneed_2.01", &trace_state);
+
+       /* my_int_field */
+       ret = synth_event_add_next_val(395, &trace_state);
+
+To assign the values in any order, synth_event_add_val() should be
+used.  Each call is passed the same synth_trace_state object used in
+the synth_event_trace_start(), along with the field name of the field
+to set and the value to set it to.  The same sequence of calls as in
+the above examples using this method would be (without error-handling
+code):
+
+       ret = synth_event_add_val("next_pid_field", 777, &trace_state);
+       ret = synth_event_add_val("next_comm_field", (u64)"silly putty",
+                                 &trace_state);
+       ret = synth_event_add_val("ts_ns", 1000000, &trace_state);
+       ret = synth_event_add_val("ts_ms", 1000, &trace_state);
+       ret = synth_event_add_val("cpu", smp_processor_id(), &trace_state);
+       ret = synth_event_add_val("my_string_field", (u64)"thneed_9",
+                                 &trace_state);
+       ret = synth_event_add_val("my_int_field", 3999, &trace_state);
+
+Note that synth_event_add_next_val() and synth_event_add_val() are
+incompatible if used within the same trace of an event - either one
+can be used but not both at the same time.
+
+Finally, the event won't be actually traced until it's 'closed',
+which is done using synth_event_trace_end(), which takes only the
+struct synth_trace_state object used in the previous calls:
+
+       ret = synth_event_trace_end(&trace_state);
+
+Note that synth_event_trace_end() must be called at the end regardless
+of whether any of the add calls failed (say due to a bad field name
+being passed in).
+
+6.3.4 Dyamically creating kprobe and kretprobe event definitions
+----------------------------------------------------------------
+
+To create a kprobe or kretprobe trace event from kernel code, the
+kprobe_event_gen_cmd_start() or kretprobe_event_gen_cmd_start()
+functions can be used.
+
+To create a kprobe event, an empty or partially empty kprobe event
+should first be created using kprobe_event_gen_cmd_start().  The name
+of the event and the probe location should be specfied along with one
+or args each representing a probe field should be supplied to this
+function.  Before calling kprobe_event_gen_cmd_start(), the user
+should create and initialize a dynevent_cmd object using
+kprobe_event_cmd_init().
+
+For example, to create a new "schedtest" kprobe event with two fields:
+
+  struct dynevent_cmd cmd;
+  char *buf;
+
+  /* Create a buffer to hold the generated command */
+  buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+
+  /* Before generating the command, initialize the cmd object */
+  kprobe_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+  /*
+   * Define the gen_kprobe_test event with the first 2 kprobe
+   * fields.
+   */
+  ret = kprobe_event_gen_cmd_start(&cmd, "gen_kprobe_test", "do_sys_open",
+                                   "dfd=%ax", "filename=%dx");
+
+Once the kprobe event object has been created, it can then be
+populated with more fields.  Fields can be added using
+kprobe_event_add_fields(), supplying the dynevent_cmd object along
+with a variable arg list of probe fields.  For example, to add a
+couple additional fields, the following call could be made:
+
+  ret = kprobe_event_add_fields(&cmd, "flags=%cx", "mode=+4($stack)");
+
+Once all the fields have been added, the event should be finalized and
+registered by calling the kprobe_event_gen_cmd_end() or
+kretprobe_event_gen_cmd_end() functions, depending on whether a kprobe
+or kretprobe command was started:
+
+  ret = kprobe_event_gen_cmd_end(&cmd);
+
+or
+
+  ret = kretprobe_event_gen_cmd_end(&cmd);
+
+At this point, the event object is ready to be used for tracing new
+events.
+
+Similarly, a kretprobe event can be created using
+kretprobe_event_gen_cmd_start() with a probe name and location and
+additional params such as $retval:
+
+  ret = kretprobe_event_gen_cmd_start(&cmd, "gen_kretprobe_test",
+                                      "do_sys_open", "$retval");
+
+Similar to the synthetic event case, code like the following can be
+used to enable the newly created kprobe event:
+
+  gen_kprobe_test = trace_get_event_file(NULL, "kprobes", "gen_kprobe_test");
+
+  ret = trace_array_set_clr_event(gen_kprobe_test->tr,
+                                  "kprobes", "gen_kprobe_test", true);
+
+Finally, also similar to synthetic events, the following code can be
+used to give the kprobe event file back and delete the event:
+
+  trace_put_event_file(gen_kprobe_test);
+
+  ret = kprobe_event_delete("gen_kprobe_test");
+
+6.3.4 The "dynevent_cmd" low-level API
+--------------------------------------
+
+Both the in-kernel synthetic event and kprobe interfaces are built on
+top of a lower-level "dynevent_cmd" interface.  This interface is
+meant to provide the basis for higher-level interfaces such as the
+synthetic and kprobe interfaces, which can be used as examples.
+
+The basic idea is simple and amounts to providing a general-purpose
+layer that can be used to generate trace event commands.  The
+generated command strings can then be passed to the command-parsing
+and event creation code that already exists in the trace event
+subystem for creating the corresponding trace events.
+
+In a nutshell, the way it works is that the higher-level interface
+code creates a struct dynevent_cmd object, then uses a couple
+functions, dynevent_arg_add() and dynevent_arg_pair_add() to build up
+a command string, which finally causes the command to be executed
+using the dynevent_create() function.  The details of the interface
+are described below.
+
+The first step in building a new command string is to create and
+initialize an instance of a dynevent_cmd.  Here, for instance, we
+create a dynevent_cmd on the stack and initialize it:
+
+  struct dynevent_cmd cmd;
+  char *buf;
+  int ret;
+
+  buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+
+  dynevent_cmd_init(cmd, buf, maxlen, DYNEVENT_TYPE_FOO,
+                    foo_event_run_command);
+
+The dynevent_cmd initialization needs to be given a user-specified
+buffer and the length of the buffer (MAX_DYNEVENT_CMD_LEN can be used
+for this purpose - at 2k it's generally too big to be comfortably put
+on the stack, so is dynamically allocated), a dynevent type id, which
+is meant to be used to check that further API calls are for the
+correct command type, and a pointer to an event-specific run_command()
+callback that will be called to actually execute the event-specific
+command function.
+
+Once that's done, the command string can by built up by successive
+calls to argument-adding functions.
+
+To add a single argument, define and initialize a struct dynevent_arg
+or struct dynevent_arg_pair object.  Here's an example of the simplest
+possible arg addition, which is simply to append the given string as
+a whitespace-separated argument to the command:
+
+  struct dynevent_arg arg;
+
+  dynevent_arg_init(&arg, NULL, 0);
+
+  arg.str = name;
+
+  ret = dynevent_arg_add(cmd, &arg);
+
+The arg object is first initialized using dynevent_arg_init() and in
+this case the parameters are NULL or 0, which means there's no
+optional sanity-checking function or separator appended to the end of
+the arg.
+
+Here's another more complicated example using an 'arg pair', which is
+used to create an argument that consists of a couple components added
+together as a unit, for example, a 'type field_name;' arg or a simple
+expression arg e.g. 'flags=%cx':
+
+  struct dynevent_arg_pair arg_pair;
+
+  dynevent_arg_pair_init(&arg_pair, dynevent_foo_check_arg_fn, 0, ';');
+
+  arg_pair.lhs = type;
+  arg_pair.rhs = name;
+
+  ret = dynevent_arg_pair_add(cmd, &arg_pair);
+
+Again, the arg_pair is first initialized, in this case with a callback
+function used to check the sanity of the args (for example, that
+neither part of the pair is NULL), along with a character to be used
+to add an operator between the pair (here none) and a separator to be
+appended onto the end of the arg pair (here ';').
+
+There's also a dynevent_str_add() function that can be used to simply
+add a string as-is, with no spaces, delimeters, or arg check.
+
+Any number of dynevent_*_add() calls can be made to build up the string
+(until its length surpasses cmd->maxlen).  When all the arguments have
+been added and the command string is complete, the only thing left to
+do is run the command, which happens by simply calling
+dynevent_create():
+
+  ret = dynevent_create(&cmd);
+
+At that point, if the return value is 0, the dynamic event has been
+created and is ready to use.
+
+See the dynevent_cmd function definitions themselves for the details
+of the API.
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 04/12] tracing: Add dynamic event command creation interface
  2020-01-24 22:56 ` [PATCH v3 04/12] tracing: Add dynamic event command creation interface Tom Zanussi
@ 2020-01-27 13:44   ` Masami Hiramatsu
  2020-01-27 15:24     ` Tom Zanussi
  2020-01-29 18:00   ` Steven Rostedt
  1 sibling, 1 reply; 22+ messages in thread
From: Masami Hiramatsu @ 2020-01-27 13:44 UTC (permalink / raw)
  To: Tom Zanussi
  Cc: rostedt, artem.bityutskiy, mhiramat, linux-kernel,
	linux-rt-users, ndesaulniers

Hi Tom,

On Fri, 24 Jan 2020 16:56:15 -0600
Tom Zanussi <zanussi@kernel.org> wrote:

> Add an interface used to build up dynamic event creation commands,
> such as synthetic and kprobe events.  Interfaces specific to those
> particular types of events and others can be built on top of this
> interface.
> 
> Command creation is started by first using the dynevent_cmd_init()
> function to initialize the dynevent_cmd object.  Following that, args
> are appended and optionally checked by the dynevent_arg_add() and
> dynevent_arg_pair_add() functions, which use objects representing
> arguments and pairs of arguments, initialized respectively by
> dynevent_arg_init() and dynevent_arg_pair_init().  Finally, once all
> args have been successfully added, the command is finalized and
> actually created using dynevent_create().
> 
> The code here for actually printing into the dyn_event->cmd buffer
> using snprintf() etc was adapted from v4 of Masami's 'tracing/boot:
> Add synthetic event support' patch.

OK, these code are only for dyn_event. Then, it should be implemented
in kernel/trace/trace_dynevent.{c,h}. Of course some exported functions
should be in include/linux/trace_events.h.
Or, would you have any reason to put them in trace_events.c ?

Thank you,

> 
> Signed-off-by: Tom Zanussi <zanussi@kernel.org>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  include/linux/trace_events.h |  23 +++++
>  kernel/trace/trace.h         |  34 ++++++
>  kernel/trace/trace_events.c  | 240 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 297 insertions(+)
> 
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index 25fe743bcbaf..651b03d5e272 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -354,6 +354,29 @@ extern struct trace_event_file *trace_get_event_file(const char *instance,
>  						     const char *event);
>  extern void trace_put_event_file(struct trace_event_file *file);
>  
> +#define MAX_DYNEVENT_CMD_LEN	(2048)
> +
> +enum dynevent_type {
> +	DYNEVENT_TYPE_NONE,
> +};
> +
> +struct dynevent_cmd;
> +
> +typedef int (*dynevent_create_fn_t)(struct dynevent_cmd *cmd);
> +
> +struct dynevent_cmd {
> +	char			*buf;
> +	const char		*event_name;
> +	int			maxlen;
> +	int			remaining;
> +	unsigned int		n_fields;
> +	enum dynevent_type	type;
> +	dynevent_create_fn_t	run_command;
> +	void			*private_data;
> +};
> +
> +extern int dynevent_create(struct dynevent_cmd *cmd);
> +
>  extern int synth_event_delete(const char *name);
>  
>  /*
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 8fe3b16c8cec..0e8afc32d972 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -1877,6 +1877,40 @@ static inline bool event_command_needs_rec(struct event_command *cmd_ops)
>  
>  extern int trace_event_enable_disable(struct trace_event_file *file,
>  				      int enable, int soft_disable);
> +
> +extern void dynevent_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen,
> +			      enum dynevent_type type,
> +			      dynevent_create_fn_t run_command);
> +
> +typedef int (*dynevent_check_arg_fn_t)(void *data);
> +
> +struct dynevent_arg {
> +	const char		*str;
> +	char			separator; /* e.g. ';', ',', or nothing */
> +	dynevent_check_arg_fn_t	check_arg;
> +};
> +
> +extern void dynevent_arg_init(struct dynevent_arg *arg,
> +			      dynevent_check_arg_fn_t check_arg,
> +			      char separator);
> +extern int dynevent_arg_add(struct dynevent_cmd *cmd,
> +			    struct dynevent_arg *arg);
> +
> +struct dynevent_arg_pair {
> +	const char		*lhs;
> +	const char		*rhs;
> +	char			operator; /* e.g. '=' or nothing */
> +	char			separator; /* e.g. ';', ',', or nothing */
> +	dynevent_check_arg_fn_t	check_arg;
> +};
> +
> +extern void dynevent_arg_pair_init(struct dynevent_arg_pair *arg_pair,
> +				   dynevent_check_arg_fn_t check_arg,
> +				   char operator, char separator);
> +extern int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
> +				 struct dynevent_arg_pair *arg_pair);
> +extern int dynevent_str_add(struct dynevent_cmd *cmd, const char *str);
> +
>  extern int tracing_alloc_snapshot(void);
>  extern void tracing_snapshot_cond(struct trace_array *tr, void *cond_data);
>  extern int tracing_snapshot_cond_enable(struct trace_array *tr, void *cond_data, cond_update_fn_t update);
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 402426a82cfb..aa56bb6ff6ca 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -3300,6 +3300,246 @@ void __init trace_event_init(void)
>  	event_trace_enable();
>  }
>  
> +/**
> + * dynevent_arg_add - Add an arg to a dynevent_cmd
> + * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd
> + * @arg: The argument to append to the current cmd
> + *
> + * Append an argument to a dynevent_cmd.  The argument string will be
> + * appended to the current cmd string, followed by a separator, if
> + * applicable.  Before the argument is added, the check_arg()
> + * function, if defined, is called.
> + *
> + * The cmd string, separator, and check_arg() function should be set
> + * using the dynevent_arg_init() before any arguments are added using
> + * this function.
> + *
> + * Return: 0 if successful, error otherwise.
> + */
> +int dynevent_arg_add(struct dynevent_cmd *cmd,
> +		     struct dynevent_arg *arg)
> +{
> +	int ret = 0;
> +	int delta;
> +	char *q;
> +
> +	if (arg->check_arg) {
> +		ret = arg->check_arg(arg);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	q = cmd->buf + (cmd->maxlen - cmd->remaining);
> +
> +	delta = snprintf(q, cmd->remaining, " %s%c", arg->str, arg->separator);
> +	if (delta >= cmd->remaining) {
> +		pr_err("String is too long: %s\n", arg->str);
> +		return -E2BIG;
> +	}
> +	cmd->remaining -= delta;
> +
> +	return ret;
> +}
> +
> +/**
> + * dynevent_arg_pair_add - Add an arg pair to a dynevent_cmd
> + * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd
> + * @arg_pair: The argument pair to append to the current cmd
> + *
> + * Append an argument pair to a dynevent_cmd.  An argument pair
> + * consists of a left-hand-side argument and a right-hand-side
> + * argument separated by an operator, which can be whitespace, all
> + * followed by a separator, if applicable.  This can be used to add
> + * arguments of the form 'type variable_name;' or 'x+y'.
> + *
> + * The lhs argument string will be appended to the current cmd string,
> + * followed by an operator, if applicable, followd by the rhs string,
> + * followed finally by a separator, if applicable.  Before anything is
> + * added, the check_arg() function, if defined, is called.
> + *
> + * The cmd strings, operator, separator, and check_arg() function
> + * should be set using the dynevent_arg_pair_init() before any arguments
> + * are added using this function.
> + *
> + * Return: 0 if successful, error otherwise.
> + */
> +int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
> +			  struct dynevent_arg_pair *arg_pair)
> +{
> +	int ret = 0;
> +	int delta;
> +	char *q;
> +
> +	if (arg_pair->check_arg) {
> +		ret = arg_pair->check_arg(arg_pair);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	q = cmd->buf + (cmd->maxlen - cmd->remaining);
> +
> +	delta = snprintf(q, cmd->remaining, " %s%c", arg_pair->lhs,
> +			 arg_pair->operator);
> +	if (delta >= cmd->remaining) {
> +		pr_err("field string is too long: %s\n", arg_pair->lhs);
> +		return -E2BIG;
> +	}
> +	cmd->remaining -= delta; q += delta;
> +
> +	delta = snprintf(q, cmd->remaining, "%s%c", arg_pair->rhs,
> +			 arg_pair->separator);
> +	if (delta >= cmd->remaining) {
> +		pr_err("field string is too long: %s\n", arg_pair->rhs);
> +		return -E2BIG;
> +	}
> +	cmd->remaining -= delta; q += delta;
> +
> +	return ret;
> +}
> +
> +/**
> + * dynevent_str_add - Add a string to a dynevent_cmd
> + * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd
> + * @str: The string to append to the current cmd
> + *
> + * Append a string to a dynevent_cmd.  The string will be appended to
> + * the current cmd string as-is, with nothing prepended or appended.
> + *
> + * Return: 0 if successful, error otherwise.
> + */
> +int dynevent_str_add(struct dynevent_cmd *cmd, const char *str)
> +{
> +	int ret = 0;
> +	int delta;
> +	char *q;
> +
> +	q = cmd->buf + (cmd->maxlen - cmd->remaining);
> +
> +	delta = snprintf(q, cmd->remaining, "%s", str);
> +	if (delta >= cmd->remaining) {
> +		pr_err("String is too long: %s\n", str);
> +		return -E2BIG;
> +	}
> +	cmd->remaining -= delta;
> +
> +	return ret;
> +}
> +
> +/**
> + * dynevent_cmd_init - Initialize a dynevent_cmd object
> + * @cmd: A pointer to the dynevent_cmd struct representing the cmd
> + * @buf: A pointer to the buffer to generate the command into
> + * @maxlen: The length of the buffer the command will be generated into
> + * @type: The type of the cmd, checked against further operations
> + * @run_command: The type-specific function that will actually run the command
> + *
> + * Initialize a dynevent_cmd.  A dynevent_cmd is used to build up and
> + * run dynamic event creation commands, such as commands for creating
> + * synthetic and kprobe events.  Before calling any of the functions
> + * used to build the command, a dynevent_cmd object should be
> + * instantiated and initialized using this function.
> + *
> + * The initialization sets things up by saving a pointer to the
> + * user-supplied buffer and its length via the @buf and @maxlen
> + * params, and by saving the cmd-specific @type and @run_command
> + * params which are used to check subsequent dynevent_cmd operations
> + * and actually run the command when complete.
> + */
> +void dynevent_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen,
> +		       enum dynevent_type type,
> +		       dynevent_create_fn_t run_command)
> +{
> +	memset(cmd, '\0', sizeof(*cmd));
> +
> +	cmd->buf = buf;
> +	cmd->maxlen = maxlen;
> +	cmd->remaining = cmd->maxlen;
> +	cmd->type = type;
> +	cmd->run_command = run_command;
> +}
> +
> +/**
> + * dynevent_arg_init - Initialize a dynevent_arg object
> + * @arg: A pointer to the dynevent_arg struct representing the arg
> + * @check_arg: An (optional) pointer to a function checking arg sanity
> + * @separator: An (optional) separator, appended after adding the arg
> + *
> + * Initialize a dynevent_arg object.  A dynevent_arg represents an
> + * object used to append single arguments to the current command
> + * string.  The @check_arg function, if present, will be used to check
> + * the sanity of the current arg string (which is directly set by the
> + * caller).  After the arg string is successfully appended to the
> + * command string, the optional @separator is appended.  If no
> + * separator was specified when initializing the arg, a space will be
> + * appended.
> + */
> +void dynevent_arg_init(struct dynevent_arg *arg,
> +		       dynevent_check_arg_fn_t check_arg,
> +		       char separator)
> +{
> +	memset(arg, '\0', sizeof(*arg));
> +
> +	if (!separator)
> +		separator = ' ';
> +	arg->separator = separator;
> +
> +	arg->check_arg = check_arg;
> +}
> +
> +/**
> + * dynevent_arg_pair_init - Initialize a dynevent_arg_pair object
> + * @arg_pair: A pointer to the dynevent_arg_pair struct representing the arg
> + * @check_arg: An (optional) pointer to a function checking arg sanity
> + * @operator: An (optional) operator, appended after adding the first arg
> + * @separator: An (optional) separator, appended after adding the second arg
> + *
> + * Initialize a dynevent_arg_pair object.  A dynevent_arg_pair
> + * represents an object used to append argument pairs such as 'type
> + * variable_name;' or 'x+y' to the current command string.  An
> + * argument pair consists of a left-hand-side argument and a
> + * right-hand-side argument separated by an operator, which can be
> + * whitespace, all followed by a separator, if applicable. The
> + * @check_arg function, if present, will be used to check the sanity
> + * of the current arg strings (which is directly set by the caller).
> + * After the first arg string is successfully appended to the command
> + * string, the optional @operator is appended, followed by the second
> + * arg and and optional @separator.  If no separator was specified
> + * when initializing the arg, a space will be appended.
> + */
> +void dynevent_arg_pair_init(struct dynevent_arg_pair *arg_pair,
> +			    dynevent_check_arg_fn_t check_arg,
> +			    char operator, char separator)
> +{
> +	memset(arg_pair, '\0', sizeof(*arg_pair));
> +
> +	if (!operator)
> +		operator = ' ';
> +	arg_pair->operator = operator;
> +
> +	if (!separator)
> +		separator = ' ';
> +	arg_pair->separator = separator;
> +
> +	arg_pair->check_arg = check_arg;
> +}
> +
> +/**
> + * dynevent_create - Create the dynamic event contained in dynevent_cmd
> + * @cmd: The dynevent_cmd object containing the dynamic event creation command
> + *
> + * Once a dynevent_cmd object has been successfully built up via the
> + * dynevent_cmd_init(), dynevent_arg_add() and dynevent_arg_pair_add()
> + * functions, this function runs the final command to actually create
> + * the event.
> + *
> + * Return: 0 if the event was successfully created, error otherwise.
> + */
> +int dynevent_create(struct dynevent_cmd *cmd)
> +{
> +	return cmd->run_command(cmd);
> +}
> +EXPORT_SYMBOL_GPL(dynevent_create);
> +
>  #ifdef CONFIG_EVENT_TRACE_STARTUP_TEST
>  
>  static DEFINE_SPINLOCK(test_spinlock);
> -- 
> 2.14.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API
  2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
                   ` (11 preceding siblings ...)
  2020-01-24 22:56 ` [PATCH v3 12/12] tracing: Documentation for in-kernel synthetic event API Tom Zanussi
@ 2020-01-27 13:48 ` Masami Hiramatsu
  2020-01-27 15:28   ` Tom Zanussi
  12 siblings, 1 reply; 22+ messages in thread
From: Masami Hiramatsu @ 2020-01-27 13:48 UTC (permalink / raw)
  To: Tom Zanussi
  Cc: rostedt, artem.bityutskiy, mhiramat, linux-kernel,
	linux-rt-users, ndesaulniers

Hi Tom,

On Fri, 24 Jan 2020 16:56:11 -0600
Tom Zanussi <zanussi@kernel.org> wrote:

> Hi,
> 
> This is v3 of 'tracing: Add support for in-kernel dynamic event API',
> incorporating changes based on suggestions from Masami and Steven.
> 
>   - Rebased to trace/for-next
> 
>   - Regularized the entire API to use synth_event_*, kprobe_event_*,
>     dynevent_*, and added some new macros and functions to make things
>     more consistent
> 
>   - Introduced trace_array_find_get() and used it in
>     trace_array_get_file() as suggested
>   
>   - Removed exports from dynevent_cmd functions that didn't need to be
>     exported
> 
>   - Switched the param order of __kprobe_event_gen_cmd_start() to fix
>     a problem found building with clang.  Apparently varargs and
>     implicit promotion of types like bool don't mix.  Thanks to Nick
>     Desaulniers for pointing that out.
> 
>   - Updated the documentation for all of the above
> 
> Text from the v2 posting:
> 
> This is v2 of the previous 'tracing: Add support for in-kernel
> synthetic event API', and is largely a rewrite based on suggestions
> from Masami to expand the scope to include other types of dynamic
> events such as kprobe events, in addition to the original sythetic
> event focus.

Yeah, v3 basically looks good to me now :)
Please see my comment on [4/12]. Others are good. You can put my
Acked-by to those patches. 

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
except for [04/12].

Thank you!

> 
> The functionality of the original API remains available, though it's
> in a slightly different form due to the use of the new dynevent_cmd
> API it's now based on.  The dynevent_cmd API provides a common dynamic
> event command generation capability that both the synthetic event API
> and the kprobe event API are now based on, and there are now test
> modules demonstrating both the synthetic event and kprobes APIs.
> 
> A couple of the patches are snippets from Masami's 'tracing:
> bootconfig: Boot-time tracing and Extra boot config' series, and the
> patch implementing the dynevent_cmd API includes some of the
> spnprintf() generating code from that patchset.
> 
> Because I used Masami's gen_*_cmd() naming suggestion for generating
> the commands, the previous patchset's generate_*() functions were
> renamed to trace_*() to avoid confusion, and probably is better naming
> anyway.
> 
> An overview of the user-visible changes in comparison to v1:
> 
>   - create_synth_event() using an array of synth_desc_fields remains
>     unchanged and works the same way as previously
> 
>   - gen_synth_cmd() takes a variable-length number of args which
>     represent 'type field;' pairs.  Using this with no field args
>     basically replaces the previous 'create_empty_synth_event()'
> 
>   - The 'add_synth_field()' and 'add_synth_fields()' function from v1
>     are essentially the same except that they now take a dynevent_cmd
>     instead of a synth_event pointer
> 
>   - The finalize_synth_event() from v1 is replaced by
>     create_dynevent() in the new version.
>   
>   - The new v2 API includes some additional functions to initialize
>     the dynevent_cmd - synth_dynevent_cmd() is used to do that.  While
>     it's an extra step, it makes it easier to do error handling.
> 
>   - There's a new trace_synth_event() function that traces a synthetic
>     event using a variable-arg list of values.
> 
>   - The original generate_synth_event() using an array of values is
>     now called trace_synth_event_array().
> 
>   - For piecewise event tracing, the original
>     generate_synth_event_start() and generate_synth_event_end() have
>     now been renamed to trace_synth_event_end().
> 
>   - add_next_synth_val() and add_synth_val() remain the same.
> 
>   - A similar API and test module demonstrating the API has been added
>     for kprobe events
> 
>   - Both the synthetic events and kprobe events API is based on the
>     dynevent_cmd API, newly added
> 
>   - The Documentation for all of the above has been updated
> 
> Text from the orginal v1 posting:
> 
> I've recently had several requests and suggestions from users to add
> support for the creation and generation of synthetic events from
> kernel code such as modules, and not just from the available command
> line commands.
> 
> This patchset adds support for that.  The first three patches add some
> minor preliminary setup, followed by the two main patches that add the
> ability to create and generate synthetic events from the kernel.  The
> next patch adds a test module that demonstrates actual use of the API
> and verifies that it works as intended, followed by Documentation.
> 
> Special thanks to Artem Bityutskiy, who worked with me over several
> iterations of the API, and who had many great suggestions on the
> details of the interface, and pointed out several problems with the
> code itself.
> 
> The following changes since commit 659ded30272d67a04b3692f0bfa12263be20d790:
> 
>   trace/kprobe: Remove unused MAX_KPROBE_CMDLINE_SIZE (2020-01-22 07:07:38 -0500)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.git ftrace/synth-event-gen-v3
> 
> Tom Zanussi (12):
>   tracing: Add trace_array_find/_get() to find instance trace arrays
>   tracing: Add trace_get/put_event_file()
>   tracing: Add synth_event_delete()
>   tracing: Add dynamic event command creation interface
>   tracing: Add synthetic event command generation functions
>   tracing: Change trace_boot to use synth_event interface
>   tracing: Add synth_event_trace() and related functions
>   tracing: Add synth event generation test module
>   tracing: Add kprobe event command generation functions
>   tracing: Change trace_boot to use kprobe_event interface
>   tracing: Add kprobe event command generation test module
>   tracing: Documentation for in-kernel synthetic event API
> 
>  Documentation/trace/events.rst       | 515 ++++++++++++++++++++
>  include/linux/trace_events.h         | 124 +++++
>  kernel/trace/Kconfig                 |  25 +
>  kernel/trace/Makefile                |   2 +
>  kernel/trace/kprobe_event_gen_test.c | 225 +++++++++
>  kernel/trace/synth_event_gen_test.c  | 523 ++++++++++++++++++++
>  kernel/trace/trace.c                 |  43 +-
>  kernel/trace/trace.h                 |  36 ++
>  kernel/trace/trace_boot.c            |  66 ++-
>  kernel/trace/trace_events.c          | 325 +++++++++++++
>  kernel/trace/trace_events_hist.c     | 896 ++++++++++++++++++++++++++++++++++-
>  kernel/trace/trace_kprobe.c          | 160 ++++++-
>  12 files changed, 2868 insertions(+), 72 deletions(-)
>  create mode 100644 kernel/trace/kprobe_event_gen_test.c
>  create mode 100644 kernel/trace/synth_event_gen_test.c
> 
> -- 
> 2.14.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 04/12] tracing: Add dynamic event command creation interface
  2020-01-27 13:44   ` Masami Hiramatsu
@ 2020-01-27 15:24     ` Tom Zanussi
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-27 15:24 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: rostedt, artem.bityutskiy, linux-kernel, linux-rt-users, ndesaulniers

Hi Masami,

On Mon, 2020-01-27 at 22:44 +0900, Masami Hiramatsu wrote:
> Hi Tom,
> 
> On Fri, 24 Jan 2020 16:56:15 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
> 
> > Add an interface used to build up dynamic event creation commands,
> > such as synthetic and kprobe events.  Interfaces specific to those
> > particular types of events and others can be built on top of this
> > interface.
> > 
> > Command creation is started by first using the dynevent_cmd_init()
> > function to initialize the dynevent_cmd object.  Following that,
> > args
> > are appended and optionally checked by the dynevent_arg_add() and
> > dynevent_arg_pair_add() functions, which use objects representing
> > arguments and pairs of arguments, initialized respectively by
> > dynevent_arg_init() and dynevent_arg_pair_init().  Finally, once
> > all
> > args have been successfully added, the command is finalized and
> > actually created using dynevent_create().
> > 
> > The code here for actually printing into the dyn_event->cmd buffer
> > using snprintf() etc was adapted from v4 of Masami's 'tracing/boot:
> > Add synthetic event support' patch.
> 
> OK, these code are only for dyn_event. Then, it should be implemented
> in kernel/trace/trace_dynevent.{c,h}. Of course some exported
> functions
> should be in include/linux/trace_events.h.
> Or, would you have any reason to put them in trace_events.c ?
> 

I put this code in trace_events.c since it's about dynamic trace
events, but I agree it would be nicer to have it in separate files. 
I'll move them there in the next version.

Thanks,

Tom

> Thank you,
> 
> > 
> > Signed-off-by: Tom Zanussi <zanussi@kernel.org>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  include/linux/trace_events.h |  23 +++++
> >  kernel/trace/trace.h         |  34 ++++++
> >  kernel/trace/trace_events.c  | 240
> > +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 297 insertions(+)
> > 
> > diff --git a/include/linux/trace_events.h
> > b/include/linux/trace_events.h
> > index 25fe743bcbaf..651b03d5e272 100644
> > --- a/include/linux/trace_events.h
> > +++ b/include/linux/trace_events.h
> > @@ -354,6 +354,29 @@ extern struct trace_event_file
> > *trace_get_event_file(const char *instance,
> >  						     const char
> > *event);
> >  extern void trace_put_event_file(struct trace_event_file *file);
> >  
> > +#define MAX_DYNEVENT_CMD_LEN	(2048)
> > +
> > +enum dynevent_type {
> > +	DYNEVENT_TYPE_NONE,
> > +};
> > +
> > +struct dynevent_cmd;
> > +
> > +typedef int (*dynevent_create_fn_t)(struct dynevent_cmd *cmd);
> > +
> > +struct dynevent_cmd {
> > +	char			*buf;
> > +	const char		*event_name;
> > +	int			maxlen;
> > +	int			remaining;
> > +	unsigned int		n_fields;
> > +	enum dynevent_type	type;
> > +	dynevent_create_fn_t	run_command;
> > +	void			*private_data;
> > +};
> > +
> > +extern int dynevent_create(struct dynevent_cmd *cmd);
> > +
> >  extern int synth_event_delete(const char *name);
> >  
> >  /*
> > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> > index 8fe3b16c8cec..0e8afc32d972 100644
> > --- a/kernel/trace/trace.h
> > +++ b/kernel/trace/trace.h
> > @@ -1877,6 +1877,40 @@ static inline bool
> > event_command_needs_rec(struct event_command *cmd_ops)
> >  
> >  extern int trace_event_enable_disable(struct trace_event_file
> > *file,
> >  				      int enable, int
> > soft_disable);
> > +
> > +extern void dynevent_cmd_init(struct dynevent_cmd *cmd, char *buf,
> > int maxlen,
> > +			      enum dynevent_type type,
> > +			      dynevent_create_fn_t run_command);
> > +
> > +typedef int (*dynevent_check_arg_fn_t)(void *data);
> > +
> > +struct dynevent_arg {
> > +	const char		*str;
> > +	char			separator; /* e.g. ';', ',',
> > or nothing */
> > +	dynevent_check_arg_fn_t	check_arg;
> > +};
> > +
> > +extern void dynevent_arg_init(struct dynevent_arg *arg,
> > +			      dynevent_check_arg_fn_t check_arg,
> > +			      char separator);
> > +extern int dynevent_arg_add(struct dynevent_cmd *cmd,
> > +			    struct dynevent_arg *arg);
> > +
> > +struct dynevent_arg_pair {
> > +	const char		*lhs;
> > +	const char		*rhs;
> > +	char			operator; /* e.g. '=' or
> > nothing */
> > +	char			separator; /* e.g. ';', ',',
> > or nothing */
> > +	dynevent_check_arg_fn_t	check_arg;
> > +};
> > +
> > +extern void dynevent_arg_pair_init(struct dynevent_arg_pair
> > *arg_pair,
> > +				   dynevent_check_arg_fn_t
> > check_arg,
> > +				   char operator, char separator);
> > +extern int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
> > +				 struct dynevent_arg_pair
> > *arg_pair);
> > +extern int dynevent_str_add(struct dynevent_cmd *cmd, const char
> > *str);
> > +
> >  extern int tracing_alloc_snapshot(void);
> >  extern void tracing_snapshot_cond(struct trace_array *tr, void
> > *cond_data);
> >  extern int tracing_snapshot_cond_enable(struct trace_array *tr,
> > void *cond_data, cond_update_fn_t update);
> > diff --git a/kernel/trace/trace_events.c
> > b/kernel/trace/trace_events.c
> > index 402426a82cfb..aa56bb6ff6ca 100644
> > --- a/kernel/trace/trace_events.c
> > +++ b/kernel/trace/trace_events.c
> > @@ -3300,6 +3300,246 @@ void __init trace_event_init(void)
> >  	event_trace_enable();
> >  }
> >  
> > +/**
> > + * dynevent_arg_add - Add an arg to a dynevent_cmd
> > + * @cmd: A pointer to the dynevent_cmd struct representing the new
> > event cmd
> > + * @arg: The argument to append to the current cmd
> > + *
> > + * Append an argument to a dynevent_cmd.  The argument string will
> > be
> > + * appended to the current cmd string, followed by a separator, if
> > + * applicable.  Before the argument is added, the check_arg()
> > + * function, if defined, is called.
> > + *
> > + * The cmd string, separator, and check_arg() function should be
> > set
> > + * using the dynevent_arg_init() before any arguments are added
> > using
> > + * this function.
> > + *
> > + * Return: 0 if successful, error otherwise.
> > + */
> > +int dynevent_arg_add(struct dynevent_cmd *cmd,
> > +		     struct dynevent_arg *arg)
> > +{
> > +	int ret = 0;
> > +	int delta;
> > +	char *q;
> > +
> > +	if (arg->check_arg) {
> > +		ret = arg->check_arg(arg);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	q = cmd->buf + (cmd->maxlen - cmd->remaining);
> > +
> > +	delta = snprintf(q, cmd->remaining, " %s%c", arg->str,
> > arg->separator);
> > +	if (delta >= cmd->remaining) {
> > +		pr_err("String is too long: %s\n", arg->str);
> > +		return -E2BIG;
> > +	}
> > +	cmd->remaining -= delta;
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * dynevent_arg_pair_add - Add an arg pair to a dynevent_cmd
> > + * @cmd: A pointer to the dynevent_cmd struct representing the new
> > event cmd
> > + * @arg_pair: The argument pair to append to the current cmd
> > + *
> > + * Append an argument pair to a dynevent_cmd.  An argument pair
> > + * consists of a left-hand-side argument and a right-hand-side
> > + * argument separated by an operator, which can be whitespace, all
> > + * followed by a separator, if applicable.  This can be used to
> > add
> > + * arguments of the form 'type variable_name;' or 'x+y'.
> > + *
> > + * The lhs argument string will be appended to the current cmd
> > string,
> > + * followed by an operator, if applicable, followd by the rhs
> > string,
> > + * followed finally by a separator, if applicable.  Before
> > anything is
> > + * added, the check_arg() function, if defined, is called.
> > + *
> > + * The cmd strings, operator, separator, and check_arg() function
> > + * should be set using the dynevent_arg_pair_init() before any
> > arguments
> > + * are added using this function.
> > + *
> > + * Return: 0 if successful, error otherwise.
> > + */
> > +int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
> > +			  struct dynevent_arg_pair *arg_pair)
> > +{
> > +	int ret = 0;
> > +	int delta;
> > +	char *q;
> > +
> > +	if (arg_pair->check_arg) {
> > +		ret = arg_pair->check_arg(arg_pair);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	q = cmd->buf + (cmd->maxlen - cmd->remaining);
> > +
> > +	delta = snprintf(q, cmd->remaining, " %s%c", arg_pair-
> > >lhs,
> > +			 arg_pair->operator);
> > +	if (delta >= cmd->remaining) {
> > +		pr_err("field string is too long: %s\n", arg_pair-
> > >lhs);
> > +		return -E2BIG;
> > +	}
> > +	cmd->remaining -= delta; q += delta;
> > +
> > +	delta = snprintf(q, cmd->remaining, "%s%c", arg_pair->rhs,
> > +			 arg_pair->separator);
> > +	if (delta >= cmd->remaining) {
> > +		pr_err("field string is too long: %s\n", arg_pair-
> > >rhs);
> > +		return -E2BIG;
> > +	}
> > +	cmd->remaining -= delta; q += delta;
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * dynevent_str_add - Add a string to a dynevent_cmd
> > + * @cmd: A pointer to the dynevent_cmd struct representing the new
> > event cmd
> > + * @str: The string to append to the current cmd
> > + *
> > + * Append a string to a dynevent_cmd.  The string will be appended
> > to
> > + * the current cmd string as-is, with nothing prepended or
> > appended.
> > + *
> > + * Return: 0 if successful, error otherwise.
> > + */
> > +int dynevent_str_add(struct dynevent_cmd *cmd, const char *str)
> > +{
> > +	int ret = 0;
> > +	int delta;
> > +	char *q;
> > +
> > +	q = cmd->buf + (cmd->maxlen - cmd->remaining);
> > +
> > +	delta = snprintf(q, cmd->remaining, "%s", str);
> > +	if (delta >= cmd->remaining) {
> > +		pr_err("String is too long: %s\n", str);
> > +		return -E2BIG;
> > +	}
> > +	cmd->remaining -= delta;
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * dynevent_cmd_init - Initialize a dynevent_cmd object
> > + * @cmd: A pointer to the dynevent_cmd struct representing the cmd
> > + * @buf: A pointer to the buffer to generate the command into
> > + * @maxlen: The length of the buffer the command will be generated
> > into
> > + * @type: The type of the cmd, checked against further operations
> > + * @run_command: The type-specific function that will actually run
> > the command
> > + *
> > + * Initialize a dynevent_cmd.  A dynevent_cmd is used to build up
> > and
> > + * run dynamic event creation commands, such as commands for
> > creating
> > + * synthetic and kprobe events.  Before calling any of the
> > functions
> > + * used to build the command, a dynevent_cmd object should be
> > + * instantiated and initialized using this function.
> > + *
> > + * The initialization sets things up by saving a pointer to the
> > + * user-supplied buffer and its length via the @buf and @maxlen
> > + * params, and by saving the cmd-specific @type and @run_command
> > + * params which are used to check subsequent dynevent_cmd
> > operations
> > + * and actually run the command when complete.
> > + */
> > +void dynevent_cmd_init(struct dynevent_cmd *cmd, char *buf, int
> > maxlen,
> > +		       enum dynevent_type type,
> > +		       dynevent_create_fn_t run_command)
> > +{
> > +	memset(cmd, '\0', sizeof(*cmd));
> > +
> > +	cmd->buf = buf;
> > +	cmd->maxlen = maxlen;
> > +	cmd->remaining = cmd->maxlen;
> > +	cmd->type = type;
> > +	cmd->run_command = run_command;
> > +}
> > +
> > +/**
> > + * dynevent_arg_init - Initialize a dynevent_arg object
> > + * @arg: A pointer to the dynevent_arg struct representing the arg
> > + * @check_arg: An (optional) pointer to a function checking arg
> > sanity
> > + * @separator: An (optional) separator, appended after adding the
> > arg
> > + *
> > + * Initialize a dynevent_arg object.  A dynevent_arg represents an
> > + * object used to append single arguments to the current command
> > + * string.  The @check_arg function, if present, will be used to
> > check
> > + * the sanity of the current arg string (which is directly set by
> > the
> > + * caller).  After the arg string is successfully appended to the
> > + * command string, the optional @separator is appended.  If no
> > + * separator was specified when initializing the arg, a space will
> > be
> > + * appended.
> > + */
> > +void dynevent_arg_init(struct dynevent_arg *arg,
> > +		       dynevent_check_arg_fn_t check_arg,
> > +		       char separator)
> > +{
> > +	memset(arg, '\0', sizeof(*arg));
> > +
> > +	if (!separator)
> > +		separator = ' ';
> > +	arg->separator = separator;
> > +
> > +	arg->check_arg = check_arg;
> > +}
> > +
> > +/**
> > + * dynevent_arg_pair_init - Initialize a dynevent_arg_pair object
> > + * @arg_pair: A pointer to the dynevent_arg_pair struct
> > representing the arg
> > + * @check_arg: An (optional) pointer to a function checking arg
> > sanity
> > + * @operator: An (optional) operator, appended after adding the
> > first arg
> > + * @separator: An (optional) separator, appended after adding the
> > second arg
> > + *
> > + * Initialize a dynevent_arg_pair object.  A dynevent_arg_pair
> > + * represents an object used to append argument pairs such as
> > 'type
> > + * variable_name;' or 'x+y' to the current command string.  An
> > + * argument pair consists of a left-hand-side argument and a
> > + * right-hand-side argument separated by an operator, which can be
> > + * whitespace, all followed by a separator, if applicable. The
> > + * @check_arg function, if present, will be used to check the
> > sanity
> > + * of the current arg strings (which is directly set by the
> > caller).
> > + * After the first arg string is successfully appended to the
> > command
> > + * string, the optional @operator is appended, followed by the
> > second
> > + * arg and and optional @separator.  If no separator was specified
> > + * when initializing the arg, a space will be appended.
> > + */
> > +void dynevent_arg_pair_init(struct dynevent_arg_pair *arg_pair,
> > +			    dynevent_check_arg_fn_t check_arg,
> > +			    char operator, char separator)
> > +{
> > +	memset(arg_pair, '\0', sizeof(*arg_pair));
> > +
> > +	if (!operator)
> > +		operator = ' ';
> > +	arg_pair->operator = operator;
> > +
> > +	if (!separator)
> > +		separator = ' ';
> > +	arg_pair->separator = separator;
> > +
> > +	arg_pair->check_arg = check_arg;
> > +}
> > +
> > +/**
> > + * dynevent_create - Create the dynamic event contained in
> > dynevent_cmd
> > + * @cmd: The dynevent_cmd object containing the dynamic event
> > creation command
> > + *
> > + * Once a dynevent_cmd object has been successfully built up via
> > the
> > + * dynevent_cmd_init(), dynevent_arg_add() and
> > dynevent_arg_pair_add()
> > + * functions, this function runs the final command to actually
> > create
> > + * the event.
> > + *
> > + * Return: 0 if the event was successfully created, error
> > otherwise.
> > + */
> > +int dynevent_create(struct dynevent_cmd *cmd)
> > +{
> > +	return cmd->run_command(cmd);
> > +}
> > +EXPORT_SYMBOL_GPL(dynevent_create);
> > +
> >  #ifdef CONFIG_EVENT_TRACE_STARTUP_TEST
> >  
> >  static DEFINE_SPINLOCK(test_spinlock);
> > -- 
> > 2.14.1
> > 
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API
  2020-01-27 13:48 ` [PATCH v3 00/12] tracing: Add support for in-kernel dynamic " Masami Hiramatsu
@ 2020-01-27 15:28   ` Tom Zanussi
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-27 15:28 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: rostedt, artem.bityutskiy, linux-kernel, linux-rt-users, ndesaulniers

Hi Masami,

On Mon, 2020-01-27 at 22:48 +0900, Masami Hiramatsu wrote:
> Hi Tom,
> 
> On Fri, 24 Jan 2020 16:56:11 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
> 
> > Hi,
> > 
> > This is v3 of 'tracing: Add support for in-kernel dynamic event
> > API',
> > incorporating changes based on suggestions from Masami and Steven.
> > 
> >   - Rebased to trace/for-next
> > 
> >   - Regularized the entire API to use synth_event_*,
> > kprobe_event_*,
> >     dynevent_*, and added some new macros and functions to make
> > things
> >     more consistent
> > 
> >   - Introduced trace_array_find_get() and used it in
> >     trace_array_get_file() as suggested
> >   
> >   - Removed exports from dynevent_cmd functions that didn't need to
> > be
> >     exported
> > 
> >   - Switched the param order of __kprobe_event_gen_cmd_start() to
> > fix
> >     a problem found building with clang.  Apparently varargs and
> >     implicit promotion of types like bool don't mix.  Thanks to
> > Nick
> >     Desaulniers for pointing that out.
> > 
> >   - Updated the documentation for all of the above
> > 
> > Text from the v2 posting:
> > 
> > This is v2 of the previous 'tracing: Add support for in-kernel
> > synthetic event API', and is largely a rewrite based on suggestions
> > from Masami to expand the scope to include other types of dynamic
> > events such as kprobe events, in addition to the original sythetic
> > event focus.
> 
> Yeah, v3 basically looks good to me now :)
> Please see my comment on [4/12]. Others are good. You can put my
> Acked-by to those patches. 
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> except for [04/12].
> 
> Thank you!
> 

OK, I'll add your acks for those.

Thanks for all your comments!

Tom

> > 
> > The functionality of the original API remains available, though
> > it's
> > in a slightly different form due to the use of the new dynevent_cmd
> > API it's now based on.  The dynevent_cmd API provides a common
> > dynamic
> > event command generation capability that both the synthetic event
> > API
> > and the kprobe event API are now based on, and there are now test
> > modules demonstrating both the synthetic event and kprobes APIs.
> > 
> > A couple of the patches are snippets from Masami's 'tracing:
> > bootconfig: Boot-time tracing and Extra boot config' series, and
> > the
> > patch implementing the dynevent_cmd API includes some of the
> > spnprintf() generating code from that patchset.
> > 
> > Because I used Masami's gen_*_cmd() naming suggestion for
> > generating
> > the commands, the previous patchset's generate_*() functions were
> > renamed to trace_*() to avoid confusion, and probably is better
> > naming
> > anyway.
> > 
> > An overview of the user-visible changes in comparison to v1:
> > 
> >   - create_synth_event() using an array of synth_desc_fields
> > remains
> >     unchanged and works the same way as previously
> > 
> >   - gen_synth_cmd() takes a variable-length number of args which
> >     represent 'type field;' pairs.  Using this with no field args
> >     basically replaces the previous 'create_empty_synth_event()'
> > 
> >   - The 'add_synth_field()' and 'add_synth_fields()' function from
> > v1
> >     are essentially the same except that they now take a
> > dynevent_cmd
> >     instead of a synth_event pointer
> > 
> >   - The finalize_synth_event() from v1 is replaced by
> >     create_dynevent() in the new version.
> >   
> >   - The new v2 API includes some additional functions to initialize
> >     the dynevent_cmd - synth_dynevent_cmd() is used to do
> > that.  While
> >     it's an extra step, it makes it easier to do error handling.
> > 
> >   - There's a new trace_synth_event() function that traces a
> > synthetic
> >     event using a variable-arg list of values.
> > 
> >   - The original generate_synth_event() using an array of values is
> >     now called trace_synth_event_array().
> > 
> >   - For piecewise event tracing, the original
> >     generate_synth_event_start() and generate_synth_event_end()
> > have
> >     now been renamed to trace_synth_event_end().
> > 
> >   - add_next_synth_val() and add_synth_val() remain the same.
> > 
> >   - A similar API and test module demonstrating the API has been
> > added
> >     for kprobe events
> > 
> >   - Both the synthetic events and kprobe events API is based on the
> >     dynevent_cmd API, newly added
> > 
> >   - The Documentation for all of the above has been updated
> > 
> > Text from the orginal v1 posting:
> > 
> > I've recently had several requests and suggestions from users to
> > add
> > support for the creation and generation of synthetic events from
> > kernel code such as modules, and not just from the available
> > command
> > line commands.
> > 
> > This patchset adds support for that.  The first three patches add
> > some
> > minor preliminary setup, followed by the two main patches that add
> > the
> > ability to create and generate synthetic events from the
> > kernel.  The
> > next patch adds a test module that demonstrates actual use of the
> > API
> > and verifies that it works as intended, followed by Documentation.
> > 
> > Special thanks to Artem Bityutskiy, who worked with me over several
> > iterations of the API, and who had many great suggestions on the
> > details of the interface, and pointed out several problems with the
> > code itself.
> > 
> > The following changes since commit
> > 659ded30272d67a04b3692f0bfa12263be20d790:
> > 
> >   trace/kprobe: Remove unused MAX_KPROBE_CMDLINE_SIZE (2020-01-22
> > 07:07:38 -0500)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-
> > trace.git ftrace/synth-event-gen-v3
> > 
> > Tom Zanussi (12):
> >   tracing: Add trace_array_find/_get() to find instance trace
> > arrays
> >   tracing: Add trace_get/put_event_file()
> >   tracing: Add synth_event_delete()
> >   tracing: Add dynamic event command creation interface
> >   tracing: Add synthetic event command generation functions
> >   tracing: Change trace_boot to use synth_event interface
> >   tracing: Add synth_event_trace() and related functions
> >   tracing: Add synth event generation test module
> >   tracing: Add kprobe event command generation functions
> >   tracing: Change trace_boot to use kprobe_event interface
> >   tracing: Add kprobe event command generation test module
> >   tracing: Documentation for in-kernel synthetic event API
> > 
> >  Documentation/trace/events.rst       | 515 ++++++++++++++++++++
> >  include/linux/trace_events.h         | 124 +++++
> >  kernel/trace/Kconfig                 |  25 +
> >  kernel/trace/Makefile                |   2 +
> >  kernel/trace/kprobe_event_gen_test.c | 225 +++++++++
> >  kernel/trace/synth_event_gen_test.c  | 523 ++++++++++++++++++++
> >  kernel/trace/trace.c                 |  43 +-
> >  kernel/trace/trace.h                 |  36 ++
> >  kernel/trace/trace_boot.c            |  66 ++-
> >  kernel/trace/trace_events.c          | 325 +++++++++++++
> >  kernel/trace/trace_events_hist.c     | 896
> > ++++++++++++++++++++++++++++++++++-
> >  kernel/trace/trace_kprobe.c          | 160 ++++++-
> >  12 files changed, 2868 insertions(+), 72 deletions(-)
> >  create mode 100644 kernel/trace/kprobe_event_gen_test.c
> >  create mode 100644 kernel/trace/synth_event_gen_test.c
> > 
> > -- 
> > 2.14.1
> > 
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 02/12] tracing: Add trace_get/put_event_file()
  2020-01-24 22:56 ` [PATCH v3 02/12] tracing: Add trace_get/put_event_file() Tom Zanussi
@ 2020-01-29 17:52   ` Steven Rostedt
  2020-01-29 19:05     ` Tom Zanussi
  0 siblings, 1 reply; 22+ messages in thread
From: Steven Rostedt @ 2020-01-29 17:52 UTC (permalink / raw)
  To: Tom Zanussi
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

On Fri, 24 Jan 2020 16:56:13 -0600
Tom Zanussi <zanussi@kernel.org> wrote:

> +/**
> + * trace_put_event_file - Release a file from trace_get_event_file()
> + * @file: The trace event file
> + *
> + * If a file was retrieved using trace_get_event_file(), this should
> + * be called when it's no longer needed.  It will cancel the previous
> + * trace_array_get() called by that function, and decrement the
> + * event's module refcount.
> + */
> +void trace_put_event_file(struct trace_event_file *file)
> +{
> +	trace_array_put(file->tr);
> +
> +	mutex_lock(&event_mutex);
> +	module_put(file->event_call->mod);
> +	mutex_unlock(&event_mutex);

I believe the trace_array_put() needs to be at the end. Otherwise, I
believe the file could be freed before the event_mutex is taken.

I'll swap it, as I'm trying to get this into the merge window.

-- Steve

> +}
> +EXPORT_SYMBOL_GPL(trace_put_event_file);
> +
>  #ifdef CONFIG_DYNAMIC_FTRACE
>  
>  /* Avoid typos */


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 04/12] tracing: Add dynamic event command creation interface
  2020-01-24 22:56 ` [PATCH v3 04/12] tracing: Add dynamic event command creation interface Tom Zanussi
  2020-01-27 13:44   ` Masami Hiramatsu
@ 2020-01-29 18:00   ` Steven Rostedt
  2020-01-29 19:02     ` Tom Zanussi
  1 sibling, 1 reply; 22+ messages in thread
From: Steven Rostedt @ 2020-01-29 18:00 UTC (permalink / raw)
  To: Tom Zanussi
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

On Fri, 24 Jan 2020 16:56:15 -0600
Tom Zanussi <zanussi@kernel.org> wrote:

> +/**
> + * dynevent_arg_add - Add an arg to a dynevent_cmd
> + * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd
> + * @arg: The argument to append to the current cmd
> + *
> + * Append an argument to a dynevent_cmd.  The argument string will be
> + * appended to the current cmd string, followed by a separator, if
> + * applicable.  Before the argument is added, the check_arg()
> + * function, if defined, is called.
> + *
> + * The cmd string, separator, and check_arg() function should be set
> + * using the dynevent_arg_init() before any arguments are added using
> + * this function.
> + *
> + * Return: 0 if successful, error otherwise.
> + */
> +int dynevent_arg_add(struct dynevent_cmd *cmd,
> +		     struct dynevent_arg *arg)
> +{
> +	int ret = 0;
> +	int delta;
> +	char *q;
> +
> +	if (arg->check_arg) {
> +		ret = arg->check_arg(arg);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	q = cmd->buf + (cmd->maxlen - cmd->remaining);
> +
> +	delta = snprintf(q, cmd->remaining, " %s%c", arg->str, arg->separator);
> +	if (delta >= cmd->remaining) {
> +		pr_err("String is too long: %s\n", arg->str);
> +		return -E2BIG;
> +	}
> +	cmd->remaining -= delta;

This looks like you can take advantage of the seq_buf code
(see /lib/seq_buf.c), as that handles a lot of this for you.

As I'm trying to get this into the merge window, I won't delay this for
that (unless I find something else to delay this for), but this should
definitely incorporate seq_buf and not handle its own buffering.

-- Steve



> +
> +	return ret;
> +}

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 04/12] tracing: Add dynamic event command creation interface
  2020-01-29 18:00   ` Steven Rostedt
@ 2020-01-29 19:02     ` Tom Zanussi
  2020-01-29 19:55       ` Steven Rostedt
  0 siblings, 1 reply; 22+ messages in thread
From: Tom Zanussi @ 2020-01-29 19:02 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

Hi Steve,

On Wed, 2020-01-29 at 13:00 -0500, Steven Rostedt wrote:
> On Fri, 24 Jan 2020 16:56:15 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
> 
> > +/**
> > + * dynevent_arg_add - Add an arg to a dynevent_cmd
> > + * @cmd: A pointer to the dynevent_cmd struct representing the new
> > event cmd
> > + * @arg: The argument to append to the current cmd
> > + *
> > + * Append an argument to a dynevent_cmd.  The argument string will
> > be
> > + * appended to the current cmd string, followed by a separator, if
> > + * applicable.  Before the argument is added, the check_arg()
> > + * function, if defined, is called.
> > + *
> > + * The cmd string, separator, and check_arg() function should be
> > set
> > + * using the dynevent_arg_init() before any arguments are added
> > using
> > + * this function.
> > + *
> > + * Return: 0 if successful, error otherwise.
> > + */
> > +int dynevent_arg_add(struct dynevent_cmd *cmd,
> > +		     struct dynevent_arg *arg)
> > +{
> > +	int ret = 0;
> > +	int delta;
> > +	char *q;
> > +
> > +	if (arg->check_arg) {
> > +		ret = arg->check_arg(arg);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	q = cmd->buf + (cmd->maxlen - cmd->remaining);
> > +
> > +	delta = snprintf(q, cmd->remaining, " %s%c", arg->str,
> > arg->separator);
> > +	if (delta >= cmd->remaining) {
> > +		pr_err("String is too long: %s\n", arg->str);
> > +		return -E2BIG;
> > +	}
> > +	cmd->remaining -= delta;
> 
> This looks like you can take advantage of the seq_buf code
> (see /lib/seq_buf.c), as that handles a lot of this for you.
> 
> As I'm trying to get this into the merge window, I won't delay this
> for
> that (unless I find something else to delay this for), but this
> should
> definitely incorporate seq_buf and not handle its own buffering.
> 

OK, I can send a follow-on patch that does this.

Note that I just sent a v4 series which moves most of the code in this
patch to trace_dynevent.{c,h} as requested by Masami.  No other changes
from v3 were made other than adding acks.

Thanks,

Tom  


> -- Steve
> 
> 
> 
> > +
> > +	return ret;
> > +}

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 02/12] tracing: Add trace_get/put_event_file()
  2020-01-29 17:52   ` Steven Rostedt
@ 2020-01-29 19:05     ` Tom Zanussi
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Zanussi @ 2020-01-29 19:05 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

On Wed, 2020-01-29 at 12:52 -0500, Steven Rostedt wrote:
> On Fri, 24 Jan 2020 16:56:13 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
> 
> > +/**
> > + * trace_put_event_file - Release a file from
> > trace_get_event_file()
> > + * @file: The trace event file
> > + *
> > + * If a file was retrieved using trace_get_event_file(), this
> > should
> > + * be called when it's no longer needed.  It will cancel the
> > previous
> > + * trace_array_get() called by that function, and decrement the
> > + * event's module refcount.
> > + */
> > +void trace_put_event_file(struct trace_event_file *file)
> > +{
> > +	trace_array_put(file->tr);
> > +
> > +	mutex_lock(&event_mutex);
> > +	module_put(file->event_call->mod);
> > +	mutex_unlock(&event_mutex);
> 
> I believe the trace_array_put() needs to be at the end. Otherwise, I
> believe the file could be freed before the event_mutex is taken.
> 
> I'll swap it, as I'm trying to get this into the merge window.

OK, thanks,

Tom

> 
> -- Steve
> 
> > +}
> > +EXPORT_SYMBOL_GPL(trace_put_event_file);
> > +
> >  #ifdef CONFIG_DYNAMIC_FTRACE
> >  
> >  /* Avoid typos */
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 04/12] tracing: Add dynamic event command creation interface
  2020-01-29 19:02     ` Tom Zanussi
@ 2020-01-29 19:55       ` Steven Rostedt
  0 siblings, 0 replies; 22+ messages in thread
From: Steven Rostedt @ 2020-01-29 19:55 UTC (permalink / raw)
  To: Tom Zanussi
  Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users, ndesaulniers

On Wed, 29 Jan 2020 13:02:52 -0600
Tom Zanussi <zanussi@kernel.org> wrote:

> 
> OK, I can send a follow-on patch that does this.

Thanks!

> 
> Note that I just sent a v4 series which moves most of the code in this
> patch to trace_dynevent.{c,h} as requested by Masami.  No other changes
> from v3 were made other than adding acks.

Luckily, I had a meeting and stopped at patch 5. I reverted and started
again with your v4 series, and did the swap again too.

-- Steve

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-01-29 19:55 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-24 22:56 [PATCH v3 00/12] tracing: Add support for in-kernel dynamic event API Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 01/12] tracing: Add trace_array_find/_get() to find instance trace arrays Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 02/12] tracing: Add trace_get/put_event_file() Tom Zanussi
2020-01-29 17:52   ` Steven Rostedt
2020-01-29 19:05     ` Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 03/12] tracing: Add synth_event_delete() Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 04/12] tracing: Add dynamic event command creation interface Tom Zanussi
2020-01-27 13:44   ` Masami Hiramatsu
2020-01-27 15:24     ` Tom Zanussi
2020-01-29 18:00   ` Steven Rostedt
2020-01-29 19:02     ` Tom Zanussi
2020-01-29 19:55       ` Steven Rostedt
2020-01-24 22:56 ` [PATCH v3 05/12] tracing: Add synthetic event command generation functions Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 06/12] tracing: Change trace_boot to use synth_event interface Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 07/12] tracing: Add synth_event_trace() and related functions Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 08/12] tracing: Add synth event generation test module Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 09/12] tracing: Add kprobe event command generation functions Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 10/12] tracing: Change trace_boot to use kprobe_event interface Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 11/12] tracing: Add kprobe event command generation test module Tom Zanussi
2020-01-24 22:56 ` [PATCH v3 12/12] tracing: Documentation for in-kernel synthetic event API Tom Zanussi
2020-01-27 13:48 ` [PATCH v3 00/12] tracing: Add support for in-kernel dynamic " Masami Hiramatsu
2020-01-27 15:28   ` Tom Zanussi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).