2012-10-17 17:22:07

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 00/11] perf/urgent fixes

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit 95cf59ea72331d0093010543b8951bb43f262cac:

perf: Fix perf_cgroup_switch for sw-events (2012-10-05 13:59:07 +0200)

are available in the git repository at:

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

for you to fetch changes up to 88a21d2f07d2a4bec2e3e03dd50a39683b938b10:

perf hists browser: Add back callchain folding symbol (2012-10-17 13:54:08 -0300)

----------------------------------------------------------------
perf/urgent fixes:

. The python binding needs to link with libtraceevent and to initialize
the 'page_size' variable so that mmaping works again.

. The callchain folding character that appears on the TUI just before
the overhead had disappeared due to recent changes, add it back.

. Intel PEBS in VT-x context uses the DS address as a guest linear address,
even though its programmed by the host as a host linear address. This either
results in guest memory corruption and or the hardware faulting and 'crashing'
the virtual machine. Therefore we have to disable PEBS on VT-x enter and
re-enable on VT-x exit, enforcing a strict exclude_guest.

Kernel side enforcement fix by Peter Zijlstra, tooling side fix by David Ahern.

. Fix build on sparc due to UAPI, fix from David Miller.

. Fixes for the srclike sort key for unresolved symbols and when processing
samples in JITted code, where we don't have an ELF file, just an special
symbol table, fixes from Namhyung Kim.

. Fix some leaks in libtraceevent, from Steven Rostedt.

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

----------------------------------------------------------------
Arnaldo Carvalho de Melo (3):
perf python: Initialize 'page_size' variable
perf python: Link with libtraceevent
perf hists browser: Add back callchain folding symbol

David Ahern (1):
perf tool: Precise mode requires exclude_guest

David Miller (1):
perf tools: Fix build on sparc.

Namhyung Kim (3):
perf tools: Fix segfault when using srcline sort key
perf tools: Remove warnings on JIT samples for srcline sort key
perf hists browser: Fix off-by-two bug on the first column

Peter Zijlstra (1):
perf: Require exclude_guest to use PEBS - kernel side enforcement

Steven Rostedt (2):
lib tools traceevent: Add back pevent assignment in __pevent_parse_format()
tools lib traceevent: Fix missed freeing of subargs in free_arg() in filter

arch/x86/kernel/cpu/perf_event.c | 6 ++++++
tools/lib/traceevent/event-parse.c | 9 ++++++---
tools/lib/traceevent/parse-filter.c | 15 +++++++++++++++
tools/perf/perf.h | 2 +-
tools/perf/ui/browsers/hists.c | 6 ++++--
tools/perf/util/parse-events.c | 3 +++
tools/perf/util/python.c | 2 ++
tools/perf/util/setup.py | 1 +
tools/perf/util/sort.c | 6 ++++++
9 files changed, 44 insertions(+), 6 deletions(-)


2012-10-17 17:20:08

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 02/11] perf: Require exclude_guest to use PEBS - kernel side enforcement

From: Peter Zijlstra <[email protected]>

Intel PEBS in VT-x context uses the DS address as a guest linear
address, even though its programmed by the host as a host linear
address. This either results in guest memory corruption and or the
hardware faulting and 'crashing' the virtual machine. Therefore we have
to disable PEBS on VT-x enter and re-enable on VT-x exit, enforcing a
strict exclude_guest.

This patch enforces exclude_guest kernel side.

Signed-off-by: Peter Zijlstra <[email protected]>
Cc: Avi Kivity <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Gleb Natapov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Robert Richter <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: David Ahern <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
arch/x86/kernel/cpu/perf_event.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 915b876..3373f84 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -338,6 +338,9 @@ int x86_setup_perfctr(struct perf_event *event)
/* BTS is currently only allowed for user-mode. */
if (!attr->exclude_kernel)
return -EOPNOTSUPP;
+
+ if (!attr->exclude_guest)
+ return -EOPNOTSUPP;
}

