Re: [PATCH v1] perf evlist: Force adding default events only to core PMUs

From: Arnaldo Carvalho de Melo
Date: Thu Jun 06 2024 - 09:51:33 EST


On Thu, Jun 06, 2024 at 10:42:33AM +0100, James Clark wrote:
> On 06/06/2024 08:09, Namhyung Kim wrote:
> > On Wed, Jun 5, 2024 at 4:02 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> >> 2) Ignore failures, possibly hiding user errors.

> >> I would prefer for (2) the errors were pr_err rather than pr_debug,
> >> something the user can clean up by getting rid of warned about PMUs.
> >> This will avoid hiding the error, but then on Neoverse cycles will
> >> warn about the arm_dsu PMU's cycles event for exactly Linus' test
> >> case. My understanding is that this is deemed a regression, hence
> >> Arnaldo proposing pr_debug to hide it.

> > Right, if we use pr_err() then users will complain. If we use
> > pr_debug() then errors will be hidden silently.

> I'm not sure if anyone would really complain about warnings for
> attempting to open but not succeeding, as long as the event that they
> really wanted is there. I'm imagining output like this:

> $ perf record -e cycles -- ls

> Warning: skipped arm_dsu/cycles/ event(s), recording on
> armv8_pmuv3_0/cycles/, armv8_pmuv3_1/cycles/

> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.008 MB perf.data (30 samples) ]

> You only really need to worry when no events can be opened, but
> presumably that was a warning anyway.

Agreed, while we don't find a way, old or new to autoritatively skip the
event, when that pr_warning() gets turned into a pr_debug() so that
people expecting that those skipped events were included get a message
telling why they were not.

> And in stat mode I wouldn't expect any warnings.

Right.

- Arnaldo