[PATCH 0/4] [GIT PULL] tracing: more fixes for v2.6.32

[PATCH 0/4] [GIT PULL] tracing: more fixes for v2.6.32

[Steven Rostedt]

Ingo,

This patch set is based off of tip/tracing/core.

This also includes Li's fix for the perf counter module race.

Please pull the latest tip/tracing/core2 tree, which can be found at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/tracing/core2


Jiri Olsa (1):
      tracing/function-graph: x86_64 stack allocation cleanup

John Reiser (1):
      ftrace: __start_mcount_loc should be .init.rodata

Li Zefan (1):
      tracing/profile: fix profile_disable vs module_unload

Tom Zanussi (1):
      tracing/filters: add filter Documentation

----
 Documentation/trace/events.txt     |  184 +++++++++++++++++++++++++++++++++++-
 arch/x86/kernel/entry_64.S         |    6 +-
 include/asm-generic/vmlinux.lds.h  |    5 +-
 kernel/trace/trace_event_profile.c |    5 +-
 4 files changed, 193 insertions(+), 7 deletions(-)
-- 

[PATCH 1/4] ftrace: __start_mcount_loc should be .init.rodata

[Steven Rostedt]

[-- Attachment #1: 0001-ftrace-__start_mcount_loc-should-be-.init.rodata.patch --]
[-- Type: text/plain, Size: 2131 bytes --]

From: John Reiser <jreiser@bitwagon.com>

__start_mcount_loc[] is unused after init, yet occupies RAM forever
as part of .rodata.  152kiB is typical on a 64-bit architecture.  Instead,
__start_mcount_loc should be in the interval [__init_begin, __init_end)
so that the space is reclaimed after init.

__start_mcount_loc[] is generated during the load portion
of kernel build, and is used only by ftrace_init().  ftrace_init is declared
'__init' and is in .init.text, which is freed after init.
__start_mcount_loc is placed into .rodata by a call to MCOUNT_REC inside
the RO_DATA macro of include/asm-generic/vmlinux.lds.h.  The array *is*
read-only, but more importantly it is not used after init.  So the call to
MCOUNT_REC should be moved from RO_DATA to INIT_DATA.

This patch has been tested on x86_64 with CONFIG_DEBUG_PAGEALLOC=y
which verifies that the address range never is accessed after init.

Signed-off-by: John Reiser <jreiser@BitWagon.com>
LKML-Reference: <4A6DF0B6.7080402@bitwagon.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/asm-generic/vmlinux.lds.h |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 6ad76bf..98b37cf 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -91,7 +91,8 @@
 #endif
 
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
-#define MCOUNT_REC()	VMLINUX_SYMBOL(__start_mcount_loc) = .; \
+#define MCOUNT_REC()	. = ALIGN(8);				\
+			VMLINUX_SYMBOL(__start_mcount_loc) = .; \
 			*(__mcount_loc)				\
 			VMLINUX_SYMBOL(__stop_mcount_loc) = .;
 #else
@@ -331,7 +332,6 @@
 	/* __*init sections */						\
 	__init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) {		\
 		*(.ref.rodata)						\
-		MCOUNT_REC()						\
 		DEV_KEEP(init.rodata)					\
 		DEV_KEEP(exit.rodata)					\
 		CPU_KEEP(init.rodata)					\
@@ -455,6 +455,7 @@
 	MEM_DISCARD(init.data)						\
 	KERNEL_CTORS()							\
 	*(.init.rodata)							\
+	MCOUNT_REC()							\
 	DEV_DISCARD(init.rodata)					\
 	CPU_DISCARD(init.rodata)					\
 	MEM_DISCARD(init.rodata)
-- 
1.6.3.3

-- 

[PATCH 2/4] tracing/function-graph: x86_64 stack allocation cleanup

[Steven Rostedt]

[-- Attachment #1: 0002-tracing-function-graph-x86_64-stack-allocation-clean.patch --]
[-- Type: text/plain, Size: 981 bytes --]

From: Jiri Olsa <jolsa@redhat.com>

Only 24 bytes needs to be reserved on the stack for the function graph
tracer on x86_64.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
LKML-Reference: <20090729085837.GB4998@jolsa.lab.eng.brq.redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/entry_64.S |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index c251be7..d59fe32 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -146,7 +146,7 @@ ENTRY(ftrace_graph_caller)
 END(ftrace_graph_caller)
 
 GLOBAL(return_to_handler)
-	subq  $80, %rsp
+	subq  $24, %rsp
 
 	/* Save the return values */
 	movq %rax, (%rsp)
@@ -155,10 +155,10 @@ GLOBAL(return_to_handler)
 
 	call ftrace_return_to_handler
 
-	movq %rax, 72(%rsp)
+	movq %rax, 16(%rsp)
 	movq 8(%rsp), %rdx
 	movq (%rsp), %rax
-	addq $72, %rsp
+	addq $16, %rsp
 	retq
 #endif
 
-- 
1.6.3.3

-- 

[PATCH 3/4] tracing/profile: fix profile_disable vs module_unload

[Steven Rostedt]

[-- Attachment #1: 0003-tracing-profile-fix-profile_disable-vs-module_unload.patch --]
[-- Type: text/plain, Size: 1513 bytes --]

From: Li Zefan <lizf@cn.fujitsu.com>

If the correspoding module is unloaded before ftrace_profile_disable()
is called, event->profile_disable() won't be called, which can
cause oops:

  # insmod trace-events-sample.ko
  # perf record -f -a -e sample:foo_bar sleep 3 &
  # sleep 1
  # rmmod trace_events_sample
  # insmod trace-events-sample.ko
  OOPS!

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
LKML-Reference: <4A9214E3.2070807@cn.fujitsu.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_event_profile.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_event_profile.c b/kernel/trace/trace_event_profile.c
index 11ba5bb..55a25c9 100644
--- a/kernel/trace/trace_event_profile.c
+++ b/kernel/trace/trace_event_profile.c
@@ -5,6 +5,7 @@
  *
  */
 
+#include <linux/module.h>
 #include "trace.h"
 
 int ftrace_profile_enable(int event_id)
@@ -14,7 +15,8 @@ int ftrace_profile_enable(int event_id)
 
 	mutex_lock(&event_mutex);
 	list_for_each_entry(event, &ftrace_events, list) {
-		if (event->id == event_id && event->profile_enable) {
+		if (event->id == event_id && event->profile_enable &&
+		    try_module_get(event->mod)) {
 			ret = event->profile_enable(event);
 			break;
 		}
@@ -32,6 +34,7 @@ void ftrace_profile_disable(int event_id)
 	list_for_each_entry(event, &ftrace_events, list) {
 		if (event->id == event_id) {
 			event->profile_disable(event);
+			module_put(event->mod);
 			break;
 		}
 	}
-- 
1.6.3.3

-- 

[PATCH 4/4] tracing/filters: add filter Documentation

[Steven Rostedt]

[-- Attachment #1: 0004-tracing-filters-add-filter-Documentation.patch --]
[-- Type: text/plain, Size: 7470 bytes --]

From: Tom Zanussi <tzanussi@gmail.com>

Documentation for event filters and formats.

v2 changes: fix a few problems noticed by Randy Dunlap.

Reviewed-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <1252642431.8016.9.camel@tropicana>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/trace/events.txt |  184 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 183 insertions(+), 1 deletions(-)

diff --git a/Documentation/trace/events.txt b/Documentation/trace/events.txt
index 2bcc8d4..6e5f35e 100644
--- a/Documentation/trace/events.txt
+++ b/Documentation/trace/events.txt
@@ -1,7 +1,7 @@
 			     Event Tracing
 
 		Documentation written by Theodore Ts'o
-			Updated by Li Zefan
+		Updated by Li Zefan and Tom Zanussi
 
 1. Introduction
 ===============
@@ -97,3 +97,185 @@ The format of this boot option is the same as described in section 2.1.
 
 See The example provided in samples/trace_events
 
+4. Event formats
+================
+
+Each trace event has a 'format' file associated with it that contains
+a description of each field in a logged event.  This information can
+be used to parse the binary trace stream, and is also the place to
+find the field names that can be used in event filters (see section 5).
+
+It also displays the format string that will be used to print the
+event in text mode, along with the event name and ID used for
+profiling.
+
+Every event has a set of 'common' fields associated with it; these are
+the fields prefixed with 'common_'.  The other fields vary between
+events and correspond to the fields defined in the TRACE_EVENT
+definition for that event.
+
+Each field in the format has the form:
+
+     field:field-type field-name; offset:N; size:N;
+
+where offset is the offset of the field in the trace record and size
+is the size of the data item, in bytes.
+
+For example, here's the information displayed for the 'sched_wakeup'
+event:
+
+# cat /debug/tracing/events/sched/sched_wakeup/format
+
+name: sched_wakeup
+ID: 60
+format:
+	field:unsigned short common_type;	offset:0;	size:2;
+	field:unsigned char common_flags;	offset:2;	size:1;
+	field:unsigned char common_preempt_count;	offset:3;	size:1;
+	field:int common_pid;	offset:4;	size:4;
+	field:int common_tgid;	offset:8;	size:4;
+
+	field:char comm[TASK_COMM_LEN];	offset:12;	size:16;
+	field:pid_t pid;	offset:28;	size:4;
+	field:int prio;	offset:32;	size:4;
+	field:int success;	offset:36;	size:4;
+	field:int cpu;	offset:40;	size:4;
+
+print fmt: "task %s:%d [%d] success=%d [%03d]", REC->comm, REC->pid,
+	   REC->prio, REC->success, REC->cpu
+
+This event contains 10 fields, the first 5 common and the remaining 5
+event-specific.  All the fields for this event are numeric, except for
+'comm' which is a string, a distinction important for event filtering.
+
+5. Event filtering
+==================
+
+Trace events can be filtered in the kernel by associating boolean
+'filter expressions' with them.  As soon as an event is logged into
+the trace buffer, its fields are checked against the filter expression
+associated with that event type.  An event with field values that
+'match' the filter will appear in the trace output, and an event whose
+values don't match will be discarded.  An event with no filter
+associated with it matches everything, and is the default when no
+filter has been set for an event.
+
+5.1 Expression syntax
+---------------------
+
+A filter expression consists of one or more 'predicates' that can be
+combined using the logical operators '&&' and '||'.  A predicate is
+simply a clause that compares the value of a field contained within a
+logged event with a constant value and returns either 0 or 1 depending
+on whether the field value matched (1) or didn't match (0):
+
+	  field-name relational-operator value
+
+Parentheses can be used to provide arbitrary logical groupings and
+double-quotes can be used to prevent the shell from interpreting
+operators as shell metacharacters.
+
+The field-names available for use in filters can be found in the
+'format' files for trace events (see section 4).
+
+The relational-operators depend on the type of the field being tested:
+
+The operators available for numeric fields are:
+
+==, !=, <, <=, >, >=
+
+And for string fields they are:
+
+==, !=
+
+Currently, only exact string matches are supported.
+
+Currently, the maximum number of predicates in a filter is 16.
+
+5.2 Setting filters
+-------------------
+
+A filter for an individual event is set by writing a filter expression
+to the 'filter' file for the given event.
+
+For example:
+
+# cd /debug/tracing/events/sched/sched_wakeup
+# echo "common_preempt_count > 4" > filter
+
+A slightly more involved example:
+
+# cd /debug/tracing/events/sched/sched_signal_send
+# echo "((sig >= 10 && sig < 15) || sig == 17) && comm != bash" > filter
+
+If there is an error in the expression, you'll get an 'Invalid
+argument' error when setting it, and the erroneous string along with
+an error message can be seen by looking at the filter e.g.:
+
+# cd /debug/tracing/events/sched/sched_signal_send
+# echo "((sig >= 10 && sig < 15) || dsig == 17) && comm != bash" > filter
+-bash: echo: write error: Invalid argument
+# cat filter
+((sig >= 10 && sig < 15) || dsig == 17) && comm != bash
+^
+parse_error: Field not found
+
+Currently the caret ('^') for an error always appears at the beginning of
+the filter string; the error message should still be useful though
+even without more accurate position info.
+
+5.3 Clearing filters
+--------------------
+
+To clear the filter for an event, write a '0' to the event's filter
+file.
+
+To clear the filters for all events in a subsystem, write a '0' to the
+subsystem's filter file.
+
+5.3 Subsystem filters
+---------------------
+
+For convenience, filters for every event in a subsystem can be set or
+cleared as a group by writing a filter expression into the filter file
+at the root of the subsytem.  Note however, that if a filter for any
+event within the subsystem lacks a field specified in the subsystem
+filter, or if the filter can't be applied for any other reason, the
+filter for that event will retain its previous setting.  This can
+result in an unintended mixture of filters which could lead to
+confusing (to the user who might think different filters are in
+effect) trace output.  Only filters that reference just the common
+fields can be guaranteed to propagate successfully to all events.
+
+Here are a few subsystem filter examples that also illustrate the
+above points:
+
+Clear the filters on all events in the sched subsytem:
+
+# cd /sys/kernel/debug/tracing/events/sched
+# echo 0 > filter
+# cat sched_switch/filter
+none
+# cat sched_wakeup/filter
+none
+
+Set a filter using only common fields for all events in the sched
+subsytem (all events end up with the same filter):
+
+# cd /sys/kernel/debug/tracing/events/sched
+# echo common_pid == 0 > filter
+# cat sched_switch/filter
+common_pid == 0
+# cat sched_wakeup/filter
+common_pid == 0
+
+Attempt to set a filter using a non-common field for all events in the
+sched subsytem (all events but those that have a prev_pid field retain
+their old filters):
+
+# cd /sys/kernel/debug/tracing/events/sched
+# echo prev_pid == 0 > filter
+# cat sched_switch/filter
+prev_pid == 0
+# cat sched_wakeup/filter
+common_pid == 0
-- 
1.6.3.3

-- 

Re: [PATCH 2/4] tracing/function-graph: x86_64 stack allocation cleanup

[Frederic Weisbecker]

On Sat, Sep 12, 2009 at 11:05:45PM -0400, Steven Rostedt wrote:
> From: Jiri Olsa <jolsa@redhat.com>
> 
> Only 24 bytes needs to be reserved on the stack for the function graph
> tracer on x86_64.
> 
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> LKML-Reference: <20090729085837.GB4998@jolsa.lab.eng.brq.redhat.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  arch/x86/kernel/entry_64.S |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index c251be7..d59fe32 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -146,7 +146,7 @@ ENTRY(ftrace_graph_caller)
>  END(ftrace_graph_caller)
>  
>  GLOBAL(return_to_handler)
> -	subq  $80, %rsp
> +	subq  $24, %rsp


That's theoretically a good fix.

But Steve, do you remember the weird issues we had while only
saving the theoretically strict needed stack space here?

It made the function graph tracer crashing in x86-64, and we
never found out why we needed to save more stack than needed.

Sorry that may sound like a FUD message but I can't explain
the reason of this, and I fear we may met it again.

Well, at least that may help us finding out the real resons of
such crashes, but...

Re: [PATCH 2/4] tracing/function-graph: x86_64 stack allocation cleanup

[Steven Rostedt]

On Sun, 2009-09-13 at 19:05 +0200, Frederic Weisbecker wrote:
> On Sat, Sep 12, 2009 at 11:05:45PM -0400, Steven Rostedt wrote:
> > From: Jiri Olsa <jolsa@redhat.com>
> > 
> > Only 24 bytes needs to be reserved on the stack for the function graph
> > tracer on x86_64.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> > LKML-Reference: <20090729085837.GB4998@jolsa.lab.eng.brq.redhat.com>
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > ---
> >  arch/x86/kernel/entry_64.S |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> > index c251be7..d59fe32 100644
> > --- a/arch/x86/kernel/entry_64.S
> > +++ b/arch/x86/kernel/entry_64.S
> > @@ -146,7 +146,7 @@ ENTRY(ftrace_graph_caller)
> >  END(ftrace_graph_caller)
> >  
> >  GLOBAL(return_to_handler)
> > -	subq  $80, %rsp
> > +	subq  $24, %rsp
> 
> 
> That's theoretically a good fix.
> 
> But Steve, do you remember the weird issues we had while only
> saving the theoretically strict needed stack space here?
> 
> It made the function graph tracer crashing in x86-64, and we
> never found out why we needed to save more stack than needed.
> 
> Sorry that may sound like a FUD message but I can't explain
> the reason of this, and I fear we may met it again.
> 
> Well, at least that may help us finding out the real resons of
> such crashes, but...

I did not forget about them, and that's the reason that I did not apply
them in the beginning. But that was long ago, and we fixed lots of
issues. I remember hitting crashes with the patch too, but I've applied
this and ran it on those same machines and I no longer get those
crashes. Thus, my thinking is that we already fixed the bug that was
causing it.

Only way to know for sure is to apply it and let it out into the
wild ;-)

-- Steve

Re: [PATCH 2/4] tracing/function-graph: x86_64 stack allocation cleanup

[Frederic Weisbecker]

On Sun, Sep 13, 2009 at 01:47:27PM -0400, Steven Rostedt wrote:
> On Sun, 2009-09-13 at 19:05 +0200, Frederic Weisbecker wrote:
> > On Sat, Sep 12, 2009 at 11:05:45PM -0400, Steven Rostedt wrote:
> > > From: Jiri Olsa <jolsa@redhat.com>
> > > 
> > > Only 24 bytes needs to be reserved on the stack for the function graph
> > > tracer on x86_64.
> > > 
> > > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> > > LKML-Reference: <20090729085837.GB4998@jolsa.lab.eng.brq.redhat.com>
> > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > > ---
> > >  arch/x86/kernel/entry_64.S |    6 +++---
> > >  1 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> > > index c251be7..d59fe32 100644
> > > --- a/arch/x86/kernel/entry_64.S
> > > +++ b/arch/x86/kernel/entry_64.S
> > > @@ -146,7 +146,7 @@ ENTRY(ftrace_graph_caller)
> > >  END(ftrace_graph_caller)
> > >  
> > >  GLOBAL(return_to_handler)
> > > -	subq  $80, %rsp
> > > +	subq  $24, %rsp
> > 
> > 
> > That's theoretically a good fix.
> > 
> > But Steve, do you remember the weird issues we had while only
> > saving the theoretically strict needed stack space here?
> > 
> > It made the function graph tracer crashing in x86-64, and we
> > never found out why we needed to save more stack than needed.
> > 
> > Sorry that may sound like a FUD message but I can't explain
> > the reason of this, and I fear we may met it again.
> > 
> > Well, at least that may help us finding out the real resons of
> > such crashes, but...
> 
> I did not forget about them, and that's the reason that I did not apply
> them in the beginning. But that was long ago, and we fixed lots of
> issues. I remember hitting crashes with the patch too, but I've applied
> this and ran it on those same machines and I no longer get those
> crashes. Thus, my thinking is that we already fixed the bug that was
> causing it.
> 
> Only way to know for sure is to apply it and let it out into the
> wild ;-)


Ok, fine then :)