| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZrE7EWyFJ3hThopM@google.com>
Date: Mon, 5 Aug 2024 13:50:25 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Kan Liang <kan.liang@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 5/5] perf annotate: Add --skip-empty option
On Mon, Aug 05, 2024 at 05:23:51PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Aug 05, 2024 at 01:14:27PM -0700, Namhyung Kim wrote:
> > On Mon, Aug 05, 2024 at 04:22:12PM -0300, Arnaldo Carvalho de Melo wrote:
> > > On Sat, Aug 03, 2024 at 02:13:32PM -0700, Namhyung Kim wrote:
> > > > Like in perf report, we want to hide empty events in the perf annotate
> > > > output. This is consistent when the option is set in perf report.
> > > >
> > > > For example, the following command would use 3 events including dummy.
> > > >
> > > > $ perf mem record -a -- perf test -w noploop
> > > >
> > > > $ perf evlist
> > > > cpu/mem-loads,ldlat=30/P
> > > > cpu/mem-stores/P
> > > > dummy:u
> > > >
> > > > Just using perf annotate with --group will show the all 3 events.
> > >
> > > Seems unrelated, just before compiling with this patch:
> > >
> > > root@x1:~# perf mem record -a -- perf test -w noploop
> > > Memory events are enabled on a subset of CPUs: 4-11
> > > [ perf record: Woken up 1 times to write data ]
> > > [ perf record: Captured and wrote 10.506 MB perf.data (2775 samples) ]
> > > root@x1:~#
> > >
> > > root@x1:~# perf annotate --group --stdio2 sched_clock
> > > root@x1:~# perf annotate --stdio2 sched_clock
> > > Samples: 178 of event 'cpu_atom/mem-stores/P', 4000 Hz, Event count (approx.): 565268, [percent: local period]
> > > sched_clock() /usr/lib/debug/lib/modules/6.8.11-200.fc39.x86_64/vmlinux
> > > Percent 0xffffffff810511e0 <sched_clock>:
> > > endbr64
> > > 5.76 incl pcpu_hot+0x8
> > > 5.47 → callq sched_clock_noinstr
> > > 88.78 decl pcpu_hot+0x8
> > > ↓ je 1e
> > > → jmp __x86_return_thunk
> > > 1e: → callq __SCT__preempt_schedule_notrace
> > > → jmp __x86_return_thunk
> > > root@x1:~# perf annotate --group --stdio2 sched_clock
> > > root@x1:~# perf annotate --group --stdio sched_clock
> > > root@x1:~# perf annotate --group sched_clock
> > > root@x1:~#
> > >
> > > root@x1:~# perf evlist
> > > cpu_atom/mem-loads,ldlat=30/P
> > > cpu_atom/mem-stores/P
> > > dummy:u
> > > root@x1:~#
> > >
> > > root@x1:~# perf report --header-only | grep cmdline
> > > # cmdline : /home/acme/bin/perf mem record -a -- perf test -w noploop
> > > root@x1:~#
> > >
> > > I thought it would be some hybrid oddity but seems to be just --group
> > > related, seems like it stops if the first event has no samples? Because
> > > it works with another symbol:
> >
> > Good catch. Yeah I found it only checked the first event. Something
> > like below should fix the issue.
>
> Nope, with the patch applied:
>
> root@x1:~# perf annotate --group --stdio sched_clock
> root@x1:~# perf annotate --stdio sched_clock
> Percent | Source code & Disassembly of vmlinux for cpu_atom/mem-stores/P (147 samples, percent: local period)
> -------------------------------------------------------------------------------------------------------------------
> : 0 0xffffffff810511e0 <sched_clock>:
> 0.00 : ffffffff810511e0: endbr64
> 5.11 : ffffffff810511e4: incl %gs:0x7efe2d5d(%rip) # 33f48 <pcpu_hot+0x8>
> 0.13 : ffffffff810511eb: callq 0xffffffff821350d0
> 94.76 : ffffffff810511f0: decl %gs:0x7efe2d51(%rip) # 33f48 <pcpu_hot+0x8>
> 0.00 : ffffffff810511f7: je 0xffffffff810511fe
> 0.00 : ffffffff810511f9: jmp 0xffffffff82153320
> 0.00 : ffffffff810511fe: callq 0xffffffff82153990
> 0.00 : ffffffff81051203: jmp 0xffffffff82153320
> root@x1:~# perf annotate --group --stdio sched_clock
> root@x1:~# perf annotate --group --stdio2 sched_clock
> root@x1:~# perf annotate --group sched_clock
> root@x1:~#
Oh ok, it was not enough. It should call evsel__output_resort() after
hists__match() and hists__link(). Use this instead.
Thanks,
Namhyung
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index efcadb7620b8..1bfe41783a7c 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -632,13 +632,23 @@ static int __cmd_annotate(struct perf_annotate *ann)
evlist__for_each_entry(session->evlist, pos) {
struct hists *hists = evsel__hists(pos);
u32 nr_samples = hists->stats.nr_samples;
+ struct ui_progress prog;
+ struct evsel *evsel;
- if (nr_samples == 0)
+ if (!symbol_conf.event_group || !evsel__is_group_leader(pos))
continue;
- if (!symbol_conf.event_group || !evsel__is_group_leader(pos))
+ for_each_group_member(evsel, pos)
+ nr_samples += evsel__hists(evsel)->stats.nr_samples;
+
+ if (nr_samples == 0)
continue;
+ ui_progress__init(&prog, nr_samples,
+ "Sorting group events for output...");
+ evsel__output_resort(pos, &prog);
+ ui_progress__finish();
+
hists__find_annotations(hists, pos, ann);
}
Powered by blists - more mailing lists