hwc->config |= config;
@@ -380,6 +383,9 @@ int x86_pmu_hw_config(struct perf_event *event)
if (event->attr.precise_ip) {
int precise = 0;

+ if (!event->attr.exclude_guest)
+ return -EOPNOTSUPP;
+
/* Support for constant skid */
if (x86_pmu.pebs_active && !x86_pmu.pebs_broken) {
precise++;
--
1.7.1

2012-10-17 17:20:17

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 11/11] perf hists browser: Add back callchain folding symbol

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

The commit 5395a04841fc ("perf hists: Separate overhead and baseline
columns") makes the "Overhead" column no more the first one, this
caused the test that checks if it is time to show if a histogram
entry has callchains never hits.

Fix it by checking if the 'i' variable is equal to PERF_HPP__OVERHEAD
instead of 0.

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]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browsers/hists.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fe4430a..ef2f93c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -647,7 +647,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,

ui_browser__set_percent_color(&browser->b, percent, current_entry);

- if (i == 0 && symbol_conf.use_callchain) {
+ if (i == PERF_HPP__OVERHEAD && symbol_conf.use_callchain) {
slsmg_printf("%c ", folded_sign);
width -= 2;
}
--
1.7.1

2012-10-17 17:20:34

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 03/11] perf tools: Fix segfault when using srcline sort key

From: Namhyung Kim <[email protected]>

The srcline sort key is for grouping samples based on their source file
and line number. It use addr2line tool to get the information but it
requires dso name. It caused a segfault when a sample does not have the
name by dereferencing a NULL pointer. Fix it by using raw ip addresses
for those samples.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Ingo Molnar <[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/sort.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index b5b1b92..dd68f11 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -260,6 +260,9 @@ static int hist_entry__srcline_snprintf(struct hist_entry *self, char *bf,
if (path != NULL)
goto out_path;

+ if (!self->ms.map)
+ goto out_ip;
+
snprintf(cmd, sizeof(cmd), "addr2line -e %s %016" PRIx64,
self->ms.map->dso->long_name, self->ip);
fp = popen(cmd, "r");
--
1.7.1

2012-10-17 17:20:47

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 09/11] perf python: Link with libtraceevent

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

The evsel methods to read tracepoint fields uses libtraceevent
functions, becoming needed by the python binding as well.

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]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/setup.py | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index d0f9f29..09c3cea 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -31,6 +31,7 @@ perf = Extension('perf',
sources = ext_sources,
include_dirs = ['util/include'],
extra_compile_args = cflags,
+ extra_objects = [build_tmp + '/../../libtraceevent.a'],
)

setup(name='perf',
--
1.7.1

2012-10-17 17:20:45

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 08/11] perf python: Initialize 'page_size' variable

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

The commit 0c1fe6b:

'perf tools: Have the page size value available for all tools'

Broke the python binding because the global variable 'page_size' is
initialized on the main() routine, that is not called when using
just the python binding, causing evlist.mmap() to fail because it
expects that variable to be initialized to the system's page size.

Fix it by initializing it on the binding init routine.

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]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/python.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 9181bf2..a2657fd 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1015,6 +1015,8 @@ PyMODINIT_FUNC initperf(void)
pyrf_cpu_map__setup_types() < 0)
return;

+ page_size = sysconf(_SC_PAGE_SIZE);
+
Py_INCREF(&pyrf_evlist__type);
PyModule_AddObject(module, "evlist", (PyObject*)&pyrf_evlist__type);

--
1.7.1

2012-10-17 17:20:43

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 05/11] perf hists browser: Fix off-by-two bug on the first column

From: Namhyung Kim <[email protected]>

