[GIT PULL 0/9] perf/core fixes and improvements

[GIT PULL 0/9] perf/core fixes and improvements

[Arnaldo Carvalho de Melo]

Hi Ingo,

        Please consider pulling.

Regards,

- Arnaldo

The following changes since commit b9f616bbf4a917398aa09db89efbdf9a204e80dc:

  Merge branch 'perf/core' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2012-01-08 12:36:18 +0100)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core

for you to fetch changes up to 172d1b0b73256551f100fc00c69e356d047103f5:

  perf tools: Fix compile error on x86_64 Ubuntu (2012-01-08 13:34:55 -0200)

----------------------------------------------------------------
Namhyung Kim (9):
      perf script: Add missing closedir() calls
      perf test: Change type of '-v' option to INCR
      perf top: Add error message for EMFILE
      perf kmem: Add missing closedir() calls
      perf kmem: Fix a memory leak
      perf annotate: Fix usage string
      perf annotate: Get rid of field_sep check
      perf report: Fix --stdio output alignment when --showcpuutilization used
      perf tools: Fix compile error on x86_64 Ubuntu

 tools/perf/builtin-annotate.c      |    7 +------
 tools/perf/builtin-kmem.c          |    3 +++
 tools/perf/builtin-script.c        |    4 ++++
 tools/perf/builtin-test.c          |    2 +-
 tools/perf/builtin-top.c           |    4 ++++
 tools/perf/util/hist.c             |   34 ++++++++++++++++++----------------
 tools/perf/util/trace-event-info.c |    1 -
 7 files changed, 31 insertions(+), 24 deletions(-)

[PATCH 1/9] perf script: Add missing closedir() calls

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

The get_script_path() calls opendir() but misses corresponding
closedir()'s. Add them.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1325957132-10600-1-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-script.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index fd1909a..bb68ddf 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1018,13 +1018,17 @@ static char *get_script_path(const char *script_root, const char *suffix)
 			__script_root = get_script_root(&script_dirent, suffix);
 			if (__script_root && !strcmp(script_root, __script_root)) {
 				free(__script_root);
+				closedir(lang_dir);
+				closedir(scripts_dir);
 				snprintf(script_path, MAXPATHLEN, "%s/%s",
 					 lang_path, script_dirent.d_name);
 				return strdup(script_path);
 			}
 			free(__script_root);
 		}
+		closedir(lang_dir);
 	}
+	closedir(scripts_dir);
 
 	return NULL;
 }
-- 
1.7.8.GIT

[PATCH 2/9] perf test: Change type of '-v' option to INCR

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

The '-v' option is usually defined via OPT_INCR not _INTEGER.  Follow
the trend :).

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1325957132-10600-2-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-test.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 2b9a7f4..3854e86 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1396,7 +1396,7 @@ int cmd_test(int argc, const char **argv, const char *prefix __used)
 	NULL,
 	};
 	const struct option test_options[] = {
-	OPT_INTEGER('v', "verbose", &verbose,
+	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show symbol address, etc)"),
 	OPT_END()
 	};
-- 
1.7.8.GIT

[PATCH 3/9] perf top: Add error message for EMFILE

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

When a user tries to open so many events, perf_event_open syscall may
fail with EMFILE. Provide advise for that case.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1325957132-10600-3-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index d89dec9..8f80df8 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -888,6 +888,10 @@ try_again:
 				ui__warning("The %s event is not supported.\n",
 					    event_name(counter));
 				goto out_err;
+			} else if (err == EMFILE) {
+				ui__warning("Too many events are opened.\n"
+					    "Try again after reducing the number of events\n");
+				goto out_err;
 			}
 
 			ui__warning("The sys_perf_event_open() syscall "
-- 
1.7.8.GIT

[PATCH 4/9] perf kmem: Add missing closedir() calls

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

The setup_cpunode_map() calls opendir() but misses corresponding
closedir(). Add them.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1325957132-10600-4-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-kmem.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index fe1ad8f..7a9b5c5 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -108,7 +108,9 @@ static void setup_cpunode_map(void)
 				continue;
 			cpunode_map[cpu] = mem;
 		}
