[for-next][PATCH 0/6] tracing: Some last minute fixes for 5.6

[for-next][PATCH 0/6] tracing: Some last minute fixes for 5.6

[Steven Rostedt]

Tom Zanussi (6):
      tracing: Change trace_boot to use synth_event interface
      tracing: Fix now invalid var_ref_vals assumption in trace action
      tracing: Consolidate some synth_event_trace code
      tracing: Remove check_arg() callbacks from dynevent args
      tracing: Remove useless code in dynevent_arg_pair_add()
      tracing: Use seq_buf for building dynevent_cmd string

----
 include/linux/trace_events.h     |   4 +-
 kernel/trace/trace_boot.c        |  31 +++---
 kernel/trace/trace_dynevent.c    | 110 ++++++++-----------
 kernel/trace/trace_dynevent.h    |  11 +-
 kernel/trace/trace_events_hist.c | 221 +++++++++++++++++++--------------------
 kernel/trace/trace_kprobe.c      |  12 +--
 6 files changed, 171 insertions(+), 218 deletions(-)

[for-next][PATCH 1/6] tracing: Change trace_boot to use synth_event interface

[Steven Rostedt]

From: Tom Zanussi <zanussi@kernel.org>

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.

Link: http://lkml.kernel.org/r/94f1fa0e31846d0bddca916b8663404b20559e34.1580323897.git.zanussi@kernel.org

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.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 2298a70cdda6..06d7feb5255f 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -125,38 +125,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 4d56a4f0310d..2e88c9805f4b 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1755,12 +1755,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;
@@ -1790,7 +1785,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.24.1

[for-next][PATCH 2/6] tracing: Fix now invalid var_ref_vals assumption in trace action

[Steven Rostedt]

From: Tom Zanussi <zanussi@kernel.org>

The patch 'tracing: Fix histogram code when expression has same var as
value' added code to return an existing variable reference when
creating a new variable reference, which resulted in var_ref_vals
slots being reused instead of being duplicated.

The implementation of the trace action assumes that the end of the
var_ref_vals array starting at action_data.var_ref_idx corresponds to
the values that will be assigned to the trace params. The patch
mentioned above invalidates that assumption, which means that each
param needs to explicitly specify its index into var_ref_vals.

This fix changes action_data.var_ref_idx to an array of var ref
indexes to account for that.

Link: https://lore.kernel.org/r/1580335695.6220.8.camel@kernel.org

