Received-SPF: pass (google.com: domain of linux-kernel+bounces-49737-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <ce398eb3-2ded-4149-9ac8-4eb60a474a0d@intel.com>
Date: Fri, 2 Feb 2024 13:16:58 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] perf header: Set proper symbol name for vdso when
 build-id event found
Content-Language: en-US
To: Like Xu <like.xu.linux@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
 linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
 Arnaldo Carvalho de Melo <acme@kernel.org>,
 Namhyung Kim <namhyung@kernel.org>
References: <20231201111506.37155-1-likexu@tencent.com>
 <f50149fb-967c-4987-8d08-d6bb2a69bcf3@intel.com>
 <9d8c8900-699e-4879-8aca-d9371e77f097@gmail.com>
From: Adrian Hunter <adrian.hunter@intel.com>
Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki,
 Business Identity Code: 0357606 - 4, Domiciled in Helsinki
In-Reply-To: <9d8c8900-699e-4879-8aca-d9371e77f097@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 14/12/23 04:48, Like Xu wrote:
> On 14/12/2023 3:26 am, Adrian Hunter wrote:
>> On 1/12/23 13:15, Like Xu wrote:
>>> From: Like Xu <likexu@tencent.com>
>>>
>>> If using perf-record to sample a cpu-cycles:P event on a hypervisor process
>>> when '--kcore' is not enabled, user may find some surprise in perf-report:
>>>
>>> # perf report -i perf.data -v:
>>> # Overhead Command   Shared Object                Symbol
>>>      99.71%  vcpu0    arch/x86/kvm/kvm-intel.ko  0xffffffffa10d1e30 B [k] 0x0000000000034ed0
>>>
>>> build id event received for vmlinux: d12116149f511f7dbd0b21c45d38d3d2ec09b87f [20]
>>> build id event received for kvm-intel.ko: a8fc0213abbafd97b10ce58ce84bec8519f9abce [20]
>>> build id event received for [vdso]: 4d56e381df8d2c051f6bc1ef69c0118c59d5c49f [20]
>>>
>>> # perf report:
>>> # Overhead  Command  Shared Object     Symbol
>>> # ........  .......  ................  .......................................
>>> #
>>>      99.71%  vcpu0    [kvm_intel]       [k] 0x0000000000034ed0
>>>       0.10%  vcpu0    [kernel.vmlinux]  [k] __lock_acquire.isra.29
>>>
>>> Users may be curious as to how 0x34ed0 was generated and wondered if this
>>> RIP came from the guest application but perf-script-D does not point to any
>>> samples of this address.
>>>
>>> Based on perf/tool implementation, this is actually an offset pointing to
>>> the vdso object (in this case it is the assembly __vmx_vcpu_run defined
>>> in arch/x86/kvm/vmx/vmenter.S). The pattern is not reproduced on perf-tool
>>> of some distributions, and git-bisect quickly identified the possible root
>>> cause, which leds to this straightforward fix and after this change:
>>>
>>> # perf report -i perf.data -v:
>>> # Overhead Command   Shared Object                Symbol
>>>      99.71%  vcpu0    arch/x86/kvm/kvm-intel.ko  0x34ed0            B [k] __vmx_vcpu_run
>>>
>>> # perf report:
>>> # Overhead  Command  Shared Object     Symbol
>>> # ........  .......  ................  .......................................
>>> #
>>>      99.71%  vcpu0    [kvm_intel]       [k] __vmx_vcpu_run
>>>
>>> The fix also gets commit 1deec1bd96cc ("perf header: Set proper module name
>>> when build-id event found") lit again.
>>>
>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Ian Rogers <irogers@google.com>
>>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>>> Fixes: b2fe96a350de ("perf tools: Fix module symbol processing")
>>> Signed-off-by: Like Xu <likexu@tencent.com>
>>> ---
>>>   tools/perf/util/header.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>>> index e86b9439ffee..a33d589511ff 100644
>>> --- a/tools/perf/util/header.c
>>> +++ b/tools/perf/util/header.c
>>> @@ -2305,8 +2305,8 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
>>>                 if (!kmod_path__parse_name(&m, filename) && m.kmod)
>>>                   dso__set_module_info(dso, &m, machine);
>>> -
>>> -            dso->kernel = dso_space;
>>> +            else
>>> +                dso->kernel = dso_space;
>>
>> This is undoing some of b2fe96a350de ("perf tools: Fix module
>> symbol processing") without explanation.
> 
> Thanks for your comments.
> W/ this fix, "perf test -v object" looks OK.
> 
>>
>> Symbols in the .noinstr.text section don't seem to be
>> being resolved, so that could be the issue.  perf synthesizes
>> an MMAP record from /proc/modules, which works for .text
>> but perhaps not for .noinstr.text
> 
> Not sure if it covers the potentially broken features you mentioned above,
> would you be willing to provide a more detailed test command ?

I sent a patch that might help a bit:

https://lore.kernel.org/linux-perf-users/20240202110130.3553-3-adrian.hunter@intel.com/