[PATCH 2/2] tracing: Have existing event_command implementations use helpers

From: Tom Zanussi
Date: Fri Nov 05 2021 - 17:20:29 EST

Next message: Kuogee Hsieh: "[PATCH v2] drm/msm/dp: employ bridge mechanism for display enable and disable"
Previous message: Tom Zanussi: "[PATCH 1/2] tracing: Add helper functions to simplify event_command callback handling"
In reply to: Tom Zanussi: "[PATCH 1/2] tracing: Add helper functions to simplify event_command callback handling"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Simplify the existing event_command implementations by making use of
the helper functions previously introduced.

Signed-off-by: Tom Zanussi <zanussi@xxxxxxxxxx>
---
kernel/trace/trace_events_hist.c | 52 +++------
kernel/trace/trace_events_trigger.c | 158 ++++++----------------------
2 files changed, 51 insertions(+), 159 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 8ff572a31fd3..c99b1d075d9d 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6128,10 +6128,11 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
struct hist_trigger_attrs *attrs;
struct event_trigger_ops *trigger_ops;
struct hist_trigger_data *hist_data;
+ char *trigger, *p, *start;
struct synth_event *se;
const char *se_name;
- bool remove = false;
- char *trigger, *p, *start;
+ int n_registered;
+ bool remove;
int ret = 0;

lockdep_assert_held(&event_mutex);
@@ -6141,11 +6142,9 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
last_cmd_set(file, param);
}

- if (!param)
- return -EINVAL;
-
- if (glob[0] == '!')
- remove = true;
+ ret = event_trigger_check(glob, &trigger, &param, &remove, true, false);
+ if (ret)
+ return ret;

/*
* separate the trigger from the filter (k:v [if filter])
@@ -6202,29 +6201,15 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
return PTR_ERR(hist_data);
}

- trigger_ops = cmd_ops->get_trigger_ops(cmd, trigger);
-
- trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
+ trigger_data = event_trigger_alloc(cmd_ops, trigger, cmd, hist_data);
if (!trigger_data) {
ret = -ENOMEM;
goto out_free;
}

- trigger_data->count = -1;
- trigger_data->ops = trigger_ops;
- trigger_data->cmd_ops = cmd_ops;
-
- INIT_LIST_HEAD(&trigger_data->list);
- RCU_INIT_POINTER(trigger_data->filter, NULL);
-
- trigger_data->private_data = hist_data;
-
- /* if param is non-empty, it's supposed to be a filter */
- if (param && cmd_ops->set_filter) {
- ret = cmd_ops->set_filter(param, trigger_data, file);
- if (ret < 0)
- goto out_free;
- }
+ ret = event_trigger_set_filter(cmd_ops, file, param, trigger_data);
+ if (ret < 0)
+ goto out_free;

if (remove) {
if (!have_hist_trigger_match(trigger_data, file))
@@ -6244,18 +6229,14 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
goto out_free;
}

- ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
- /*
- * The above returns on success the # of triggers registered,
- * but if it didn't register any it returns zero. Consider no
- * triggers registered a failure too.
- */
- if (!ret) {
+ ret = event_trigger_register(cmd_ops, file, glob, cmd, trigger, trigger_data, &n_registered);
+ if (ret)
+ goto out_free;
+ if ((ret == 0) && (n_registered == 0)) {
if (!(attrs->pause || attrs->cont || attrs->clear))
ret = -ENOENT;
goto out_free;
- } else if (ret < 0)
- goto out_free;
+ }

if (get_named_trigger_data(trigger_data))
goto enable;
@@ -6289,8 +6270,7 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
out_unreg:
cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file);
out_free:
- if (cmd_ops->set_filter)
- cmd_ops->set_filter(NULL, trigger_data, NULL);
+ event_trigger_reset_filter(cmd_ops, trigger_data);

