[RFC][PATCH 0/3] tracing: Make error_log per instance

[RFC][PATCH 0/3] tracing: Make error_log per instance

[Steven Rostedt]

Hi Tom,

I noticed that you created an error_log file in every instance, but
they all show the same errors. These three patches make it so that
the errors appear in the instance directory that they happened in.
If you write a bad error to

   /sys/kernel/tracing/instance/foo/events/sched/sched_switch/hist

It appears only in

  /sys/kernel/tracing/instances/foo/error_log

Which I think is the proper approach, especially instances should not
affect the top directory or other instances.

For those errors that do not have an associated instance (creating a
kprobe/uprobe event or perf), a NULL passed to tracing_log_err() will
result in the error message in the top level error message.

Do you (or Masami) have any issues with this patch set?

If not, please add a "reviewed-by" or "acked-by" and I'll add it
to your patch series and push them to for-next (after more testing).

If this isn't obvious, this patch series is on top of:

  http://lkml.kernel.org/r/cover.1554072478.git.tom.zanussi@linux.intel.com

Actually, I added it right after patch 5 of that series (before the
selftests and documentation).

Thanks!

-- Steve



Steven Rostedt (VMware) (3):
      tracing: Add trace_array parameter to create_event_filter()
      tracing: Have histogram code pass around trace_array for error handling
      tracing: Have the error logs show up in the proper instances

----
 kernel/trace/trace.c                |  55 +++++++++-----
 kernel/trace/trace.h                |   8 +-
 kernel/trace/trace_events_filter.c  |  25 ++++---
 kernel/trace/trace_events_hist.c    | 145 ++++++++++++++++++++----------------
 kernel/trace/trace_events_trigger.c |   3 +-
 kernel/trace/trace_probe.c          |   2 +-
 6 files changed, 142 insertions(+), 96 deletions(-)

[RFC][PATCH 1/3] tracing: Add trace_array parameter to create_event_filter()

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Pass in the trace_array that represents the instance the filter being
changed is in to create_event_filter(). This will allow for error messages
that happen when writing to the filter can be displayed in the proper
instance "error_log" file.

