linux-kernel - Re: [PATCH 5/5] perf annotate: Add --skip-empty option

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZrEyo4CSm6nKU20s@google.com>
Date: Mon, 5 Aug 2024 13:14:27 -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 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.

Thanks,
Namhyung


diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index efcadb7620b8..8d3ec439b783 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -632,11 +632,15 @@ 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 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;
 
 		hists__find_annotations(hists, pos, ann);