remove_hist_vars(hist_data);

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index e3977b0a3e74..7920a0b1ded5 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -799,89 +799,45 @@ event_trigger_callback(struct event_command *cmd_ops,
struct event_trigger_data *trigger_data;
struct event_trigger_ops *trigger_ops;
char *trigger = NULL;
- char *number;
+ bool remove;
int ret;

- /* separate the trigger from the filter (t:n [if filter]) */
- if (param && isdigit(param[0])) {
- trigger = strsep(&param, " \t");
- if (param) {
- param = skip_spaces(param);
- if (!*param)
- param = NULL;
- }
- }
-
- trigger_ops = cmd_ops->get_trigger_ops(cmd, trigger);
+ ret = event_trigger_check(glob, &trigger, &param, &remove, false, true);
+ if (ret)
+ return ret;

ret = -ENOMEM;
- trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
+ trigger_data = event_trigger_alloc(cmd_ops, trigger, cmd, file);
if (!trigger_data)
goto out;

- trigger_data->count = -1;
- trigger_data->ops = trigger_ops;
- trigger_data->cmd_ops = cmd_ops;
- trigger_data->private_data = file;
- INIT_LIST_HEAD(&trigger_data->list);
- INIT_LIST_HEAD(&trigger_data->named_list);
-
- if (glob[0] == '!') {
+ if (remove) {
cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file);
- kfree(trigger_data);
ret = 0;
goto out;
}

- if (trigger) {
- number = strsep(&trigger, ":");
-
- ret = -EINVAL;
- if (!strlen(number))
- goto out_free;
-
- /*
- * We use the callback data field (which is a pointer)
- * as our counter.
- */
- ret = kstrtoul(number, 0, &trigger_data->count);
- if (ret)
- goto out_free;
- }
-
- if (!param) /* if param is non-empty, it's supposed to be a filter */
- goto out_reg;
-
- if (!cmd_ops->set_filter)
- goto out_reg;
+ ret = event_trigger_parse_num(trigger, trigger_data);
+ if (ret)
+ goto out_free;

- ret = cmd_ops->set_filter(param, trigger_data, file);
+ ret = event_trigger_set_filter(cmd_ops, file, param, trigger_data);
if (ret < 0)
goto out_free;

- out_reg:
/* Up the trigger_data count to make sure reg doesn't free it on failure */
event_trigger_init(trigger_ops, trigger_data);
- ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
- /*
- * The above returns on success the # of functions enabled,
- * but if it didn't find any functions it returns zero.
- * Consider no functions a failure too.
- */
- if (!ret) {
- cmd_ops->unreg(glob, trigger_ops, trigger_data, file);
- ret = -ENOENT;
- } else if (ret > 0)
- ret = 0;
+
+ ret = event_trigger_register(cmd_ops, file, glob, cmd, trigger, trigger_data, NULL);
+ if (ret)
+ goto out_free;

/* Down the counter of trigger_data or free it if not used anymore */
event_trigger_free(trigger_ops, trigger_data);
out:
return ret;
-
out_free:
- if (cmd_ops->set_filter)
- cmd_ops->set_filter(NULL, trigger_data, NULL);
+ event_trigger_reset_filter(cmd_ops, trigger_data);
kfree(trigger_data);
goto out;
}
@@ -1543,26 +1499,16 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
struct event_trigger_data *trigger_data;
struct event_trigger_ops *trigger_ops;
struct trace_array *tr = file->tr;
+ bool enable, remove;
const char *system;
const char *event;
bool hist = false;
char *trigger;
- char *number;
- bool enable;
int ret;

- if (!param)
- return -EINVAL;
-
- /* separate the trigger from the filter (s:e:n [if filter]) */
- trigger = strsep(&param, " \t");
- if (!trigger)
- return -EINVAL;
- if (param) {
- param = skip_spaces(param);
- if (!*param)
- param = NULL;
- }
+ ret = event_trigger_check(glob, &trigger, &param, &remove, true, true);
+ if (ret)
+ return ret;

