tracing: Remove logic for registering multiple event triggers at a time

Code for registering triggers assumes it's possible to register more
than one trigger at a time.  In fact, it's unimplemented and there
doesn't seem to be a reason to do that.

Remove the n_registered param from event_trigger_register() and fix up
callers.

Doing so simplifies the logic in event_trigger_register to the point
that it just becomes a wrapper calling event_command.reg().

It also removes the problematic call to event_command.unreg() in case
of failure.  A new function, event_trigger_unregister() is also added
for callers to call themselves.

The changes to trace_events_hist.c simply allow compilation; a
separate patch follows which updates the hist triggers to work
correctly with the new changes.

Link: https://lkml.kernel.org/r/6149fec7a139d93e84fa4535672fb5bef88006b0.1644010575.git.zanussi@kernel.org
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 07d990270e2aa1c7a01f65b55795ca5ece08f2e6..2bc8036b06593f6642905c633176d4df63aa6afc 100644 (file)
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1629,10 +1629,11 @@ extern void event_trigger_reset_filter(struct event_command *cmd_ops,
 extern int event_trigger_register(struct event_command *cmd_ops,
                                  struct trace_event_file *file,
                                  char *glob,
-                                 char *cmd,
-                                 char *trigger,
-                                 struct event_trigger_data *trigger_data,
-                                 int *n_registered);
+                                 struct event_trigger_data *trigger_data);
+extern void event_trigger_unregister(struct event_command *cmd_ops,
+                                    struct trace_event_file *file,
+                                    char *glob,
+                                    struct event_trigger_data *trigger_data);
 
 /**
  * struct event_trigger_ops - callbacks for trace event triggers

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 44db5ba9cabb885a8538b882c28f04aaa1f40fa3..b18d00905eae47cc460fa0771f0d616c2c751d6d 100644 (file)
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6287,7 +6287,7 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
                        goto out_free;
                }
 
-               cmd_ops->unreg(glob+1, trigger_data, file);
+               event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
                se_name = trace_event_name(file->event_call);
                se = find_synth_event(se_name);
                if (se)
@@ -6296,13 +6296,10 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
                goto out_free;
        }
 
-       ret = cmd_ops->reg(glob, 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, trigger_data);
+       if (ret)
+               goto out_free;
+       if (ret == 0) {
                if (!(attrs->pause || attrs->cont || attrs->clear))
                        ret = -ENOENT;
                goto out_free;
@@ -6331,15 +6328,13 @@ enable:
        se = find_synth_event(se_name);
        if (se)
                se->ref++;
-       /* Just return zero, not the number of registered triggers */
-       ret = 0;
  out:
        if (ret == 0)
                hist_err_clear();
 
        return ret;
  out_unreg:
-       cmd_ops->unreg(glob+1, trigger_data, file);
+       event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
  out_free:
        if (cmd_ops->set_filter)
                cmd_ops->set_filter(NULL, trigger_data, NULL);

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 7eb9d04f1c2ea27c0396901eaea9d434b9f55535..ea13679054a85d543a9244f812cf8808f91b6e8a 100644 (file)
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -587,13 +587,12 @@ static int register_trigger(char *glob,
        }
 
        list_add_rcu(&data->list, &file->triggers);
-       ret++;
 
        update_cond_flag(file);
-       if (trace_event_trigger_enable_disable(file, 1) < 0) {
+       ret = trace_event_trigger_enable_disable(file, 1);
+       if (ret < 0) {
                list_del_rcu(&data->list);
                update_cond_flag(file);
-               ret--;
        }
 out:
        return ret;
@@ -927,48 +926,37 @@ void event_trigger_reset_filter(struct event_command *cmd_ops,
  * @cmd_ops: The event_command operations for the trigger
  * @file: The event file for the trigger's event
  * @glob: The trigger command string, with optional remove(!) operator
- * @cmd: The cmd string
- * @param: The param string
  * @trigger_data: The trigger_data for the trigger
- * @n_registered: optional outparam, the number of triggers registered
  *
  * Register an event trigger.  The @cmd_ops are used to call the
- * cmd_ops->reg() function which actually does the registration. The
- * cmd_ops->reg() function returns the number of triggers registered,
- * which is assigned to n_registered, if n_registered is non-NULL.
+ * cmd_ops->reg() function which actually does the registration.
  *
  * Return: 0 on success, errno otherwise
  */
 int event_trigger_register(struct event_command *cmd_ops,
                           struct trace_event_file *file,
                           char *glob,
-                          char *cmd,
-                          char *param,
-                          struct event_trigger_data *trigger_data,
-                          int *n_registered)
+                          struct event_trigger_data *trigger_data)
 {
-       int ret;
-
-       if (n_registered)
-               *n_registered = 0;
-
-       ret = cmd_ops->reg(glob, 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_data, file);
-               ret = -ENOENT;
-       } else if (ret > 0) {
-               if (n_registered)
-                       *n_registered = ret;
-               /* Just return zero, not the number of enabled functions */
-               ret = 0;
-       }
+       return cmd_ops->reg(glob, trigger_data, file);
+}
 
-       return ret;
+/**
+ * event_trigger_unregister - unregister an event trigger
+ * @cmd_ops: The event_command operations for the trigger
+ * @file: The event file for the trigger's event
+ * @glob: The trigger command string, with optional remove(!) operator
+ * @trigger_data: The trigger_data for the trigger
+ *
+ * Unregister an event trigger.  The @cmd_ops are used to call the
+ * cmd_ops->unreg() function which actually does the unregistration.
+ */
+void event_trigger_unregister(struct event_command *cmd_ops,
+                             struct trace_event_file *file,
+                             char *glob,
+                             struct event_trigger_data *trigger_data)
+{
+       cmd_ops->unreg(glob, trigger_data, file);
 }
 
 /*
@@ -1027,7 +1015,7 @@ event_trigger_parse(struct event_command *cmd_ops,
        INIT_LIST_HEAD(&trigger_data->named_list);
 
        if (glob[0] == '!') {
-               cmd_ops->unreg(glob+1, trigger_data, file);
+               event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
                kfree(trigger_data);
                ret = 0;
                goto out;
@@ -1062,17 +1050,10 @@ event_trigger_parse(struct event_command *cmd_ops,
  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_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_data, file);
-               ret = -ENOENT;
-       } else if (ret > 0)
-               ret = 0;
+
+       ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
+       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);
@@ -1854,7 +1835,7 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
        trigger_data->private_data = enable_data;
 
        if (glob[0] == '!') {
-               cmd_ops->unreg(glob+1, trigger_data, file);
+               event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
                kfree(trigger_data);
                kfree(enable_data);
                ret = 0;
@@ -1901,19 +1882,11 @@ int event_enable_trigger_parse(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_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, trigger_data);
+       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;
@@ -1959,13 +1932,12 @@ int event_enable_register_trigger(char *glob,
        }
 
        list_add_rcu(&data->list, &file->triggers);
-       ret++;
 
        update_cond_flag(file);
-       if (trace_event_trigger_enable_disable(file, 1) < 0) {
+       ret = trace_event_trigger_enable_disable(file, 1);
+       if (ret < 0) {
                list_del_rcu(&data->list);
                update_cond_flag(file);
-               ret--;
        }
 out:
        return ret;