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

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 for Android: free password hash cracker in your pocket

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

Date: Tue, 28 May 2024 13:03:11 -0700
From: Ian Rogers <irogers@...gle.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Leo Yan <leo.yan@...ux.dev>, 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>, 
	James Clark <james.clark@....com>, Dominique Martinet <asmadeus@...ewreck.org>, 
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] perf evlist: Force adding default events only to core PMUs

On Tue, May 28, 2024 at 12:45 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Tue, 28 May 2024 at 11:59, Ian Rogers <irogers@...gle.com> 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
>