linux-kernel - Re: [PATCH 1/1] Revert "perf hist: Fix bogus profiles when filters are enabled"

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] [day] [month] [year] [list]

Message-ID: <CACT4Y+ZxqgqAGoSgUaVKk6_=h1dO7iV8qrVHDOrmbBS3VW8-=g@mail.gmail.com>
Date: Fri, 29 Aug 2025 07:59:19 +0200
From: Dmitry Vyukov <dvyukov@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>, 
	Ian Rogers <irogers@...gle.com>, James Clark <james.clark@...aro.org>, 
	Jiri Olsa <jolsa@...nel.org>, Kan Liang <kan.liang@...ux.intel.com>, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 1/1] Revert "perf hist: Fix bogus profiles when filters
 are enabled"

On Mon, 25 Aug 2025 at 08:13, Namhyung Kim <namhyung@...nel.org> wrote:
>
> Hello,
>
> On Wed, Aug 20, 2025 at 06:14:08PM -0700, Dmitry Vyukov wrote:
> > On Wed, 20 Aug 2025 at 10:23, Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> > >
> > > This reverts commit 8b4799e4f0f40a4ec737bf870aa38d06288bf0fb.
> > >
> > > Not combining entries in 'perf top', so we're getting multiple lines for
> > > the same symbol, with the same address.
> > >
> > > To test it, simply run 'perf top', then do /acpi to see just symbols
> > > starting with acpi_ and notice that there are various lines with the
> > > same symbol, press V to see the address and its the same.
> >
> > With this revert, does it show 1 entry but with a wrong percent?
> > I am not sure why there are 2 entries for the same symbol, but if we
> > merge them, we can sum of percents. Is it the right thing to do?
>
> I don't think it'd have a wrong percent.  The hists maintain stats for
> filtered entries separately.

I still don't fully follow.

If we merge a filtered entry into non-filtered entry (with the
revert), now we attribute what was filtered out to the non-filtered
entry, and the non-filtered entry has wrong overhead, no?

If we merge the other way around: non-filtered entry into filtered
entry, then we won't show it at all.

> Based on the position of filtered entries in the RB tree, I think it
> might not merge correct samples together and create multiple entries
> with the same info.

The second thing I don't understand: without this revert we don't
merge filtered and non-filtered entries, top shows duplicate entries,
does it mean it shows filtered out entries? This also looks wrong...

> Filtering by unused sort keys would be undefined.  We probably want to
> warn users instead.

Do you mean that the filtered=1 is set incorrectly in this case?
Do you mean that with this revert 2 bugs just compensate each other by
luck: we wrongly set filtered=1, and then wrongly merge them together,
so it all works out in the end?