2014-01-21 20:43:43

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 0/8] perf/core fixes

From: Arnaldo Carvalho de Melo <[email protected]>

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit 45e6af06367e7b2eb8dc49671092462d8f8a5f47:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2014-01-19 13:09:01 +0100)

are available in the git repository at:


git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo

for you to fetch changes up to 578c03c86fadcc6fd7319ddf41dd4d1d88aab77a:

perf symbols: Fix JIT symbol resolution on heap (2014-01-21 10:56:05 -0300)

----------------------------------------------------------------
perf/core fixes:

. Fix JIT symbol resolution on heap (Namhyung Kim)

. Fix wrong SVG height in 'timechart' (Stanislav Fomichev)

. Free temp cpu_map in perf_session__cpu_bitmap (Stanislav Fomichev)

. Fix NULL pointer reference bug with event unit in 'stat' (Stephane Eranian)

. Fix memory corruption of xyarray when cpumask is used (Stephane Eranian)

. Ensure sscanf does not overrun the "mem" field (Alan Cox)

. Add support for the xtensa architecture (Baruch Siach)

Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

----------------------------------------------------------------
Alan Cox (1):
perf tools: Ensure sscanf does not overrun the "mem" field

Baruch Siach (1):
perf tools: Add support for the xtensa architecture

Namhyung Kim (1):
perf symbols: Fix JIT symbol resolution on heap

Stanislav Fomichev (2):
perf timechart: Fix wrong SVG height
perf session: Free cpu_map in perf_session__cpu_bitmap

Stephane Eranian (3):
perf stat: fix NULL pointer reference bug with event unit
perf evsel: Remove duplicate member zeroing after free
perf stat: Fix memory corruption of xyarray when cpumask is used

tools/perf/builtin-timechart.c | 3 +++
tools/perf/perf.h | 7 +++++++
tools/perf/util/evlist.c | 7 +++++--
tools/perf/util/evsel.c | 1 -
tools/perf/util/header.c | 2 +-
tools/perf/util/map.c | 4 ++--
tools/perf/util/parse-events.c | 2 +-
tools/perf/util/pmu.c | 24 ++++++++++++++++++++----
tools/perf/util/pmu.h | 2 +-
tools/perf/util/session.c | 10 +++++++---
10 files changed, 47 insertions(+), 15 deletions(-)


2014-01-21 20:43:28

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 3/8] perf tools: Add support for the xtensa architecture

From: Baruch Siach <[email protected]>

Tested using kernel tracepoints on a QEMU simulated environment.

Kernel support for perf depends on the patch "xtensa: enable
HAVE_PERF_EVENTS", which is scheduled for v3.14.

Hardware performance counters are not supported under xtensa yet.

Acked-by: Ingo Molnar <[email protected]>
Acked-by: Max Filippov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/aafcdb22f04e2d3188d2938528939481be56b649.1389608855.git.baruch@tkos.co.il
Signed-off-by: Baruch Siach <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/perf.h | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 3c2f213e979d..7daa806d9050 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -132,6 +132,13 @@
#define CPUINFO_PROC "CPU"
#endif

+#ifdef __xtensa__
+#define mb() asm volatile("memw" ::: "memory")
+#define wmb() asm volatile("memw" ::: "memory")
+#define rmb() asm volatile("" ::: "memory")
+#define CPUINFO_PROC "core ID"
+#endif
+
#define barrier() asm volatile ("" ::: "memory")

#ifndef cpu_relax
--
1.8.1.4

2014-01-21 20:43:33

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 8/8] perf symbols: Fix JIT symbol resolution on heap

From: Namhyung Kim <[email protected]>

Gaurav reported that perf cannot profile JIT program if it executes the
code on heap. This was because current map__new() only handle JIT on
anon mappings - extends it to handle no_dso (heap, stack) case too.

This patch assumes JIT profiling only provides dynamic function symbols
so check the mapping type to distinguish the case. It'd provide no
symbols for data mapping - if we need to support symbols on data
mappings later it should be changed.