+		closedir(dir2);
 	}
+	closedir(dir1);
 }
 
 static void insert_alloc_stat(unsigned long call_site, unsigned long ptr,
-- 
1.7.8.GIT

[PATCH 5/9] perf kmem: Fix a memory leak

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

The 'str' should be freed when sort_dimension__add() failed too.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1325957132-10600-5-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-kmem.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 7a9b5c5..39104c0 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -647,6 +647,7 @@ static int setup_sorting(struct list_head *sort_list, const char *arg)
 			break;
 		if (sort_dimension__add(tok, sort_list) < 0) {
 			error("Unknown --sort key: '%s'", tok);
+			free(str);
 			return -1;
 		}
 	}
-- 
1.7.8.GIT

[PATCH 6/9] perf annotate: Fix usage string

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

The annotate command doesn't take non-option arguments.

In fact, it can take last argument as a symbol filter though, but that's
a special case and, IMHO, it should be discouraged in favor of the -s
option.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1325957132-10600-6-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 214ba7f..3ec2496 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -235,7 +235,7 @@ out_delete:
 }
 
 static const char * const annotate_usage[] = {
-	"perf annotate [<options>] <command>",
+	"perf annotate [<options>]",
 	NULL
 };
 
-- 
1.7.8.GIT

[PATCH 7/9] perf annotate: Get rid of field_sep check

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

The 'field_sep' variable is not set anywhere. Just remove the
conditional.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1325957132-10600-7-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 3ec2496..806e0a2 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -313,10 +313,5 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __used)
 		annotate.sym_hist_filter = argv[0];
 	}
 
-	if (field_sep && *field_sep == '.') {
-		pr_err("'.' is the only non valid --field-separator argument\n");
-		return -1;
-	}
-
 	return __cmd_annotate(&annotate);
 }
-- 
1.7.8.GIT

[PATCH 8/9] perf report: Fix --stdio output alignment when --showcpuutilization used

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

Current perf report output is broken if --showcpuutilization is used.
Combination with -n and/or --show-total-period make things worse.
This patch fixes it as follows:

before:
    48.25%    48.25%     0.00%    sleep  [kernel.kallsyms]  [k] trace_hardirqs_off
    34.99%    34.99%     0.00%    sleep  [kernel.kallsyms]  [k] __find_get_block_slow
    15.99%    15.99%     0.00%    sleep  [kernel.kallsyms]  [k] lock_release_holdtime
     0.77%     0.77%     0.00%    sleep  [kernel.kallsyms]  [k] native_write_msr_safe

after:
    48.25%    48.25%     0.00%    sleep  [kernel.kallsyms]  [k] trace_hardirqs_off
    34.99%    34.99%     0.00%    sleep  [kernel.kallsyms]  [k] __find_get_block_slow
    15.99%    15.99%     0.00%    sleep  [kernel.kallsyms]  [k] lock_release_holdtime
     0.77%     0.77%     0.00%    sleep  [kernel.kallsyms]  [k] native_write_msr_safe

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1325957132-10600-8-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 4df4495..6f505d1 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -919,20 +919,6 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 
 	fprintf(fp, "# %s", pair ? "Baseline" : "Overhead");
 
-	if (symbol_conf.show_nr_samples) {
-		if (sep)
-			fprintf(fp, "%cSamples", *sep);
-		else
-			fputs("  Samples  ", fp);
-	}
-
-	if (symbol_conf.show_total_period) {
-		if (sep)
-			ret += fprintf(fp, "%cPeriod", *sep);
-		else
-			ret += fprintf(fp, "   Period    ");
-	}
-
 	if (symbol_conf.show_cpu_utilization) {
 		if (sep) {
 			ret += fprintf(fp, "%csys", *sep);
@@ -942,8 +928,8 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 				ret += fprintf(fp, "%cguest us", *sep);
 			}
 		} else {
-			ret += fprintf(fp, "  sys  ");
-			ret += fprintf(fp, "  us  ");
+			ret += fprintf(fp, "     sys  ");
+			ret += fprintf(fp, "      us  ");
 			if (perf_guest) {
 				ret += fprintf(fp, "  guest sys  ");
 				ret += fprintf(fp, "  guest us  ");
@@ -951,6 +937,20 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 		}
 	}
 