The commit 5395a04841fc ("perf hists: Separate overhead and baseline
columns") makes the "Overhead" column no more the first one. So it
resulted in the mis-aligned column in the normal (non-diff) output.

Signed-off-by: Namhyung Kim <[email protected]>
Reported-by: Markus Trippelsdorf <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/None
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browsers/hists.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 0568536..fe4430a 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -610,6 +610,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
char folded_sign = ' ';
bool current_entry = ui_browser__is_current_entry(&browser->b, row);
off_t row_offset = entry->row_offset;
+ bool first = true;

if (current_entry) {
browser->he_selection = entry;
@@ -633,10 +634,11 @@ static int hist_browser__show_entry(struct hist_browser *browser,
if (!perf_hpp__format[i].cond)
continue;

- if (i) {
+ if (!first) {
slsmg_printf(" ");
width -= 2;
}
+ first = false;

if (perf_hpp__format[i].color) {
hpp.ptr = &percent;
--
1.7.1

2012-10-17 17:20:14

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 10/11] perf tools: Fix build on sparc.

From: David Miller <[email protected]>

More UAPI stuff.

Signed-off-by: David S. Miller <[email protected]>
Cc: Ingo Molnar <[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/perf.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 87f4ec6..8421c20 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -57,7 +57,7 @@ void get_term_dimensions(struct winsize *ws);
#endif

#ifdef __sparc__
-#include "../../arch/sparc/include/asm/unistd.h"
+#include "../../arch/sparc/include/uapi/asm/unistd.h"
#define rmb() asm volatile("":::"memory")
#define cpu_relax() asm volatile("":::"memory")
#define CPUINFO_PROC "cpu"
--
1.7.1

2012-10-17 17:21:46

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 04/11] perf tools: Remove warnings on JIT samples for srcline sort key

From: Namhyung Kim <[email protected]>

When using the srcline sort key with perf report, I see many lines of
warning related to JIT samples like below:

addr2line: '/tmp/perf-1397.map': No such file

Since it's not a ELF binary and doesn't provide such information, just
use the raw ip address.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Irina Tirdea <[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/sort.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index dd68f11..cfd1c0f 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -263,6 +263,9 @@ static int hist_entry__srcline_snprintf(struct hist_entry *self, char *bf,
if (!self->ms.map)
goto out_ip;

+ if (!strncmp(self->ms.map->dso->long_name, "/tmp/perf-", 10))
+ goto out_ip;
+
snprintf(cmd, sizeof(cmd), "addr2line -e %s %016" PRIx64,
self->ms.map->dso->long_name, self->ip);
fp = popen(cmd, "r");
--
1.7.1

2012-10-17 17:22:36

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 07/11] tools lib traceevent: Fix missed freeing of subargs in free_arg() in filter

From: Steven Rostedt <[email protected]>

Some of args were missed in free_args(), as well as subargs.

That is args like FILTER_ARG_NUM have left and right pointers to other
args that also need to be freed.

Signed-off-by: Steven Rostedt <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/parse-filter.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index ad17855..5ea4326 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -209,7 +209,16 @@ static void free_arg(struct filter_arg *arg)
switch (arg->type) {
case FILTER_ARG_NONE:
case FILTER_ARG_BOOLEAN:
+ break;
+
case FILTER_ARG_NUM:
+ free_arg(arg->num.left);
+ free_arg(arg->num.right);
+ break;
+
+ case FILTER_ARG_EXP:
+ free_arg(arg->exp.left);
+ free_arg(arg->exp.right);
break;

case FILTER_ARG_STR:
@@ -218,6 +227,12 @@ static void free_arg(struct filter_arg *arg)
free(arg->str.buffer);
break;

+ case FILTER_ARG_VALUE:
+ if (arg->value.type == FILTER_STRING ||
+ arg->value.type == FILTER_CHAR)
+ free(arg->value.str);
+ break;
+
case FILTER_ARG_OP:
free_arg(arg->op.left);
free_arg(arg->op.right);
--
1.7.1

2012-10-17 17:22:49

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 06/11] lib tools traceevent: Add back pevent assignment in __pevent_parse_format()

From: Steven Rostedt <[email protected]>

Even though with the change of commit commit 2b29175 "tools lib
traceevent: Carve out events format parsing routine", allowed
__pevent_parse_format() to parse an event without the need of a pevent
handler, the event still needs to assign the pevent handed to it.

There's no problem with assigning it if the pevent is NULL, as the
event->pevent would be NULL without the assignment. But function parsing
handlers may be assigned to the pevent handler to help in parsing the
event. If there's no pevent then there would not be any function
handlers, but if the pevent isn't assigned first before parsing the
event, it wont honor the function handlers that were assigned.

Worse yet, the current code crashes if an event has a function that it
tries to parse. For example:

# perf record -e scsi:scsi_dispatch_cmd_timeout
Segmentation fault (core dumped)

This happens because the scsi_dispatch_cmd_timeout event format has the following:

scsi_trace_parse_cdb(p, __get_dynamic_array(cmnd), REC->cmd_len)

which hasn't been defined by the pevent code.

Signed-off-by: Steven Rostedt <[email protected]>
Reviewed-by: Namhyung Kim <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 47264b4..f2989c5 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2602,6 +2602,9 @@ find_func_handler(struct pevent *pevent, char *func_name)
{
struct pevent_function_handler *func;

+ if (!pevent)
+ return NULL;
+
for (func = pevent->func_handlers; func; func = func->next) {
if (strcmp(func->name, func_name) == 0)
break;
@@ -4938,6 +4941,9 @@ enum pevent_errno __pevent_parse_format(struct event_format **eventp,
goto event_alloc_failed;
}

+ /* Add pevent to event so that it can be referenced */
+ event->pevent = pevent;
+
ret = event_read_format(event);
if (ret < 0) {
ret = PEVENT_ERRNO__READ_FORMAT_FAILED;
@@ -5041,9 +5047,6 @@ enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
if (event == NULL)
return ret;

- /* Add pevent to event so that it can be referenced */
- event->pevent = pevent;
-
if (add_event(pevent, event)) {
ret = PEVENT_ERRNO__MEM_ALLOC_FAILED;
goto event_add_failed;
--
1.7.1

2012-10-17 17:20:05

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 01/11] perf tool: Precise mode requires exclude_guest

From: David Ahern <[email protected]>

Summary of events per Peter:

"Intel PEBS in VT-x context uses the DS address as a guest linear address,
even though its programmed by the host as a host linear address. This
either results in guest memory corruption and or the hardware faulting and
'crashing' the virtual machine. Therefore we have to disable PEBS on VT-x
enter and re-enable on VT-x exit, enforcing a strict exclude_guest.

AMB IBS does work but doesn't currently support exclude_* at all,
setting an exclude_* bit will make it fail."

This patch handles userspace perf command, setting the exclude_guest
attribute if precise mode is requested, but only if a user has not
specified a request for guest or host only profiling (G or H attribute).

Kernel side AMD currently ignores all exclude_* bits, so there is no impact
to existing IBS code paths. Robert Richter has a patch where IBS code will
return EINVAL if an exclude_* bit is set. When this goes in it means use
of :p on AMD with IBS will first fail with EINVAL (because exclude_guest
will be set). Then the existing fallback code within perf will unset
exclude_guest and try again. The second attempt will succeed if the CPU
supports IBS profiling.

Signed-off-by: David Ahern <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Acked-by: Robert Richter <[email protected]>
Tested-by: Robert Richter <[email protected]>
Reviewed-by: Robert Richter <[email protected]>
Cc: Avi Kivity <[email protected]>
Cc: Gleb Natapov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Robert Richter <[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 | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index aed38e4..75c7b0f 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -690,6 +690,9 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
eH = 0;
} else if (*str == 'p') {
precise++;
+ /* use of precise requires exclude_guest */
+ if (!exclude_GH)
+ eG = 1;
} else
break;

--
1.7.1

2012-10-18 01:32:02

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 08/11] perf python: Initialize 'page_size' variable

Hi Arnaldo,

On Wed, 17 Oct 2012 14:19:44 -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> The commit 0c1fe6b:
>
> 'perf tools: Have the page size value available for all tools'

Is that commit in your perf/urgent branch? I got this:

GEN python/perf.so
util/python.c: In function ‘initperf’:
util/python.c:1018:2: error: ‘page_size’ undeclared (first use in this function)
util/python.c:1018:2: note: each undeclared identifier is reported only once for each function it appears in
error: command 'gcc' failed with exit status 1
cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
make: *** [python/perf.so] Error 1

Adding '#include "util.h"' didn't work. But manually adding 'extern
unsigned int page_size;' did.

Thanks,
Namhyung


>
> Broke the python binding because the global variable 'page_size' is
> initialized on the main() routine, that is not called when using
> just the python binding, causing evlist.mmap() to fail because it
> expects that variable to be initialized to the system's page size.
>
> Fix it by initializing it on the binding init routine.
>
> 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]>
> Cc: Stephane Eranian <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/perf/util/python.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 9181bf2..a2657fd 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -1015,6 +1015,8 @@ PyMODINIT_FUNC initperf(void)
> pyrf_cpu_map__setup_types() < 0)
> return;
>
> + page_size = sysconf(_SC_PAGE_SIZE);
> +
> Py_INCREF(&pyrf_evlist__type);
> PyModule_AddObject(module, "evlist", (PyObject*)&pyrf_evlist__type);

2012-10-18 01:38:23

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf python: Link with libtraceevent

On Wed, 17 Oct 2012 14:19:45 -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> The evsel methods to read tracepoint fields uses libtraceevent
> functions, becoming needed by the python binding as well.

I got a build error after adding 'extern unsigned int page_size' to
util/python.c - please see my previous reply.

namhyung@sejong:perf$ make
SUBDIR ../lib/traceevent/
LINK perf
GEN python/perf.so
gcc: error: python_ext_build/tmp//../../libtraceevent.a: No such file or directory
error: command 'gcc' failed with exit status 1
cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
make: *** [python/perf.so] Error 1

The libtraceevent.a will be in
python_ext_build/tmp/../../../lib/traceevent/libtraceevent.a if make was
invoked without the O.

Makefile contains following:

TRACE_EVENT_DIR = ../lib/traceevent/

ifneq ($(OUTPUT),)
TE_PATH=$(OUTPUT)
else
TE_PATH=$(TRACE_EVENT_DIR)
endif

LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
TE_LIB := -L$(TE_PATH) -ltraceevent

Thanks,
Namhyung

>
> 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]>
> Cc: Stephane Eranian <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/perf/util/setup.py | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> index d0f9f29..09c3cea 100644
> --- a/tools/perf/util/setup.py
> +++ b/tools/perf/util/setup.py
> @@ -31,6 +31,7 @@ perf = Extension('perf',
> sources = ext_sources,
> include_dirs = ['util/include'],
> extra_compile_args = cflags,
> + extra_objects = [build_tmp + '/../../libtraceevent.a'],
> )
>
> setup(name='perf',

2012-10-18 01:39:53

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 10/11] perf tools: Fix build on sparc.

On Wed, 17 Oct 2012 14:19:46 -0300, Arnaldo Carvalho de Melo wrote:
> From: David Miller <[email protected]>
>
> More UAPI stuff.

What about other architectures? Don't they have the same problem?

Thanks,
Namhyung

2012-10-18 02:15:26

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 10/11] perf tools: Fix build on sparc.

From: Namhyung Kim <[email protected]>
Date: Thu, 18 Oct 2012 10:39:49 +0900

> On Wed, 17 Oct 2012 14:19:46 -0300, Arnaldo Carvalho de Melo wrote:
>> From: David Miller <[email protected]>
>>
>> More UAPI stuff.
>
> What about other architectures? Don't they have the same problem?

If they took in the per-architecture UAPI pull from David Howells,
then yes. But I don't think all of them have, and if that's true
then just blinding converting all of them is the wrong thing to do.

2012-10-18 13:59:12

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 08/11] perf python: Initialize 'page_size' variable

Em Thu, Oct 18, 2012 at 10:31:57AM +0900, Namhyung Kim escreveu:
> On Wed, 17 Oct 2012 14:19:44 -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <[email protected]>

> > The commit 0c1fe6b:

> > 'perf tools: Have the page size value available for all tools'

> Is that commit in your perf/urgent branch? I got this:

> GEN python/perf.so

Ouch, brown paper bag grade stuff detected, I reverted it now in my perf/urgent
branch.

Ingo, I'm on a conference here in Brazil, just pushed this revert to
perf/urgent, up to you if you want to pull from it or apply all the
csets in this branch except for the one reverted:

commit 356712f6e296fdae1edae51b96b485ed830bdc0c
Author: Arnaldo Carvalho de Melo <[email protected]>
Date: Tue Oct 16 14:51:04 2012 -0300

perf python: Initialize 'page_size' variable

-----------------------

I'll keep it on my perf/core branch, where it is needed.

Thanks Namhyung!

- Arnaldo

> util/python.c: In function ‘initperf’:
> util/python.c:1018:2: error: ‘page_size’ undeclared (first use in this function)
> util/python.c:1018:2: note: each undeclared identifier is reported only once for each function it appears in
> error: command 'gcc' failed with exit status 1
> cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
> make: *** [python/perf.so] Error 1
>
> Adding '#include "util.h"' didn't work. But manually adding 'extern
> unsigned int page_size;' did.
>
> Thanks,
> Namhyung
>
>
> >
> > Broke the python binding because the global variable 'page_size' is
> > initialized on the main() routine, that is not called when using
> > just the python binding, causing evlist.mmap() to fail because it
> > expects that variable to be initialized to the system's page size.
> >
> > Fix it by initializing it on the binding init routine.
> >
> > 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]>
> > Cc: Stephane Eranian <[email protected]>
> > Link: http://lkml.kernel.org/n/[email protected]
> > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> > ---
> > tools/perf/util/python.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > index 9181bf2..a2657fd 100644
> > --- a/tools/perf/util/python.c
> > +++ b/tools/perf/util/python.c
> > @@ -1015,6 +1015,8 @@ PyMODINIT_FUNC initperf(void)
> > pyrf_cpu_map__setup_types() < 0)
> > return;
> >
> > + page_size = sysconf(_SC_PAGE_SIZE);
> > +
> > Py_INCREF(&pyrf_evlist__type);
> > PyModule_AddObject(module, "evlist", (PyObject*)&pyrf_evlist__type);

