Re: Event reordering regression for software events

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Thu, Jul 13, 2023 at 01:37:00PM -0700, Andi Kleen escreveu:
> Hi Ian,
> 
> The current post 6.3 linux-perf-next causes some fairly drastic
> reordering in output for some event combinations. I bisected it down to
> your patch:
> 
> commit 347c2f0a0988c59c402148054ef54648853fa669
> Author: Ian Rogers <irogers@google.com>
> Date:   Sat Mar 11 18:15:40 2023 -0800
> 
>     perf parse-events: Sort and group parsed events
> 
>     This change is intended to be a no-op for most current cases, the
>     default sort order is the order the events were parsed. Where it
>     varies is in how groups are handled. Previously an uncore and core
>     event that are grouped would most often cause the group to be removed:
> 
> I currently only have a large test case, but for the test case [1] below 
> most of the "dummy"s and some other software events end up in a row and also in the
> wrong place, instead of the expected order.
> 
> good:
> 
> ...
> 986476553;ns;duration_time;986476553;100.00;;
> 0;;dummy;983952237;100.00;;
> 0;;cs:u;983952237;100.00;;
> 16532;;minor-faults:u;983952237;100.00;;
> 0;;major-faults:u;983952237;100.00;;
> 0;;migrations:u;983952237;100.00;;
> 3532509496;;cycles:u;23999092;2.00;;
> ....
> 
> bad:
> 
> 1010119571;ns;duration_time;1010119571;100.00;;
> 0;;dummy [software];1007748753;100.00;;
> 0;;cs:u [software];1007748753;100.00;;
> 16496;;minor-faults:u [software];1007748753;100.00;;
> 0;;major-faults:u [software];1007748753;100.00;;
> 0;;migrations:u [software];1007748753;100.00;;
> 0;;dummy [software];1007748753;100.00;;
> 0;;dummy [software];1007748753;100.00;;
> 0;;dummy [software];1007748753;100.00;;
> 0;;dummy [software];1007748753;100.00;;
> 0;;emulation-faults [software];1007748753;100.00;;
> 0;;dummy [software];1007748753;100.00;;
> 0;;dummy [software];1007748753;100.00;;
> 0;;emulation-faults [software];1007748753;100.00;;
> 0;;emulation-faults [software];1007748753;100.00;;
> 3603193382;;cycles:u;23996241;2.00;;
> 2277091922;;cpu/event=0x0,umask=0x3/u;23996241;2.00;;
> 4182126406;;cpu/event=0xc2,umask=0x2/u;23996241;2.00;;
> 4364677170;;cpu/event=0xc0,umask=0x0/u;23996241;2.00;;
> 
> 
> Unfortunately this totally breaks toplev. It needs to have the dummies
> in the right location.`
> 
> Another problem is that it also now adds [software] to lots of software
> events, which of course also breaks any parsing tools. I haven't bisected
> that too, but it needs fixing too.
> 
> I must say using perf recently is frustrating with all the regressions!
> Please be a bit more careful with compatibility.

That is really unfortunate and indeed perf passed thru a lot of changes
trying to properly support hybrid systems and verify reference counting
correctness, a necessary step to resume attempts at being multithreaded.

We have seen a growing number of 'perf test' entries being submitted,
with writing shell scripts that check if the output from various command
lines produce expected results.

It would really be great to have new entries that exercise what is
expected by toplev so that we really are careful with compatibility.

- Arnaldo