2013-10-26 12:24:20

by Ingo Molnar

[permalink] [raw]
Subject: [GIT PULL] perf fixes

Linus,

Please pull the latest perf-urgent-for-linus git tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf-urgent-for-linus

# HEAD: e4f8eaad70ea186b8da290c99239dce721a34f88 Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent

The tree contains three fixes:

- Two tooling fixes

- Reversal of the new 'MMAP2' extended mmap record ABI, introduced
in this merge window. (Patches were proposed to fix it but it was
all a bit late and we felt it's safer to just delay the ABI one
more kernel release and do it right.)

Thanks,

Ingo

------------------>
Arnaldo Carvalho de Melo (1):
perf scripting perl: Fix build error on Fedora 12

Masami Hiramatsu (1):
perf probe: Fix to initialize fname always before use it

Stephane Eranian (1):
perf: Disable PERF_RECORD_MMAP2 support


kernel/events/core.c | 4 +++
tools/perf/util/event.c | 30 ++++++++++------------
tools/perf/util/evsel.c | 1 -
tools/perf/util/probe-finder.c | 2 +-
.../perf/util/scripting-engines/trace-event-perl.c | 2 +-
5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index d49a9d2..953c143 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6767,6 +6767,10 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
if (ret)
return -EFAULT;

+ /* disabled for now */
+ if (attr->mmap2)
+ return -EINVAL;
+
if (attr->__reserved_1)
return -EINVAL;

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 9b393e7..63df031 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -187,7 +187,7 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
return -1;
}

- event->header.type = PERF_RECORD_MMAP2;
+ event->header.type = PERF_RECORD_MMAP;
/*
* Just like the kernel, see __perf_event_mmap in kernel/perf_event.c
*/
@@ -198,7 +198,6 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
char prot[5];
char execname[PATH_MAX];
char anonstr[] = "//anon";
- unsigned int ino;
size_t size;
ssize_t n;

@@ -209,13 +208,10 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
strcpy(execname, "");

/* 00400000-0040c000 r-xp 00000000 fd:01 41038 /bin/cat */
- n = sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %x:%x %u %s\n",
- &event->mmap2.start, &event->mmap2.len, prot,
- &event->mmap2.pgoff, &event->mmap2.maj,
- &event->mmap2.min,
- &ino, execname);
-
- event->mmap2.ino = (u64)ino;
+ n = sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %*x:%*x %*u %s\n",
+ &event->mmap.start, &event->mmap.len, prot,
+ &event->mmap.pgoff,
+ execname);

if (n != 8)
continue;
@@ -227,15 +223,15 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
strcpy(execname, anonstr);

size = strlen(execname) + 1;
- memcpy(event->mmap2.filename, execname, size);
+ memcpy(event->mmap.filename, execname, size);
size = PERF_ALIGN(size, sizeof(u64));
- event->mmap2.len -= event->mmap.start;
- event->mmap2.header.size = (sizeof(event->mmap2) -
- (sizeof(event->mmap2.filename) - size));
- memset(event->mmap2.filename + size, 0, machine->id_hdr_size);
- event->mmap2.header.size += machine->id_hdr_size;
- event->mmap2.pid = tgid;
- event->mmap2.tid = pid;
+ event->mmap.len -= event->mmap.start;
+ event->mmap.header.size = (sizeof(event->mmap) -
+ (sizeof(event->mmap.filename) - size));
+ memset(event->mmap.filename + size, 0, machine->id_hdr_size);
+ event->mmap.header.size += machine->id_hdr_size;
+ event->mmap.pid = tgid;
+ event->mmap.tid = pid;

