[PATCH] perf tools: Add missing parameters documentation

[PATCH] perf tools: Add missing parameters documentation

[Frederic Weisbecker]

Add missing documentation for the following parameters:

- perf record -R
- perf report -g

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt |    4 ++++
 tools/perf/Documentation/perf-report.txt |   10 ++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 6be696b..0ff23de 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -91,6 +91,10 @@ OPTIONS
 --no-samples::
 	Don't sample.
 
+-R::
+--raw-samples::
+Collect raw sample records from all opened counters (typically for tracepoint counters).
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 370344a..59f0b84 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -51,6 +51,16 @@ OPTIONS
 	all occurances of this separator in symbol names (and other output)
 	with a '.' character, that thus it's the only non valid separator.
 
+-g [type,min]::
+--call-graph::
+        Display callchains using type and min percent threshold.
+	type can be either:
+	- flat: single column, linear exposure of callchains.
+	- graph: use a graph tree, displaying absolute overhead rates.
+	- fractal: like graph, but displays relative rates. Each branch of
+		 the tree is considered as a new profiled object. +
+	Default: fractal,0.5.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1]
-- 
1.6.2.3

[PATCH 1/4] perf tools: Librarize idle thread registration

[Frederic Weisbecker]

Librarize register_idle_thread() used by annotate and report.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c |   13 +------------
 tools/perf/builtin-report.c   |   32 +++++++++++++++-----------------
 tools/perf/util/thread.c      |   13 +++++++++++++
 tools/perf/util/thread.h      |    2 ++
 4 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4c7bc44..043d85b 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -472,17 +472,6 @@ static void output__resort(void)
 	}
 }
 
-static void register_idle_thread(void)
-{
-	struct thread *thread = threads__findnew(0, &threads, &last_match);
-
-	if (thread == NULL ||
-			thread__set_comm(thread, "[idle]")) {
-		fprintf(stderr, "problem inserting idle task.\n");
-		exit(-1);
-	}
-}
-
 static unsigned long total = 0,
 		     total_mmap = 0,
 		     total_comm = 0,
@@ -970,7 +959,7 @@ static int __cmd_annotate(void)
 	uint32_t size;
 	char *buf;
 
-	register_idle_thread();
+	register_idle_thread(&threads, &last_match);
 
 	input = open(input_name, O_RDONLY);
 	if (input < 0) {
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index cdd46ab..cdf9a8d 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -666,12 +666,9 @@ static void dso__calc_col_width(struct dso *self)
 	self->slen_calculated = 1;
 }
 
-static int thread__set_comm_adjust(struct thread *self, const char *comm)
+static void thread__comm_adjust(struct thread *self)
 {
-	int ret = thread__set_comm(self, comm);
-
-	if (ret)
-		return ret;
+	char *comm = self->comm;
 
 	if (!col_width_list_str && !field_sep &&
 	    (!comm_list || strlist__has_entry(comm_list, comm))) {
@@ -682,6 +679,16 @@ static int thread__set_comm_adjust(struct thread *self, const char *comm)
 			threads__col_width = slen + 6;
 		}
 	}
+}
+
+static int thread__set_comm_adjust(struct thread *self, const char *comm)
+{
+	int ret = thread__set_comm(self, comm);
+
+	if (ret)
+		return ret;
+
+	thread__comm_adjust(self);
 
 	return 0;
 }
@@ -1073,17 +1080,6 @@ print_entries:
 	return ret;
 }
 
-static void register_idle_thread(void)
-{
-	struct thread *thread = threads__findnew(0, &threads, &last_match);
-
-	if (thread == NULL ||
-			thread__set_comm_adjust(thread, "[idle]")) {
-		fprintf(stderr, "problem inserting idle task.\n");
-		exit(-1);
-	}
-}
-
 static unsigned long total = 0,
 		     total_mmap = 0,
 		     total_comm = 0,
@@ -1381,11 +1377,13 @@ static int __cmd_report(void)
 	unsigned long offset = 0;
 	unsigned long head, shift;
 	struct stat input_stat;
+	struct thread *idle;
 	event_t *event;
 	uint32_t size;
 	char *buf;
 
-	register_idle_thread();
+	idle = register_idle_thread(&threads, &last_match);
+	thread__comm_adjust(idle);
 
 	if (show_threads)
 		perf_read_values_init(&show_threads_values);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index f98032c..3acd37f 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -80,6 +80,19 @@ threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match)
 	return th;
 }
 