Note, for calls to create_filter() (that was also modified to support
create_event_filter()), that changes filters that do not exist in a instance
(for perf for example), NULL may be passed in, which means that there will
not be any message to log for that filter.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.h                |  3 ++-
 kernel/trace/trace_events_filter.c  | 21 ++++++++++++---------
 kernel/trace/trace_events_trigger.c |  3 ++-
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b711edbef7e7..809c5d7f0064 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1553,7 +1553,8 @@ extern int apply_subsystem_event_filter(struct trace_subsystem_dir *dir,
 extern void print_subsystem_event_filter(struct event_subsystem *system,
 					 struct trace_seq *s);
 extern int filter_assign_type(const char *type);
-extern int create_event_filter(struct trace_event_call *call,
+extern int create_event_filter(struct trace_array *tr,
+			       struct trace_event_call *call,
 			       char *filter_str, bool set_str,
 			       struct event_filter **filterp);
 extern void free_event_filter(struct event_filter *filter);
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 290d42c59101..b1033638e499 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -920,7 +920,8 @@ static void remove_filter_string(struct event_filter *filter)
 	filter->filter_string = NULL;
 }
 
-static void append_filter_err(struct filter_parse_error *pe,
+static void append_filter_err(struct trace_array *tr,
+			      struct filter_parse_error *pe,
 			      struct event_filter *filter)
 {
 	struct trace_seq *s;
@@ -1607,7 +1608,7 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
 		if (err) {
 			filter_disable(file);
 			parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
-			append_filter_err(pe, filter);
+			append_filter_err(tr, pe, filter);
 		} else
 			event_set_filtered_flag(file);
 
@@ -1719,7 +1720,8 @@ static void create_filter_finish(struct filter_parse_error *pe)
  * information if @set_str is %true and the caller is responsible for
  * freeing it.
  */
-static int create_filter(struct trace_event_call *call,
+static int create_filter(struct trace_array *tr,
+			 struct trace_event_call *call,
 			 char *filter_string, bool set_str,
 			 struct event_filter **filterp)
 {
@@ -1736,17 +1738,18 @@ static int create_filter(struct trace_event_call *call,
 
 	err = process_preds(call, filter_string, *filterp, pe);
 	if (err && set_str)
-		append_filter_err(pe, *filterp);
+		append_filter_err(tr, pe, *filterp);
 	create_filter_finish(pe);
 
 	return err;
 }
 
-int create_event_filter(struct trace_event_call *call,
+int create_event_filter(struct trace_array *tr,
+			struct trace_event_call *call,
 			char *filter_str, bool set_str,
 			struct event_filter **filterp)
 {
-	return create_filter(call, filter_str, set_str, filterp);
+	return create_filter(tr, call, filter_str, set_str, filterp);
 }
 
 /**
@@ -1773,7 +1776,7 @@ static int create_system_filter(struct trace_subsystem_dir *dir,
 			kfree((*filterp)->filter_string);
 			(*filterp)->filter_string = NULL;
 		} else {
-			append_filter_err(pe, *filterp);
+			append_filter_err(tr, pe, *filterp);
 		}
 	}
 	create_filter_finish(pe);
@@ -1804,7 +1807,7 @@ int apply_event_filter(struct trace_event_file *file, char *filter_string)
 		return 0;
 	}
 
-	err = create_filter(call, filter_string, true, &filter);
+	err = create_filter(file->tr, call, filter_string, true, &filter);
 
 	/*
 	 * Always swap the call filter with the new filter
@@ -2060,7 +2063,7 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
 	if (event->filter)
 		goto out_unlock;
 
-	err = create_filter(call, filter_str, false, &filter);
+	err = create_filter(NULL, call, filter_str, false, &filter);
 	if (err)
 		goto free_filter;
 
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index cd12ecb66eb9..2a2912cb4533 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -731,7 +731,8 @@ int set_trigger_filter(char *filter_str,
 		goto out;
 
 	/* The filter is for the 'trigger' event, not the triggered event */
-	ret = create_event_filter(file->event_call, filter_str, false, &filter);
+	ret = create_event_filter(file->tr, file->event_call,
+				  filter_str, false, &filter);
 	/*
 	 * If create_event_filter() fails, filter still needs to be freed.
 	 * Which the calling code will do with data->filter.
-- 
2.20.1

[RFC][PATCH 2/3] tracing: Have histogram code pass around trace_array for error handling

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Have the trace_array that associates the trace instance of the histogram
passed around to functions so that error handling can display the error
message in the proper instance.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 142 +++++++++++++++++--------------
 1 file changed, 80 insertions(+), 62 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 071c62cacba7..49d373792721 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -619,7 +619,7 @@ static void last_cmd_set(struct trace_event_file *file, char *str)
 		snprintf(last_cmd_loc, MAX_FILTER_STR_VAL, "hist:%s:%s", system, name);
 }
 
-static void hist_err(u8 err_type, u8 err_pos)
+static void hist_err(struct trace_array *tr, u8 err_type, u8 err_pos)
 {
 	tracing_log_err(last_cmd_loc, last_cmd, err_text, err_type, err_pos);
 }
@@ -1756,7 +1756,7 @@ static struct trace_event_file *find_var_file(struct trace_array *tr,
 
 		if (find_var_field(var_hist_data, var_name)) {
 			if (found) {
-				hist_err(HIST_ERR_VAR_NOT_UNIQUE, errpos(var_name));
+				hist_err(tr, HIST_ERR_VAR_NOT_UNIQUE, errpos(var_name));
 				return NULL;
 			}
 
@@ -1807,7 +1807,8 @@ find_match_var(struct hist_trigger_data *hist_data, char *var_name)
 			hist_field = find_file_var(file, var_name);
 			if (hist_field) {
 				if (found) {
-					hist_err(HIST_ERR_VAR_NOT_UNIQUE, errpos(var_name));
+					hist_err(tr, HIST_ERR_VAR_NOT_UNIQUE,
+						 errpos(var_name));
 					return ERR_PTR(-EINVAL);
 				}
 
@@ -2042,7 +2043,8 @@ static int parse_action(char *str, struct hist_trigger_attrs *attrs)
 	return ret;
 }
 
-static int parse_assignment(char *str, struct hist_trigger_attrs *attrs)
+static int parse_assignment(struct trace_array *tr,
+			    char *str, struct hist_trigger_attrs *attrs)
 {
 	int ret = 0;
 
@@ -2098,7 +2100,7 @@ static int parse_assignment(char *str, struct hist_trigger_attrs *attrs)
 		char *assignment;
 
 		if (attrs->n_assignments == TRACING_MAP_VARS_MAX) {
-			hist_err(HIST_ERR_TOO_MANY_VARS, errpos(str));
+			hist_err(tr, HIST_ERR_TOO_MANY_VARS, errpos(str));
 			ret = -EINVAL;
 			goto out;
 		}
@@ -2115,7 +2117,8 @@ static int parse_assignment(char *str, struct hist_trigger_attrs *attrs)
 	return ret;
 }
 
-static struct hist_trigger_attrs *parse_hist_trigger_attrs(char *trigger_str)
+static struct hist_trigger_attrs *
+parse_hist_trigger_attrs(struct trace_array *tr, char *trigger_str)
 {
 	struct hist_trigger_attrs *attrs;
 	int ret = 0;
@@ -2128,7 +2131,7 @@ static struct hist_trigger_attrs *parse_hist_trigger_attrs(char *trigger_str)
 		char *str = strsep(&trigger_str, ":");
 
 		if (strchr(str, '=')) {
-			ret = parse_assignment(str, attrs);
+			ret = parse_assignment(tr, str, attrs);
 			if (ret)
 				goto free;
 		} else if (strcmp(str, "pause") == 0)
@@ -2684,6 +2687,7 @@ static struct hist_field *parse_var_ref(struct hist_trigger_data *hist_data,
 					char *var_name)
 {
 	struct hist_field *var_field = NULL, *ref_field = NULL;
+	struct trace_array *tr = hist_data->event_file->tr;
 
 	if (!is_var_ref(var_name))
 		return NULL;
@@ -2696,7 +2700,7 @@ static struct hist_field *parse_var_ref(struct hist_trigger_data *hist_data,
 					   system, event_name);
 
 	if (!ref_field)
-		hist_err(HIST_ERR_VAR_NOT_FOUND, errpos(var_name));
+		hist_err(tr, HIST_ERR_VAR_NOT_FOUND, errpos(var_name));
 
 	return ref_field;
 }
@@ -2707,6 +2711,7 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
 {
 	struct ftrace_event_field *field = NULL;
 	char *field_name, *modifier, *str;
+	struct trace_array *tr = file->tr;
 
 	modifier = str = kstrdup(field_str, GFP_KERNEL);
 	if (!modifier)
@@ -2730,7 +2735,7 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
 		else if (strcmp(modifier, "usecs") == 0)
 			*flags |= HIST_FIELD_FL_TIMESTAMP_USECS;
 		else {
-			hist_err(HIST_ERR_BAD_FIELD_MODIFIER, errpos(modifier));
+			hist_err(tr, HIST_ERR_BAD_FIELD_MODIFIER, errpos(modifier));
 			field = ERR_PTR(-EINVAL);
 			goto out;
 		}
@@ -2746,7 +2751,7 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
 	else {
 		field = trace_find_event_field(file->event_call, field_name);
 		if (!field || !field->size) {
-			hist_err(HIST_ERR_FIELD_NOT_FOUND, errpos(field_name));
+			hist_err(tr, HIST_ERR_FIELD_NOT_FOUND, errpos(field_name));
 			field = ERR_PTR(-EINVAL);
 			goto out;
 		}
@@ -2808,7 +2813,8 @@ static struct hist_field *parse_atom(struct hist_trigger_data *hist_data,
 
 	s = local_field_var_ref(hist_data, ref_system, ref_event, ref_var);
 	if (!s) {
-		hist_field = parse_var_ref(hist_data, ref_system, ref_event, ref_var);
+		hist_field = parse_var_ref(hist_data, ref_system,
+					   ref_event, ref_var);
 		if (hist_field) {
 			if (var_name) {
 				hist_field = create_alias(hist_data, hist_field, var_name);
@@ -2857,7 +2863,7 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data,
 	/* we support only -(xxx) i.e. explicit parens required */
 
 	if (level > 3) {
-		hist_err(HIST_ERR_TOO_MANY_SUBEXPR, errpos(str));
+		hist_err(file->tr, HIST_ERR_TOO_MANY_SUBEXPR, errpos(str));
 		ret = -EINVAL;
 		goto free;
 	}
@@ -2912,7 +2918,8 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data,
 	return ERR_PTR(ret);
 }
 
-static int check_expr_operands(struct hist_field *operand1,
+static int check_expr_operands(struct trace_array *tr,
+			       struct hist_field *operand1,
 			       struct hist_field *operand2)
 {
 	unsigned long operand1_flags = operand1->flags;
@@ -2940,7 +2947,7 @@ static int check_expr_operands(struct hist_field *operand1,
 
 	if ((operand1_flags & HIST_FIELD_FL_TIMESTAMP_USECS) !=
 	    (operand2_flags & HIST_FIELD_FL_TIMESTAMP_USECS)) {
-		hist_err(HIST_ERR_TIMESTAMP_MISMATCH, 0);
+		hist_err(tr, HIST_ERR_TIMESTAMP_MISMATCH, 0);
 		return -EINVAL;
 	}
 
@@ -2958,7 +2965,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 	char *sep, *operand1_str;
 
 	if (level > 3) {
-		hist_err(HIST_ERR_TOO_MANY_SUBEXPR, errpos(str));
+		hist_err(file->tr, HIST_ERR_TOO_MANY_SUBEXPR, errpos(str));
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -3003,7 +3010,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 		goto free;
 	}
 
-	ret = check_expr_operands(operand1, operand2);
+	ret = check_expr_operands(file->tr, operand1, operand2);
 	if (ret)
 		goto free;
 
@@ -3196,14 +3203,14 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
 	int ret;
 
 	if (target_hist_data->n_field_var_hists >= SYNTH_FIELDS_MAX) {
-		hist_err(HIST_ERR_TOO_MANY_FIELD_VARS, errpos(field_name));
+		hist_err(tr, HIST_ERR_TOO_MANY_FIELD_VARS, errpos(field_name));
 		return ERR_PTR(-EINVAL);
 	}
 
 	file = event_file(tr, subsys_name, event_name);
 
 	if (IS_ERR(file)) {
-		hist_err(HIST_ERR_EVENT_FILE_NOT_FOUND, errpos(field_name));
+		hist_err(tr, HIST_ERR_EVENT_FILE_NOT_FOUND, errpos(field_name));
 		ret = PTR_ERR(file);
 		return ERR_PTR(ret);
 	}
@@ -3216,7 +3223,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
 	 */
 	hist_data = find_compatible_hist(target_hist_data, file);
 	if (!hist_data) {
-		hist_err(HIST_ERR_HIST_NOT_FOUND, errpos(field_name));
+		hist_err(tr, HIST_ERR_HIST_NOT_FOUND, errpos(field_name));
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -3277,7 +3284,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
 		kfree(cmd);
 		kfree(var_hist->cmd);
 		kfree(var_hist);
-		hist_err(HIST_ERR_HIST_CREATE_FAIL, errpos(field_name));
+		hist_err(tr, HIST_ERR_HIST_CREATE_FAIL, errpos(field_name));
 		return ERR_PTR(ret);
 	}
 
@@ -3289,7 +3296,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
 	if (IS_ERR_OR_NULL(event_var)) {
 		kfree(var_hist->cmd);
 		kfree(var_hist);
-		hist_err(HIST_ERR_SYNTH_VAR_NOT_FOUND, errpos(field_name));
+		hist_err(tr, HIST_ERR_SYNTH_VAR_NOT_FOUND, errpos(field_name));
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -3422,25 +3429,26 @@ static struct field_var *create_field_var(struct hist_trigger_data *hist_data,
 {
 	struct hist_field *val = NULL, *var = NULL;
 	unsigned long flags = HIST_FIELD_FL_VAR;
+	struct trace_array *tr = file->tr;
 	struct field_var *field_var;
 	int ret = 0;
 
 	if (hist_data->n_field_vars >= SYNTH_FIELDS_MAX) {
-		hist_err(HIST_ERR_TOO_MANY_FIELD_VARS, errpos(field_name));
+		hist_err(tr, HIST_ERR_TOO_MANY_FIELD_VARS, errpos(field_name));
 		ret = -EINVAL;
 		goto err;
 	}
 
 	val = parse_atom(hist_data, file, field_name, &flags, NULL);
 	if (IS_ERR(val)) {
-		hist_err(HIST_ERR_FIELD_VAR_PARSE_FAIL, errpos(field_name));
+		hist_err(tr, HIST_ERR_FIELD_VAR_PARSE_FAIL, errpos(field_name));
 		ret = PTR_ERR(val);
 		goto err;
 	}
 
 	var = create_var(hist_data, file, field_name, val->size, val->type);
 	if (IS_ERR(var)) {
-		hist_err(HIST_ERR_VAR_CREATE_FIND_FAIL, errpos(field_name));
+		hist_err(tr, HIST_ERR_VAR_CREATE_FIND_FAIL, errpos(field_name));
 		kfree(val);
 		ret = PTR_ERR(var);
 		goto err;
@@ -3767,19 +3775,20 @@ static int track_data_create(struct hist_trigger_data *hist_data,
 {
 	struct hist_field *var_field, *ref_field, *track_var = NULL;
 	struct trace_event_file *file = hist_data->event_file;
+	struct trace_array *tr = file->tr;
 	char *track_data_var_str;
 	int ret = 0;
 
 	track_data_var_str = data->track_data.var_str;
 	if (track_data_var_str[0] != '$') {
-		hist_err(HIST_ERR_ONX_NOT_VAR, errpos(track_data_var_str));
+		hist_err(tr, HIST_ERR_ONX_NOT_VAR, errpos(track_data_var_str));
 		return -EINVAL;
 	}
 	track_data_var_str++;
 
 	var_field = find_target_event_var(hist_data, NULL, NULL, track_data_var_str);
 	if (!var_field) {
-		hist_err(HIST_ERR_ONX_VAR_NOT_FOUND, errpos(track_data_var_str));
+		hist_err(tr, HIST_ERR_ONX_VAR_NOT_FOUND, errpos(track_data_var_str));
 		return -EINVAL;
 	}
 
@@ -3792,7 +3801,7 @@ static int track_data_create(struct hist_trigger_data *hist_data,
 	if (data->handler == HANDLER_ONMAX)
 		track_var = create_var(hist_data, file, "__max", sizeof(u64), "u64");
 	if (IS_ERR(track_var)) {
-		hist_err(HIST_ERR_ONX_VAR_CREATE_FAIL, 0);
+		hist_err(tr, HIST_ERR_ONX_VAR_CREATE_FAIL, 0);
 		ret = PTR_ERR(track_var);
 		goto out;
 	}
@@ -3800,7 +3809,7 @@ static int track_data_create(struct hist_trigger_data *hist_data,
 	if (data->handler == HANDLER_ONCHANGE)
 		track_var = create_var(hist_data, file, "__change", sizeof(u64), "u64");
 	if (IS_ERR(track_var)) {
-		hist_err(HIST_ERR_ONX_VAR_CREATE_FAIL, 0);
+		hist_err(tr, HIST_ERR_ONX_VAR_CREATE_FAIL, 0);
 		ret = PTR_ERR(track_var);
 		goto out;
 	}
@@ -3811,7 +3820,8 @@ static int track_data_create(struct hist_trigger_data *hist_data,
 	return ret;
 }
 
-static int parse_action_params(char *params, struct action_data *data)
+static int parse_action_params(struct trace_array *tr, char *params,
+			       struct action_data *data)
 {
 	char *param, *saved_param;
 	bool first_param = true;
@@ -3819,20 +3829,20 @@ static int parse_action_params(char *params, struct action_data *data)
 
 	while (params) {
 		if (data->n_params >= SYNTH_FIELDS_MAX) {
-			hist_err(HIST_ERR_TOO_MANY_PARAMS, 0);
+			hist_err(tr, HIST_ERR_TOO_MANY_PARAMS, 0);
 			goto out;
 		}
 
 		param = strsep(&params, ",");
 		if (!param) {
-			hist_err(HIST_ERR_PARAM_NOT_FOUND, 0);
+			hist_err(tr, HIST_ERR_PARAM_NOT_FOUND, 0);
 			ret = -EINVAL;
 			goto out;
 		}
 
 		param = strstrip(param);
 		if (strlen(param) < 2) {
-			hist_err(HIST_ERR_INVALID_PARAM, errpos(param));
+			hist_err(tr, HIST_ERR_INVALID_PARAM, errpos(param));
 			ret = -EINVAL;
 			goto out;
 		}
@@ -3856,7 +3866,7 @@ static int parse_action_params(char *params, struct action_data *data)
 	return ret;
 }
 
-static int action_parse(char *str, struct action_data *data,
+static int action_parse(struct trace_array *tr, char *str, struct action_data *data,
 			enum handler_id handler)
 {
 	char *action_name;
@@ -3864,14 +3874,14 @@ static int action_parse(char *str, struct action_data *data,
 
 	strsep(&str, ".");
 	if (!str) {
-		hist_err(HIST_ERR_ACTION_NOT_FOUND, 0);
+		hist_err(tr, HIST_ERR_ACTION_NOT_FOUND, 0);
 		ret = -EINVAL;
 		goto out;
 	}
 
 	action_name = strsep(&str, "(");
 	if (!action_name || !str) {
-		hist_err(HIST_ERR_ACTION_NOT_FOUND, 0);
+		hist_err(tr, HIST_ERR_ACTION_NOT_FOUND, 0);
 		ret = -EINVAL;
 		goto out;
 	}
@@ -3880,12 +3890,12 @@ static int action_parse(char *str, struct action_data *data,
 		char *params = strsep(&str, ")");
 
 		if (!params) {
-			hist_err(HIST_ERR_NO_SAVE_PARAMS, 0);
+			hist_err(tr, HIST_ERR_NO_SAVE_PARAMS, 0);
 			ret = -EINVAL;
 			goto out;
 		}
 
-		ret = parse_action_params(params, data);
+		ret = parse_action_params(tr, params, data);
 		if (ret)
 			goto out;
 
@@ -3894,7 +3904,7 @@ static int action_parse(char *str, struct action_data *data,
 		else if (handler == HANDLER_ONCHANGE)
 			data->track_data.check_val = check_track_val_changed;
 		else {
-			hist_err(HIST_ERR_ACTION_MISMATCH, errpos(action_name));
+			hist_err(tr, HIST_ERR_ACTION_MISMATCH, errpos(action_name));
 			ret = -EINVAL;
 			goto out;
 		}
@@ -3906,7 +3916,7 @@ static int action_parse(char *str, struct action_data *data,
 		char *params = strsep(&str, ")");
 
 		if (!str) {
-			hist_err(HIST_ERR_NO_CLOSING_PAREN, errpos(params));
+			hist_err(tr, HIST_ERR_NO_CLOSING_PAREN, errpos(params));
 			ret = -EINVAL;
 			goto out;
 		}
@@ -3916,7 +3926,7 @@ static int action_parse(char *str, struct action_data *data,
 		else if (handler == HANDLER_ONCHANGE)
 			data->track_data.check_val = check_track_val_changed;
 		else {
-			hist_err(HIST_ERR_ACTION_MISMATCH, errpos(action_name));
+			hist_err(tr, HIST_ERR_ACTION_MISMATCH, errpos(action_name));
 			ret = -EINVAL;
 			goto out;
 		}
@@ -3931,7 +3941,7 @@ static int action_parse(char *str, struct action_data *data,
 			data->use_trace_keyword = true;
 
 		if (params) {
-			ret = parse_action_params(params, data);
+			ret = parse_action_params(tr, params, data);
 			if (ret)
 				goto out;
 		}
@@ -3984,7 +3994,7 @@ static struct action_data *track_data_parse(struct hist_trigger_data *hist_data,
 		goto free;
 	}
 
-	ret = action_parse(str, data, handler);
+	ret = action_parse(hist_data->event_file->tr, str, data, handler);
 	if (ret)
 		goto free;
  out:
@@ -4054,6 +4064,7 @@ trace_action_find_var(struct hist_trigger_data *hist_data,
 		      struct action_data *data,
 		      char *system, char *event, char *var)
 {
+	struct trace_array *tr = hist_data->event_file->tr;
 	struct hist_field *hist_field;
 
 	var++; /* skip '$' */
@@ -4069,7 +4080,7 @@ trace_action_find_var(struct hist_trigger_data *hist_data,
 	}
 
 	if (!hist_field)
-		hist_err(HIST_ERR_PARAM_NOT_FOUND, errpos(var));
+		hist_err(tr, HIST_ERR_PARAM_NOT_FOUND, errpos(var));
 
 	return hist_field;
 }
@@ -4127,6 +4138,7 @@ trace_action_create_field_var(struct hist_trigger_data *hist_data,
 static int trace_action_create(struct hist_trigger_data *hist_data,
 			       struct action_data *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;
@@ -4144,7 +4156,7 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
 
 	event = find_synth_event(synth_event_name);
 	if (!event) {
-		hist_err(HIST_ERR_SYNTH_EVENT_NOT_FOUND, errpos(synth_event_name));
+		hist_err(tr, HIST_ERR_SYNTH_EVENT_NOT_FOUND, errpos(synth_event_name));
 		return -EINVAL;
 	}
 
@@ -4205,14 +4217,14 @@ static int trace_action_create(struct hist_trigger_data *hist_data,
 			continue;
 		}
 
-		hist_err(HIST_ERR_SYNTH_TYPE_MISMATCH, errpos(param));
+		hist_err(tr, HIST_ERR_SYNTH_TYPE_MISMATCH, errpos(param));
 		kfree(p);
 		ret = -EINVAL;
 		goto err;
 	}
 
 	if (field_pos != event->n_fields) {
-		hist_err(HIST_ERR_SYNTH_COUNT_MISMATCH, errpos(event->name));
+		hist_err(tr, HIST_ERR_SYNTH_COUNT_MISMATCH, errpos(event->name));
 		ret = -EINVAL;
 		goto err;
 	}
@@ -4231,6 +4243,7 @@ static int action_create(struct hist_trigger_data *hist_data,
 			 struct action_data *data)
 {
 	struct trace_event_file *file = hist_data->event_file;
+	struct trace_array *tr = file->tr;
 	struct track_data *track_data;
 	struct field_var *field_var;
 	unsigned int i;
@@ -4258,7 +4271,7 @@ static int action_create(struct hist_trigger_data *hist_data,
 	if (data->action == ACTION_SAVE) {
 		if (hist_data->n_save_vars) {
 			ret = -EEXIST;
-			hist_err(HIST_ERR_TOO_MANY_SAVE_ACTIONS, 0);
+			hist_err(tr, HIST_ERR_TOO_MANY_SAVE_ACTIONS, 0);
 			goto out;
 		}
 
@@ -4271,7 +4284,8 @@ static int action_create(struct hist_trigger_data *hist_data,
 
 			field_var = create_target_field_var(hist_data, NULL, NULL, param);
 			if (IS_ERR(field_var)) {
-				hist_err(HIST_ERR_FIELD_VAR_CREATE_FAIL, errpos(param));
+				hist_err(tr, HIST_ERR_FIELD_VAR_CREATE_FAIL,
+					 errpos(param));
 				ret = PTR_ERR(field_var);
 				kfree(param);
 				goto out;
@@ -4305,18 +4319,18 @@ static struct action_data *onmatch_parse(struct trace_array *tr, char *str)
 
 	match_event = strsep(&str, ")");
 	if (!match_event || !str) {
-		hist_err(HIST_ERR_NO_CLOSING_PAREN, errpos(match_event));
+		hist_err(tr, HIST_ERR_NO_CLOSING_PAREN, errpos(match_event));
 		goto free;
 	}
 
 	match_event_system = strsep(&match_event, ".");
 	if (!match_event) {
-		hist_err(HIST_ERR_SUBSYS_NOT_FOUND, errpos(match_event_system));
+		hist_err(tr, HIST_ERR_SUBSYS_NOT_FOUND, errpos(match_event_system));
 		goto free;
 	}
 
 	if (IS_ERR(event_file(tr, match_event_system, match_event))) {
-		hist_err(HIST_ERR_INVALID_SUBSYS_EVENT, errpos(match_event));
+		hist_err(tr, HIST_ERR_INVALID_SUBSYS_EVENT, errpos(match_event));
 		goto free;
 	}
 
@@ -4332,7 +4346,7 @@ static struct action_data *onmatch_parse(struct trace_array *tr, char *str)
 		goto free;
 	}
 
-	ret = action_parse(str, data, HANDLER_ONMATCH);
+	ret = action_parse(tr, str, data, HANDLER_ONMATCH);
 	if (ret)
 		goto free;
  out:
@@ -4401,13 +4415,14 @@ static int create_var_field(struct hist_trigger_data *hist_data,
 			    struct trace_event_file *file,
 			    char *var_name, char *expr_str)
 {
+	struct trace_array *tr = hist_data->event_file->tr; 
 	unsigned long flags = 0;
 
 	if (WARN_ON(val_idx >= TRACING_MAP_VALS_MAX + TRACING_MAP_VARS_MAX))
 		return -EINVAL;
 
 	if (find_var(hist_data, file, var_name) && !hist_data->remove) {
-		hist_err(HIST_ERR_DUPLICATE_VAR, errpos(var_name));
+		hist_err(tr, HIST_ERR_DUPLICATE_VAR, errpos(var_name));
 		return -EINVAL;
 	}
 
@@ -4464,8 +4479,8 @@ static int create_key_field(struct hist_trigger_data *hist_data,
 			    struct trace_event_file *file,
 			    char *field_str)
 {
+	struct trace_array *tr = hist_data->event_file->tr;
 	struct hist_field *hist_field = NULL;
-
 	unsigned long flags = 0;
 	unsigned int key_size;
 	int ret = 0;
@@ -4488,7 +4503,7 @@ static int create_key_field(struct hist_trigger_data *hist_data,
 		}
 
 		if (hist_field->flags & HIST_FIELD_FL_VAR_REF) {
-			hist_err(HIST_ERR_INVALID_REF_KEY, errpos(field_str));
+			hist_err(tr, HIST_ERR_INVALID_REF_KEY, errpos(field_str));
 			destroy_hist_field(hist_field, 0);
 			ret = -EINVAL;
 			goto out;
@@ -4589,6 +4604,7 @@ static void free_var_defs(struct hist_trigger_data *hist_data)
 
 static int parse_var_defs(struct hist_trigger_data *hist_data)
 {
+	struct trace_array *tr = hist_data->event_file->tr;
 	char *s, *str, *var_name, *field_str;
 	unsigned int i, j, n_vars = 0;
 	int ret = 0;
@@ -4602,13 +4618,14 @@ static int parse_var_defs(struct hist_trigger_data *hist_data)
 
 			var_name = strsep(&field_str, "=");
 			if (!var_name || !field_str) {
-				hist_err(HIST_ERR_MALFORMED_ASSIGNMENT, errpos(var_name));
+				hist_err(tr, HIST_ERR_MALFORMED_ASSIGNMENT,
+					 errpos(var_name));
 				ret = -EINVAL;
 				goto free;
 			}
 
 			if (n_vars == TRACING_MAP_VARS_MAX) {
-				hist_err(HIST_ERR_TOO_MANY_VARS, errpos(var_name));
+				hist_err(tr, HIST_ERR_TOO_MANY_VARS, errpos(var_name));
 				ret = -EINVAL;
 				goto free;
 			}
@@ -5829,6 +5846,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
 {
 	struct hist_trigger_data *hist_data = data->private_data;
 	struct event_trigger_data *test, *named_data = NULL;
+	struct trace_array *tr = file->tr;
 	int ret = 0;
 
 	if (hist_data->attrs->name) {
@@ -5836,7 +5854,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
 		if (named_data) {
 			if (!hist_trigger_match(data, named_data, named_data,
 						true)) {
-				hist_err(HIST_ERR_NAMED_MISMATCH, errpos(hist_data->attrs->name));
+				hist_err(tr, HIST_ERR_NAMED_MISMATCH, errpos(hist_data->attrs->name));
 				ret = -EINVAL;
 				goto out;
 			}
@@ -5857,7 +5875,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
 			else if (hist_data->attrs->clear)
 				hist_clear(test);
 			else {
-				hist_err(HIST_ERR_TRIGGER_EEXIST, 0);
+				hist_err(tr, HIST_ERR_TRIGGER_EEXIST, 0);
 				ret = -EEXIST;
 			}
 			goto out;
@@ -5865,7 +5883,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
 	}
  new:
 	if (hist_data->attrs->cont || hist_data->attrs->clear) {
-		hist_err(HIST_ERR_TRIGGER_ENOENT_CLEAR, 0);
+		hist_err(tr, HIST_ERR_TRIGGER_ENOENT_CLEAR, 0);
 		ret = -ENOENT;
 		goto out;
 	}
@@ -5890,7 +5908,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
 
 		ret = tracing_set_clock(file->tr, hist_data->attrs->clock);
 		if (ret) {
-			hist_err(HIST_ERR_SET_CLOCK_FAIL, errpos(clock));
+			hist_err(tr, HIST_ERR_SET_CLOCK_FAIL, errpos(clock));
 			goto out;
 		}
 
@@ -6108,7 +6126,7 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
 		trigger = strstrip(trigger);
 	}
 
-	attrs = parse_hist_trigger_attrs(trigger);
+	attrs = parse_hist_trigger_attrs(file->tr, trigger);
 	if (IS_ERR(attrs))
 		return PTR_ERR(attrs);
 
-- 
2.20.1

[RFC][PATCH 3/3] tracing: Have the error logs show up in the proper instances

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

As each instance has their own error_log file, it makes more sense that the
instances show the errors of their own instead of all error_logs having the
same data. Make it that the errors show up in the instance error_log file
that the error happens in. If no instance trace_array is available, then
NULL can be passed in which will create the error in the top level instance
(the one at the top of the tracefs directory).

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c               | 55 ++++++++++++++++++++----------
 kernel/trace/trace.h               |  5 ++-
 kernel/trace/trace_events_filter.c |  4 +--
 kernel/trace/trace_events_hist.c   |  3 +-
 kernel/trace/trace_probe.c         |  2 +-
 5 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 7978168f5041..3d55e9daae8c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6897,25 +6897,22 @@ struct tracing_log_err {
 	char			cmd[MAX_FILTER_STR_VAL]; /* what caused err */
 };
 
-static LIST_HEAD(tracing_err_log);
 static DEFINE_MUTEX(tracing_err_log_lock);
 
-static unsigned int n_tracing_err_log_entries;
-
-struct tracing_log_err *get_tracing_log_err(void)
+struct tracing_log_err *get_tracing_log_err(struct trace_array *tr)
 {
 	struct tracing_log_err *err;
 
-	if (n_tracing_err_log_entries < TRACING_LOG_ERRS_MAX) {
+	if (tr->n_err_log_entries < TRACING_LOG_ERRS_MAX) {
 		err = kzalloc(sizeof(*err), GFP_KERNEL);
 		if (!err)
 			err = ERR_PTR(-ENOMEM);
-		n_tracing_err_log_entries++;
+		tr->n_err_log_entries++;
 
 		return err;
 	}
 
-	err = list_first_entry(&tracing_err_log, struct tracing_log_err, list);
+	err = list_first_entry(&tr->err_log, struct tracing_log_err, list);
 	list_del(&err->list);
 
 	return err;
@@ -6949,6 +6946,7 @@ unsigned int err_pos(char *cmd, const char *str)
 
 /**
  * tracing_log_err - write an error to the tracing error log
+ * @tr: The associated trace array for the error (NULL for top level array)
  * @loc: A string describing where the error occurred
  * @cmd: The tracing command that caused the error
  * @errs: The array of loc-specific static error strings
@@ -6973,13 +6971,17 @@ unsigned int err_pos(char *cmd, const char *str)
  * existing callers for examples of how static strings are typically
  * defined for use with tracing_log_err().
  */
-void tracing_log_err(const char *loc, const char *cmd,
+void tracing_log_err(struct trace_array *tr,
+		     const char *loc, const char *cmd,
 		     const char **errs, u8 type, u8 pos)
 {
 	struct tracing_log_err *err;
 
+	if (!tr)
+		tr = &global_trace;
+
 	mutex_lock(&tracing_err_log_lock);
-	err = get_tracing_log_err();
+	err = get_tracing_log_err(tr);
 	if (PTR_ERR(err) == -ENOMEM) {
 		mutex_unlock(&tracing_err_log_lock);
 		return;
@@ -6993,34 +6995,38 @@ void tracing_log_err(const char *loc, const char *cmd,
 	err->info.pos = pos;
 	err->info.ts = local_clock();
 
-	list_add_tail(&err->list, &tracing_err_log);
+	list_add_tail(&err->list, &tr->err_log);
 	mutex_unlock(&tracing_err_log_lock);
 }
 
-static void clear_tracing_err_log(void)
+static void clear_tracing_err_log(struct trace_array *tr)
 {
 	struct tracing_log_err *err, *next;
 
 	mutex_lock(&tracing_err_log_lock);
-	list_for_each_entry_safe(err, next, &tracing_err_log, list) {
+	list_for_each_entry_safe(err, next, &tr->err_log, list) {
 		list_del(&err->list);
 		kfree(err);
 	}
 
-	n_tracing_err_log_entries = 0;
+	tr->n_err_log_entries = 0;
 	mutex_unlock(&tracing_err_log_lock);
 }
 
 static void *tracing_err_log_seq_start(struct seq_file *m, loff_t *pos)
 {
+	struct trace_array *tr = m->private;
+
 	mutex_lock(&tracing_err_log_lock);
 
-	return seq_list_start(&tracing_err_log, *pos);
+	return seq_list_start(&tr->err_log, *pos);
 }
 
 static void *tracing_err_log_seq_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	return seq_list_next(v, &tracing_err_log, pos);
+	struct trace_array *tr = m->private;
+
+	return seq_list_next(v, &tr->err_log, pos);
 }
 
 static void tracing_err_log_seq_stop(struct seq_file *m, void *v)
@@ -7067,15 +7073,25 @@ static const struct seq_operations tracing_err_log_seq_ops = {
 
 static int tracing_err_log_open(struct inode *inode, struct file *file)
 {
+	struct trace_array *tr = inode->i_private;
 	int ret = 0;
 
+	if (trace_array_get(tr) < 0)
+		return -ENODEV;
+
 	/* If this file was opened for write, then erase contents */
 	if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC))
-		clear_tracing_err_log();
+		clear_tracing_err_log(tr);
 
-	if (file->f_mode & FMODE_READ)
+	if (file->f_mode & FMODE_READ) {
 		ret = seq_open(file, &tracing_err_log_seq_ops);
-
+		if (!ret) {
+			struct seq_file *m = file->private_data;
+			m->private = tr;
+		} else {
+			trace_array_put(tr);
+		}
+	}
 	return ret;
 }
 
@@ -7091,6 +7107,7 @@ static const struct file_operations tracing_err_log_fops = {
 	.write		= tracing_err_log_write,
 	.read           = seq_read,
 	.llseek         = seq_lseek,
+	.release	= tracing_release_generic_tr,
 };
 
 static int tracing_buffers_open(struct inode *inode, struct file *filp)
@@ -8293,6 +8310,7 @@ struct trace_array *trace_array_create(const char *name)
 	INIT_LIST_HEAD(&tr->systems);
 	INIT_LIST_HEAD(&tr->events);
 	INIT_LIST_HEAD(&tr->hist_vars);
+	INIT_LIST_HEAD(&tr->err_log);
 
 	if (allocate_trace_buffers(tr, trace_buf_size) < 0)
 		goto out_free_tr;
@@ -9087,6 +9105,7 @@ __init static int tracer_alloc_buffers(void)
 	INIT_LIST_HEAD(&global_trace.systems);
 	INIT_LIST_HEAD(&global_trace.events);
 	INIT_LIST_HEAD(&global_trace.hist_vars);
+	INIT_LIST_HEAD(&global_trace.err_log);
 	list_add(&global_trace.list, &ftrace_trace_arrays);
 
 	apply_trace_boot_options();
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 809c5d7f0064..da00a3d508c1 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -293,11 +293,13 @@ struct trace_array {
 	int			nr_topts;
 	bool			clear_trace;
 	int			buffer_percent;
+	unsigned int		n_err_log_entries;
 	struct tracer		*current_trace;
 	unsigned int		trace_flags;
 	unsigned char		trace_flags_index[TRACE_FLAGS_MAX_SIZE];
 	unsigned int		flags;
 	raw_spinlock_t		start_lock;
+	struct list_head	err_log;
 	struct dentry		*dir;
 	struct dentry		*options;
 	struct dentry		*percpu_dir;
@@ -1886,7 +1888,8 @@ extern ssize_t trace_parse_run_command(struct file *file,
 		int (*createfn)(int, char**));
 
 extern unsigned int err_pos(char *cmd, const char *str);
-extern void tracing_log_err(const char *loc, const char *cmd,
+extern void tracing_log_err(struct trace_array *tr,
+			    const char *loc, const char *cmd,
 			    const char **errs, u8 type, u8 pos);
 
 /*
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index b1033638e499..d07bfa5c0ee5 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -949,12 +949,12 @@ static void append_filter_err(struct trace_array *tr,
 	if (pe->lasterr > 0) {
 		trace_seq_printf(s, "\n%*s", pos, "^");
 		trace_seq_printf(s, "\nparse_error: %s\n", err_text[pe->lasterr]);
-		tracing_log_err("event filter parse error",
+		tracing_log_err(tr, "event filter parse error",
 				filter->filter_string, err_text,
 				pe->lasterr, pe->lasterr_pos);
 	} else {
 		trace_seq_printf(s, "\nError: (%d)\n", pe->lasterr);
-		tracing_log_err("event filter parse error",
+		tracing_log_err(tr, "event filter parse error",
 				filter->filter_string, err_text,
 				FILT_ERR_ERRNO, 0);
 	}
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 49d373792721..a07a5b6ebd98 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -621,7 +621,8 @@ static void last_cmd_set(struct trace_event_file *file, char *str)
 
 static void hist_err(struct trace_array *tr, u8 err_type, u8 err_pos)
 {
-	tracing_log_err(last_cmd_loc, last_cmd, err_text, err_type, err_pos);
+	tracing_log_err(tr, last_cmd_loc, last_cmd, err_text,
+			err_type, err_pos);
 }
 
 static void hist_err_clear(void)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index e11f98c49d72..4cc2d467d34c 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -186,7 +186,7 @@ void __trace_probe_log_err(int offset, int err_type)
 	}
 	*(p - 1) = '\0';
 
-	tracing_log_err(trace_probe_log.subsystem, command,
+	tracing_log_err(NULL, trace_probe_log.subsystem, command,
 			trace_probe_err_text, err_type, pos + offset);
 
 	kfree(command);
-- 
2.20.1

Re: [RFC][PATCH 0/3] tracing: Make error_log per instance

[Tom Zanussi]

Hi Steve,

On Tue, 2019-04-02 at 14:29 -0400, Steven Rostedt wrote:
> Hi Tom,
> 
> I noticed that you created an error_log file in every instance, but
> they all show the same errors. These three patches make it so that
> the errors appear in the instance directory that they happened in.
> If you write a bad error to
> 
>    /sys/kernel/tracing/instance/foo/events/sched/sched_switch/hist
> 
> It appears only in
> 
>   /sys/kernel/tracing/instances/foo/error_log
> 
> Which I think is the proper approach, especially instances should not
> affect the top directory or other instances.
> 
> For those errors that do not have an associated instance (creating a
> kprobe/uprobe event or perf), a NULL passed to tracing_log_err() will
> result in the error message in the top level error message.
> 
> Do you (or Masami) have any issues with this patch set?
> 
> If not, please add a "reviewed-by" or "acked-by" and I'll add it
> to your patch series and push them to for-next (after more testing).
> 

Looks good, thanks for doing this.  You can add my:

Reviewed-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Tom Zanussi <tom.zanussi@linux.intel.com>

FYI, I noticed one bit of trailing whitespace in 2/3 - you might want
to run it through checkpatch again before you merge ;-)

Thanks,

Tom


> If this isn't obvious, this patch series is on top of:
> 
>   http://lkml.kernel.org/r/cover.1554072478.git.tom.zanussi@linux.int
> el.com
> 
> Actually, I added it right after patch 5 of that series (before the
> selftests and documentation).
> 
> Thanks!
> 
> -- Steve
> 
> 
> 
> Steven Rostedt (VMware) (3):
>       tracing: Add trace_array parameter to create_event_filter()
>       tracing: Have histogram code pass around trace_array for error
> handling
>       tracing: Have the error logs show up in the proper instances
> 
> ----
>  kernel/trace/trace.c                |  55 +++++++++-----
>  kernel/trace/trace.h                |   8 +-
>  kernel/trace/trace_events_filter.c  |  25 ++++---
>  kernel/trace/trace_events_hist.c    | 145 ++++++++++++++++++++----
> ------------
>  kernel/trace/trace_events_trigger.c |   3 +-
>  kernel/trace/trace_probe.c          |   2 +-
>  6 files changed, 142 insertions(+), 96 deletions(-)

Re: [RFC][PATCH 0/3] tracing: Make error_log per instance

[Steven Rostedt]

On Tue, 02 Apr 2019 15:42:16 -0500
Tom Zanussi <zanussi@kernel.org> wrote:

> Hi Steve,
> 
> On Tue, 2019-04-02 at 14:29 -0400, Steven Rostedt wrote:
> > Hi Tom,
> > 
> > I noticed that you created an error_log file in every instance, but
> > they all show the same errors. These three patches make it so that
> > the errors appear in the instance directory that they happened in.
> > If you write a bad error to
> > 
> >    /sys/kernel/tracing/instance/foo/events/sched/sched_switch/hist
> > 
> > It appears only in
> > 
> >   /sys/kernel/tracing/instances/foo/error_log
> > 
> > Which I think is the proper approach, especially instances should not
> > affect the top directory or other instances.
> > 
> > For those errors that do not have an associated instance (creating a
> > kprobe/uprobe event or perf), a NULL passed to tracing_log_err() will
> > result in the error message in the top level error message.
> > 
> > Do you (or Masami) have any issues with this patch set?
> > 
> > If not, please add a "reviewed-by" or "acked-by" and I'll add it
> > to your patch series and push them to for-next (after more testing).
> >   
> 
> Looks good, thanks for doing this.  You can add my:
> 
> Reviewed-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> Tested-by: Tom Zanussi <tom.zanussi@linux.intel.com>

Thanks, will do.

> 
> FYI, I noticed one bit of trailing whitespace in 2/3 - you might want
> to run it through checkpatch again before you merge ;-)

Hmm, strange that git didn't complain about that. It usually does on a
commit. Or perhaps (more likely) it did, and I didn't notice ;-)

-- Steve

Re: [RFC][PATCH 0/3] tracing: Make error_log per instance

[Masami Hiramatsu]

Hi Steve,

Sorry for replying late.

On Tue, 02 Apr 2019 14:29:51 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Hi Tom,
> 
> I noticed that you created an error_log file in every instance, but
> they all show the same errors. These three patches make it so that
> the errors appear in the instance directory that they happened in.
> If you write a bad error to
> 
>    /sys/kernel/tracing/instance/foo/events/sched/sched_switch/hist
> 
> It appears only in
> 
>   /sys/kernel/tracing/instances/foo/error_log
> 
> Which I think is the proper approach, especially instances should not
> affect the top directory or other instances.

Agreed, leaking the probe-event error on instance will not be good.

> 
> For those errors that do not have an associated instance (creating a
> kprobe/uprobe event or perf), a NULL passed to tracing_log_err() will
> result in the error message in the top level error message.
> 
> Do you (or Masami) have any issues with this patch set?
> 
> If not, please add a "reviewed-by" or "acked-by" and I'll add it
> to your patch series and push them to for-next (after more testing).

This looks good to me.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you!

> 
> If this isn't obvious, this patch series is on top of:
> 
>   http://lkml.kernel.org/r/cover.1554072478.git.tom.zanussi@linux.intel.com
> 
> Actually, I added it right after patch 5 of that series (before the
> selftests and documentation).
> 
> Thanks!
> 
> -- Steve
> 
> 
> 
> Steven Rostedt (VMware) (3):
>       tracing: Add trace_array parameter to create_event_filter()
>       tracing: Have histogram code pass around trace_array for error handling
>       tracing: Have the error logs show up in the proper instances
> 
> ----
>  kernel/trace/trace.c                |  55 +++++++++-----
>  kernel/trace/trace.h                |   8 +-
>  kernel/trace/trace_events_filter.c  |  25 ++++---
>  kernel/trace/trace_events_hist.c    | 145 ++++++++++++++++++++----------------
>  kernel/trace/trace_events_trigger.c |   3 +-
>  kernel/trace/trace_probe.c          |   2 +-
>  6 files changed, 142 insertions(+), 96 deletions(-)


-- 
Masami Hiramatsu <mhiramat@kernel.org>