Re: [V5 PATCH] perf parse-events: Specially handle uncore event alias in small groups

[Liang, Kan]

On 5/1/2018 12:33 PM, Jiri Olsa wrote:
> On Tue, May 01, 2018 at 07:31:36AM -0700, kan.liang@xxxxxxxxxxxxxxx wrote:
> > From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> >
> > Perf stat doesn't count the uncore event aliases from the same uncore
> > block in a group, for example:
> >
> >    perf stat -e '{unc_m_cas_count.all,unc_m_clockticks}' -a -I 1000
> >    #           time             counts unit events
> >         1.000447342      <not counted>      unc_m_cas_count.all
> >         1.000447342      <not counted>      unc_m_clockticks
> >         2.000740654      <not counted>      unc_m_cas_count.all
> >         2.000740654      <not counted>      unc_m_clockticks
> >
> > The output is very misleading. It gives a wrong impression that the
> > uncore event doesn't work.
> >
> > An uncore block could be composed by several PMUs. An uncore event alias
> > is a joint name which means the same event runs on all PMUs of a block.
> > Perf doesn't support mixed events from different PMUs in the same group.
> > It is wrong to put uncore event aliases in a big group.
> >
> > The right way is to split the big group into multiple small groups which
> > only include the events from the same PMU.
> > Only uncore event aliases from the same uncore block should be specially
> > handled here. It doesn't make sense to mix the uncore events with other
> > uncore events from different blocks or even core events in a group.
> >
> > With the patch:
> >    #           time             counts unit events
> >       1.001557653            140,833      unc_m_cas_count.all
> >       1.001557653      1,330,231,332      unc_m_clockticks
> >       2.002709483             85,007      unc_m_cas_count.all
> >       2.002709483      1,429,494,563      unc_m_clockticks
> >
> > Reported-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> > Signed-off-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> > ---
> >
> > Changes since V4:
> > - Add WARN_ON when failing to alloc an array for leaders
> > - Add comments to explain why it has to go through the whole group
>
> Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>

Thanks Jirka.

Hi Arnaldo,

Are you OK with the patch?
It looks like Jirka has another patch which will touch the same code.
If it's OK for you, could you please merge the patch? So he can do the 
changes on top of this patch.

Thanks,
Kan