+struct thread *
+register_idle_thread(struct rb_root *threads, struct thread **last_match)
+{
+	struct thread *thread = threads__findnew(0, threads, last_match);
+
+	if (!thread || thread__set_comm(thread, "[idle]")) {
+		fprintf(stderr, "problem inserting idle task.\n");
+		exit(-1);
+	}
+
+	return thread;
+}
+
 void thread__insert_map(struct thread *self, struct map *map)
 {
 	struct map *pos, *tmp;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index b1c6671..634f280 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -13,6 +13,8 @@ struct thread {
 int thread__set_comm(struct thread *self, const char *comm);
 struct thread *
 threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match);
+struct thread *
+register_idle_thread(struct rb_root *threads, struct thread **last_match);
 void thread__insert_map(struct thread *self, struct map *map);
 int thread__fork(struct thread *self, struct thread *parent);
 struct map *thread__find_map(struct thread *self, u64 ip);
-- 
1.6.2.3

[PATCH 2/4] perf tools: Resolve idle thread cmdline for perf trace

[Frederic Weisbecker]

The cmd-trace tool used the cmdline file and resolved the idle thread
using a hardcoded check for the 0 task pid.

Now we have a centralized way to do that from perf using
register_idle_thread() API.

Before:
	:0-0     [000]     0.000000: irq_handler_entry: irq=0 handler=name
	:0-0     [000]     0.000000: irq_handler_entry: irq=0 handler=name

After:
	[idle]-0     [000]     0.000000: irq_handler_entry: irq=0 handler=name
	[idle]-0     [000]     0.000000: irq_handler_entry: irq=0 handler=name

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-trace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index dd3c2e7..8247fd0 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -164,6 +164,7 @@ static int __cmd_trace(void)
 	char *buf;
 
 	trace_report();
+	register_idle_thread(&threads, &last_match);
 
 	input = open(input_name, O_RDONLY);
 	if (input < 0) {
-- 
1.6.2.3

[PATCH 3/4] perf tools: Unify swapper tasks naming

[Frederic Weisbecker]

In perf tools, we hardcode the pid 0 cmdline resolving to "idle"
because the init task is not included in the COMM events.

But the idle tasks secondary cpus are resolved into their "init"
name through the COMM events.
We have then such strange result in perf report (ditto with trace):

    19.66%       init    [kernel]          [k] acpi_idle_enter_c1
    17.32%       [idle]  [kernel]          [k] acpi_idle_enter_c1

It's then better to unify the swapper tasks into a single init name.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 3acd37f..7635928 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -85,7 +85,7 @@ register_idle_thread(struct rb_root *threads, struct thread **last_match)
 {
 	struct thread *thread = threads__findnew(0, threads, last_match);
 
-	if (!thread || thread__set_comm(thread, "[idle]")) {
+	if (!thread || thread__set_comm(thread, "[init]")) {
 		fprintf(stderr, "problem inserting idle task.\n");
 		exit(-1);
 	}
-- 
1.6.2.3

[PATCH 4/4] perf tools: Complete support for dynamic strings

[Frederic Weisbecker]

Complete support for __str_loc type strings of ftrace events which
have dynamic offsets values set for each of them inside their sammples.

Before:
        geany-5759  [000]     0.000000: lock_release: name
        geany-5759  [000]     0.000000: lock_release: name
        geany-5759  [000]     0.000000: lock_release: name
  kondemand/0-362   [000]     0.000000: lock_release: name
      pdflush-421   [000]     0.000000: lock_release: name

After:
        geany-5759  [000]     0.000000: lock_release: &u->lock
        geany-5759  [000]     0.000000: lock_release: key
        geany-5759  [000]     0.000000: lock_release: &group->notification_mutex
  kondemand/0-362   [000]     0.000000: lock_release: &rq->lock
      pdflush-421   [000]     0.000000: lock_release: &rq->lock

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 tools/perf/util/trace-event-parse.c |   16 ++++++++++++++--
 tools/perf/util/trace-event.h       |    1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 96c5e97..665dac2 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -1548,6 +1548,7 @@ process_str(struct event *event __unused, struct print_arg *arg, char **tok)
 
 	arg->type = PRINT_STRING;
 	arg->string.string = token;
+	arg->string.offset = -1;
 
 	if (read_expected(EVENT_DELIM, (char *)")") < 0)
 		return EVENT_ERROR;
@@ -2031,9 +2032,20 @@ static void print_str_arg(void *data, int size,
 
 	case PRINT_TYPE:
 		break;
-	case PRINT_STRING:
-		printf("%s", arg->string.string);
+	case PRINT_STRING: {
+		int str_offset;
+
+		if (arg->string.offset == -1) {
+			struct format_field *f;
+
+			f = find_any_field(event, arg->string.string);
+			arg->string.offset = f->offset;
+		}
+		str_offset = *(int *)(data + arg->string.offset);
+		str_offset &= 0xffff;
+		printf("%s", ((char *)data) + str_offset);
 		break;
+	}
 	case PRINT_OP:
 		/*
 		 * The only op for string should be ? :
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index 051fcf3..420294a 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -50,6 +50,7 @@ struct print_arg_atom {
 
 struct print_arg_string {
 	char			*string;
+	int			offset;
 };
 
 struct print_arg_field {
-- 
1.6.2.3

Re: [PATCH 3/4] perf tools: Unify swapper tasks naming

[Frederic Weisbecker]

On Mon, Aug 31, 2009 at 06:45:20AM +0200, Frederic Weisbecker wrote:
> In perf tools, we hardcode the pid 0 cmdline resolving to "idle"
> because the init task is not included in the COMM events.
> 
> But the idle tasks secondary cpus are resolved into their "init"
> name through the COMM events.
> We have then such strange result in perf report (ditto with trace):
> 
>     19.66%       init    [kernel]          [k] acpi_idle_enter_c1
>     17.32%       [idle]  [kernel]          [k] acpi_idle_enter_c1
>


And BTW, idle/boot cpu is the only task that has square brackets.
Either these should be removed or we could add these brackets
to every kernel task coms.

I've tried with the following patch. It doesn't change anything, I'm not
sure why, I haven't yet investigate much for now:

diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index d988dfb..3753c39 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -3230,10 +3230,15 @@ static void perf_counter_comm_event(struct perf_comm_event *comm_event)
 	struct perf_cpu_context *cpuctx;
 	struct perf_counter_context *ctx;
 	unsigned int size;
-	char comm[TASK_COMM_LEN];
+	char comm[TASK_COMM_LEN + 2]; /* Also reserve square backets */
 
 	memset(comm, 0, sizeof(comm));
-	strncpy(comm, comm_event->task->comm, sizeof(comm));
+
+	if (comm_event->task->mm)
+		strncpy(comm, comm_event->task->comm, sizeof(comm));
+	else
+		snprintf(comm, sizeof(comm), "[%s]", comm_event->task->comm);
+
 	size = ALIGN(strlen(comm)+1, sizeof(u64));
 
 	comm_event->comm = comm;

Re: [PATCH 4/4] perf tools: Complete support for dynamic strings

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Complete support for __str_loc type strings of ftrace events which
> have dynamic offsets values set for each of them inside their sammples.
> 
> Before:
>         geany-5759  [000]     0.000000: lock_release: name
>         geany-5759  [000]     0.000000: lock_release: name
>         geany-5759  [000]     0.000000: lock_release: name
>   kondemand/0-362   [000]     0.000000: lock_release: name
>       pdflush-421   [000]     0.000000: lock_release: name
> 
> After:
>         geany-5759  [000]     0.000000: lock_release: &u->lock
>         geany-5759  [000]     0.000000: lock_release: key
>         geany-5759  [000]     0.000000: lock_release: &group->notification_mutex
>   kondemand/0-362   [000]     0.000000: lock_release: &rq->lock
>       pdflush-421   [000]     0.000000: lock_release: &rq->lock

I've applied all five patches, thanks Frederic!

Also, i think this makes perf trace quite functional already so i 
merged that topic (and these commits) into tracing/core, for v2.6.32 
merging.

One thing that i noticed which is still quite quirky are the 
parameters to perf record. (needing -R and the :r postfix to events) 
Could something more intuitive be done here?

Also, i think people would like to use wildcards in event 
specifiers, such as:

  perf record -e timer/*

To capture all timer events:

 hrtimer_cancel
 hrtimer_expire_entry
 hrtimer_expire_exit
 hrtimer_init
 hrtimer_start
 itimer_expire
 itimer_state
 timer_cancel
 timer_expire_entry
 timer_expire_exit
 timer_init
 timer_start

Instead of having to type:

perf record -e hrtimer_cancel -e hrtimer_expire_entry \
 -e hrtimer_expire_exit -e hrtimer_init -e hrtimer_start \
 -e itimer_expire -e itimer_state -e timer_cancel \
 -e timer_expire_entry -e timer_expire_exit -e timer_init \
 -e timer_start

which is not quite realistic.

	Ingo

[tip:perfcounters/core] perf tools: Add missing parameters documentation

[tip-bot for Frederic Weisbecker]

Commit-ID:  ec7ba4ea1d605029fb09601ab4ad3053bc1f519c
Gitweb:     http://git.kernel.org/tip/ec7ba4ea1d605029fb09601ab4ad3053bc1f519c
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 31 Aug 2009 03:32:03 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 31 Aug 2009 10:04:47 +0200

perf tools: Add missing parameters documentation

Add missing documentation for the following parameters:

- perf record -R
- perf report -g

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
LKML-Reference: <1251682323-10395-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 tools/perf/Documentation/perf-record.txt |    4 ++++
 tools/perf/Documentation/perf-report.txt |   10 ++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 6be696b..0ff23de 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -91,6 +91,10 @@ OPTIONS
 --no-samples::
 	Don't sample.
 
+-R::
+--raw-samples::
+Collect raw sample records from all opened counters (typically for tracepoint counters).
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 370344a..59f0b84 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -51,6 +51,16 @@ OPTIONS
 	all occurances of this separator in symbol names (and other output)
 	with a '.' character, that thus it's the only non valid separator.
 
+-g [type,min]::
+--call-graph::
+        Display callchains using type and min percent threshold.
+	type can be either:
+	- flat: single column, linear exposure of callchains.
+	- graph: use a graph tree, displaying absolute overhead rates.
+	- fractal: like graph, but displays relative rates. Each branch of
+		 the tree is considered as a new profiled object. +
+	Default: fractal,0.5.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1]

[tip:perfcounters/core] perf tools: Librarize idle thread registration

[tip-bot for Frederic Weisbecker]

Commit-ID:  5b447a6a13ea823b698bf4c01193654fd7ebf4ec
Gitweb:     http://git.kernel.org/tip/5b447a6a13ea823b698bf4c01193654fd7ebf4ec
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 31 Aug 2009 06:45:18 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 31 Aug 2009 10:04:48 +0200

perf tools: Librarize idle thread registration

Librarize register_idle_thread() used by annotate and report.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
LKML-Reference: <1251693921-6579-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 tools/perf/builtin-annotate.c |   13 +------------
 tools/perf/builtin-report.c   |   32 +++++++++++++++-----------------
 tools/perf/util/thread.c      |   13 +++++++++++++
 tools/perf/util/thread.h      |    2 ++
 4 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4c7bc44..043d85b 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -472,17 +472,6 @@ static void output__resort(void)
 	}
 }
 
-static void register_idle_thread(void)
-{
-	struct thread *thread = threads__findnew(0, &threads, &last_match);
-
-	if (thread == NULL ||
-			thread__set_comm(thread, "[idle]")) {
-		fprintf(stderr, "problem inserting idle task.\n");
-		exit(-1);
-	}
-}
-
 static unsigned long total = 0,
 		     total_mmap = 0,
 		     total_comm = 0,
@@ -970,7 +959,7 @@ static int __cmd_annotate(void)
 	uint32_t size;
 	char *buf;
 
-	register_idle_thread();
+	register_idle_thread(&threads, &last_match);
 
 	input = open(input_name, O_RDONLY);
 	if (input < 0) {
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index cdd46ab..cdf9a8d 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -666,12 +666,9 @@ static void dso__calc_col_width(struct dso *self)
 	self->slen_calculated = 1;
 }
 
-static int thread__set_comm_adjust(struct thread *self, const char *comm)
+static void thread__comm_adjust(struct thread *self)
 {
-	int ret = thread__set_comm(self, comm);
-
-	if (ret)
-		return ret;
+	char *comm = self->comm;
 
 	if (!col_width_list_str && !field_sep &&
 	    (!comm_list || strlist__has_entry(comm_list, comm))) {
@@ -682,6 +679,16 @@ static int thread__set_comm_adjust(struct thread *self, const char *comm)
 			threads__col_width = slen + 6;
 		}
 	}
+}
+
+static int thread__set_comm_adjust(struct thread *self, const char *comm)
+{
+	int ret = thread__set_comm(self, comm);
+
+	if (ret)
+		return ret;
+
+	thread__comm_adjust(self);
 
 	return 0;
 }
@@ -1073,17 +1080,6 @@ print_entries:
 	return ret;
 }
 
-static void register_idle_thread(void)
-{
-	struct thread *thread = threads__findnew(0, &threads, &last_match);
-
-	if (thread == NULL ||
-			thread__set_comm_adjust(thread, "[idle]")) {
-		fprintf(stderr, "problem inserting idle task.\n");
-		exit(-1);
-	}
-}
-
 static unsigned long total = 0,
 		     total_mmap = 0,
 		     total_comm = 0,
@@ -1381,11 +1377,13 @@ static int __cmd_report(void)
 	unsigned long offset = 0;
 	unsigned long head, shift;
 	struct stat input_stat;
+	struct thread *idle;
 	event_t *event;
 	uint32_t size;
 	char *buf;
 
-	register_idle_thread();
+	idle = register_idle_thread(&threads, &last_match);
+	thread__comm_adjust(idle);
 
 	if (show_threads)
 		perf_read_values_init(&show_threads_values);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index f98032c..3acd37f 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -80,6 +80,19 @@ threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match)
 	return th;
 }
 
+struct thread *
+register_idle_thread(struct rb_root *threads, struct thread **last_match)
+{
+	struct thread *thread = threads__findnew(0, threads, last_match);
+
+	if (!thread || thread__set_comm(thread, "[idle]")) {
+		fprintf(stderr, "problem inserting idle task.\n");
+		exit(-1);
+	}
+
+	return thread;
+}
+
 void thread__insert_map(struct thread *self, struct map *map)
 {
 	struct map *pos, *tmp;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index b1c6671..634f280 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -13,6 +13,8 @@ struct thread {
 int thread__set_comm(struct thread *self, const char *comm);
 struct thread *
 threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match);
+struct thread *
+register_idle_thread(struct rb_root *threads, struct thread **last_match);
 void thread__insert_map(struct thread *self, struct map *map);
 int thread__fork(struct thread *self, struct thread *parent);
 struct map *thread__find_map(struct thread *self, u64 ip);

[tip:perfcounters/core] perf tools: Resolve idle thread cmdline for perf trace

[tip-bot for Frederic Weisbecker]

Commit-ID:  3a2684ca58e06941ff00e3f096ca44f898a6d13e
Gitweb:     http://git.kernel.org/tip/3a2684ca58e06941ff00e3f096ca44f898a6d13e
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 31 Aug 2009 06:45:19 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 31 Aug 2009 10:04:48 +0200

perf tools: Resolve idle thread cmdline for perf trace

The cmd-trace tool used the cmdline file and resolved the idle
thread using a hardcoded check for the 0 task pid.

Now we have a centralized way to do that from perf using
register_idle_thread() API.

Before:
	:0-0     [000]     0.000000: irq_handler_entry: irq=0 handler=name
	:0-0     [000]     0.000000: irq_handler_entry: irq=0 handler=name

After:
	[idle]-0     [000]     0.000000: irq_handler_entry: irq=0 handler=name
	[idle]-0     [000]     0.000000: irq_handler_entry: irq=0 handler=name

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
LKML-Reference: <1251693921-6579-2-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 tools/perf/builtin-trace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index dd3c2e7..8247fd0 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -164,6 +164,7 @@ static int __cmd_trace(void)
 	char *buf;
 
 	trace_report();
+	register_idle_thread(&threads, &last_match);
 
 	input = open(input_name, O_RDONLY);
 	if (input < 0) {

[tip:perfcounters/core] perf tools: Unify swapper tasks naming

[tip-bot for Frederic Weisbecker]

Commit-ID:  9b8055a52c8986167e0a7357460d528a00db67e6
Gitweb:     http://git.kernel.org/tip/9b8055a52c8986167e0a7357460d528a00db67e6
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 31 Aug 2009 06:45:20 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 31 Aug 2009 10:04:49 +0200

perf tools: Unify swapper tasks naming

In perf tools, we hardcode the pid 0 cmdline resolving to
"idle" because the init task is not included in the COMM
events.

But the idle tasks secondary cpus are resolved into their
"init" name through the COMM events.

We have then such strange result in perf report (ditto with
trace):

    19.66%       init    [kernel]          [k] acpi_idle_enter_c1
    17.32%       [idle]  [kernel]          [k] acpi_idle_enter_c1

It's then better to unify the swapper tasks into a single init
name.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
LKML-Reference: <1251693921-6579-3-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>


---
 tools/perf/util/thread.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 3acd37f..7635928 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -85,7 +85,7 @@ register_idle_thread(struct rb_root *threads, struct thread **last_match)
 {
 	struct thread *thread = threads__findnew(0, threads, last_match);
 
-	if (!thread || thread__set_comm(thread, "[idle]")) {
+	if (!thread || thread__set_comm(thread, "[init]")) {
 		fprintf(stderr, "problem inserting idle task.\n");
 		exit(-1);
 	}

[tip:perfcounters/core] perf tools: Complete support for dynamic strings

[tip-bot for Frederic Weisbecker]

Commit-ID:  561f732c1233f6bf7c3c5c5fb9b4d90bb6c107aa
Gitweb:     http://git.kernel.org/tip/561f732c1233f6bf7c3c5c5fb9b4d90bb6c107aa
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 31 Aug 2009 06:45:21 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 31 Aug 2009 10:04:49 +0200

perf tools: Complete support for dynamic strings

Complete support for __str_loc type strings of ftrace events
which have dynamic offsets values set for each of them inside
their sammples.

Before:
        geany-5759  [000]     0.000000: lock_release: name
        geany-5759  [000]     0.000000: lock_release: name
        geany-5759  [000]     0.000000: lock_release: name
  kondemand/0-362   [000]     0.000000: lock_release: name
      pdflush-421   [000]     0.000000: lock_release: name

After:
        geany-5759  [000]     0.000000: lock_release: &u->lock
        geany-5759  [000]     0.000000: lock_release: key
        geany-5759  [000]     0.000000: lock_release: &group->notification_mutex
  kondemand/0-362   [000]     0.000000: lock_release: &rq->lock
      pdflush-421   [000]     0.000000: lock_release: &rq->lock

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <1251693921-6579-4-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>


---
 tools/perf/util/trace-event-parse.c |   16 ++++++++++++++--
 tools/perf/util/trace-event.h       |    1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 96c5e97..665dac2 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -1548,6 +1548,7 @@ process_str(struct event *event __unused, struct print_arg *arg, char **tok)
 
 	arg->type = PRINT_STRING;
 	arg->string.string = token;
+	arg->string.offset = -1;
 
 	if (read_expected(EVENT_DELIM, (char *)")") < 0)
 		return EVENT_ERROR;
@@ -2031,9 +2032,20 @@ static void print_str_arg(void *data, int size,
 
 	case PRINT_TYPE:
 		break;
-	case PRINT_STRING:
-		printf("%s", arg->string.string);
+	case PRINT_STRING: {
+		int str_offset;
+
+		if (arg->string.offset == -1) {
+			struct format_field *f;
+
+			f = find_any_field(event, arg->string.string);
+			arg->string.offset = f->offset;
+		}
+		str_offset = *(int *)(data + arg->string.offset);
+		str_offset &= 0xffff;
+		printf("%s", ((char *)data) + str_offset);
 		break;
+	}
 	case PRINT_OP:
 		/*
 		 * The only op for string should be ? :
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index 051fcf3..420294a 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -50,6 +50,7 @@ struct print_arg_atom {
 
 struct print_arg_string {
 	char			*string;
+	int			offset;
 };
 
 struct print_arg_field {

Re: [PATCH 4/4] perf tools: Complete support for dynamic strings

[Frederic Weisbecker]

On Mon, Aug 31, 2009 at 10:11:33AM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > Complete support for __str_loc type strings of ftrace events which
> > have dynamic offsets values set for each of them inside their sammples.
> > 
> > Before:
> >         geany-5759  [000]     0.000000: lock_release: name
> >         geany-5759  [000]     0.000000: lock_release: name
> >         geany-5759  [000]     0.000000: lock_release: name
> >   kondemand/0-362   [000]     0.000000: lock_release: name
> >       pdflush-421   [000]     0.000000: lock_release: name
> > 
> > After:
> >         geany-5759  [000]     0.000000: lock_release: &u->lock
> >         geany-5759  [000]     0.000000: lock_release: key
> >         geany-5759  [000]     0.000000: lock_release: &group->notification_mutex
> >   kondemand/0-362   [000]     0.000000: lock_release: &rq->lock
> >       pdflush-421   [000]     0.000000: lock_release: &rq->lock
> 
> I've applied all five patches, thanks Frederic!
> 
> Also, i think this makes perf trace quite functional already so i 
> merged that topic (and these commits) into tracing/core, for v2.6.32 
> merging.



Ok.


 
> One thing that i noticed which is still quite quirky are the 
> parameters to perf record. (needing -R and the :r postfix to events) 
> Could something more intuitive be done here?


I hope. It's not that easy, at least for me, to find something intuitive
there.

I've thought about

  -e *sys:event  (because the * suggests a deref that takes the content)
                 But that's a bit convoluted. And moreover we may want
                 to support basic regex later for event selection.

If someone has an idea, please tell me.

Another thing. We could, by default display a small comment while
selecting an event without its content:

"Opening counter sys:event. Type -R if you want raw sampling."

The user could shut that up with a -q option.

> Also, i think people would like to use wildcards in event 
> specifiers, such as:
> 
>   perf record -e timer/*
> 
> To capture all timer events:
> 
>  hrtimer_cancel
>  hrtimer_expire_entry
>  hrtimer_expire_exit
>  hrtimer_init
>  hrtimer_start
>  itimer_expire
>  itimer_state
>  timer_cancel
>  timer_expire_entry
>  timer_expire_exit
>  timer_init
>  timer_start
>
> Instead of having to type:
> 
> perf record -e hrtimer_cancel -e hrtimer_expire_entry \
>  -e hrtimer_expire_exit -e hrtimer_init -e hrtimer_start \
>  -e itimer_expire -e itimer_state -e timer_cancel \
>  -e timer_expire_entry -e timer_expire_exit -e timer_init \
>  -e timer_start
> 
> which is not quite realistic.


Totally agreed, it was in my todo list.

But don't you think
	perf record -e timer:*
is more intuitive and follows the current logic?

Re: [PATCH 4/4] perf tools: Complete support for dynamic strings

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Mon, Aug 31, 2009 at 10:11:33AM +0200, Ingo Molnar wrote:
> > 
> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > 
> > > Complete support for __str_loc type strings of ftrace events which
> > > have dynamic offsets values set for each of them inside their sammples.
> > > 
> > > Before:
> > >         geany-5759  [000]     0.000000: lock_release: name
> > >         geany-5759  [000]     0.000000: lock_release: name
> > >         geany-5759  [000]     0.000000: lock_release: name
> > >   kondemand/0-362   [000]     0.000000: lock_release: name
> > >       pdflush-421   [000]     0.000000: lock_release: name
> > > 
> > > After:
> > >         geany-5759  [000]     0.000000: lock_release: &u->lock
> > >         geany-5759  [000]     0.000000: lock_release: key
> > >         geany-5759  [000]     0.000000: lock_release: &group->notification_mutex
> > >   kondemand/0-362   [000]     0.000000: lock_release: &rq->lock
> > >       pdflush-421   [000]     0.000000: lock_release: &rq->lock
> > 
> > I've applied all five patches, thanks Frederic!
> > 
> > Also, i think this makes perf trace quite functional already so i 
> > merged that topic (and these commits) into tracing/core, for v2.6.32 
> > merging.
> 
> 
> 
> Ok.
> 
> 
>  
> > One thing that i noticed which is still quite quirky are the 
> > parameters to perf record. (needing -R and the :r postfix to events) 
> > Could something more intuitive be done here?
> 
> 
> I hope. It's not that easy, at least for me, to find something intuitive
> there.
> 
> I've thought about
> 
>   -e *sys:event  (because the * suggests a deref that takes the content)
>                  But that's a bit convoluted. And moreover we may want
>                  to support basic regex later for event selection.
> 
> If someone has an idea, please tell me.
> 
> Another thing. We could, by default display a small comment while
> selecting an event without its content:
> 
> "Opening counter sys:event. Type -R if you want raw sampling."
> 
> The user could shut that up with a -q option.

The confusion comes from the multi-purpose nature of 'perf record'. 
It is not clear from that workflow whether the resulting perf.data 
will be for trace reporting or for profiling/call-graph analysis.

I think we could solve some of this confusion by always gathering 
the most information we can - and allow the user to trim down the 
data if he wants to.

I.e. PERF_SAMPLE_RAW should be implicit if tracepoints are 
specified.

An additional measure would be to simplify the tracing workflow: a 
simple 'perf trace -e sched:switch /bin/ls' should really do 
implicit recording and should report the trace after that.

	Ingo