2012-10-18 14:00:53

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf python: Link with libtraceevent

Em Thu, Oct 18, 2012 at 10:38:18AM +0900, Namhyung Kim escreveu:
> On Wed, 17 Oct 2012 14:19:45 -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <[email protected]>
> >
> > The evsel methods to read tracepoint fields uses libtraceevent
> > functions, becoming needed by the python binding as well.
>
> I got a build error after adding 'extern unsigned int page_size' to
> util/python.c - please see my previous reply.
>
> namhyung@sejong:perf$ make
> SUBDIR ../lib/traceevent/
> LINK perf
> GEN python/perf.so
> gcc: error: python_ext_build/tmp//../../libtraceevent.a: No such file or directory
> error: command 'gcc' failed with exit status 1
> cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
> make: *** [python/perf.so] Error 1
>
> The libtraceevent.a will be in
> python_ext_build/tmp/../../../lib/traceevent/libtraceevent.a if make was
> invoked without the O.
>
> Makefile contains following:
>
> TRACE_EVENT_DIR = ../lib/traceevent/
>
> ifneq ($(OUTPUT),)
> TE_PATH=$(OUTPUT)
> else
> TE_PATH=$(TRACE_EVENT_DIR)
> endif
>
> LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
> TE_LIB := -L$(TE_PATH) -ltraceevent