Reported-by: Gaurav Jain <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
Tested-by: Gaurav Jain <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Gaurav Jain <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/map.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 9b9bd719aa19..ee1dd687a262 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -69,7 +69,7 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
map->ino = ino;
map->ino_generation = ino_gen;

- if (anon) {
+ if ((anon || no_dso) && type == MAP__FUNCTION) {
snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", pid);
filename = newfilename;
}
@@ -93,7 +93,7 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
* functions still return NULL, and we avoid the
* unnecessary map__load warning.
*/
- if (no_dso)
+ if (type != MAP__FUNCTION)
dso__set_loaded(dso, map->type);
}
}
--
1.8.1.4

2014-01-21 20:43:24

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 5/8] perf tools: Ensure sscanf does not overrun the "mem" field

From: Alan Cox <[email protected]>

Make the parsing robust.

(perf has some other assumptions that BUFSIZE <= MAX_PATH which are
not touched here)

Reported-by: Jackie Chang
Signed-off-by: Alan Cox <[email protected]>
Cc: Alan Cox <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/header.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index bb3e0ede6183..893f8e2df928 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -930,7 +930,7 @@ static int write_topo_node(int fd, int node)
/* skip over invalid lines */
if (!strchr(buf, ':'))
continue;
- if (sscanf(buf, "%*s %*d %s %"PRIu64, field, &mem) != 2)
+ if (sscanf(buf, "%*s %*d %31s %"PRIu64, field, &mem) != 2)
goto done;
if (!strcmp(field, "MemTotal:"))
mem_total = mem;
--
1.8.1.4

2014-01-21 20:44:15

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 4/8] perf stat: fix NULL pointer reference bug with event unit

From: Stephane Eranian <[email protected]>

This patch fixes a problem with the handling of the newly introduced
optional event unit. The following cmdline caused a segfault:

$ perf stat -e cpu/event-0x3c/ ls

This patch fixes the problem with the default setting for alias->unit
which was eventually causing the segfault.

Signed-off-by: Stephane Eranian <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/parse-events.c | 2 +-
tools/perf/util/pmu.c | 24 ++++++++++++++++++++----
tools/perf/util/pmu.h | 2 +-
3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a7f1b6a91fdd..d248fca6d7ed 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -635,7 +635,7 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
struct perf_event_attr attr;
struct perf_pmu *pmu;
struct perf_evsel *evsel;
- char *unit;
+ const char *unit;
double scale;

pmu = perf_pmu__find(name);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index d9cab4d27192..b752ecb40d86 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -105,7 +105,7 @@ static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char *
char scale[128];
int fd, ret = -1;
char path[PATH_MAX];
- char *lc;
+ const char *lc;

snprintf(path, PATH_MAX, "%s/%s.scale", dir, name);

@@ -609,7 +609,7 @@ static struct perf_pmu_alias *pmu_find_alias(struct perf_pmu *pmu,


static int check_unit_scale(struct perf_pmu_alias *alias,
- char **unit, double *scale)
+ const char **unit, double *scale)
{
/*
* Only one term in event definition can
@@ -634,14 +634,18 @@ static int check_unit_scale(struct perf_pmu_alias *alias,
* defined for the alias
*/
int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
- char **unit, double *scale)
+ const char **unit, double *scale)
{
struct parse_events_term *term, *h;
struct perf_pmu_alias *alias;
int ret;

+ /*
+ * Mark unit and scale as not set
+ * (different from default values, see below)
+ */
*unit = NULL;
- *scale = 0;
+ *scale = 0.0;

list_for_each_entry_safe(term, h, head_terms, list) {
alias = pmu_find_alias(pmu, term);
@@ -658,6 +662,18 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
list_del(&term->list);
free(term);
}
+
+ /*
+ * if no unit or scale foundin aliases, then
+ * set defaults as for evsel
+ * unit cannot left to NULL
+ */
+ if (*unit == NULL)
+ *unit = "";
+
+ if (*scale == 0.0)
+ *scale = 1.0;
+
return 0;
}

diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 9183380e2038..8b64125a9281 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -29,7 +29,7 @@ int perf_pmu__config_terms(struct list_head *formats,
struct perf_event_attr *attr,
struct list_head *head_terms);
int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
- char **unit, double *scale);
+ const char **unit, double *scale);
struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
struct list_head *head_terms);
int perf_pmu_wrap(void);
--
1.8.1.4

2014-01-21 20:43:20

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/8] perf session: Free cpu_map in perf_session__cpu_bitmap

From: Stanislav Fomichev <[email protected]>

This method uses a temporary struct cpu_map to figure out the cpus
present in the received cpu list in string form, but it failed to free
it after returning. Fix it.

Signed-off-by: Stanislav Fomichev <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ Use goto + err = -1 to do the delete just once, in the normal exit path ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/session.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 7acc03e8f3b2..0b39a48e5110 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1573,7 +1573,7 @@ next:
int perf_session__cpu_bitmap(struct perf_session *session,
const char *cpu_list, unsigned long *cpu_bitmap)
{
- int i;
+ int i, err = -1;
struct cpu_map *map;

for (i = 0; i < PERF_TYPE_MAX; ++i) {
@@ -1602,13 +1602,17 @@ int perf_session__cpu_bitmap(struct perf_session *session,
if (cpu >= MAX_NR_CPUS) {
pr_err("Requested CPU %d too large. "
"Consider raising MAX_NR_CPUS\n", cpu);
- return -1;
+ goto out_delete_map;
}

set_bit(cpu, cpu_bitmap);
}

- return 0;
+ err = 0;
+
+out_delete_map:
+ cpu_map__delete(map);
+ return err;
}

void perf_session__fprintf_info(struct perf_session *session, FILE *fp,
--
1.8.1.4

2014-01-21 20:44:39

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 6/8] perf evsel: Remove duplicate member zeroing after free

From: Stephane Eranian <[email protected]>

No need to set evsel->fd to NULL after calling perf_evsel__free_fd(), as
this method already does that.

Cc: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/evsel.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 22e18a26b7e6..55407c594b87 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1081,7 +1081,6 @@ void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads)

perf_evsel__close_fd(evsel, ncpus, nthreads);
perf_evsel__free_fd(evsel);
- evsel->fd = NULL;
}

static struct {
--
1.8.1.4

2014-01-21 20:44:37

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 7/8] perf stat: Fix memory corruption of xyarray when cpumask is used

From: Stephane Eranian <[email protected]>

This patch fixes a memory corruption problem with the xyarray when the
evsel fds get closed at the end of the run_perf_stat() call.

It could be triggered with:

# perf stat -a -e power/energy-cores/ ls

When cpumask are used by events (.e.g, RAPL or uncores) then the evsel
fds are allocated based on the actual number of CPUs monitored. That
number can be smaller than the total number of CPUs on the system.

The problem arises at the end by perf stat closes the fds twice. When
fds are closed, their entry in the xyarray are set to -1.

The first close() on the evsel is made from __run_perf_stat() and it
uses the actual number of CPUS for the event which is how the xyarray
was allocated for.

The second is from perf_evlist_close() but that one is on the total
number of CPUs in the system, so it assume the xyarray was allocated to
cover it. However it was not, and some writes corrupt memory.

The fix is in perf_evlist_close() is to first try with the evsel->cpus
if present, if not use the evlist->cpus. That fixes the problem.

Signed-off-by: Stephane Eranian <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/evlist.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 40bd2c04df8a..59ef2802fcf6 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1003,9 +1003,12 @@ void perf_evlist__close(struct perf_evlist *evlist)
struct perf_evsel *evsel;
int ncpus = cpu_map__nr(evlist->cpus);
int nthreads = thread_map__nr(evlist->threads);
+ int n;

