openkylin
/
linux
mirror of https://gitee.com/openkylin/linux.git
9
0
Fork 0
Code Issues Projects Releases Wiki Activity

tracing: trigger: Replace unneeded RCU-list traversals 

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>
This commit is contained in:
Masami Hiramatsu 2019-12-20 11:31:43 +09:00 committed by Steven Rostedt (VMware)
parent 4778194794
commit 3b42a4c83a
2 changed files with 45 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
kernel/trace/trace_events_hist.c
View File

@ -1771,11 +1771,13 @@ static struct hist_field *find_var(struct hist_trigger_data *hist_data,
struct event_trigger_data *test; struct event_trigger_data *test;
struct hist_field *hist_field; struct hist_field *hist_field;
lockdep_assert_held(&event_mutex);
hist_field = find_var_field(hist_data, var_name); hist_field = find_var_field(hist_data, var_name);
if (hist_field) if (hist_field)
return 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) { if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
test_data = test->private_data; test_data = test->private_data;
hist_field = find_var_field(test_data, var_name); 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 event_trigger_data *test;
struct hist_field *hist_field; 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) { if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
test_data = test->private_data; test_data = test->private_data;
hist_field = find_var_field(test_data, var_name); 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; 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->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (test->private_data == hist_data) if (test->private_data == hist_data)
return test->filter_str; return test->filter_str;
@ -3171,9 +3177,11 @@ find_compatible_hist(struct hist_trigger_data *target_hist_data,
struct event_trigger_data *test; struct event_trigger_data *test;
unsigned int n_keys; unsigned int n_keys;
lockdep_assert_held(&event_mutex);
n_keys = target_hist_data->n_fields - target_hist_data->n_vals; 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) { if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
hist_data = test->private_data; hist_data = test->private_data;
@ -5536,7 +5544,7 @@ static int hist_show(struct seq_file *m, void *v)
goto out_unlock; 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) if (data->cmd_ops->trigger_type == ETT_EVENT_HIST)
hist_trigger_show(m, data, n++); 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) if (hist_data->attrs->name && !named_data)
goto new; 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 (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (!hist_trigger_match(data, test, named_data, false)) if (!hist_trigger_match(data, test, named_data, false))
continue; continue;
@ -6013,10 +6023,12 @@ static bool have_hist_trigger_match(struct event_trigger_data *data,
struct event_trigger_data *test, *named_data = NULL; struct event_trigger_data *test, *named_data = NULL;
bool match = false; bool match = false;
lockdep_assert_held(&event_mutex);
if (hist_data->attrs->name) if (hist_data->attrs->name)
named_data = find_named_trigger(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 (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (hist_trigger_match(data, test, named_data, false)) { if (hist_trigger_match(data, test, named_data, false)) {
match = true; 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 hist_trigger_data *hist_data = data->private_data;
struct event_trigger_data *test, *named_data = NULL; struct event_trigger_data *test, *named_data = NULL;
lockdep_assert_held(&event_mutex);
if (hist_data->attrs->name) if (hist_data->attrs->name)
named_data = find_named_trigger(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 (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (!hist_trigger_match(data, test, named_data, false)) if (!hist_trigger_match(data, test, named_data, false))
continue; 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; struct event_trigger_data *test, *named_data = NULL;
bool unregistered = false; bool unregistered = false;
lockdep_assert_held(&event_mutex);
if (hist_data->attrs->name) if (hist_data->attrs->name)
named_data = find_named_trigger(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 (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (!hist_trigger_match(data, test, named_data, false)) if (!hist_trigger_match(data, test, named_data, false))
continue; continue;
@ -6088,7 +6104,9 @@ static bool hist_file_check_refs(struct trace_event_file *file)
struct hist_trigger_data *hist_data; struct hist_trigger_data *hist_data;
struct event_trigger_data *test; 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->cmd_ops->trigger_type == ETT_EVENT_HIST) {
hist_data = test->private_data; hist_data = test->private_data;
if (check_var_refs(hist_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 enable_trigger_data *enable_data = data->private_data;
struct event_trigger_data *test; 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 (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
if (enable_data->enable) if (enable_data->enable)
test->paused = false; test->paused = false;

20
kernel/trace/trace_events_trigger.c
View File

@ -501,7 +501,9 @@ void update_cond_flag(struct trace_event_file *file)
struct event_trigger_data *data; struct event_trigger_data *data;
bool set_cond = false; 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) || if (data->filter || event_command_post_trigger(data->cmd_ops) ||
event_command_needs_rec(data->cmd_ops)) { event_command_needs_rec(data->cmd_ops)) {
set_cond = true; set_cond = true;
@ -536,7 +538,9 @@ static int register_trigger(char *glob, struct event_trigger_ops *ops,
struct event_trigger_data *test; struct event_trigger_data *test;
int ret = 0; 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) { if (test->cmd_ops->trigger_type == data->cmd_ops->trigger_type) {
ret = -EEXIST; ret = -EEXIST;
goto out; goto out;
@ -581,7 +585,9 @@ static void unregister_trigger(char *glob, struct event_trigger_ops *ops,
struct event_trigger_data *data; struct event_trigger_data *data;
bool unregistered = false; 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) { if (data->cmd_ops->trigger_type == test->cmd_ops->trigger_type) {
unregistered = true; unregistered = true;
list_del_rcu(&data->list); list_del_rcu(&data->list);
@ -1497,7 +1503,9 @@ int event_enable_register_trigger(char *glob,
struct event_trigger_data *test; struct event_trigger_data *test;
int ret = 0; 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; test_enable_data = test->private_data;
if (test_enable_data && if (test_enable_data &&
(test->cmd_ops->trigger_type == (test->cmd_ops->trigger_type ==
@ -1537,7 +1545,9 @@ void event_enable_unregister_trigger(char *glob,
struct event_trigger_data *data; struct event_trigger_data *data;
bool unregistered = false; 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; enable_data = data->private_data;
if (enable_data && if (enable_data &&
(data->cmd_ops->trigger_type == (data->cmd_ops->trigger_type ==