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

[Dmitry Vyukov]

On Mon, 25 Aug 2025 at 08:13, Namhyung Kim <namhyung@xxxxxxxxxx> 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@xxxxxxxxxx> 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?