+	if (symbol_conf.show_nr_samples) {
+		if (sep)
+			fprintf(fp, "%cSamples", *sep);
+		else
+			fputs("  Samples  ", fp);
+	}
+
+	if (symbol_conf.show_total_period) {
+		if (sep)
+			ret += fprintf(fp, "%cPeriod", *sep);
+		else
+			ret += fprintf(fp, "   Period    ");
+	}
+
 	if (pair) {
 		if (sep)
 			ret += fprintf(fp, "%cDelta", *sep);
@@ -995,6 +995,8 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
 		goto print_entries;
 
 	fprintf(fp, "# ........");
+	if (symbol_conf.show_cpu_utilization)
+		fprintf(fp, "   .......   .......");
 	if (symbol_conf.show_nr_samples)
 		fprintf(fp, " ..........");
 	if (symbol_conf.show_total_period)
-- 
1.7.8.GIT

[PATCH 9/9] perf tools: Fix compile error on x86_64 Ubuntu

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@gmail.com>

The ctype.h include is not needed here and it breaks build on some systems (at
least 64bit Ubuntu 10.04) like below. Just get rid of it.

    CC util/trace-event-info.o
cc1: warnings being treated as errors
util/trace-event-info.c: In function ‘record_file’:
util/trace-event-info.c:192: error: implicit declaration of function ‘pwrite’
util/trace-event-info.c:192: error: nested extern declaration of ‘pwrite’
make: *** [util/trace-event-info.o] Error 1

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Joerg Roedel <joerg.roedel@amd.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1326035430-7621-1-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/trace-event-info.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index ac6830d..fc22cf5 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -18,7 +18,6 @@
  *
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
-#include <ctype.h>
 #include "util.h"
 #include <dirent.h>
 #include <mntent.h>
-- 
1.7.8.GIT

Re: [GIT PULL 0/9] perf/core fixes and improvements

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> Hi Ingo,
> 
>         Please consider pulling.
> 
> Regards,
> 
> - Arnaldo
> 
> The following changes since commit b9f616bbf4a917398aa09db89efbdf9a204e80dc:
> 
>   Merge branch 'perf/core' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2012-01-08 12:36:18 +0100)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core
> 
> for you to fetch changes up to 172d1b0b73256551f100fc00c69e356d047103f5:
> 
>   perf tools: Fix compile error on x86_64 Ubuntu (2012-01-08 13:34:55 -0200)
> 
> ----------------------------------------------------------------
> Namhyung Kim (9):
>       perf script: Add missing closedir() calls
>       perf test: Change type of '-v' option to INCR
>       perf top: Add error message for EMFILE
>       perf kmem: Add missing closedir() calls
>       perf kmem: Fix a memory leak
>       perf annotate: Fix usage string
>       perf annotate: Get rid of field_sep check
>       perf report: Fix --stdio output alignment when --showcpuutilization used
>       perf tools: Fix compile error on x86_64 Ubuntu
> 
>  tools/perf/builtin-annotate.c      |    7 +------
>  tools/perf/builtin-kmem.c          |    3 +++
>  tools/perf/builtin-script.c        |    4 ++++
>  tools/perf/builtin-test.c          |    2 +-
>  tools/perf/builtin-top.c           |    4 ++++
>  tools/perf/util/hist.c             |   34 ++++++++++++++++++----------------
>  tools/perf/util/trace-event-info.c |    1 -
>  7 files changed, 31 insertions(+), 24 deletions(-)

Pulled, thanks Arnaldo!

	Ingo

Re: [PATCH 7/9] perf annotate: Get rid of field_sep check

[Namhyung Kim]

2012-01-08 (Sun), 13:42 -0200, Arnaldo Carvalho de Melo wrote:
> From: Namhyung Kim <namhyung@gmail.com>
> 
> The 'field_sep' variable is not set anywhere. Just remove the
> conditional.
> 

BTW, it seems there's no user of the variable now. Maybe a candidate
for removal?

Thanks.

-- 
Regards,
Namhyung Kim