linux-kernel - Re: [PATCH v1 2/2] perf script: Fix for `perf script +F metric` with leader sampling

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=fXNvR-XaYAZrJKaX=nHU2FrmFKCYoX+UyEBv2=TNq4wcw@mail.gmail.com>
Date: Tue, 23 Jul 2024 08:49:34 -0700
From: Ian Rogers <irogers@...gle.com>
To: Andi Kleen <ak@...ux.intel.com>
Cc: James Clark <james.clark@...aro.org>, 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>, 
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
Subject: Re: [PATCH v1 2/2] perf script: Fix for `perf script +F metric` with
 leader sampling

On Tue, Jul 23, 2024 at 8:40 AM Andi Kleen <ak@...ux.intel.com> wrote:
>
> > Yeah I suppose it's strictly better now without the segfault. Could you pull
> > in the test and update it to add -a? At least then that behavior will be
> > locked down and we can extend it later without -a.
>
> Ian's patch really implements completely new functionality (supporting
> metrics over multiple groups). It isn't a regression fix, but completely
> redesigns the old "single group" feature.

I don't understand this, I don't understand "single group". I couldn't
understand the old logic using period. It is true in this patch and
others we've been trying to remove duplicated count values, such as
"saved_value" in shadow stats. Having a single value has led to things
being more robust, for example, counts getting reset in perf stat's
interval mode. The stat-display is still a mess, the stat-shadow code
is also still a mess with bugs in things like column header printing.
It won't get any better by having something as simple as a count
duplicated in at least 3 places.

> > I also tested Andi's V5 and still got the segfault.
>
> Yes the earlier versions worked, but they broke perf stat report.
> I think the right short term fix is to use V3 or so, but with a
> check that perf stat report isn't used.
>
> Then perhaps the new functionality of multiple groups can be considered
> over it.

I look forward to seeing the changes. I was trying to do a service
here as you were randomly jamming 0 in as an aggregation index and
hoping for some magic to come from it. The code here shows how things
can be wired up and it works, but just not in the way the original
stuff did because I didn't understand how the original stuff was
expecting to work. Feel free to ignore it and not see it merged.

Thanks,
Ian