I see, so we need to somehow propagate this TE_PATH variable to the setup.py file...

- Arnaldo

2012-10-18 14:38:51

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf python: Link with libtraceevent

Em Thu, Oct 18, 2012 at 11:00:33AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Oct 18, 2012 at 10:38:18AM +0900, Namhyung Kim escreveu:
> > On Wed, 17 Oct 2012 14:19:45 -0300, Arnaldo Carvalho de Melo wrote:
> > I got a build error after adding 'extern unsigned int page_size' to
> > util/python.c - please see my previous reply.
> >
> > namhyung@sejong:perf$ make
> > SUBDIR ../lib/traceevent/
> > LINK perf
> > GEN python/perf.so
> > gcc: error: python_ext_build/tmp//../../libtraceevent.a: No such file or directory
> > error: command 'gcc' failed with exit status 1
> > cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
> > make: *** [python/perf.so] Error 1
> > TRACE_EVENT_DIR = ../lib/traceevent/

> > ifneq ($(OUTPUT),)
> > TE_PATH=$(OUTPUT)
> > else
> > TE_PATH=$(TRACE_EVENT_DIR)
> > endif
> >
> > LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
>
> I see, so we need to somehow propagate this TE_PATH variable to the setup.py file...

Can you try with the attached patch? I tested it here and works with:

