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

[Ian Rogers]

On Tue, May 28, 2024 at 12:45 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, 28 May 2024 at 11:59, Ian Rogers <irogers@xxxxxxxxxx> wrote:
> >
> > But nobody else ever reported the issue, even ARM who maintain the PMU
> > driver whose event name conflicts. This hasn't been a problem for
> > anybody else.
>
> I'm not blaming you for having had a bug.
>
> I'm blaming you for NOT DEALING WITH THE BUG APPROPRIATELY.
>
> Taking some time to fix the bug is fine.
>
> But that's not what you've been doing.

On LKML:

Issue reported:
2024-05-25  1:31 ` Linus Torvalds [this message]..
Fix posted:
2024-05-25 15:32 ` Ian Rogers [this message]

The only thing I've tried to clear up is the ambiguity of when an
event doesn't have a PMU what does it mean? Perf's metrics don't
specify PMUs and have uncore events. We can't restrict non-PMU
specifying events to just core events without rewriting them even if
it best matches your mental model, perf has never worked this way.

> Since then, pretty much ALL you have done is argue about irrelevant
> thingas that weren't about the regression in question.
>
> The fact that you still don't agree, having broken documented
> behavior, and still argue against just having it fixed, I can't do
> anything about.
>
> > So I think the revert is a real regression for a larger user base.
>
> I didn't have much choice, did I? You refuse to even acknowledge the
> bug I hit. I'd have been happy if you had just fixed the bug. You
> didn't.  You just argued.
>
> > There is a testing issue here, not least I don't possess an Apple ARM
> > machine.
>
> This is not an Apple ARM machine. I have one of those too, but this
> isn't it. It's an Ampere Computing system, based on an ARM Neoverse N1
> (and the ARM PMU's both for the core and for the interconnect).
>
> But that is pretty much irrelevant by now.
>
> The issue is that you don't fix bugs you leave behind, forcing the revert.

But you've traded a fix for one set of users with a fix for another. I
suspect the number of ARM neoverse N1 users of the PMU are small, not
least as these devices tend to be in the cloud where PMU support is
deliberately limited. As test expectations were for the patch applied,
I think things are further regressed. I'm glad you're happy.

Thanks,
Ian

> I'm happy to test any patches. But I'm done arguing. The "cycles"
> thing needs to work. This is not a "pretty please".
>
> This is a "if you can't understand that and acknowledge that without
> arguing, just work on something else, ok?"
>
>                   Linus
>