if (process(tool, event, &synth_sample, machine) != 0) {
rc = -1;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0ce9feb..9f1ef9b 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -678,7 +678,6 @@ void perf_evsel__config(struct perf_evsel *evsel,
attr->sample_type |= PERF_SAMPLE_WEIGHT;

attr->mmap = track;
- attr->mmap2 = track && !perf_missing_features.mmap2;
attr->comm = track;

/*
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index c09e0a9..f069273 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1357,10 +1357,10 @@ int debuginfo__find_probe_point(struct debuginfo *self, unsigned long addr,
goto post;
}

+ fname = dwarf_decl_file(&spdie);
if (addr == (unsigned long)baseaddr) {
/* Function entry - Relative line number is 0 */
lineno = baseline;
- fname = dwarf_decl_file(&spdie);
goto post;
}

diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
index a85e4ae..c0c9795 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -282,7 +282,7 @@ static void perl_process_tracepoint(union perf_event *perf_event __maybe_unused,

event = find_cache_event(evsel);
if (!event)
- die("ug! no event found for type %" PRIu64, evsel->attr.config);
+ die("ug! no event found for type %" PRIu64, (u64)evsel->attr.config);

pid = raw_field_value(event, "common_pid", data);


2013-10-28 08:28:53

by Markus Trippelsdorf

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes

On 2013.10.26 at 14:24 +0200, Ingo Molnar wrote:
> - Reversal of the new 'MMAP2' extended mmap record ABI, introduced
> in this merge window. (Patches were proposed to fix it but it was
> all a bit late and we felt it's safer to just delay the ABI one
> more kernel release and do it right.)

commit 3090ffb5a2515990182f3f55b0688a7817325488
Author: Stephane Eranian <[email protected]>
Date: Thu Oct 17 19:32:15 2013 +0200

perf: Disable PERF_RECORD_MMAP2 support

The commit above breaks "perf top" on my system, because it causes only
[kernel] symbols to show up in the list. When I revert the commit I get
the usual mixture of userspace and kernel symbols in the list.

--
Markus

2013-10-28 09:02:41

by Markus Trippelsdorf

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes

On 2013.10.28 at 09:28 +0100, Markus Trippelsdorf wrote:
> On 2013.10.26 at 14:24 +0200, Ingo Molnar wrote:
> > - Reversal of the new 'MMAP2' extended mmap record ABI, introduced
> > in this merge window. (Patches were proposed to fix it but it was
> > all a bit late and we felt it's safer to just delay the ABI one
> > more kernel release and do it right.)
>
> commit 3090ffb5a2515990182f3f55b0688a7817325488
> Author: Stephane Eranian <[email protected]>
> Date: Thu Oct 17 19:32:15 2013 +0200
>
> perf: Disable PERF_RECORD_MMAP2 support
>
> The commit above breaks "perf top" on my system, because it causes only
> [kernel] symbols to show up in the list. When I revert the commit I get
> the usual mixture of userspace and kernel symbols in the list.

Forgot to mention that this happens on an otherwise idle machine. If I
run CPU intensive apps they'll show up eventually.

Example (current Linus tree):

PerfTop: 593 irqs/sec kernel:80.6% exact: 0.0% [4000Hz cycles], (all, 4 CPUs)
----------------------------------------------------------------------------------------------

57.75% [kernel] [k] amd_e400_idle
28.66% [unknown] [.] 0x0000000000450bc8
1.51% [kernel] [k] kallsyms_expand_symbol.const
1.22% [kernel] [k] cpu_startup_entry
1.13% [kernel] [k] number.isra.1
0.77% [kernel] [k] clear_page_c
0.74% [kernel] [k] memcpy
0.46% [kernel] [k] task_waking_fair
0.45% [kernel] [k] _raw_spin_lock_irqsave
0.42% [kernel] [k] _raw_spin_unlock_irqrestore
0.39% [kernel] [k] format_decode
0.34% [kernel] [k] do_sys_poll
0.31% [kernel] [k] current_kernel_time
0.30% [kernel] [k] vsnprintf
0.29% [kernel] [k] native_apic_mem_write
0.28% [kernel] [k] select_task_rq_fair
0.26% [kernel] [k] swiotlb_free_coherent
0.26% [kernel] [k] irq_entries_start
0.2n% [kernel] [k] strnlen

With the commit above reverted:

PerfTop: 907 irqs/sec kernel:73.3% exact: 0.0% [4000Hz cycles], (all, 4 CPUs)
-----------------------------------------------------------------------------------------------

30.89% [kernel] [k] amd_e400_idle
6.88% [unknown] [.] 0x00007f346f2ef3a6
5.22% libc-2.18.90.so [.] strcmp
3.65% libc-2.18.90.so [.] strstr
2.65% libbfd-2.24.51.20131018.so [.] d_print_comp.part.8
2.61% perf [.] symbols__insert
2.18% [kernel] [k] cpu_startup_entry
1.82% perf [.] symbol_filter
1.73% perf [.] rb_next
1.58% libc-2.18.90.so [.] critical_factorization
1.34% libelf-0.156.so [.] gelf_getsym
1.28% libQtGui.so.4.8.5 [.] qt_alphargbblit_quint32(QRasterBuffer*, int, int, unsigned int, unsigned int const*, int, int, int, QClipData
1.25% libc-2.18.90.so [.] __libc_calloc
1.17% libc-2.18.90.so [.] _int_malloc
1.15% libc-2.18.90.so [.] strchr
0.97% xmobar [.] 0x000000000000c922
0.97% [kernel] [k] number.isra.1
0.96% libpthread-2.18.90.so [.] pthread_rwlock_rdlock
0.96% libc-2.18.90.so [.] memcpy@@GLIBC_2.14
0.94% perf [.] dso__load_sym
0.93% libc-2.18.90.so [.] memchr
0.84% libbfd-2.24.51.20131018.so [.] d_name
0.75% [kernel] [k] clear_page_c
0.73% libpthread-2.18.90.so [.] pthread_rwlock_unlock
0.66% libc-2.18.90.so [.] memset

--
Markus

2013-10-28 09:34:33

by Markus Trippelsdorf

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes

On 2013.10.28 at 10:02 +0100, ------------------------------ Markus Trippelsdorf wrote:
> On 2013.10.28 at 09:28 +0100, Markus Trippelsdorf wrote:
> > On 2013.10.26 at 14:24 +0200, Ingo Molnar wrote:
> > > - Reversal of the new 'MMAP2' extended mmap record ABI, introduced
> > > in this merge window. (Patches were proposed to fix it but it was
> > > all a bit late and we felt it's safer to just delay the ABI one
> > > more kernel release and do it right.)
> >
> > commit 3090ffb5a2515990182f3f55b0688a7817325488
> > Author: Stephane Eranian <[email protected]>
> > Date: Thu Oct 17 19:32:15 2013 +0200
> >
> > perf: Disable PERF_RECORD_MMAP2 support
> >
> > The commit above breaks "perf top" on my system, because it causes only
> > [kernel] symbols to show up in the list. When I revert the commit I get
> > the usual mixture of userspace and kernel symbols in the list.

The following patch fixes the issue:

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 63df031fc9c7..811f3950654d 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -199,7 +199,6 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
char execname[PATH_MAX];
char anonstr[] = "//anon";
size_t size;
- ssize_t n;

if (fgets(bf, sizeof(bf), fp) == NULL)
break;
@@ -208,14 +207,11 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
strcpy(execname, "");

/* 00400000-0040c000 r-xp 00000000 fd:01 41038 /bin/cat */
- n = sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %*x:%*x %*u %s\n",
+ sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %*x:%*x %*u %s\n",
&event->mmap.start, &event->mmap.len, prot,
&event->mmap.pgoff,
execname);

- if (n != 8)
- continue;
-
if (prot[2] != 'x')
continue;

--
Markus

2013-10-28 12:34:21

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes

Em Mon, Oct 28, 2013 at 10:34:30AM +0100, Markus Trippelsdorf escreveu:
> On 2013.10.28 at 10:02 +0100, Markus Trippelsdorf wrote:
> > On 2013.10.28 at 09:28 +0100, Markus Trippelsdorf wrote:
> > > On 2013.10.26 at 14:24 +0200, Ingo Molnar wrote:
> > > > - Reversal of the new 'MMAP2' extended mmap record ABI, introduced

> > > commit 3090ffb5a2515990182f3f55b0688a7817325488
> > > Author: Stephane Eranian <[email protected]>

> > > perf: Disable PERF_RECORD_MMAP2 support

> The following patch fixes the issue:

> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> /* 00400000-0040c000 r-xp 00000000 fd:01 41038 /bin/cat */
> - n = sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %*x:%*x %*u %s\n",
> + sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %*x:%*x %*u %s\n",
> &event->mmap.start, &event->mmap.len, prot,
> &event->mmap.pgoff,
> execname);

> - if (n != 8)
> - continue;
> -

Yeah, this one was added in the patch that introduced MMAP2 support in
the tooling side, it was unrelated to what the patch needed, i.e. should
have come in a separate patch, checking sscanf number of itens
successfully matched/assigned.

And then, when reverting it, Stephane forgot to match the number number
of expected entries to be matched/assigned from 8 to 5, can you try with
the following patch instead?

- Arnaldo

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 63df031fc9c7..49096ea58a15 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -213,7 +213,7 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
&event->mmap.pgoff,
execname);

