From: Masami Hiramatsu <mhiramat@kernel.org> To: Steven Rostedt <rostedt@goodmis.org> Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>, Tom Zanussi <tom.zanussi@linux.intel.com>, Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> Subject: [PATCH v2 09/12] tracing: Remove unneeded synth_event_mutex Date: Mon, 5 Nov 2018 18:04:01 +0900 Message-ID: <154140864134.17322.4796059721306031894.stgit@devbox> (raw) In-Reply-To: <154140838606.17322.15294184388075458777.stgit@devbox> 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. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- kernel/trace/trace_events_hist.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 414aabd67d1f..21e4954375a1 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -444,8 +444,6 @@ static bool have_hist_err(void) return false; } -static DEFINE_MUTEX(synth_event_mutex); - struct synth_trace_event { struct trace_entry ent; u64 fields[]; @@ -1077,7 +1075,6 @@ static int __create_synth_event(int argc, const char *name, const char **argv) return -EINVAL; mutex_lock(&event_mutex); - mutex_lock(&synth_event_mutex); event = find_synth_event(name); if (event) { @@ -1119,7 +1116,6 @@ static int __create_synth_event(int argc, const char *name, const char **argv) else free_synth_event(event); out: - mutex_unlock(&synth_event_mutex); mutex_unlock(&event_mutex); return ret; @@ -1139,7 +1135,6 @@ static int create_or_delete_synth_event(int argc, char **argv) /* trace_run_command() ensures argc != 0 */ if (name[0] == '!') { mutex_lock(&event_mutex); - mutex_lock(&synth_event_mutex); event = find_synth_event(name + 1); if (event) { if (event->ref) @@ -1153,7 +1148,6 @@ static int create_or_delete_synth_event(int argc, char **argv) } } else ret = -ENOENT; - mutex_unlock(&synth_event_mutex); mutex_unlock(&event_mutex); return ret; } @@ -3535,7 +3529,7 @@ static void onmatch_destroy(struct action_data *data) { unsigned int i; - mutex_lock(&synth_event_mutex); + lockdep_assert_held(&event_mutex); kfree(data->onmatch.match_event); kfree(data->onmatch.match_event_system); @@ -3548,8 +3542,6 @@ static void onmatch_destroy(struct action_data *data) data->onmatch.synth_event->ref--; kfree(data); - - mutex_unlock(&synth_event_mutex); } 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; int ret = 0; - mutex_lock(&synth_event_mutex); + lockdep_assert_held(&event_mutex); + event = find_synth_event(data->onmatch.synth_event_name); if (!event) { hist_err("onmatch: Couldn't find synthetic event: ", data->onmatch.synth_event_name); - mutex_unlock(&synth_event_mutex); return -EINVAL; } event->ref++; - mutex_unlock(&synth_event_mutex); var_ref_idx = hist_data->n_var_refs; @@ -3782,9 +3773,7 @@ static int onmatch_create(struct hist_trigger_data *hist_data, out: return ret; err: - mutex_lock(&synth_event_mutex); event->ref--; - mutex_unlock(&synth_event_mutex); goto out; } @@ -5492,6 +5481,8 @@ static void hist_unreg_all(struct trace_event_file *file) struct synth_event *se; const char *se_name; + lockdep_assert_held(&event_mutex); + if (hist_file_check_refs(file)) return; @@ -5501,12 +5492,10 @@ static void hist_unreg_all(struct trace_event_file *file) list_del_rcu(&test->list); trace_event_trigger_enable_disable(file, 0); - mutex_lock(&synth_event_mutex); se_name = trace_event_name(file->event_call); se = find_synth_event(se_name); if (se) se->ref--; - mutex_unlock(&synth_event_mutex); update_cond_flag(file); if (hist_data->enable_timestamps) @@ -5532,6 +5521,8 @@ static int event_hist_trigger_func(struct event_command *cmd_ops, char *trigger, *p; int ret = 0; + lockdep_assert_held(&event_mutex); + if (glob && strlen(glob)) { last_cmd_set(param); 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); - - mutex_lock(&synth_event_mutex); se_name = trace_event_name(file->event_call); se = find_synth_event(se_name); if (se) se->ref--; - mutex_unlock(&synth_event_mutex); - ret = 0; goto out_free; } @@ -5665,13 +5652,10 @@ static int event_hist_trigger_func(struct event_command *cmd_ops, if (ret) goto out_unreg; - mutex_lock(&synth_event_mutex); se_name = trace_event_name(file->event_call); se = find_synth_event(se_name); if (se) se->ref++; - mutex_unlock(&synth_event_mutex); - /* Just return zero, not the number of registered triggers */ ret = 0; out:
next prev parent reply index Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-05 8:59 [PATCH v2 00/12] tracing: Unifying dynamic event interface Masami Hiramatsu 2018-11-05 9:00 ` [PATCH v2 01/12] tracing/uprobes: Add busy check when cleanup all uprobes Masami Hiramatsu 2018-12-04 17:43 ` Steven Rostedt 2018-12-07 2:19 ` Masami Hiramatsu 2018-11-05 9:00 ` [PATCH v2 02/12] tracing: Lock event_mutex before synth_event_mutex Masami Hiramatsu 2018-11-05 9:01 ` [PATCH v2 03/12] tracing: Simplify creation and deletion of synthetic event Masami Hiramatsu 2018-11-05 9:01 ` [PATCH v2 04/12] tracing: Integrate similar probe argument parsers Masami Hiramatsu 2018-11-05 9:02 ` [PATCH v2 05/12] tracing: Add unified dynamic event framework Masami Hiramatsu 2018-11-05 9:02 ` [PATCH v2 06/12] tracing/kprobes: Use dyn_event framework for kprobe events Masami Hiramatsu 2018-11-05 9:03 ` [PATCH v2 07/12] tracing/uprobes: Use dyn_event framework for uprobe events Masami Hiramatsu 2018-11-05 9:03 ` [PATCH v2 08/12] tracing: Use dyn_event framework for synthetic events Masami Hiramatsu 2018-11-05 9:04 ` Masami Hiramatsu [this message] 2018-11-05 9:04 ` [PATCH v2 10/12] tracing: Remove orphaned trace_add/remove_event_call functions Masami Hiramatsu 2018-12-04 18:51 ` Steven Rostedt 2018-12-07 2:22 ` Masami Hiramatsu 2018-11-05 9:04 ` [PATCH v2 11/12] tracing: Add generic event-name based remove event method Masami Hiramatsu 2018-11-05 9:05 ` [PATCH v2 12/12] selftests/ftrace: Add testcases for dynamic event Masami Hiramatsu 2018-11-28 7:31 ` [PATCH v2 00/12] tracing: Unifying dynamic event interface Masami Hiramatsu 2018-11-28 23:42 ` Tom Zanussi 2018-11-29 3:46 ` Steven Rostedt 2018-11-29 5:20 ` Masami Hiramatsu 2018-11-29 15:08 ` Tom Zanussi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=154140864134.17322.4796059721306031894.stgit@devbox \ --to=mhiramat@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mingo@kernel.org \ --cc=ravi.bangoria@linux.vnet.ibm.com \ --cc=rostedt@goodmis.org \ --cc=tom.zanussi@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
LKML Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \ linux-kernel@vger.kernel.org public-inbox-index lkml Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git