2013-10-07 19:03:25

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 0/6] perf/urgent fixes

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

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit d8b11a0cbd1c66ce283eb9dabe0498dfa6483f32:

perf/x86: Clean up cap_user_time* setting (2013-10-04 09:58:55 +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 b314e5cfd11fd78545ce6c2be42646254390c1aa:

perf session: Fix infinite loop on invalid perf.data file (2013-10-04 15:17:46 -0300)

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

. The libaudit test was failing in some systems due to a unescaped newline, fix
it so that the 'trace' tool can be built in such systems.

. Fix installation of libexec components.

. Add default handler for mmap2 events so that tools that don't explicitely
define an MMAP2 handler don't crash, fix from David Ahern.

. Fix to find line information for probe list, from Masami Hiramatsu.

. Set child_pid after perf_evlist__prepare_workload(), fix from Namhyung Kim.

. Fix infinite loop on invalid perf.data file, from Namhyung Kim.

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

----------------------------------------------------------------
Arnaldo Carvalho de Melo (2):
perf tools: Fix libaudit test
perf tools: Fix installation of libexec components

David Ahern (1):
perf tools: Add default handler for mmap2 events

Masami Hiramatsu (1):
perf probe: Fix to find line information for probe list

Namhyung Kim (2):
perf stat: Set child_pid after perf_evlist__prepare_workload()
perf session: Fix infinite loop on invalid perf.data file

tools/perf/Makefile | 1 +
tools/perf/builtin-stat.c | 1 +
tools/perf/config/feature-tests.mak | 2 +-
tools/perf/util/dwarf-aux.c | 25 ++++++++++++++++---
tools/perf/util/dwarf-aux.h | 6 ++++-
tools/perf/util/header.c | 12 +++++++++
tools/perf/util/probe-finder.c | 49 +++++++++++++++++++++++++------------
tools/perf/util/session.c | 4 ++-
8 files changed, 77 insertions(+), 23 deletions(-)


2013-10-07 19:02:36

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/6] perf tools: Add default handler for mmap2 events

From: David Ahern <[email protected]>

Commands that do not implement an mmap2 handler should at least not die
with a segfault when processing files with MMAP2 events.

Signed-off-by: David Ahern <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/session.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 70ffa41518f3..37c4718f6be1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -256,6 +256,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
tool->sample = process_event_sample_stub;
if (tool->mmap == NULL)
tool->mmap = process_event_stub;
+ if (tool->mmap2 == NULL)
+ tool->mmap2 = process_event_stub;
if (tool->comm == NULL)
tool->comm = process_event_stub;
if (tool->fork == NULL)
--
1.8.1.4

2013-10-07 19:02:53

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 3/6] perf tools: Fix libaudit test

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

In ubuntu systems the libaudit test was always failing due to the
newline in the printf call not being escaped, which somehow didn't
prevented the test from working as expected on other systems, such
as fedora18.

Fix it by removing the newline, as this is just a test, that program is
just a compile test.

The error messages, obtained using 'make V=1':

CHK libaudit
<stdin>: In function ‘main’:
<stdin>:5:9: error: missing terminating " character [-Werror]
<stdin>:5:2: error: missing terminating " character
<stdin>:6:1: error: missing terminating " character [-Werror]
<stdin>:6:1: error: missing terminating " character
<stdin>:7:2: error: expected expression before ‘return’
<stdin>:8:1: error: expected ‘;’ before ‘}’ token
cc1: all warnings being treated as errors
config/Makefile:241: No libaudit.h found, disables 'trace' tool, please install audit-libs-devel or libaudit-dev

After this change the test works as expected in all systems tested and the
'trace' tool is built when the needed devel packages are installed.

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: 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/config/feature-tests.mak | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index d5a8dd44945f..f79305739ecc 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -219,7 +219,7 @@ define SOURCE_LIBAUDIT

int main(void)
{
- printf(\"error message: %s\n\", audit_errno_to_name(0));
+ printf(\"error message: %s\", audit_errno_to_name(0));
return audit_open();
}
endef
--
1.8.1.4

2013-10-07 19:02:56

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 4/6] perf probe: Fix to find line information for probe list

From: Masami Hiramatsu <[email protected]>

Fix to find the correct (as much as possible) line information for
listing probes. Without this fix, perf probe --list action will show
incorrect line information as below;

probe:getname_flags (on getname_flags@ksrc/linux-3/fs/namei.c)
probe:getname_flags_1 (on getname:-89@x86/include/asm/current.h)
probe:getname_flags_2 (on user_path_at_empty:-2054@x86/include/asm/current.h)

The minus line number is obviously wrong, and current.h is not related
to the probe point. Deeper investigation discovered that there were 2
issues related to this bug, and minor typos too.

The 1st issue is the rack of considering about nested inlined functions,
which causes the wrong (relative) line number.

The 2nd issue is that the dwarf line info is not correct at those
points. It points 14th line of current.h.

Since it seems that the line info includes somewhat unreliable
information, this fixes perf to try to find correct line information
from both of debuginfo and line info as below.

1) Probe address is the entry of a function instance