- if (n != 8)
+ if (n != 5)
continue;

if (prot[2] != 'x')

2013-10-28 12:42:26

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes

Em Mon, Oct 28, 2013 at 09:34:11AM -0300, Arnaldo Carvalho de Melo escreveu:
> And then, when reverting it, Stephane forgot to match the number number
> of expected entries to be matched/assigned from 8 to 5, can you try with
> the following patch instead?

Does the wording on this commit log sounds about right to you?

- Arnaldo


Attachments:
(No filename) (332.00 B)
a.patch (1.44 kB)
Download all attachments

2013-10-28 12:59:04

by Markus Trippelsdorf

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes

On 2013.10.28 at 09:42 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 28, 2013 at 09:34:11AM -0300, Arnaldo Carvalho de Melo escreveu:
> > And then, when reverting it, Stephane forgot to match the number number
> > of expected entries to be matched/assigned from 8 to 5, can you try with
> > the following patch instead?
>
> Does the wording on this commit log sounds about right to you?

Yes. Your patch is fine. Thanks.

--
Markus

2013-10-29 09:50:32

by Stephane Eranian

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes

On Mon, Oct 28, 2013 at 1:42 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Mon, Oct 28, 2013 at 09:34:11AM -0300, Arnaldo Carvalho de Melo escreveu:
>> And then, when reverting it, Stephane forgot to match the number number
>> of expected entries to be matched/assigned from 8 to 5, can you try with
>> the following patch instead?
>
> Does the wording on this commit log sounds about right to you?
>
Thanks for fixing this issue.

