Re: [PATCH v2] perf stat: Introduce skippable evsels

[Ian Rogers]

On Fri, Apr 21, 2023 at 10:11 AM Liang, Kan <kan.liang@xxxxxxxxxxxxxxx> wrote:
>
>
>
> On 2023-04-21 11:49 a.m., Ian Rogers wrote:
> >> Can we take a step back? Create a perf/json_metric or whatever branch,
> >> fix all the issues thoroughly, and then merge to the mainline.
> >>
> >> I think the default of perf stat is frequently used by not only the
> >> newbees but also the veterans. That could have a big user-visible impact.
> >>
> >> The 6.4 merge window is approaching. Can we at least revert the patches
> >> for 6.4?
> >>
> >> Arnaldo, what do you think?
> > Revert is a complete no go, we'd be reintroducing the bugs that are
> > fixed and who will then fix those? How will they be fixed in any way
> > other than how they've already been fixed? I've yet to see a
> > description of a bug that isn't either:
> > 1) an issue because the output formatting changed - the fix here is to
> > use CSV or json output, using the hard coded metrics was a bug here
> > anyway, not least as the metrics are wrong,
> > 2) a pre-existing issue, such as hybrid is broken,
> > 3) a non-issue, such as multiplexing on Skylake.
> > That's not to say everything couldn't run better, which was the issue
> > this patch was looking to address.
>
> Sigh, we really need a clear design and definition regarding the default
> of perf stat, especially what is included, what's the expected layout,
> the forbidden. So everyone from the developers to the users is on the
> same page. Could you please update the document for the default of perf
> stat?
>
>
> One more thing I want to point out is that the json metric may keep
> changing. While the kernel metics (only includes the core part of the
> json metric) is quite stable. For the default of perf stat, the kernel
> metics should be a better choice.
>
>
> Clarification: I'm not looking for reverting all the patches of the json
> metrics in the perf-tool-next branch. The request is to revert the
> patches which impact the default of perf stat. More specifically, the
> patch 39 and 41.
> https://lore.kernel.org/lkml/20230219092848.639226-40-irogers@xxxxxxxxxx/

This is silly. You can't just revert those patches. The removal of the
hard coded logic was so that it didn't need to get ported when the
later patches removed saved values. You are asking for a far bigger
revert.

Thanks,
Ian

> This is really a long discussion. I think we all express the arguments
> clearly. Let's make the final decision and stop here.
>
> Arnaldo? Peter? Ingo?
>
> Could you please share your opinions?
>
> Thanks,
> Kan