Fixes: 8bcebc77e85f ("tracing: Fix histogram code when expression has same var as value")
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 53 +++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 2e88c9805f4b..5b4e04780411 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -476,11 +476,12 @@ struct action_data {
 	 * When a histogram trigger is hit, the values of any
 	 * references to variables, including variables being passed
 	 * as parameters to synthetic events, are collected into a
-	 * var_ref_vals array.  This var_ref_idx is the index of the
-	 * first param in the array to be passed to the synthetic
-	 * event invocation.
+	 * var_ref_vals array.  This var_ref_idx array is an array of
+	 * indices into the var_ref_vals array, one for each synthetic
+	 * event param, and is passed to the synthetic event
+	 * invocation.
 	 */
-	unsigned int		var_ref_idx;
+	unsigned int		var_ref_idx[TRACING_MAP_VARS_MAX];
 	struct synth_event	*synth_event;
 	bool			use_trace_keyword;
 	char			*synth_event_name;
@@ -884,14 +885,14 @@ static struct trace_event_functions synth_event_funcs = {
 
 static notrace void trace_event_raw_event_synth(void *__data,
 						u64 *var_ref_vals,
-						unsigned int var_ref_idx)
+						unsigned int *var_ref_idx)
 {
 	struct trace_event_file *trace_file = __data;
 	struct synth_trace_event *entry;
 	struct trace_event_buffer fbuffer;
 	struct trace_buffer *buffer;
 	struct synth_event *event;
-	unsigned int i, n_u64;
+	unsigned int i, n_u64, val_idx;
 	int fields_size = 0;
 
 	event = trace_file->event_call->data;
@@ -914,15 +915,16 @@ static notrace void trace_event_raw_event_synth(void *__data,
 		goto out;
 
 	for (i = 0, n_u64 = 0; i < event->n_fields; i++) {
+		val_idx = var_ref_idx[i];
 		if (event->fields[i]->is_string) {
-			char *str_val = (char *)(long)var_ref_vals[var_ref_idx + i];
+			char *str_val = (char *)(long)var_ref_vals[val_idx];
 			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 {
 			struct synth_field *field = event->fields[i];
-			u64 val = var_ref_vals[var_ref_idx + i];
+			u64 val = var_ref_vals[val_idx];
 
 			switch (field->size) {
 			case 1:
@@ -1122,10 +1124,10 @@ static struct tracepoint *alloc_synth_tracepoint(char *name)
 }
 
 typedef void (*synth_probe_func_t) (void *__data, u64 *var_ref_vals,
-				    unsigned int var_ref_idx);
+				    unsigned int *var_ref_idx);
 
 static inline void trace_synth(struct synth_event *event, u64 *var_ref_vals,
-			       unsigned int var_ref_idx)
+			       unsigned int *var_ref_idx)
 {
 	struct tracepoint *tp = event->tp;
 
@@ -3506,6 +3508,22 @@ static int init_var_ref(struct hist_field *ref_field,
 	goto out;
 }
 
+static int find_var_ref_idx(struct hist_trigger_data *hist_data,
+			    struct hist_field *var_field)
+{
+	struct hist_field *ref_field;
+	int i;
+
+	for (i = 0; i < hist_data->n_var_refs; i++) {
+		ref_field = hist_data->var_refs[i];
+		if (ref_field->var.idx == var_field->var.idx &&
+		    ref_field->var.hist_data == var_field->hist_data)
+			return i;
+	}
+
+	return -ENOENT;
+}
+
 /**
  * create_var_ref - Create a variable reference and attach it to trigger
  * @hist_data: The trigger that will be referencing the variable
@@ -5071,11 +5089,11 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
 	struct trace_array *tr = hist_data->event_file->tr;
 	char *event_name, *param, *system = NULL;
 	struct hist_field *hist_field, *var_ref;
-	unsigned int i, var_ref_idx;
+	unsigned int i;
 	unsigned int field_pos = 0;
 	struct synth_event *event;
 	char *synth_event_name;
-	int ret = 0;
+	int var_ref_idx, ret = 0;
 
 	lockdep_assert_held(&event_mutex);
 
@@ -5092,8 +5110,6 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
 
 	event->ref++;
 
-	var_ref_idx = hist_data->n_var_refs;
-
 	for (i = 0; i < data->n_params; i++) {
 		char *p;
 
@@ -5142,6 +5158,14 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
 				goto err;
 			}
 
+			var_ref_idx = find_var_ref_idx(hist_data, var_ref);
+			if (WARN_ON(var_ref_idx < 0)) {
+				ret = var_ref_idx;
+				goto err;
+			}
+
+			data->var_ref_idx[i] = var_ref_idx;
+
 			field_pos++;
 			kfree(p);
 			continue;
@@ -5160,7 +5184,6 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
 	}
 
 	data->synth_event = event;
-	data->var_ref_idx = var_ref_idx;
  out:
 	return ret;
  err:
-- 
2.24.1

[for-next][PATCH 3/6] tracing: Consolidate some synth_event_trace code

[Steven Rostedt]

From: Tom Zanussi <zanussi@kernel.org>

The synth_event trace code contains some almost identical functions
and some small functions that are called only once - consolidate the
common code into single functions and fold in the small functions to
simplify the code overall.

Link: http://lkml.kernel.org/r/d1c8d8ad124a653b7543afe801d38c199ca5c20e.1580506712.git.zanussi@kernel.org

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 141 +++++++++++++------------------
 1 file changed, 57 insertions(+), 84 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 5b4e04780411..42058a1b5146 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2053,24 +2053,72 @@ int synth_event_trace_start(struct trace_event_file *file,
 }
 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)
+static int __synth_event_add_val(const char *field_name, u64 val,
+				 struct synth_event_trace_state *trace_state)
 {
-	struct synth_trace_event *entry = trace_state->entry;
+	struct synth_field *field = NULL;
+	struct synth_trace_event *entry;
+	struct synth_event *event;
+	int i, ret = 0;
+
+	if (!trace_state) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* can't mix add_next_synth_val() with add_synth_val() */
+	if (field_name) {
+		if (trace_state->add_next) {
+			ret = -EINVAL;
+			goto out;
+		}
+		trace_state->add_name = true;
+	} else {
+		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->add_name) {
+		for (i = 0; i < event->n_fields; i++) {
+			field = event->fields[i];
+			if (strcmp(field->name, field_name) == 0)
+				break;
+		}
+		if (!field) {
+			ret = -EINVAL;
+			goto out;
+		}
+	} else {
+		if (trace_state->cur_field >= event->n_fields) {
+			ret = -EINVAL;
+			goto out;
+		}
+		field = event->fields[trace_state->cur_field++];
+	}
 
+	entry = trace_state->entry;
 	if (field->is_string) {
 		char *str_val = (char *)(long)val;
 		char *str_field;
 
-		if (!str_val)
-			return -EINVAL;
+		if (!str_val) {
+			ret = -EINVAL;
+			goto out;
+		}
 
 		str_field = (char *)&entry->fields[field->offset];
 		strscpy(str_field, str_val, STR_VAR_LEN_MAX);
 	} else
 		entry->fields[field->offset] = val;
-
-	return 0;
+ out:
+	return ret;
 }
 
 /**
@@ -2104,54 +2152,10 @@ static int save_synth_val(struct synth_field *field, u64 val,
 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;
+	return __synth_event_add_val(NULL, val, trace_state);
 }
 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
@@ -2183,38 +2187,7 @@ static struct synth_field *find_synth_field(struct synth_event *event,
 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;
+	return __synth_event_add_val(field_name, val, trace_state);
 }
 EXPORT_SYMBOL_GPL(synth_event_add_val);
 
-- 
2.24.1

[for-next][PATCH 4/6] tracing: Remove check_arg() callbacks from dynevent args

[Steven Rostedt]

From: Tom Zanussi <zanussi@kernel.org>

It's kind of strange to have check_arg() callbacks as part of the arg
objects themselves; it makes more sense to just pass these in when the
args are added instead.

Remove the check_arg() callbacks from those objects which also means
removing the check_arg() args from the init functions, adding them to
the add functions and fixing up existing callers.

Link: http://lkml.kernel.org/r/c7708d6f177fcbe1a36b6e4e8e150907df0fa5d2.1580506712.git.zanussi@kernel.org

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_dynevent.c    | 62 +++++++++++++++-----------------
 kernel/trace/trace_dynevent.h    | 11 +++---
 kernel/trace/trace_events_hist.c | 16 ++++-----
 kernel/trace/trace_kprobe.c      | 10 +++---
 4 files changed, 46 insertions(+), 53 deletions(-)

diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index 6ffdbc4fda53..f9cfcdc9d1f3 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -228,27 +228,30 @@ fs_initcall(init_dynamic_event);
  * 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
+ * @check_arg: An (optional) pointer to a function checking arg sanity
  *
  * 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.
+ * applicable.  Before the argument is added, the @check_arg function,
+ * if present, will be used to check the sanity of the current arg
+ * string.
  *
- * The cmd string, separator, and check_arg() function should be set
- * using the dynevent_arg_init() before any arguments are added using
- * this function.
+ * The cmd string and separator 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)
+		     struct dynevent_arg *arg,
+		     dynevent_check_arg_fn_t check_arg)
 {
 	int ret = 0;
 	int delta;
 	char *q;
 
-	if (arg->check_arg) {
-		ret = arg->check_arg(arg);
+	if (check_arg) {
+		ret = check_arg(arg);
 		if (ret)
 			return ret;
 	}
@@ -269,6 +272,7 @@ int dynevent_arg_add(struct dynevent_cmd *cmd,
  * 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
+ * @check_arg: An (optional) pointer to a function checking arg sanity
  *
  * Append an argument pair to a dynevent_cmd.  An argument pair
  * consists of a left-hand-side argument and a right-hand-side
@@ -278,24 +282,26 @@ int dynevent_arg_add(struct dynevent_cmd *cmd,
  *
  * 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.
+ * followed finally by a separator, if applicable.  Before the
+ * argument is added, the @check_arg function, if present, will be
+ * used to check the sanity of the current arg strings.
  *
- * 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.
+ * The cmd strings, operator, and separator 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)
+			  struct dynevent_arg_pair *arg_pair,
+			  dynevent_check_arg_fn_t check_arg)
 {
 	int ret = 0;
 	int delta;
 	char *q;
 
-	if (arg_pair->check_arg) {
-		ret = arg_pair->check_arg(arg_pair);
+	if (check_arg) {
+		ret = check_arg(arg_pair);
 		if (ret)
 			return ret;
 	}
@@ -385,20 +391,16 @@ void dynevent_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen,
 /**
  * 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
+ * string.  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));
@@ -406,14 +408,11 @@ void dynevent_arg_init(struct dynevent_arg *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
  *
@@ -422,16 +421,13 @@ void dynevent_arg_init(struct dynevent_arg *arg,
  * 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.
+ * whitespace, all followed by a separator, if applicable.  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));
@@ -443,8 +439,6 @@ void dynevent_arg_pair_init(struct dynevent_arg_pair *arg_pair,
 	if (!separator)
 		separator = ' ';
 	arg_pair->separator = separator;
-
-	arg_pair->check_arg = check_arg;
 }
 
 /**
diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
index b593fc34c5b1..d6857a254ede 100644
--- a/kernel/trace/trace_dynevent.h
+++ b/kernel/trace/trace_dynevent.h
@@ -126,28 +126,27 @@ 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 *arg,
+			    dynevent_check_arg_fn_t check_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);
+				 struct dynevent_arg_pair *arg_pair,
+				 dynevent_check_arg_fn_t check_arg);
 extern int dynevent_str_add(struct dynevent_cmd *cmd, const char *str);
 
 #endif
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 42058a1b5146..d2817fe52f32 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1334,12 +1334,12 @@ int synth_event_add_field(struct dynevent_cmd *cmd, const char *type,
 	if (!type || !name)
 		return -EINVAL;
 
-	dynevent_arg_pair_init(&arg_pair, synth_event_check_arg_fn, 0, ';');
+	dynevent_arg_pair_init(&arg_pair, 0, ';');
 
 	arg_pair.lhs = type;
 	arg_pair.rhs = name;
 
-	ret = dynevent_arg_pair_add(cmd, &arg_pair);
+	ret = dynevent_arg_pair_add(cmd, &arg_pair, synth_event_check_arg_fn);
 	if (ret)
 		return ret;
 
@@ -1377,11 +1377,11 @@ int synth_event_add_field_str(struct dynevent_cmd *cmd, const char *type_name)
 	if (!type_name)
 		return -EINVAL;
 
-	dynevent_arg_init(&arg, NULL, ';');
+	dynevent_arg_init(&arg, ';');
 
 	arg.str = type_name;
 
-	ret = dynevent_arg_add(cmd, &arg);
+	ret = dynevent_arg_add(cmd, &arg, NULL);
 	if (ret)
 		return ret;
 
@@ -1472,9 +1472,9 @@ int __synth_event_gen_cmd_start(struct dynevent_cmd *cmd, const char *name,
 	if (cmd->type != DYNEVENT_TYPE_SYNTH)
 		return -EINVAL;
 
-	dynevent_arg_init(&arg, NULL, 0);
+	dynevent_arg_init(&arg, 0);
 	arg.str = name;
-	ret = dynevent_arg_add(cmd, &arg);
+	ret = dynevent_arg_add(cmd, &arg, NULL);
 	if (ret)
 		return ret;
 
@@ -1546,9 +1546,9 @@ int synth_event_gen_cmd_array_start(struct dynevent_cmd *cmd, const char *name,
 	if (n_fields > SYNTH_FIELDS_MAX)
 		return -EINVAL;
 
-	dynevent_arg_init(&arg, NULL, 0);
+	dynevent_arg_init(&arg, 0);
 	arg.str = name;
-	ret = dynevent_arg_add(cmd, &arg);
+	ret = dynevent_arg_add(cmd, &arg, NULL);
 	if (ret)
 		return ret;
 
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 307abb724a71..fe183d4045d2 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -962,9 +962,9 @@ int __kprobe_event_gen_cmd_start(struct dynevent_cmd *cmd, bool kretprobe,
 	if (ret)
 		return ret;
 
-	dynevent_arg_init(&arg, NULL, 0);
+	dynevent_arg_init(&arg, 0);
 	arg.str = loc;
-	ret = dynevent_arg_add(cmd, &arg);
+	ret = dynevent_arg_add(cmd, &arg, NULL);
 	if (ret)
 		return ret;
 
@@ -982,7 +982,7 @@ int __kprobe_event_gen_cmd_start(struct dynevent_cmd *cmd, bool kretprobe,
 		}
 
 		arg.str = field;
-		ret = dynevent_arg_add(cmd, &arg);
+		ret = dynevent_arg_add(cmd, &arg, NULL);
 		if (ret)
 			break;
 	}
@@ -1017,7 +1017,7 @@ int __kprobe_event_add_fields(struct dynevent_cmd *cmd, ...)
 	if (cmd->type != DYNEVENT_TYPE_KPROBE)
 		return -EINVAL;
 
-	dynevent_arg_init(&arg, NULL, 0);
+	dynevent_arg_init(&arg, 0);
 
 	va_start(args, cmd);
 	for (;;) {
@@ -1033,7 +1033,7 @@ int __kprobe_event_add_fields(struct dynevent_cmd *cmd, ...)
 		}
 
 		arg.str = field;
-		ret = dynevent_arg_add(cmd, &arg);
+		ret = dynevent_arg_add(cmd, &arg, NULL);
 		if (ret)
 			break;
 	}
-- 
2.24.1

[for-next][PATCH 5/6] tracing: Remove useless code in dynevent_arg_pair_add()

[Steven Rostedt]

From: Tom Zanussi <zanussi@kernel.org>

The final addition to q is unnecessary, since q isn't ever used
afterwards.

Link: http://lkml.kernel.org/r/7880a1268217886cdba7035526650195668da856.1580506712.git.zanussi@kernel.org

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_dynevent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index f9cfcdc9d1f3..204275ec8d71 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -322,7 +322,7 @@ int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
 		pr_err("field string is too long: %s\n", arg_pair->rhs);
 		return -E2BIG;
 	}
-	cmd->remaining -= delta; q += delta;
+	cmd->remaining -= delta;
 
 	return ret;
 }
-- 
2.24.1

[for-next][PATCH 6/6] tracing: Use seq_buf for building dynevent_cmd string

[Steven Rostedt]

From: Tom Zanussi <zanussi@kernel.org>

The dynevent_cmd commands that build up the command string don't need
to do that themselves - there's a seq_buf facility that does pretty
much the same thing those command are doing manually, so use it
instead.

Link: http://lkml.kernel.org/r/eb8a6e835c964d0ab8a38cbf5ffa60746b54a465.1580506712.git.zanussi@kernel.org

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/trace_events.h     |  4 +--
 kernel/trace/trace_dynevent.c    | 48 +++++++++-----------------------
 kernel/trace/trace_events_hist.c |  2 +-
 kernel/trace/trace_kprobe.c      |  2 +-
 4 files changed, 16 insertions(+), 40 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 7c307a7c9c6a..67f528ecb9e5 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -367,10 +367,8 @@ struct dynevent_cmd;
 typedef int (*dynevent_create_fn_t)(struct dynevent_cmd *cmd);
 
 struct dynevent_cmd {
-	char			*buf;
+	struct seq_buf		seq;
 	const char		*event_name;
-	int			maxlen;
-	int			remaining;
 	unsigned int		n_fields;
 	enum dynevent_type	type;
 	dynevent_create_fn_t	run_command;
diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index 204275ec8d71..9f2e8520b748 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -247,8 +247,6 @@ int dynevent_arg_add(struct dynevent_cmd *cmd,
 		     dynevent_check_arg_fn_t check_arg)
 {
 	int ret = 0;
-	int delta;
-	char *q;
 
 	if (check_arg) {
 		ret = check_arg(arg);
@@ -256,14 +254,11 @@ int dynevent_arg_add(struct dynevent_cmd *cmd,
 			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);
+	ret = seq_buf_printf(&cmd->seq, " %s%c", arg->str, arg->separator);
+	if (ret) {
+		pr_err("String is too long: %s%c\n", arg->str, arg->separator);
 		return -E2BIG;
 	}
-	cmd->remaining -= delta;
 
 	return ret;
 }
@@ -297,8 +292,6 @@ int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
 			  dynevent_check_arg_fn_t check_arg)
 {
 	int ret = 0;
-	int delta;
-	char *q;
 
 	if (check_arg) {
 		ret = check_arg(arg_pair);
@@ -306,23 +299,15 @@ int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
 			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);
+	ret = seq_buf_printf(&cmd->seq, " %s%c%s%c", arg_pair->lhs,
+			     arg_pair->operator, arg_pair->rhs,
+			     arg_pair->separator);
+	if (ret) {
+		pr_err("field string is too long: %s%c%s%c\n", arg_pair->lhs,
+		       arg_pair->operator, arg_pair->rhs,
+		       arg_pair->separator);
 		return -E2BIG;
 	}
-	cmd->remaining -= delta;
 
 	return ret;
 }
@@ -340,17 +325,12 @@ int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
 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) {
+	ret = seq_buf_puts(&cmd->seq, str);
+	if (ret) {
 		pr_err("String is too long: %s\n", str);
 		return -E2BIG;
 	}
-	cmd->remaining -= delta;
 
 	return ret;
 }
@@ -381,9 +361,7 @@ void dynevent_cmd_init(struct dynevent_cmd *cmd, char *buf, int maxlen,
 {
 	memset(cmd, '\0', sizeof(*cmd));
 
-	cmd->buf = buf;
-	cmd->maxlen = maxlen;
-	cmd->remaining = cmd->maxlen;
+	seq_buf_init(&cmd->seq, buf, maxlen);
 	cmd->type = type;
 	cmd->run_command = run_command;
 }
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index d2817fe52f32..b3bcfd8c7332 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1762,7 +1762,7 @@ static int synth_event_run_command(struct dynevent_cmd *cmd)
 	struct synth_event *se;
 	int ret;
 
-	ret = trace_run_command(cmd->buf, create_or_delete_synth_event);
+	ret = trace_run_command(cmd->seq.buffer, create_or_delete_synth_event);
 	if (ret)
 		return ret;
 
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index fe183d4045d2..51efc790aea8 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -903,7 +903,7 @@ static int create_or_delete_trace_kprobe(int argc, char **argv)
 
 static int trace_kprobe_run_command(struct dynevent_cmd *cmd)
 {
-	return trace_run_command(cmd->buf, create_or_delete_trace_kprobe);
+	return trace_run_command(cmd->seq.buffer, create_or_delete_trace_kprobe);
 }
 
 /**
-- 
2.24.1