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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

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@google.com> 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