> - Arnaldo

2013-10-29 10:06:30

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes


* Stephane Eranian <[email protected]> wrote:

> On Mon, Oct 28, 2013 at 1:42 PM, Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> > Em Mon, Oct 28, 2013 at 09:34:11AM -0300, Arnaldo Carvalho de Melo escreveu:
> >> And then, when reverting it, Stephane forgot to match the number number
> >> of expected entries to be matched/assigned from 8 to 5, can you try with
> >> the following patch instead?
> >
> > Does the wording on this commit log sounds about right to you?
> >
> Thanks for fixing this issue.

Thanks guys for testing it - I just sent the pending fixes to Linus
so it should soon be resolved upstream as well.

Thanks,

Ingo

2013-10-29 12:48:08

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [GIT PULL] perf fixes

Em Tue, Oct 29, 2013 at 10:50:30AM +0100, Stephane Eranian escreveu:
> On Mon, Oct 28, 2013 at 1:42 PM, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Oct 28, 2013 at 09:34:11AM -0300, Arnaldo Carvalho de Melo escreveu:
> >> And then, when reverting it, Stephane forgot to match the number number
> >> of expected entries to be matched/assigned from 8 to 5, can you try with
> >> the following patch instead?

> > Does the wording on this commit log sounds about right to you?

> Thanks for fixing this issue.

You're welcome! :-)

- Arnaldo