make -C tools/perf

and with O=

- Arnaldo


Attachments:
(No filename) (1.11 kB)
python-libtraceevent.patch (1.85 kB)
Download all attachments

2012-10-18 15:40:34

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf python: Link with libtraceevent

2012-10-18 (목), 11:38 -0300, Arnaldo Carvalho de Melo:
> Em Thu, Oct 18, 2012 at 11:00:33AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Oct 18, 2012 at 10:38:18AM +0900, Namhyung Kim escreveu:
> > > On Wed, 17 Oct 2012 14:19:45 -0300, Arnaldo Carvalho de Melo wrote:
> > > I got a build error after adding 'extern unsigned int page_size' to
> > > util/python.c - please see my previous reply.
> > >
> > > namhyung@sejong:perf$ make
> > > SUBDIR ../lib/traceevent/
> > > LINK perf
> > > GEN python/perf.so
> > > gcc: error: python_ext_build/tmp//../../libtraceevent.a: No such file or directory
> > > error: command 'gcc' failed with exit status 1
> > > cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
> > > make: *** [python/perf.so] Error 1
> > > TRACE_EVENT_DIR = ../lib/traceevent/
>
> > > ifneq ($(OUTPUT),)
> > > TE_PATH=$(OUTPUT)
> > > else
> > > TE_PATH=$(TRACE_EVENT_DIR)
> > > endif
> > >
> > > LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
> >
> > I see, so we need to somehow propagate this TE_PATH variable to the setup.py file...
>
> Can you try with the attached patch? I tested it here and works with:
>
> make -C tools/perf
>
> and with O=

