Re: [PATCH 2/3] perf mem: Fix missed p-core mem events on ADL and RPL

[Arnaldo Carvalho de Melo]

On Sun, Sep 08, 2024 at 04:30:44PM -0400, Liang, Kan wrote:
> 
> 
> On 2024-09-06 4:06 p.m., Arnaldo Carvalho de Melo wrote:
> > On Fri, Sep 06, 2024 at 12:08:52PM -0400, Liang, Kan wrote:
> >> On 2024-09-06 10:17 a.m., Arnaldo Carvalho de Melo wrote:
> >>> On Thu, Sep 05, 2024 at 03:47:03PM -0400, Liang, Kan wrote:
> >>>> On 2024-09-05 3:33 p.m., Arnaldo Carvalho de Melo wrote:
> >>>>> But can we reconstruct the events relationship (group, :S, etc) from
> >>>>> what we have in the perf.data header?
> > 
> >>>> Do you mean show the group relation in the perf evlist?
> > 
> >>>> $perf mem record sleep 1
> >>>> [ perf record: Woken up 1 times to write data ]
> >>>> [ perf record: Captured and wrote 0.027 MB perf.data (10 samples) ]
> > 
> >>>> $perf evlist -g
> >>>> cpu_atom/mem-loads,ldlat=30/P
> >>>> cpu_atom/mem-stores/P
> >>>> {cpu_core/mem-loads-aux/,cpu_core/mem-loads,ldlat=30/}
> >>>> cpu_core/mem-stores/P
> >>>> dummy:u
> > 
> >>>> The -g option already did it, although the group modifier looks lost.
> > 
> >>> Right, I can reproduce that, but I wonder if we shouldn't make this '-g'
> >>> option the default?
> > 
> >> I think the evlist means a list of events. Only outputting the events
> >> makes sense to me.
> >> With -g, the extra relationship information is provided.
> > 
> > At first 'perf evlist' showing just the events present in the perf.data
> > file seems enough, and maybe it should continue like that.
> > 
> > It is just that this relationship is so critical that not showing it by
> > default looks suboptimal :-\
> > 
> > Perhaps we should add some warning at the end mentioning the special
> > relationships present and suggest using '-g' to see it?
> > 
> 
> Agree, and we already did a similar hint for tracepoint events.
> 
> Here is the patch to add a hint for '-g'.
> https://lore.kernel.org/lkml/20240908202847.176280-1-kan.liang@xxxxxxxxxxxxxxx/

Thanks for doing that, patch applied!

- Arnaldo