From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Tom Zanussi <zanussi@kernel.org>,
Masami Hiramatsu <mhiramat@kernel.org>
Subject: [for-next][PATCH 26/26] tracing: trigger: Replace unneeded RCU-list traversals
Date: Tue, 14 Jan 2020 16:03:42 -0500 [thread overview]
Message-ID: <20200114210339.492724604@goodmis.org> (raw)
In-Reply-To: <20200114210316.450821675@goodmis.org>
From: Masami Hiramatsu <mhiramat@kernel.org>
With CONFIG_PROVE_RCU_LIST, I had many suspicious RCU warnings
when I ran ftracetest trigger testcases.
-----
# dmesg -c > /dev/null
# ./ftracetest test.d/trigger
...
# dmesg | grep "RCU-list traversed" | cut -f 2 -d ] | cut -f 2 -d " "
kernel/trace/trace_events_hist.c:6070
kernel/trace/trace_events_hist.c:1760
kernel/trace/trace_events_hist.c:5911
kernel/trace/trace_events_trigger.c:504
kernel/trace/trace_events_hist.c:1810
kernel/trace/trace_events_hist.c:3158
kernel/trace/trace_events_hist.c:3105
kernel/trace/trace_events_hist.c:5518
kernel/trace/trace_events_hist.c:5998
kernel/trace/trace_events_hist.c:6019
kernel/trace/trace_events_hist.c:6044
kernel/trace/trace_events_trigger.c:1500
kernel/trace/trace_events_trigger.c:1540
kernel/trace/trace_events_trigger.c:539
kernel/trace/trace_events_trigger.c:584
-----
I investigated those warnings and found that the RCU-list
traversals in event trigger and hist didn't need to use
RCU version because those were called only under event_mutex.
I also checked other RCU-list traversals related to event
trigger list, and found that most of them were called from
event_hist_trigger_func() or hist_unregister_trigger() or
register/unregister functions except for a few cases.
Replace these unneeded RCU-list traversals with normal list
traversal macro and lockdep_assert_held() to check the
event_mutex is held.
Link: http://lkml.kernel.org/r/157680910305.11685.15110237954275915782.stgit@devnote2
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 41 +++++++++++++++++++++--------
kernel/trace/trace_events_trigger.c | 20 ++++++++++----
2 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 8e90f1ada437..117a1202a6b9 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1771,11 +1771,13 @@ static struct hist_field *find_var(struct hist_trigger_data *hist_data,
struct event_trigger_data *test;
struct hist_field *hist_field;
+ lockdep_assert_held(&event_mutex);
+
hist_field = find_var_field(hist_data, var_name);
if (hist_field)
return hist_field;
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
test_data = test->private_data;
hist_field = find_var_field(test_data, var_name);
@@ -1825,7 +1827,9 @@ static struct hist_field *find_file_var(struct trace_event_file *file,
struct event_trigger_data *test;
struct hist_field *hist_field;
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
test_data = test->private_data;
hist_field = find_var_field(test_data, var_name);
@@ -3120,7 +3124,9 @@ static char *find_trigger_filter(struct hist_trigger_data *hist_data,
{
struct event_trigger_data *test;
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (test->private_data == hist_data)
return test->filter_str;
@@ -3171,9 +3177,11 @@ find_compatible_hist(struct hist_trigger_data *target_hist_data,
struct event_trigger_data *test;
unsigned int n_keys;
+ lockdep_assert_held(&event_mutex);
+
n_keys = target_hist_data->n_fields - target_hist_data->n_vals;
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
hist_data = test->private_data;
@@ -5536,7 +5544,7 @@ static int hist_show(struct seq_file *m, void *v)
goto out_unlock;
}
- list_for_each_entry_rcu(data, &event_file->triggers, list) {
+ list_for_each_entry(data, &event_file->triggers, list) {
if (data->cmd_ops->trigger_type == ETT_EVENT_HIST)
hist_trigger_show(m, data, n++);
}
@@ -5929,7 +5937,9 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
if (hist_data->attrs->name && !named_data)
goto new;
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (!hist_trigger_match(data, test, named_data, false))
continue;
@@ -6013,10 +6023,12 @@ static bool have_hist_trigger_match(struct event_trigger_data *data,
struct event_trigger_data *test, *named_data = NULL;
bool match = false;
+ lockdep_assert_held(&event_mutex);
+
if (hist_data->attrs->name)
named_data = find_named_trigger(hist_data->attrs->name);
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (hist_trigger_match(data, test, named_data, false)) {
match = true;
@@ -6034,10 +6046,12 @@ static bool hist_trigger_check_refs(struct event_trigger_data *data,
struct hist_trigger_data *hist_data = data->private_data;
struct event_trigger_data *test, *named_data = NULL;
+ lockdep_assert_held(&event_mutex);
+
if (hist_data->attrs->name)
named_data = find_named_trigger(hist_data->attrs->name);
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (!hist_trigger_match(data, test, named_data, false))
continue;
@@ -6059,10 +6073,12 @@ static void hist_unregister_trigger(char *glob, struct event_trigger_ops *ops,
struct event_trigger_data *test, *named_data = NULL;
bool unregistered = false;
+ lockdep_assert_held(&event_mutex);
+
if (hist_data->attrs->name)
named_data = find_named_trigger(hist_data->attrs->name);
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (!hist_trigger_match(data, test, named_data, false))
continue;
@@ -6088,7 +6104,9 @@ static bool hist_file_check_refs(struct trace_event_file *file)
struct hist_trigger_data *hist_data;
struct event_trigger_data *test;
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
hist_data = test->private_data;
if (check_var_refs(hist_data))
@@ -6331,7 +6349,8 @@ hist_enable_trigger(struct event_trigger_data *data, void *rec,
struct enable_trigger_data *enable_data = data->private_data;
struct event_trigger_data *test;
- list_for_each_entry_rcu(test, &enable_data->file->triggers, list) {
+ list_for_each_entry_rcu(test, &enable_data->file->triggers, list,
+ lockdep_is_held(&event_mutex)) {
if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (enable_data->enable)
test->paused = false;
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index d8ada4c6f3f7..60959c31791d 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -501,7 +501,9 @@ void update_cond_flag(struct trace_event_file *file)
struct event_trigger_data *data;
bool set_cond = false;
- list_for_each_entry_rcu(data, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(data, &file->triggers, list) {
if (data->filter || event_command_post_trigger(data->cmd_ops) ||
event_command_needs_rec(data->cmd_ops)) {
set_cond = true;
@@ -536,7 +538,9 @@ static int register_trigger(char *glob, struct event_trigger_ops *ops,
struct event_trigger_data *test;
int ret = 0;
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(test, &file->triggers, list) {
if (test->cmd_ops->trigger_type == data->cmd_ops->trigger_type) {
ret = -EEXIST;
goto out;
@@ -581,7 +585,9 @@ static void unregister_trigger(char *glob, struct event_trigger_ops *ops,
struct event_trigger_data *data;
bool unregistered = false;
- list_for_each_entry_rcu(data, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(data, &file->triggers, list) {
if (data->cmd_ops->trigger_type == test->cmd_ops->trigger_type) {
unregistered = true;
list_del_rcu(&data->list);
@@ -1497,7 +1503,9 @@ int event_enable_register_trigger(char *glob,
struct event_trigger_data *test;
int ret = 0;
- list_for_each_entry_rcu(test, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(test, &file->triggers, list) {
test_enable_data = test->private_data;
if (test_enable_data &&
(test->cmd_ops->trigger_type ==
@@ -1537,7 +1545,9 @@ void event_enable_unregister_trigger(char *glob,
struct event_trigger_data *data;
bool unregistered = false;
- list_for_each_entry_rcu(data, &file->triggers, list) {
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(data, &file->triggers, list) {
enable_data = data->private_data;
if (enable_data &&
(data->cmd_ops->trigger_type ==
--
2.24.1
next prev parent reply other threads:[~2020-01-14 21:04 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-14 21:03 [for-next][PATCH 00/26] tracing: Updates for 5.6 Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 01/26] perf: Make struct ring_buffer less ambiguous Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 02/26] tracing: Rename trace_buffer to array_buffer Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 03/26] tracing: Make struct ring_buffer less ambiguous Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 04/26] bootconfig: Add Extra Boot Config support Steven Rostedt
2020-02-06 11:54 ` Borislav Petkov
2020-02-06 14:41 ` Masami Hiramatsu
2020-02-06 17:20 ` Geert Uytterhoeven
2020-02-07 0:30 ` Masami Hiramatsu
2020-02-07 8:49 ` Geert Uytterhoeven
2020-02-07 13:17 ` Masami Hiramatsu
2020-02-07 14:28 ` [PATCH] bootconfig: Allocate xbc_nodes array dynamically Masami Hiramatsu
2020-02-06 17:58 ` [for-next][PATCH 04/26] bootconfig: Add Extra Boot Config support Borislav Petkov
2020-02-06 18:10 ` Randy Dunlap
2020-02-06 22:39 ` Steven Rostedt
2020-02-06 22:51 ` Borislav Petkov
2020-02-07 8:59 ` Geert Uytterhoeven
2020-02-07 2:46 ` Masami Hiramatsu
2020-02-07 11:41 ` Borislav Petkov
2020-02-07 15:06 ` Masami Hiramatsu
2020-02-10 11:25 ` Borislav Petkov
2020-02-10 15:10 ` Masami Hiramatsu
2020-02-10 17:40 ` Borislav Petkov
2020-02-11 2:02 ` Masami Hiramatsu
2020-02-18 13:27 ` Borislav Petkov
2020-02-18 17:57 ` Steven Rostedt
2020-02-18 19:38 ` Borislav Petkov
2020-02-19 2:26 ` Masami Hiramatsu
2020-02-19 2:26 ` Masami Hiramatsu
2020-02-19 2:26 ` Masami Hiramatsu
2020-02-11 9:25 ` Geert Uytterhoeven
2020-01-14 21:03 ` [for-next][PATCH 05/26] bootconfig: Load boot config from the tail of initrd Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 06/26] tools: bootconfig: Add bootconfig command Steven Rostedt
2020-02-07 13:02 ` Michael Ellerman
2020-02-07 13:39 ` Masami Hiramatsu
2020-02-07 13:55 ` [PATCH] tools/bootconfig: Fix wrong __VA_ARGS__ usage Masami Hiramatsu
2020-02-08 11:10 ` Michael Ellerman
2020-02-09 4:09 ` Masami Hiramatsu
2020-02-09 13:05 ` [PATCH] tools/bootconfig: Suppress non-error messages Masami Hiramatsu
2020-02-10 2:06 ` Michael Ellerman
2020-02-10 7:18 ` Masami Hiramatsu
2020-02-10 9:50 ` Michael Ellerman
2020-02-10 9:50 ` [PATCH] tools/bootconfig: Fix wrong __VA_ARGS__ usage Michael Ellerman
2020-02-10 17:07 ` Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 07/26] tools: bootconfig: Add bootconfig test script Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 08/26] proc: bootconfig: Add /proc/bootconfig to show boot config list Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 09/26] init/main.c: Alloc initcall_command_line in do_initcall() and free it Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 10/26] bootconfig: init: Allow admin to use bootconfig for kernel command line Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 11/26] bootconfig: init: Allow admin to use bootconfig for init " Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 12/26] Documentation: bootconfig: Add a doc for extended boot config Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 13/26] tracing: Apply soft-disabled and filter to tracepoints printk Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 14/26] tracing: kprobes: Output kprobe event to printk buffer Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 15/26] tracing: kprobes: Register to dynevent earlier stage Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 16/26] tracing: Accept different type for synthetic event fields Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 17/26] tracing: Add NULL trace-array check in print_synth_event() Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 18/26] tracing/boot: Add boot-time tracing Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 19/26] tracing/boot: Add per-event settings Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 20/26] tracing/boot Add kprobe event support Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 21/26] tracing/boot: Add synthetic " Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 22/26] tracing/boot: Add instance node support Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 23/26] tracing/boot: Add cpu_mask option support Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 24/26] tracing/boot: Add function tracer filter options Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 25/26] Documentation: tracing: Add boot-time tracing document Steven Rostedt
2020-01-14 21:03 ` Steven Rostedt [this message]
2020-01-14 22:11 ` [for-next][PATCH 26/26] tracing: trigger: Replace unneeded RCU-list traversals 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=20200114210339.492724604@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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome