linux-kernel - Re: [PATCH v3 3/5] perf stat: Remove evlist__add_default

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: <CAP-5=fUbZRkoRN__ZE3YmzhM5HqQABZ7jHymgshZGDzT0LgqOg@mail.gmail.com>
Date: Wed, 29 May 2024 10:39:32 -0700
From: Ian Rogers <irogers@...gle.com>
To: James Clark <james.clark@....com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, 
	Ravi Bangoria <ravi.bangoria@....com>, Yang Jihong <yangjihong@...edance.com>, 
	Ze Gao <zegao2021@...il.com>, Leo Yan <leo.yan@...ux.dev>, Song Liu <song@...nel.org>, 
	K Prateek Nayak <kprateek.nayak@....com>, Kaige Ye <ye@...ge.org>, 
	Yicong Yang <yangyicong@...ilicon.com>, linux-perf-users@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/5] perf stat: Remove evlist__add_default_attrs use strings

On Wed, May 29, 2024 at 8:39 AM James Clark <james.clark@....com> wrote:
>
>
>
> On 10/05/2024 06:37, Ian Rogers wrote:
> > add_default_atttributes would add evsels by having pre-created
> > perf_event_attr, however, this needed fixing for hybrid as the
> > extended PMU type was necessary for each core PMU. The logic for this
> > was in an arch specific x86 function and wasn't present for ARM,
> > meaning that default events weren't being opened on all PMUs on
> > ARM. Change the creation of the default events to use parse_events and
> > strings as that will open the events on all PMUs.
> >
> > Rather than try to detect events on PMUs before parsing, parse the
> > event but skip its output in stat-display.
> >
> > The previous order of hardware events was: cycles,
> > stalled-cycles-frontend, stalled-cycles-backend, instructions. As
> > instructions is a more fundamental concept the order is changed to:
> > instructions, cycles, stalled-cycles-frontend, stalled-cycles-backend.
> >
> > Closes: https://lore.kernel.org/lkml/CAP-5=fVABSBZnsmtRn1uF-k-G1GWM-L5SgiinhPTfHbQsKXb_g@mail.gmail.com/
>
> Taking a look at this one now. I think some example commands and outputs
> in the commit message would be helpful because there are quite a few
> different things mentioned in the closes link.
>
> But I'm assuming this is just for the command without specifying an event:
>
>   $ perf stat
>
> I didn't realise that wasn't working properly and I'd missed that Mark
> spotted it in that link.

Hi James,

Do you want to take on owning fixing this? My concern is that by using
event parsing:

+                               "context-switches,"
+                               "cpu-migrations,"
+                               "page-faults,"
+                               "instructions,"
+                               "cycles,"
+                               "stalled-cycles-frontend,"
+                               "stalled-cycles-backend,"
+                               "branches,"
+                               "branch-misses",

any of the names could conflict with something advertised on an ARM
PMU somewhere. Clearly cycles has already proved to be controversial
and broken for perf record on Neoverse.

Thanks,
Ian