In this case, the line is set as the function declared line.

2) Probe address is the entry of an expanded inline function block

In this case, the line is set as the function call-site line.
This means that the line number is relative from the entry line
of caller function (which can be an inlined function if nested)

3) Probe address is inside a function instance or an expanded
inline function block

In this case, perf probe queries the line number from lineinfo
and verify the function declared file is same as the file name
queried from lineinfo.

If the file name is different, it is a failure case. The probe
address is shown as symbol+offset.

4) Probe address is not in the any function instance

This is a failure case, the probe address is shown as
symbol+offset.

With this fix, perf probe -l shows correct probe lines as below;

probe:getname_flags (on getname_flags@ksrc/linux-3/fs/namei.c)
probe:getname_flags_1 (on getname:2@ksrc/linux-3/fs/namei.c)
probe:getname_flags_2 (on user_path_at_empty:4@ksrc/linux-3/fs/namei.c)

Changes at v2:
- Fix typos in the function comments. (Thanks to Namhyung Kim)
- Use die_find_top_inlinefunc instead of die_find_inlinefunc_next.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[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/dwarf-aux.c | 25 +++++++++++++++++----
tools/perf/util/dwarf-aux.h | 6 +++++-
tools/perf/util/probe-finder.c | 49 ++++++++++++++++++++++++++++--------------
3 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index e23bde19d590..7defd77105d0 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -426,7 +426,7 @@ static int __die_search_func_cb(Dwarf_Die *fn_die, void *data)
* @die_mem: a buffer for result DIE
*
* Search a non-inlined function DIE which includes @addr. Stores the
- * DIE to @die_mem and returns it if found. Returns NULl if failed.
+ * DIE to @die_mem and returns it if found. Returns NULL if failed.
*/
Dwarf_Die *die_find_realfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
Dwarf_Die *die_mem)
@@ -454,15 +454,32 @@ static int __die_find_inline_cb(Dwarf_Die *die_mem, void *data)
}

/**
+ * die_find_top_inlinefunc - Search the top inlined function at given address
+ * @sp_die: a subprogram DIE which including @addr
+ * @addr: target address
+ * @die_mem: a buffer for result DIE
+ *
+ * Search an inlined function DIE which includes @addr. Stores the
+ * DIE to @die_mem and returns it if found. Returns NULL if failed.
+ * Even if several inlined functions are expanded recursively, this
+ * doesn't trace it down, and returns the topmost one.
+ */
+Dwarf_Die *die_find_top_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
+ Dwarf_Die *die_mem)
+{
+ return die_find_child(sp_die, __die_find_inline_cb, &addr, die_mem);
+}
+
+/**
* die_find_inlinefunc - Search an inlined function at given address
- * @cu_die: a CU DIE which including @addr
+ * @sp_die: a subprogram DIE which including @addr
* @addr: target address
* @die_mem: a buffer for result DIE
*
* Search an inlined function DIE which includes @addr. Stores the
- * DIE to @die_mem and returns it if found. Returns NULl if failed.
+ * DIE to @die_mem and returns it if found. Returns NULL if failed.
* If several inlined functions are expanded recursively, this trace
- * it and returns deepest one.
+ * it down and returns deepest one.
*/
Dwarf_Die *die_find_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
Dwarf_Die *die_mem)
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 8658d41697d2..b4fe90c6cb2d 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -79,7 +79,11 @@ extern Dwarf_Die *die_find_child(Dwarf_Die *rt_die,
extern Dwarf_Die *die_find_realfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
Dwarf_Die *die_mem);

