tracing: Remove unneeded synth_event_mutex

Rmove unneeded synth_event_mutex. This mutex protects the reference
count in synth_event, however, those operational points are already
protected by event_mutex.

1. In __create_synth_event() and create_or_delete_synth_event(),
 those synth_event_mutex clearly obtained right after event_mutex.

2. event_hist_trigger_func() is trigger_hist_cmd.func() which is
 called by trigger_process_regex(), which is a part of
 event_trigger_regex_write() and this function takes event_mutex.

3. hist_unreg_all() is trigger_hist_cmd.unreg_all() which is called
 by event_trigger_regex_open() and it takes event_mutex.

4. onmatch_destroy() and onmatch_create() have long call tree,
 but both are finally invoked from event_trigger_regex_write()
 and event_trace_del_tracer(), former takes event_mutex, and latter
 ensures called under event_mutex locked.

Finally, I ensured there is no resource conflict. For safety,
I added lockdep_assert_held(&event_mutex) for each function.

Link: http://lkml.kernel.org/r/154140864134.17322.4796059721306031894.stgit@devbox

Reviewed-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
This commit is contained in:
Masami Hiramatsu 2018-11-05 18:04:01 +09:00 committed by Steven Rostedt (VMware)
parent 7bbab38d07
commit 0e2b81f7b5
1 changed files with 7 additions and 23 deletions

View File

@ -444,8 +444,6 @@ static bool have_hist_err(void)
return false; return false;
} }
static DEFINE_MUTEX(synth_event_mutex);
struct synth_trace_event { struct synth_trace_event {
struct trace_entry ent; struct trace_entry ent;
u64 fields[]; u64 fields[];
@ -1077,7 +1075,6 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
return -EINVAL; return -EINVAL;
mutex_lock(&event_mutex); mutex_lock(&event_mutex);
mutex_lock(&synth_event_mutex);
event = find_synth_event(name); event = find_synth_event(name);
if (event) { if (event) {
@ -1119,7 +1116,6 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
else else
free_synth_event(event); free_synth_event(event);
out: out:
mutex_unlock(&synth_event_mutex);
mutex_unlock(&event_mutex); mutex_unlock(&event_mutex);
return ret; return ret;
@ -1139,7 +1135,6 @@ static int create_or_delete_synth_event(int argc, char **argv)
/* trace_run_command() ensures argc != 0 */ /* trace_run_command() ensures argc != 0 */
if (name[0] == '!') { if (name[0] == '!') {
mutex_lock(&event_mutex); mutex_lock(&event_mutex);
mutex_lock(&synth_event_mutex);
event = find_synth_event(name + 1); event = find_synth_event(name + 1);
if (event) { if (event) {
if (event->ref) if (event->ref)
@ -1153,7 +1148,6 @@ static int create_or_delete_synth_event(int argc, char **argv)
} }
} else } else
ret = -ENOENT; ret = -ENOENT;
mutex_unlock(&synth_event_mutex);
mutex_unlock(&event_mutex); mutex_unlock(&event_mutex);
return ret; return ret;
} }
@ -3535,7 +3529,7 @@ static void onmatch_destroy(struct action_data *data)
{ {
unsigned int i; unsigned int i;
mutex_lock(&synth_event_mutex); lockdep_assert_held(&event_mutex);
kfree(data->onmatch.match_event); kfree(data->onmatch.match_event);
kfree(data->onmatch.match_event_system); kfree(data->onmatch.match_event_system);
@ -3548,8 +3542,6 @@ static void onmatch_destroy(struct action_data *data)
data->onmatch.synth_event->ref--; data->onmatch.synth_event->ref--;
kfree(data); kfree(data);
mutex_unlock(&synth_event_mutex);
} }
static void destroy_field_var(struct field_var *field_var) static void destroy_field_var(struct field_var *field_var)
@ -3700,15 +3692,14 @@ static int onmatch_create(struct hist_trigger_data *hist_data,
struct synth_event *event; struct synth_event *event;
int ret = 0; int ret = 0;
mutex_lock(&synth_event_mutex); lockdep_assert_held(&event_mutex);
event = find_synth_event(data->onmatch.synth_event_name); event = find_synth_event(data->onmatch.synth_event_name);
if (!event) { if (!event) {
hist_err("onmatch: Couldn't find synthetic event: ", data->onmatch.synth_event_name); hist_err("onmatch: Couldn't find synthetic event: ", data->onmatch.synth_event_name);
mutex_unlock(&synth_event_mutex);
return -EINVAL; return -EINVAL;
} }
event->ref++; event->ref++;
mutex_unlock(&synth_event_mutex);
var_ref_idx = hist_data->n_var_refs; var_ref_idx = hist_data->n_var_refs;
@ -3782,9 +3773,7 @@ static int onmatch_create(struct hist_trigger_data *hist_data,
out: out:
return ret; return ret;
err: err:
mutex_lock(&synth_event_mutex);
event->ref--; event->ref--;
mutex_unlock(&synth_event_mutex);
goto out; goto out;
} }
@ -5492,6 +5481,8 @@ static void hist_unreg_all(struct trace_event_file *file)
struct synth_event *se; struct synth_event *se;
const char *se_name; const char *se_name;
lockdep_assert_held(&event_mutex);
if (hist_file_check_refs(file)) if (hist_file_check_refs(file))
return; return;
@ -5501,12 +5492,10 @@ static void hist_unreg_all(struct trace_event_file *file)
list_del_rcu(&test->list); list_del_rcu(&test->list);
trace_event_trigger_enable_disable(file, 0); trace_event_trigger_enable_disable(file, 0);
mutex_lock(&synth_event_mutex);
se_name = trace_event_name(file->event_call); se_name = trace_event_name(file->event_call);
se = find_synth_event(se_name); se = find_synth_event(se_name);
if (se) if (se)
se->ref--; se->ref--;
mutex_unlock(&synth_event_mutex);
update_cond_flag(file); update_cond_flag(file);
if (hist_data->enable_timestamps) if (hist_data->enable_timestamps)
@ -5532,6 +5521,8 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
char *trigger, *p; char *trigger, *p;
int ret = 0; int ret = 0;
lockdep_assert_held(&event_mutex);
if (glob && strlen(glob)) { if (glob && strlen(glob)) {
last_cmd_set(param); last_cmd_set(param);
hist_err_clear(); hist_err_clear();
@ -5622,14 +5613,10 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
} }
cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file); cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file);
mutex_lock(&synth_event_mutex);
se_name = trace_event_name(file->event_call); se_name = trace_event_name(file->event_call);
se = find_synth_event(se_name); se = find_synth_event(se_name);
if (se) if (se)
se->ref--; se->ref--;
mutex_unlock(&synth_event_mutex);
ret = 0; ret = 0;
goto out_free; goto out_free;
} }
@ -5665,13 +5652,10 @@ enable:
if (ret) if (ret)
goto out_unreg; goto out_unreg;
mutex_lock(&synth_event_mutex);
se_name = trace_event_name(file->event_call); se_name = trace_event_name(file->event_call);
se = find_synth_event(se_name); se = find_synth_event(se_name);
if (se) if (se)
se->ref++; se->ref++;
mutex_unlock(&synth_event_mutex);
/* Just return zero, not the number of registered triggers */ /* Just return zero, not the number of registered triggers */
ret = 0; ret = 0;
out: out: