Skip to contentSkip to conversation

tracing: fixes

GIT PULL New Steven Rostedt 2 participants 8 messages Dec 6, 2011

Subsystems (5)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

Ingo,

These are the patches that were also in a tip/perf/core push, but as they are all bug fixes, we should get them into 3.2.

Thanks,

Please pull the latest tip/perf/urgent tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
tip/perf/urgent

Head SHA1: ddf6e0e50723b62ac76ed18eb53e9417c6eefba7

Gleb Natapov (1):
jump_label: jump_label_inc may return before the code is patched

Ilya Dryomov (1):
tracing: fix event_subsystem ref counting

Li Zefan (1):
tracing: Restore system filter behavior

Steven Rostedt (3):
ftrace: Remove force undef config value left for testing
perf: Fix parsing of __print_flags() in TP_printk()
ftrace: Fix hash record accounting bug

----

 include/linux/ftrace_event.h        |    2 ++
 kernel/jump_label.c                 |    3 ++-
 kernel/trace/ftrace.c               |    5 +++--
 kernel/trace/trace_events.c         |    1 -
 kernel/trace/trace_events_filter.c  |    7 ++++++-
 tools/perf/util/trace-event-parse.c |    2 ++
 6 files changed, 15 insertions(+), 5 deletions(-)
author maintainer
#1 lore raw

PATCH 1/6 tracing: fix event_subsystem ref counting

From: Ilya Dryomov <[email protected]>

Fix a bug introduced by e9dbfae5, which prevents event_subsystem from ever being released.

Ref_count was added to keep track of subsystem users, not for counting events. Subsystem is created with ref_count = 1, so there is no need to increment it for every event, we have nr_events for that. Fix this by touching ref_count only when we actually have a new user - subsystem_open().

---
 kernel/trace/trace_events.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
kernel/trace/trace_events.c +0 -1
index 581876f..c212a7f 100644
@@ -1078,7 +1078,6 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
 	/* First see if we did not already create this dir */
 	list_for_each_entry(system, &event_subsystems, list) {
 		if (strcmp(system->name, name) == 0) {
-			__get_system(system);
 			system->nr_events++;
 			return system->entry;
 		}
Signature · 1 line
1.7.7.3
author maintainer
#2 lore raw

PATCH 2/6 tracing: Restore system filter behavior

In reply to Steven Rostedt

From: Li Zefan <[email protected]>

Though not all events have field 'prev_pid', it was allowed to do this:

  # echo 'prev_pid == 100' > events/sched/filter

but commit 75b8e98263fdb0bfbdeba60d4db463259f1fe8a2 (tracing/filter: Swap entire filter of events) broke it without any reason.

---
 include/linux/ftrace_event.h       |    2 ++
 kernel/trace/trace_events_filter.c |    7 ++++++-
 2 files changed, 8 insertions(+), 1 deletions(-)
include/linux/ftrace_event.h +2 -0
index 96efa67..c3da42d 100644
@@ -172,6 +172,7 @@ enum {
 	TRACE_EVENT_FL_FILTERED_BIT,
 	TRACE_EVENT_FL_RECORDED_CMD_BIT,
 	TRACE_EVENT_FL_CAP_ANY_BIT,
+	TRACE_EVENT_FL_NO_SET_FILTER_BIT,
 };
 
 enum {
@@ -179,6 +180,7 @@ enum {
 	TRACE_EVENT_FL_FILTERED		= (1 << TRACE_EVENT_FL_FILTERED_BIT),
 	TRACE_EVENT_FL_RECORDED_CMD	= (1 << TRACE_EVENT_FL_RECORDED_CMD_BIT),
 	TRACE_EVENT_FL_CAP_ANY		= (1 << TRACE_EVENT_FL_CAP_ANY_BIT),
+	TRACE_EVENT_FL_NO_SET_FILTER	= (1 << TRACE_EVENT_FL_NO_SET_FILTER_BIT),
 };
 
 struct ftrace_event_call {
kernel/trace/trace_events_filter.c +6 -1
index d6e7926..95dc31e 100644
@@ -1649,7 +1649,9 @@ static int replace_system_preds(struct event_subsystem *system,
 		 */
 		err = replace_preds(call, NULL, ps, filter_string, true);
 		if (err)
-			goto fail;
+			call->flags |= TRACE_EVENT_FL_NO_SET_FILTER;
+		else
+			call->flags &= ~TRACE_EVENT_FL_NO_SET_FILTER;
 	}
 
 	list_for_each_entry(call, &ftrace_events, list) {
@@ -1658,6 +1660,9 @@ static int replace_system_preds(struct event_subsystem *system,
 		if (strcmp(call->class->system, system->name) != 0)
 			continue;
 
+		if (call->flags & TRACE_EVENT_FL_NO_SET_FILTER)
+			continue;
+
 		filter_item = kzalloc(sizeof(*filter_item), GFP_KERNEL);
 		if (!filter_item)
 			goto fail_mem;
Signature · 1 line
1.7.7.3
author maintainer
#3 lore raw

PATCH 3/6 ftrace: Remove force undef config value left for testing

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

A forced undef of a config value was used for testing and was accidently left in during the final commit. This causes x86 to run slower than needed while running function tracing as well as causes the function graph selftest to fail when DYNMAIC_FTRACE is not set. This is because the code in MCOUNT expects the ftrace code to be processed with the config value set that happened to be forced not set.

The forced config option was left in by:
commit 6331c28c962561aee59e5a493b7556a4bb585957
ftrace: Fix dynamic selftest failure on some archs

---
 kernel/trace/ftrace.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
kernel/trace/ftrace.c +0 -1
index 900b409..1276b3c 100644
@@ -152,7 +152,6 @@ void clear_ftrace_function(void)
 	ftrace_pid_function = ftrace_stub;
 }
 
-#undef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
 #ifndef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
 /*
  * For those archs that do not test ftrace_trace_stop in their
Signature · 1 line
1.7.7.3
author maintainer
#4 lore raw

PATCH 4/6 jump_label: jump_label_inc may return before the code is patched

In reply to Steven Rostedt

From: Gleb Natapov <[email protected]>

If cpu A calls jump_label_inc() just after atomic_add_return() is called by cpu B, atomic_inc_not_zero() will return value greater then zero and jump_label_inc() will return to a caller before jump_label_update() finishes its job on cpu B.

---
 kernel/jump_label.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
kernel/jump_label.c +2 -1
index bbdfe2a..66ff710 100644
@@ -66,8 +66,9 @@ void jump_label_inc(struct jump_label_key *key)
 		return;
 
 	jump_label_lock();
-	if (atomic_add_return(1, &key->enabled) == 1)
+	if (atomic_read(&key->enabled) == 0)
 		jump_label_update(key, JUMP_LABEL_ENABLE);
+	atomic_inc(&key->enabled);
 	jump_label_unlock();
 }
 
Signature · 1 line
1.7.7.3
author maintainer
#5 lore raw

PATCH 5/6 perf: Fix parsing of __print_flags() in TP_printk()

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

A update is made to the sched:sched_switch event that adds some logic to the first parameter of the __print_flags() that shows the state of tasks. This change cause perf to fail parsing the flags.

A simple fix is needed to have the parser be able to process ops within the argument.

---
 tools/perf/util/trace-event-parse.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
tools/perf/util/trace-event-parse.c +2 -0
index 0a7ed5b..6c164dc 100644
@@ -1537,6 +1537,8 @@ process_flags(struct event *event, struct print_arg *arg, char **tok)
 	field = malloc_or_die(sizeof(*field));
 
 	type = process_arg(event, field, &token);
+	while (type == EVENT_OP)
+		type = process_op(event, field, &token);
 	if (test_type_token(type, token, EVENT_DELIM, ","))
 		goto out_free;
 
Signature · 1 line
1.7.7.3
author maintainer
#6 lore raw

PATCH 6/6 ftrace: Fix hash record accounting bug

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

If the set_ftrace_filter is cleared by writing just whitespace to it, then the filter hash refcounts will be decremented but not updated. This causes two bugs:

1) No functions will be enabled for tracing when they all should be

2) If the users clears the set_ftrace_filter twice, it will crash ftrace:

------------[ cut here ]------------
WARNING: at /home/rostedt/work/git/linux-trace.git/kernel/trace/ftrace.c:1384 __ftrace_hash_rec_update.part.27+0x157/0x1a7()
Modules linked in:
Pid: 2330, comm: bash Not tainted 3.1.0-test+ #32
Call Trace:
 [<ffffffff81051828>] warn_slowpath_common+0x83/0x9b
 [<ffffffff8105185a>] warn_slowpath_null+0x1a/0x1c
 [<ffffffff810ba362>] __ftrace_hash_rec_update.part.27+0x157/0x1a7
 [<ffffffff810ba6e8>] ? ftrace_regex_release+0xa7/0x10f
 [<ffffffff8111bdfe>] ? kfree+0xe5/0x115
 [<ffffffff810ba51e>] ftrace_hash_move+0x2e/0x151
 [<ffffffff810ba6fb>] ftrace_regex_release+0xba/0x10f
 [<ffffffff8112e49a>] fput+0xfd/0x1c2
 [<ffffffff8112b54c>] filp_close+0x6d/0x78
 [<ffffffff8113a92d>] sys_dup3+0x197/0x1c1
 [<ffffffff8113a9a6>] sys_dup2+0x4f/0x54
 [<ffffffff8150cac2>] system_call_fastpath+0x16/0x1b
---[ end trace 77a3a7ee73794a02 ]---
---
 kernel/trace/ftrace.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
kernel/trace/ftrace.c +3 -1
index 1276b3c..b1e8943 100644
@@ -1211,7 +1211,9 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable,
 	if (!src->count) {
 		free_ftrace_hash_rcu(*dst);
 		rcu_assign_pointer(*dst, EMPTY_HASH);
-		return 0;
+		/* still need to update the function records */
+		ret = 0;
+		goto out;
 	}
 
 	/*
Signature · 1 line
1.7.7.3
#7 lore raw
In reply to Steven Rostedt
Quoted 30 lines · Steven Rostedt

Ingo,

These are the patches that were also in a tip/perf/core push, but as they are all bug fixes, we should get them into 3.2.

Thanks,

Please pull the latest tip/perf/urgent tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
tip/perf/urgent

Head SHA1: ddf6e0e50723b62ac76ed18eb53e9417c6eefba7

Gleb Natapov (1):
jump_label: jump_label_inc may return before the code is patched

Ilya Dryomov (1):
tracing: fix event_subsystem ref counting

Li Zefan (1):
tracing: Restore system filter behavior

Steven Rostedt (3):
ftrace: Remove force undef config value left for testing
perf: Fix parsing of __print_flags() in TP_printk()
ftrace: Fix hash record accounting bug

Quoted 8 lines

----

 include/linux/ftrace_event.h        |    2 ++
 kernel/jump_label.c                 |    3 ++-
 kernel/trace/ftrace.c               |    5 +++--
 kernel/trace/trace_events.c         |    1 -
 kernel/trace/trace_events_filter.c  |    7 ++++++-
 tools/perf/util/trace-event-parse.c |    2 ++
 6 files changed, 15 insertions(+), 5 deletions(-)

Pulled, thanks a lot Steve!

If it's problem-free i'll get it to Linus today-ish. Note that i also merged perf/urgent into perf/core, to neutralize the conflict-creation properties of the cherry-picks.

Thanks,

Ingo