It works well for me too:

Tested-by: Namhyung Kim <[email protected]>

Thanks,
Namhyung


2012-10-20 00:36:05

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf python: Link with libtraceevent


* Namhyung Kim <[email protected]> wrote:

> 2012-10-18 (???), 11:38 -0300, Arnaldo Carvalho de Melo:
> > Em Thu, Oct 18, 2012 at 11:00:33AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Oct 18, 2012 at 10:38:18AM +0900, Namhyung Kim escreveu:
> > > > On Wed, 17 Oct 2012 14:19:45 -0300, Arnaldo Carvalho de Melo wrote:
> > > > I got a build error after adding 'extern unsigned int page_size' to
> > > > util/python.c - please see my previous reply.
> > > >
> > > > namhyung@sejong:perf$ make
> > > > SUBDIR ../lib/traceevent/
> > > > LINK perf
> > > > GEN python/perf.so
> > > > gcc: error: python_ext_build/tmp//../../libtraceevent.a: No such file or directory
> > > > error: command 'gcc' failed with exit status 1
> > > > cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
> > > > make: *** [python/perf.so] Error 1
> > > > TRACE_EVENT_DIR = ../lib/traceevent/
> >
> > > > ifneq ($(OUTPUT),)
> > > > TE_PATH=$(OUTPUT)
> > > > else
> > > > TE_PATH=$(TRACE_EVENT_DIR)
> > > > endif
> > > >
> > > > LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
> > >
> > > I see, so we need to somehow propagate this TE_PATH variable to the setup.py file...
> >
> > Can you try with the attached patch? I tested it here and works with:
> >
> > make -C tools/perf
> >
> > and with O=
>
> It works well for me too:
>
> Tested-by: Namhyung Kim <[email protected]>

Even with that I'm getting:

util/python.c: In function ‘initperf’:
util/python.c:1018:2: error: ‘page_size’ undeclared
(first use in this function)
util/python.c:1018:2: note: each undeclared identifier is
reported only once for each function it appears in
error: command 'gcc' failed with exit status 1

Thanks,

Ingo

2012-10-20 15:04:47

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf python: Link with libtraceevent

Em Sat, Oct 20, 2012 at 02:35:57AM +0200, Ingo Molnar escreveu:
> Even with that I'm getting:
>
> util/python.c: In function ‘initperf’:
> util/python.c:1018:2: error: ‘page_size’ undeclared
> (first use in this function)
> util/python.c:1018:2: note: each undeclared identifier is
> reported only once for each function it appears in
> error: command 'gcc' failed with exit status 1

Yeah, there was another problem, a patch that makes sense only for
perf/core was added, I reverted it after Namhyung reported the same
problem:

commit 9cd938a4c3b908a3ff7ef65bdc432f1915361e87
Author: Arnaldo Carvalho de Melo <[email protected]>
Date: Thu Oct 18 10:51:08 2012 -0300

Revert "perf python: Initialize 'page_size' variable"

The commit that added the page_size variable is only in the perf/core
branch, not needed for perf/urgent. Brown paper bag grade, sigh.

This reverts commit 356712f6e296fdae1edae51b96b485ed830bdc0c.

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

2012-10-21 13:34:05

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 09/11] perf python: Link with libtraceevent


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

> Em Sat, Oct 20, 2012 at 02:35:57AM +0200, Ingo Molnar escreveu:
> > Even with that I'm getting:
> >
> > util/python.c: In function ???initperf???:
> > util/python.c:1018:2: error: ???page_size??? undeclared
> > (first use in this function)
> > util/python.c:1018:2: note: each undeclared identifier is
> > reported only once for each function it appears in
> > error: command 'gcc' failed with exit status 1
>
> Yeah, there was another problem, a patch that makes sense only
> for perf/core was added, I reverted it after Namhyung reported
> the same problem:

Yeah.

So, you being on conference and me trying to not miss -rc2 with
a couple of high-profile perf fixes I ended up resolving this
bug in the merge commit, the almost-evil merge a448a03 and such.

That's now upstream - mind checking whether anything is still
amiss?

Thanks,

Ingo