-/* Search an inlined function including given address */
+/* Search the top inlined function including given address */
+extern Dwarf_Die *die_find_top_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
+ Dwarf_Die *die_mem);
+
+/* Search the deepest inlined function including given address */
extern Dwarf_Die *die_find_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
Dwarf_Die *die_mem);

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 371476cb8ddc..c09e0a9fdf4c 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1327,8 +1327,8 @@ int debuginfo__find_probe_point(struct debuginfo *self, unsigned long addr,
struct perf_probe_point *ppt)
{
Dwarf_Die cudie, spdie, indie;
- Dwarf_Addr _addr, baseaddr;
- const char *fname = NULL, *func = NULL, *tmp;
+ Dwarf_Addr _addr = 0, baseaddr = 0;
+ const char *fname = NULL, *func = NULL, *basefunc = NULL, *tmp;
int baseline = 0, lineno = 0, ret = 0;

/* Adjust address with bias */
@@ -1349,27 +1349,36 @@ int debuginfo__find_probe_point(struct debuginfo *self, unsigned long addr,
/* Find a corresponding function (name, baseline and baseaddr) */
if (die_find_realfunc(&cudie, (Dwarf_Addr)addr, &spdie)) {
/* Get function entry information */
- tmp = dwarf_diename(&spdie);
- if (!tmp ||
+ func = basefunc = dwarf_diename(&spdie);
+ if (!func ||
dwarf_entrypc(&spdie, &baseaddr) != 0 ||
- dwarf_decl_line(&spdie, &baseline) != 0)
+ dwarf_decl_line(&spdie, &baseline) != 0) {
+ lineno = 0;
goto post;
- func = tmp;
+ }

- if (addr == (unsigned long)baseaddr)
+ if (addr == (unsigned long)baseaddr) {
/* Function entry - Relative line number is 0 */
lineno = baseline;
- else if (die_find_inlinefunc(&spdie, (Dwarf_Addr)addr,
- &indie)) {
+ fname = dwarf_decl_file(&spdie);
+ goto post;
+ }
+
+ /* Track down the inline functions step by step */
+ while (die_find_top_inlinefunc(&spdie, (Dwarf_Addr)addr,
+ &indie)) {
+ /* There is an inline function */
if (dwarf_entrypc(&indie, &_addr) == 0 &&
- _addr == addr)
+ _addr == addr) {
/*
* addr is at an inline function entry.
* In this case, lineno should be the call-site
- * line number.
+ * line number. (overwrite lineinfo)
*/
lineno = die_get_call_lineno(&indie);
- else {
+ fname = die_get_call_file(&indie);
+ break;
+ } else {
/*
* addr is in an inline function body.
* Since lineno points one of the lines
@@ -1377,19 +1386,27 @@ int debuginfo__find_probe_point(struct debuginfo *self, unsigned long addr,
* be the entry line of the inline function.
*/
tmp = dwarf_diename(&indie);
- if (tmp &&
- dwarf_decl_line(&spdie, &baseline) == 0)
- func = tmp;
+ if (!tmp ||
+ dwarf_decl_line(&indie, &baseline) != 0)
+ break;
+ func = tmp;
+ spdie = indie;
}
}
+ /* Verify the lineno and baseline are in a same file */
+ tmp = dwarf_decl_file(&spdie);
+ if (!tmp || strcmp(tmp, fname) != 0)
+ lineno = 0;
}

post:
/* Make a relative line number or an offset */
if (lineno)
ppt->line = lineno - baseline;
- else if (func)
+ else if (basefunc) {
ppt->offset = addr - (unsigned long)baseaddr;
+ func = basefunc;
+ }

/* Duplicate strings */
if (func) {
--
1.8.1.4

2013-10-07 19:02:51

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 5/6] perf tools: Fix installation of libexec components

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

Doing a fresh install on a user home directory needs to first make sure
that the ~/libexec/perf-core/ directory is present so that
'perf-archive' like scripts, 'perf test' attr config files and 'perf
script' scripts can be installed.

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: 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/Makefile | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 3a0ff7fb71b6..64c043b7a438 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -770,6 +770,7 @@ check: $(OUTPUT)common-cmds.h
install-bin: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
$(INSTALL) $(OUTPUT)perf '$(DESTDIR_SQ)$(bindir_SQ)'
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)'
$(INSTALL) $(OUTPUT)perf-archive -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)'
ifndef NO_LIBPERL
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/perl/Perf-Trace-Util/lib/Perf/Trace'
--
1.8.1.4

2013-10-07 19:02:50

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 6/6] perf session: Fix infinite loop on invalid perf.data file

From: Namhyung Kim <[email protected]>

perf-record updates the header in the perf.data file at termination.
Without this update perf-report (and other processing built-ins) it
caused an infinite loop when perf report (or something like) called.

