This is a note to let you know that I've just added the patch titled
tracing: Generalize hist trigger onmax and save action
to the 4.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
tracing-generalize-hist-trigger-onmax-and-save-action.patch
and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From stable+bounces-43489-greg=kroah.com@xxxxxxxxxxxxxxx Thu May 9 04:30:50 2024
From: George Guo <dongtai.guo@xxxxxxxxx>
Date: Thu, 9 May 2024 10:29:30 +0800
Subject: tracing: Generalize hist trigger onmax and save action
To: gregkh@xxxxxxxxxxxxxxxxxxx, rostedt@xxxxxxxxxxx, mhiramat@xxxxxxxxxx, tom.zanussi@xxxxxxxxxxxxxxx
Cc: stable@xxxxxxxxxxxxxxx, George Guo <guodongtai@xxxxxxxxxx>
Message-ID: <20240509022931.3513365-13-dongtai.guo@xxxxxxxxx>
From: Tom Zanussi <tom.zanussi@xxxxxxxxxxxxxxx>
commit 466f4528fbc692ea56deca278fa6aeb79e6e8b21 upstream.
The action refactor code allowed actions and handlers to be separated,
but the existing onmax handler and save action code is still not
flexible enough to handle arbitrary coupling. This change generalizes
them and in the process makes additional handlers and actions easier
to implement.
The onmax action can be broken up and thought of as two separate
components - a variable to be tracked (the parameter given to the
onmax($var_to_track) function) and an invisible variable created to
save the ongoing result of doing something with that variable, such as
saving the max value of that variable so far seen.
Separating it out like this and renaming it appropriately allows us to
use the same code for similar tracking functions such as
onchange($var_to_track), which would just track the last value seen
rather than the max seen so far, which is useful in some situations.
Additionally, because different handlers and actions may want to save
and access data differently e.g. save and retrieve tracking values as
local variables vs something more global, save_val() and get_val()
interface functions are introduced and max-specific implementations
are used instead.
The same goes for the code that checks whether a maximum has been hit
- a generic check_val() interface and max-checking implementation is
used instead, which allows future patches to make use of he same code
using their own implemetations of similar functionality.
Link: http://lkml.kernel.org/r/980ea73dd8e3f36db3d646f99652f8fed42b77d4.1550100284.git.tom.zanussi@xxxxxxxxxxxxxxx
Signed-off-by: Tom Zanussi <tom.zanussi@xxxxxxxxxxxxxxx>
Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
Signed-off-by: George Guo <guodongtai@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
kernel/trace/trace_events_hist.c | 242 ++++++++++++++++++++++++++-------------
1 file changed, 163 insertions(+), 79 deletions(-)
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -360,6 +360,8 @@ typedef void (*action_fn_t) (struct hist
struct ring_buffer_event *rbe, void *key,
struct action_data *data, u64 *var_ref_vals);
+typedef bool (*check_track_val_fn_t) (u64 track_val, u64 var_val);
+
enum handler_id {
HANDLER_ONMATCH = 1,
HANDLER_ONMAX,
@@ -397,15 +399,35 @@ struct action_data {
} match_data;
struct {
+ /*
+ * var_str contains the $-unstripped variable
+ * name referenced by var_ref, and used when
+ * printing the action. Because var_ref
+ * creation is deferred to create_actions(),
+ * we need a per-action way to save it until
+ * then, thus var_str.
+ */
char *var_str;
- unsigned int max_var_ref_idx;
- struct hist_field *max_var;
- struct hist_field *var;
- } onmax;
+
+ /*
+ * var_ref refers to the variable being
+ * tracked e.g onmax($var).
+ */
+ struct hist_field *var_ref;
+
+ /*
+ * track_var contains the 'invisible' tracking
+ * variable created to keep the current
+ * e.g. max value.
+ */
+ struct hist_field *track_var;
+
+ check_track_val_fn_t check_val;
+ action_fn_t save_data;
+ } track_data;
};
};
-
static char last_hist_cmd[MAX_FILTER_STR_VAL];
static char hist_err_str[MAX_FILTER_STR_VAL];
@@ -3311,10 +3333,10 @@ static void update_field_vars(struct his
hist_data->n_field_vars, 0);
}
-static void update_max_vars(struct hist_trigger_data *hist_data,
- struct tracing_map_elt *elt,
- struct ring_buffer_event *rbe,
- void *rec)
+static void save_track_data_vars(struct hist_trigger_data *hist_data,
+ struct tracing_map_elt *elt, void *rec,
+ struct ring_buffer_event *rbe, void *key,
+ struct action_data *data, u64 *var_ref_vals)
{
__update_field_vars(elt, rbe, rec, hist_data->save_vars,
hist_data->n_save_vars, hist_data->n_field_var_str);
@@ -3452,14 +3474,67 @@ create_target_field_var(struct hist_trig
return create_field_var(target_hist_data, file, var_name);
}
-static void onmax_print(struct seq_file *m,
- struct hist_trigger_data *hist_data,
- struct tracing_map_elt *elt,
- struct action_data *data)
+static bool check_track_val_max(u64 track_val, u64 var_val)
+{
+ if (var_val <= track_val)
+ return false;
+
+ return true;
+}
+
+static u64 get_track_val(struct hist_trigger_data *hist_data,
+ struct tracing_map_elt *elt,
+ struct action_data *data)
+{
+ unsigned int track_var_idx = data->track_data.track_var->var.idx;
+ u64 track_val;
+
+ track_val = tracing_map_read_var(elt, track_var_idx);
+
+ return track_val;
+}
+
+static void save_track_val(struct hist_trigger_data *hist_data,
+ struct tracing_map_elt *elt,
+ struct action_data *data, u64 var_val)
+{
+ unsigned int track_var_idx = data->track_data.track_var->var.idx;
+
+ tracing_map_set_var(elt, track_var_idx, var_val);
+}
+
+static void save_track_data(struct hist_trigger_data *hist_data,
+ struct tracing_map_elt *elt, void *rec,
+ struct ring_buffer_event *rbe, void *key,
+ struct action_data *data, u64 *var_ref_vals)
+{
+ if (data->track_data.save_data)
+ data->track_data.save_data(hist_data, elt, rec, rbe, key, data, var_ref_vals);
+}
+
+static bool check_track_val(struct tracing_map_elt *elt,
+ struct action_data *data,
+ u64 var_val)
{
- unsigned int i, save_var_idx, max_idx = data->onmax.max_var->var.idx;
+ struct hist_trigger_data *hist_data;
+ u64 track_val;
+
+ hist_data = data->track_data.track_var->hist_data;
+ track_val = get_track_val(hist_data, elt, data);
+
+ return data->track_data.check_val(track_val, var_val);
+}
+
+static void track_data_print(struct seq_file *m,
+ struct hist_trigger_data *hist_data,
+ struct tracing_map_elt *elt,
+ struct action_data *data)
+{
+ u64 track_val = get_track_val(hist_data, elt, data);
+ unsigned int i, save_var_idx;
- seq_printf(m, "\n\tmax: %10llu", tracing_map_read_var(elt, max_idx));
+ if (data->handler == HANDLER_ONMAX)
+ seq_printf(m, "\n\tmax: %10llu", track_val);
for (i = 0; i < hist_data->n_save_vars; i++) {
struct hist_field *save_val = hist_data->save_vars[i]->val;
@@ -3478,25 +3553,17 @@ static void onmax_print(struct seq_file
}
}
-static void onmax_save(struct hist_trigger_data *hist_data,
- struct tracing_map_elt *elt, void *rec,
- struct ring_buffer_event *rbe, void *key,
- struct action_data *data, u64 *var_ref_vals)
-{
- unsigned int max_idx = data->onmax.max_var->var.idx;
- unsigned int max_var_ref_idx = data->onmax.max_var_ref_idx;
-
- u64 var_val, max_val;
-
- var_val = var_ref_vals[max_var_ref_idx];
- max_val = tracing_map_read_var(elt, max_idx);
-
- if (var_val <= max_val)
- return;
-
- tracing_map_set_var(elt, max_idx, var_val);
-
- update_max_vars(hist_data, elt, rbe, rec);
+static void ontrack_action(struct hist_trigger_data *hist_data,
+ struct tracing_map_elt *elt, void *rec,
+ struct ring_buffer_event *rbe, void *key,
+ struct action_data *data, u64 *var_ref_vals)
+{
+ u64 var_val = var_ref_vals[data->track_data.var_ref->var_ref_idx];
+
+ if (check_track_val(elt, data, var_val)) {
+ save_track_val(hist_data, elt, data, var_val);
+ save_track_data(hist_data, elt, rec, rbe, key, data, var_ref_vals);
+ }
}
static void action_data_destroy(struct action_data *data)
@@ -3516,12 +3583,13 @@ static void action_data_destroy(struct a
kfree(data);
}
-static void onmax_destroy(struct action_data *data)
+static void track_data_destroy(struct hist_trigger_data *hist_data,
+ struct action_data *data)
{
- destroy_hist_field(data->onmax.max_var, 0);
- destroy_hist_field(data->onmax.var, 0);
+ destroy_hist_field(data->track_data.track_var, 0);
+ destroy_hist_field(data->track_data.var_ref, 0);
- kfree(data->onmax.var_str);
+ kfree(data->track_data.var_str);
action_data_destroy(data);
}
@@ -3529,25 +3597,24 @@ static void onmax_destroy(struct action_
static int action_create(struct hist_trigger_data *hist_data,
struct action_data *data);
-static int onmax_create(struct hist_trigger_data *hist_data,
- struct action_data *data)
+static int track_data_create(struct hist_trigger_data *hist_data,
+ struct action_data *data)
{
- struct hist_field *var_field, *ref_field, *max_var = NULL;
+ struct hist_field *var_field, *ref_field, *track_var = NULL;
struct trace_event_file *file = hist_data->event_file;
- unsigned int var_ref_idx = hist_data->n_var_refs;
- char *onmax_var_str;
+ char *track_data_var_str;
int ret = 0;
- onmax_var_str = data->onmax.var_str;
- if (onmax_var_str[0] != '$') {
- hist_err("onmax: For onmax(x), x must be a variable: ", onmax_var_str);
+ track_data_var_str = data->track_data.var_str;
+ if (track_data_var_str[0] != '$') {
+ hist_err("For onmax(x), x must be a variable: ", track_data_var_str);
return -EINVAL;
}
- onmax_var_str++;
+ track_data_var_str++;
- var_field = find_target_event_var(hist_data, NULL, NULL, onmax_var_str);
+ var_field = find_target_event_var(hist_data, NULL, NULL, track_data_var_str);
if (!var_field) {
- hist_err("onmax: Couldn't find onmax variable: ", onmax_var_str);
+ hist_err("Couldn't find onmax variable: ", track_data_var_str);
return -EINVAL;
}
@@ -3555,17 +3622,16 @@ static int onmax_create(struct hist_trig
if (!ref_field)
return -ENOMEM;
- data->onmax.var = ref_field;
-
- data->onmax.max_var_ref_idx = var_ref_idx;
+ data->track_data.var_ref = ref_field;
- max_var = create_var(hist_data, file, "max", sizeof(u64), "u64");
- if (IS_ERR(max_var)) {
- hist_err("onmax: Couldn't create onmax variable: ", "max");
- ret = PTR_ERR(max_var);
+ if (data->handler == HANDLER_ONMAX)
+ track_var = create_var(hist_data, file, "__max", sizeof(u64), "u64");
+ if (IS_ERR(track_var)) {
+ hist_err("Couldn't create onmax variable: ", "__max");
+ ret = PTR_ERR(track_var);
goto out;
}
- data->onmax.max_var = max_var;
+ data->track_data.track_var = track_var;
ret = action_create(hist_data, data);
out:
@@ -3643,8 +3709,15 @@ static int action_parse(char *str, struc
goto out;
if (handler == HANDLER_ONMAX)
- data->fn = onmax_save;
+ data->track_data.check_val = check_track_val_max;
+ else {
+ hist_err("action parsing: Handler doesn't support action: ", action_name);
+ ret = -EINVAL;
+ goto out;
+ }
+ data->track_data.save_data = save_track_data_vars;
+ data->fn = ontrack_action;
data->action = ACTION_SAVE;
} else {
char *params = strsep(&str, ")");
@@ -3655,7 +3728,15 @@ static int action_parse(char *str, struc
goto out;
}
- data->fn = action_trace;
+ if (handler == HANDLER_ONMAX)
+ data->track_data.check_val = check_track_val_max;
+
+ if (handler != HANDLER_ONMATCH) {
+ data->track_data.save_data = action_trace;
+ data->fn = ontrack_action;
+ } else
+ data->fn = action_trace;
+
data->action = ACTION_TRACE;
}
@@ -3670,24 +3751,25 @@ static int action_parse(char *str, struc
return ret;
}
-static struct action_data *onmax_parse(char *str, enum handler_id handler)
+static struct action_data *track_data_parse(struct hist_trigger_data *hist_data,
+ char *str, enum handler_id handler)
{
struct action_data *data;
- char *onmax_var_str;
int ret = -EINVAL;
+ char *var_str;
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return ERR_PTR(-ENOMEM);
- onmax_var_str = strsep(&str, ")");
- if (!onmax_var_str || !str) {
+ var_str = strsep(&str, ")");
+ if (!var_str || !str) {
ret = -EINVAL;
goto free;
}
- data->onmax.var_str = kstrdup(onmax_var_str, GFP_KERNEL);
- if (!data->onmax.var_str) {
+ data->track_data.var_str = kstrdup(var_str, GFP_KERNEL);
+ if (!data->track_data.var_str) {
ret = -ENOMEM;
goto free;
}
@@ -3698,7 +3780,7 @@ static struct action_data *onmax_parse(c
out:
return data;
free:
- onmax_destroy(data);
+ track_data_destroy(hist_data, data);
data = ERR_PTR(ret);
goto out;
}
@@ -4465,7 +4547,7 @@ static void destroy_actions(struct hist_
if (data->handler == HANDLER_ONMATCH)
onmatch_destroy(data);
else if (data->handler == HANDLER_ONMAX)
- onmax_destroy(data);
+ track_data_destroy(hist_data, data);
else
kfree(data);
}
@@ -4494,7 +4576,8 @@ static int parse_actions(struct hist_tri
} else if ((len = str_has_prefix(str, "onmax("))) {
char *action_str = str + len;
- data = onmax_parse(action_str, HANDLER_ONMAX);
+ data = track_data_parse(hist_data, action_str,
+ HANDLER_ONMAX);
if (IS_ERR(data)) {
ret = PTR_ERR(data);
break;
@@ -4524,7 +4607,7 @@ static int create_actions(struct hist_tr
if (ret)
break;
} else if (data->handler == HANDLER_ONMAX) {
- ret = onmax_create(hist_data, data);
+ ret = track_data_create(hist_data, data);
if (ret)
break;
} else {
@@ -4546,7 +4629,7 @@ static void print_actions(struct seq_fil
struct action_data *data = hist_data->actions[i];
if (data->handler == HANDLER_ONMAX)
- onmax_print(m, hist_data, elt, data);
+ track_data_print(m, hist_data, elt, data);
}
}
@@ -4571,12 +4654,13 @@ static void print_action_spec(struct seq
}
}
-static void print_onmax_spec(struct seq_file *m,
- struct hist_trigger_data *hist_data,
- struct action_data *data)
-{
- seq_puts(m, ":onmax(");
- seq_printf(m, "%s", data->onmax.var_str);
+static void print_track_data_spec(struct seq_file *m,
+ struct hist_trigger_data *hist_data,
+ struct action_data *data)
+{
+ if (data->handler == HANDLER_ONMAX)
+ seq_puts(m, ":onmax(");
+ seq_printf(m, "%s", data->track_data.var_str);
seq_printf(m, ").%s(", data->action_name);
print_action_spec(m, hist_data, data);
@@ -4634,8 +4718,8 @@ static bool actions_match(struct hist_tr
data_test->match_data.event) != 0)
return false;
} else if (data->handler == HANDLER_ONMAX) {
- if (strcmp(data->onmax.var_str,
- data_test->onmax.var_str) != 0)
+ if (strcmp(data->track_data.var_str,
+ data_test->track_data.var_str) != 0)
return false;
}
}
@@ -4655,7 +4739,7 @@ static void print_actions_spec(struct se
if (data->handler == HANDLER_ONMATCH)
print_onmatch_spec(m, hist_data, data);
else if (data->handler == HANDLER_ONMAX)
- print_onmax_spec(m, hist_data, data);
+ print_track_data_spec(m, hist_data, data);
}
}
Patches currently in stable-queue which might be from kroah.com@xxxxxxxxxxxxxxx are
queue-4.19/tracing-consolidate-trace_add-remove_event_call-back-to-the-nolock-functions.patch
queue-4.19/tracing-split-up-onmatch-action-data.patch
queue-4.19/tracing-remove-unnecessary-var_ref-destroy-in-track_data_destroy.patch
queue-4.19/tracing-use-str_has_prefix-helper-for-histogram-code.patch
queue-4.19/tracing-simplify-creation-and-deletion-of-synthetic-events.patch
queue-4.19/tracing-refactor-hist-trigger-action-code.patch
queue-4.19/string.h-add-str_has_prefix-helper-function.patch
queue-4.19/tracing-remove-unneeded-synth_event_mutex.patch
queue-4.19/tracing-add-unified-dynamic-event-framework.patch
queue-4.19/tracing-use-dyn_event-framework-for-synthetic-events.patch
queue-4.19/tracing-use-str_has_prefix-instead-of-using-fixed-sizes.patch
queue-4.19/tracing-generalize-hist-trigger-onmax-and-save-action.patch
queue-4.19/tracing-have-the-historgram-use-the-result-of-str_has_prefix-for-len-of-prefix.patch
