Commit aeed8aa3 authored by Masami Hiramatsu's avatar Masami Hiramatsu Committed by Steven Rostedt (VMware)
Browse files

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



Cc: stable@vger.kernel.org
Fixes: 30350d65 ("tracing: Add variable support to hist triggers")
Reviewed-by: default avatarTom Zanussi <zanussi@kernel.org>
Signed-off-by: default avatarMasami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
parent 99c9a923

kernel/trace/trace_events_hist.c

+30 −11
Original line number Original line Diff line number Diff line
@@ -1766,11 +1766,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);
@@ -1820,7 +1822,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);
@@ -3115,7 +3119,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;
@@ -3166,9 +3172,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;
@@ -5528,7 +5536,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++);

	}

	}
@@ -5921,7 +5929,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;
@@ -6005,10 +6015,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;
@@ -6026,10 +6038,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;
@@ -6051,10 +6065,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;
@@ -6080,7 +6096,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))
@@ -6323,7 +6341,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;

kernel/trace/trace_events_trigger.c

+15 −5
Original line number Original line Diff line number Diff line
@@ -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 ==

CRA Git | Maintained and supported by SUSTech CRA and CCSE