- evlist__for_each_reverse(evlist, evsel)
- perf_evsel__close(evsel, ncpus, nthreads);
+ evlist__for_each_reverse(evlist, evsel) {
+ n = evsel->cpus ? evsel->cpus->nr : ncpus;
+ perf_evsel__close(evsel, n, nthreads);
+ }
}

int perf_evlist__open(struct perf_evlist *evlist)
--
1.8.1.4

2014-01-21 20:45:45

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/8] perf timechart: Fix wrong SVG height

From: Stanislav Fomichev <[email protected]>

If we call perf timechart with -p 0 arguments, it means we don't want
any tasks related data. It works, but space for tasks data is reserved
in the generated SVG. Remove this unused empty space via passing 0 as
count to the open_svg.

Signed-off-by: Stanislav Fomichev <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-timechart.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 652af0b66a62..25526d6eae59 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1045,6 +1045,9 @@ static void write_svg_file(struct timechart *tchart, const char *filename)
thresh /= 10;
} while (!process_filter && thresh && count < tchart->proc_num);

+ if (!tchart->proc_num)
+ count = 0;
+
open_svg(filename, tchart->numcpus, count, tchart->first_time, tchart->last_time);

svg_time_grid();
--
1.8.1.4

2014-01-23 16:45:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 0/8] perf/core fixes


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> From: Arnaldo Carvalho de Melo <[email protected]>
>
> Hi Ingo,
>
> Please consider pulling,
>
> - Arnaldo
>
> The following changes since commit 45e6af06367e7b2eb8dc49671092462d8f8a5f47:
>
> Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2014-01-19 13:09:01 +0100)
>
> are available in the git repository at:
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo
>
> for you to fetch changes up to 578c03c86fadcc6fd7319ddf41dd4d1d88aab77a:
>
> perf symbols: Fix JIT symbol resolution on heap (2014-01-21 10:56:05 -0300)
>
> ----------------------------------------------------------------
> perf/core fixes:
>
> . Fix JIT symbol resolution on heap (Namhyung Kim)
>
> . Fix wrong SVG height in 'timechart' (Stanislav Fomichev)
>
> . Free temp cpu_map in perf_session__cpu_bitmap (Stanislav Fomichev)
>
> . Fix NULL pointer reference bug with event unit in 'stat' (Stephane Eranian)
>
> . Fix memory corruption of xyarray when cpumask is used (Stephane Eranian)
>
> . Ensure sscanf does not overrun the "mem" field (Alan Cox)
>
> . Add support for the xtensa architecture (Baruch Siach)
>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ----------------------------------------------------------------
> Alan Cox (1):
> perf tools: Ensure sscanf does not overrun the "mem" field
>
> Baruch Siach (1):
> perf tools: Add support for the xtensa architecture
>
> Namhyung Kim (1):
> perf symbols: Fix JIT symbol resolution on heap
>
> Stanislav Fomichev (2):
> perf timechart: Fix wrong SVG height
> perf session: Free cpu_map in perf_session__cpu_bitmap
>
> Stephane Eranian (3):
> perf stat: fix NULL pointer reference bug with event unit
> perf evsel: Remove duplicate member zeroing after free
> perf stat: Fix memory corruption of xyarray when cpumask is used
>
> tools/perf/builtin-timechart.c | 3 +++
> tools/perf/perf.h | 7 +++++++
> tools/perf/util/evlist.c | 7 +++++--
> tools/perf/util/evsel.c | 1 -
> tools/perf/util/header.c | 2 +-
> tools/perf/util/map.c | 4 ++--
> tools/perf/util/parse-events.c | 2 +-
> tools/perf/util/pmu.c | 24 ++++++++++++++++++++----
> tools/perf/util/pmu.h | 2 +-
> tools/perf/util/session.c | 10 +++++++---
> 10 files changed, 47 insertions(+), 15 deletions(-)

Pulled, thanks a lot Arnaldo!

Ingo