system = strsep(&trigger, ":");
if (!trigger)
@@ -1587,28 +1533,23 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
trigger_ops = cmd_ops->get_trigger_ops(cmd, trigger);

ret = -ENOMEM;
- trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
- if (!trigger_data)
- goto out;
-
enable_data = kzalloc(sizeof(*enable_data), GFP_KERNEL);
if (!enable_data) {
kfree(trigger_data);
goto out;
}

- trigger_data->count = -1;
- trigger_data->ops = trigger_ops;
- trigger_data->cmd_ops = cmd_ops;
- INIT_LIST_HEAD(&trigger_data->list);
- RCU_INIT_POINTER(trigger_data->filter, NULL);
-
enable_data->hist = hist;
enable_data->enable = enable;
enable_data->file = event_enable_file;
- trigger_data->private_data = enable_data;

- if (glob[0] == '!') {
+ trigger_data = event_trigger_alloc(cmd_ops, trigger, cmd, enable_data);
+ if (!trigger_data) {
+ kfree(enable_data);
+ goto out;
+ }
+
+ if (remove) {
cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file);
kfree(trigger_data);
kfree(enable_data);
@@ -1619,33 +1560,14 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
/* Up the trigger_data count to make sure nothing frees it on failure */
event_trigger_init(trigger_ops, trigger_data);

- if (trigger) {
- number = strsep(&trigger, ":");
-
- ret = -EINVAL;
- if (!strlen(number))
- goto out_free;
-
- /*
- * We use the callback data field (which is a pointer)
- * as our counter.
- */
- ret = kstrtoul(number, 0, &trigger_data->count);
- if (ret)
- goto out_free;
- }
-
- if (!param) /* if param is non-empty, it's supposed to be a filter */
- goto out_reg;
-
- if (!cmd_ops->set_filter)
- goto out_reg;
+ ret = event_trigger_parse_num(trigger, trigger_data);
+ if (ret)
+ goto out_free;

- ret = cmd_ops->set_filter(param, trigger_data, file);
+ ret = event_trigger_set_filter(cmd_ops, file, param, trigger_data);
if (ret < 0)
goto out_free;

- out_reg:
/* Don't let event modules unload while probe registered */
ret = trace_event_try_get_ref(event_enable_file->event_call);
if (!ret) {
@@ -1656,30 +1578,20 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
ret = trace_event_enable_disable(event_enable_file, 1, 1);
if (ret < 0)
goto out_put;
- ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
- /*
- * The above returns on success the # of functions enabled,
- * but if it didn't find any functions it returns zero.
- * Consider no functions a failure too.
- */
- if (!ret) {
- ret = -ENOENT;
- goto out_disable;
- } else if (ret < 0)
+
+ ret = event_trigger_register(cmd_ops, file, glob, cmd, trigger, trigger_data, NULL);
+ if (ret)
goto out_disable;
- /* Just return zero, not the number of enabled functions */
- ret = 0;
+
event_trigger_free(trigger_ops, trigger_data);
out:
return ret;
-
out_disable:
trace_event_enable_disable(event_enable_file, 0, 1);
out_put:
trace_event_put_ref(event_enable_file->event_call);
out_free:
- if (cmd_ops->set_filter)
- cmd_ops->set_filter(NULL, trigger_data, NULL);
+ event_trigger_reset_filter(cmd_ops, trigger_data);
event_trigger_free(trigger_ops, trigger_data);
kfree(enable_data);
goto out;
--
2.17.1

Next message: Kuogee Hsieh: "[PATCH v2] drm/msm/dp: employ bridge mechanism for display enable and disable"
Previous message: Tom Zanussi: "[PATCH 1/2] tracing: Add helper functions to simplify event_command callback handling"
In reply to: Tom Zanussi: "[PATCH 1/2] tracing: Add helper functions to simplify event_command callback handling"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]