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 [thread overview]
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 other threads:[~2018-11-05 9:04 UTC|newest]
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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).