linux-kernel - Re: [PATCH v5] perf tools c2c: Add annotation support to perf c2c report

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2f981591-4159-411c-b530-e884262d1a80@amd.com>
Date: Mon, 6 Oct 2025 16:24:45 +0530
From: Ravi Bangoria <ravi.bangoria@....com>
To: Tianyou Li <tianyou.li@...el.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>, "Ravi
 Bangoria" <ravi.bangoria@....com>
Subject: Re: [PATCH v5] perf tools c2c: Add annotation support to perf c2c
 report

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