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

[Liang, Kan]

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,
Kan