This is because the algorithm in __perf_session__process_events()
depends on the data_size which is read from file header. Use file size
directly instead in this case to do the best-effort processing.

Signed-off-by: Namhyung Kim <[email protected]>
Tested-by: David Ahern <[email protected]>
Tested-by: Sonny Rao <[email protected]>
Acked-by: Ingo Molnar <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Sonny Rao <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: David Ahern <[email protected]>
[ Reworded warning as per Ingo Molnar suggestion, replaces 'perf.data'
with session->filename, to precisely identify the data file involved ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/header.c | 12 ++++++++++++
tools/perf/util/session.c | 2 +-
2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index ce69901176d8..c3e5a3b817ab 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2768,6 +2768,18 @@ int perf_session__read_header(struct perf_session *session)
if (perf_file_header__read(&f_header, header, fd) < 0)
return -EINVAL;

+ /*
+ * Sanity check that perf.data was written cleanly; data size is
+ * initialized to 0 and updated only if the on_exit function is run.
+ * If data size is still 0 then the file contains only partial
+ * information. Just warn user and process it as much as it can.
+ */
+ if (f_header.data.size == 0) {
+ pr_warning("WARNING: The %s file's data size field is 0 which is unexpected.\n"
+ "Was the 'perf record' command properly terminated?\n",
+ session->filename);
+ }
+
nr_attrs = f_header.attrs.size / f_header.attr_size;
lseek(fd, f_header.attrs.offset, SEEK_SET);

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 37c4718f6be1..568b750c01f6 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1312,7 +1312,7 @@ int __perf_session__process_events(struct perf_session *session,
file_offset = page_offset;
head = data_offset - page_offset;

- if (data_offset + data_size < file_size)
+ if (data_size && (data_offset + data_size < file_size))
file_size = data_offset + data_size;

progress_next = file_size / 16;
--
1.8.1.4

2013-10-07 19:02:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/6] perf stat: Set child_pid after perf_evlist__prepare_workload()

From: Namhyung Kim <[email protected]>

The commit acf2892270dc ("perf stat: Use perf_evlist__prepare/
start_workload()") converted to use the function but forgot to update
child_pid. Fix it.

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

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f686d5ff594e..5098f144b92d 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -457,6 +457,7 @@ static int __run_perf_stat(int argc, const char **argv)
perror("failed to prepare workload");
return -1;
}
+ child_pid = evsel_list->workload.pid;
}

if (group)
--
1.8.1.4

2013-10-08 05:32:24

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 0/6] perf/urgent 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 d8b11a0cbd1c66ce283eb9dabe0498dfa6483f32:
>
> perf/x86: Clean up cap_user_time* setting (2013-10-04 09:58:55 +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 b314e5cfd11fd78545ce6c2be42646254390c1aa:
>
> perf session: Fix infinite loop on invalid perf.data file (2013-10-04 15:17:46 -0300)
>
> ----------------------------------------------------------------
> perf/urgent fixes:
>
> . The libaudit test was failing in some systems due to a unescaped newline, fix
> it so that the 'trace' tool can be built in such systems.
>
> . Fix installation of libexec components.
>
> . Add default handler for mmap2 events so that tools that don't explicitely
> define an MMAP2 handler don't crash, fix from David Ahern.
>
> . Fix to find line information for probe list, from Masami Hiramatsu.
>
> . Set child_pid after perf_evlist__prepare_workload(), fix from Namhyung Kim.
>
> . Fix infinite loop on invalid perf.data file, from Namhyung Kim.
>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (2):
> perf tools: Fix libaudit test
> perf tools: Fix installation of libexec components
>
> David Ahern (1):
> perf tools: Add default handler for mmap2 events
>
> Masami Hiramatsu (1):
> perf probe: Fix to find line information for probe list
>
> Namhyung Kim (2):
> perf stat: Set child_pid after perf_evlist__prepare_workload()
> perf session: Fix infinite loop on invalid perf.data file
>
> tools/perf/Makefile | 1 +
> tools/perf/builtin-stat.c | 1 +
> tools/perf/config/feature-tests.mak | 2 +-
> tools/perf/util/dwarf-aux.c | 25 ++++++++++++++++---
> tools/perf/util/dwarf-aux.h | 6 ++++-
> tools/perf/util/header.c | 12 +++++++++
> tools/perf/util/probe-finder.c | 49 +++++++++++++++++++++++++------------
> tools/perf/util/session.c | 4 ++-
> 8 files changed, 77 insertions(+), 23 deletions(-)

Pulled, thanks Arnaldo!

Ingo