[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ef9b8d69-76b4-4efc-8a89-47f58423b5b1@intel.com>
Date: Thu, 9 Oct 2025 13:34:54 +0800
From: "Li, Tianyou" <tianyou.li@...el.com>
To: Ravi Bangoria <ravi.bangoria@....com>
CC: Namhyung Kim <namhyung@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>, Alexander Shishkin
<alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, "Ian
Rogers" <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>, "Kan
Liang" <kan.liang@...ux.intel.com>, <wangyang.guo@...el.com>,
<pan.deng@...el.com>, <zhiguo.zhou@...el.com>, <jiebin.sun@...el.com>,
<thomas.falcon@...el.com>, <dapeng1.mi@...el.com>,
<linux-perf-users@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5] perf tools c2c: Add annotation support to perf c2c
report
Hi Ravi,
Very appreciated for your testing. Could I add you to the 'Tested-by'
for my next revision?
I am working on the annotate changes. Hope I can get your attention
again in the next revision. Thanks.
Regards,
Tianyou
On 10/6/2025 6:54 PM, Ravi Bangoria wrote:
> Hi Tianyou,
>
> I tested this on an AMD machine and it's working fine. Few minor issues
> I would suggest to address.
>
>>> Perf c2c report currently specified the code address and source:line
>>> information in the cacheline browser, while it is lack of annotation
>>> support like perf report to directly show the disassembly code for
>>> the particular symbol shared that same cacheline. This patches add
>>> a key 'a' binding to the cacheline browser which reuse the annotation
>>> browser to show the disassembly view for easier analysis of cacheline
>>> contentions. By default, the 'TAB' key navigate to the code address
>>> where the contentions detected.
> 1. Annotate browser title always shows "Samples: 0", which is misleading:
>
> Samples: 0 of event 'ibs_op//', 100000 Hz, Event count (approx.): 0
> ^ ^
> `--- this `--- and this
>
> 2. Comment from v3:
>
> > In the annotation browser, we did not need to show the overhead of
> > particular event type, or switch among events. The only selection
> > was set to the ip addr where the hist entry indicated in the
> > cacheline browser. The hist_entry was correctly initialized with
> > .ms field by addr_location when the mem_info can be successfully
> > resolved through addr_location.
>
> It's ideal if we can show sample count/overhead along with the
> instruction (because that's what users are used to seeing with
> perf-report->annotate output). I haven't checked the code but is
> it possible to highlight the instruction by default (Annotate+TAB
> OR Set the font color to red etc.)? Waiting for user to press the
> TAB seems counterintuitive esp when instructions are not tagged
> with overhead.
>
> |1340: > callq 0x10f0 <_init+0xf0>
> |1345: v jmp 0x1376 <thread+0x12d>
> |1347: cmpl $0x1, -0xb0(%rbp)
> |134e: v jne 0x1364 <thread+0x11b>
> |1350: movq 0x2d29(%rip), %rax # 0x4080 <data> <===== Highlight it by default
> |1357: addq $0x1, %rax
> |135b: movq %rax, 0x2d1e(%rip) # 0x4080 <data>
> |1362: v jmp 0x1376 <thread+0x12d>
>
> These are not blockers if Namhyung / Arnaldo are okay with current design.
>
> Thanks,
> Ravi
>
Powered by blists - more mailing lists