All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Tom Zanussi <zanussi@kernel.org>,
	Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>,
	Tom Zanussi <tom.zanussi@linux.intel.com>
Subject: [for-next][PATCH 27/30] tracing: Remove unneeded synth_event_mutex
Date: Wed, 05 Dec 2018 18:48:16 -0500	[thread overview]
Message-ID: <20181205234832.500521609@goodmis.org> (raw)
In-Reply-To: 20181205234749.372720574@goodmis.org

From: Masami Hiramatsu <mhiramat@kernel.org>

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>
---
 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:
-- 
2.19.1



  parent reply	other threads:[~2018-12-05 23:49 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05 23:47 [for-next][PATCH 00/30] tracing: Updates for the next merge window Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 01/30] function_graph: Remove unused task_curr_ret_stack() Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 02/30] tracing: Do not line wrap short line in function_graph_enter() Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 03/30] fgraph: Create a fgraph.c file to store function graph infrastructure Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 04/30] fgraph: Have set_graph_notrace only affect function_graph tracer Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 05/30] arm64: function_graph: Remove use of FTRACE_NOTRACE_DEPTH Steven Rostedt
2018-12-05 23:47   ` Steven Rostedt
2018-12-06  3:55   ` Steven Rostedt
2018-12-06  3:55     ` Steven Rostedt
2018-12-06 15:49   ` Will Deacon
2018-12-06 15:49     ` Will Deacon
2018-12-06 15:55     ` Steven Rostedt
2018-12-06 15:55       ` Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 06/30] function_graph: Remove the " Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 07/30] ftrace: Create new ftrace_internal.h header Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 08/30] function_graph: Do not expose the graph_time option when profiler is not configured Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 09/30] fgraph: Move function graph specific code into fgraph.c Steven Rostedt
2018-12-05 23:47 ` [for-next][PATCH 10/30] tracing: Rearrange functions in trace_sched_wakeup.c Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 11/30] fgraph: Add new fgraph_ops structure to enable function graph hooks Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 12/30] function_graph: Move ftrace_graph_ret_addr() to fgraph.c Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 13/30] function_graph: Have profiler use new helper ftrace_graph_get_ret_stack() Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 14/30] tracing: Have trace_stack nr_entries compare not be so subtle Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 15/30] scripts/recordmcount.{c,pl}: support -ffunction-sections .text.* section names Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 16/30] ring-buffer: Add percentage of ring buffer full to wake up reader Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 17/30] tracing: Add tracefs file buffer_percentage Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 18/30] tracing: Change default buffer_percent to 50 Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 19/30] tracing/uprobes: Add busy check when cleanup all uprobes Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 20/30] tracing: Lock event_mutex before synth_event_mutex Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 21/30] tracing: Simplify creation and deletion of synthetic events Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 22/30] tracing: Integrate similar probe argument parsers Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 23/30] tracing: Add unified dynamic event framework Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 24/30] tracing/kprobes: Use dyn_event framework for kprobe events Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 25/30] tracing/uprobes: Use dyn_event framework for uprobe events Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 26/30] tracing: Use dyn_event framework for synthetic events Steven Rostedt
2018-12-05 23:48 ` Steven Rostedt [this message]
2018-12-05 23:48 ` [for-next][PATCH 28/30] tracing: Consolidate trace_add/remove_event_call back to the nolock functions Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 29/30] tracing: Add generic event-name based remove event method Steven Rostedt
2018-12-05 23:48 ` [for-next][PATCH 30/30] selftests/ftrace: Add testcases for dynamic event Steven Rostedt

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=20181205234832.500521609@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=ravi.bangoria@linux.vnet.ibm.com \
    --cc=tom.zanussi@linux.intel.com \
    --cc=zanussi@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.