All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf symbol: ignore $a/$d symbols for ARM modules
@ 2021-10-27  9:52 Lexi Shao
  2021-10-27 10:23 ` James Clark
  2021-10-28  8:44 ` [PATCH] perf symbol: ignore $a/$d symbols for ARM modules James Clark
  0 siblings, 2 replies; 12+ messages in thread
From: Lexi Shao @ 2021-10-27  9:52 UTC (permalink / raw)
  To: linux-perf-users, linux-kernel
  Cc: acme, mark.rutland, peterz, mingo, alexander.shishkin, jolsa,
	namhyung, shaolexi, qiuxi1, nixiaoming, wangbing6

On ARM machine, kernel symbols from modules can be resolved to $a
instead of printing the actual symbol name. Ignore symbols starting with
"$" when building kallsyms rbtree.

A sample stacktrace is shown as follows:

c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
bf4a66d8 $a+0x78 ([test_module])
c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])

On ARM machine, $a/$d symbols are used by the compiler to mark the
beginning of code/data part in code section. These symbols are filtered
out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
are left on modules. So there are $a symbols in /proc/kallsyms which
share the same addresses with the actual module symbols and confuses perf
when resolving symbols.

After this patch, the module symbol name is printed:

c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
bf4a66d8 test_func+0x78 ([test_module])
c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])

Signed-off-by: Lexi Shao <shaolexi@huawei.com>
---
 tools/perf/util/symbol.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0fc9a5410739..35116aed74eb 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
 	if (!symbol_type__filter(type))
 		return 0;
 
+	/* Ignore local symbols for ARM modules */
+	if (name[0] == '$')
+		return 0;
+
 	/*
 	 * module symbols are not sorted so we add all
 	 * symbols, setting length to 0, and rely on
-- 
2.12.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules
  2021-10-27  9:52 [PATCH] perf symbol: ignore $a/$d symbols for ARM modules Lexi Shao
@ 2021-10-27 10:23 ` James Clark
  2021-10-27 12:31   ` Lexi Shao
  2021-10-28  8:44 ` [PATCH] perf symbol: ignore $a/$d symbols for ARM modules James Clark
  1 sibling, 1 reply; 12+ messages in thread
From: James Clark @ 2021-10-27 10:23 UTC (permalink / raw)
  To: Lexi Shao, linux-perf-users, linux-kernel
  Cc: acme, mark.rutland, peterz, mingo, alexander.shishkin, jolsa,
	namhyung, qiuxi1, nixiaoming, wangbing6



On 27/10/2021 10:52, Lexi Shao wrote:
> On ARM machine, kernel symbols from modules can be resolved to $a
> instead of printing the actual symbol name. Ignore symbols starting with
> "$" when building kallsyms rbtree.
> 
> A sample stacktrace is shown as follows:
> 
> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
> bf4a66d8 $a+0x78 ([test_module])
> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
> 
> On ARM machine, $a/$d symbols are used by the compiler to mark the
> beginning of code/data part in code section. These symbols are filtered
> out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
> are left on modules. So there are $a symbols in /proc/kallsyms which
> share the same addresses with the actual module symbols and confuses perf
> when resolving symbols.

Hi Lexi,

Is it worth using or re-implementing the entire is_ignored_symbol() function
from scripts/kallsyms.c? It seems like this change only fixes one occurrence,
but is_ignored_symbol() has a big list of other cases.

Unless those cases are different?

Thanks
James

> 
> After this patch, the module symbol name is printed:
> 
> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
> bf4a66d8 test_func+0x78 ([test_module])
> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
> 
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
> ---
>  tools/perf/util/symbol.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 0fc9a5410739..35116aed74eb 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
>  	if (!symbol_type__filter(type))
>  		return 0;
>  
> +	/* Ignore local symbols for ARM modules */
> +	if (name[0] == '$')
> +		return 0;
> +
>  	/*
>  	 * module symbols are not sorted so we add all
>  	 * symbols, setting length to 0, and rely on
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules
  2021-10-27 10:23 ` James Clark
@ 2021-10-27 12:31   ` Lexi Shao
  2021-10-27 15:10     ` James Clark
  0 siblings, 1 reply; 12+ messages in thread
From: Lexi Shao @ 2021-10-27 12:31 UTC (permalink / raw)
  To: james.clark
  Cc: acme, alexander.shishkin, jolsa, linux-kernel, linux-perf-users,
	mark.rutland, mingo, namhyung, nixiaoming, peterz, qiuxi1,
	shaolexi, wangbing6

On 27/10/2021 18:24, James Clark wrote:
>On 27/10/2021 10:52, Lexi Shao wrote:
>> On ARM machine, kernel symbols from modules can be resolved to $a
>> instead of printing the actual symbol name. Ignore symbols starting with
>> "$" when building kallsyms rbtree.
>> 
>> A sample stacktrace is shown as follows:
>> 
>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>> bf4a66d8 $a+0x78 ([test_module])
>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>> 
>> On ARM machine, $a/$d symbols are used by the compiler to mark the
>> beginning of code/data part in code section. These symbols are filtered
>> out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
>> are left on modules. So there are $a symbols in /proc/kallsyms which
>> share the same addresses with the actual module symbols and confuses perf
>> when resolving symbols.
>
>Hi Lexi,
>
>Is it worth using or re-implementing the entire is_ignored_symbol() function
>from scripts/kallsyms.c? It seems like this change only fixes one occurrence,
>but is_ignored_symbol() has a big list of other cases.
>
>Unless those cases are different?
>
>Thanks
>James

Hi James,

I don't think it's necessary to cover all the cases listed in
is_ignored_symbol(). As long as the symbols are unique and not overlapping
with each other, they should't cause problems in resolving symbols. So most
of the cases in is_ignored_symbol() should be irrelevant.

Lexi

>
>> 
>> After this patch, the module symbol name is printed:
>> 
>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>> bf4a66d8 test_func+0x78 ([test_module])
>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>> 
>> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
>> ---
>>  tools/perf/util/symbol.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 0fc9a5410739..35116aed74eb 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
>>  	if (!symbol_type__filter(type))
>>  		return 0;
>>  
>> +	/* Ignore local symbols for ARM modules */
>> +	if (name[0] == '$')
>> +		return 0;
>> +
>>  	/*
>>  	 * module symbols are not sorted so we add all
>>  	 * symbols, setting length to 0, and rely on
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules
  2021-10-27 12:31   ` Lexi Shao
@ 2021-10-27 15:10     ` James Clark
  2021-10-28  2:05       ` Lexi Shao
  0 siblings, 1 reply; 12+ messages in thread
From: James Clark @ 2021-10-27 15:10 UTC (permalink / raw)
  To: Lexi Shao
  Cc: acme, alexander.shishkin, jolsa, linux-kernel, linux-perf-users,
	mark.rutland, mingo, namhyung, nixiaoming, peterz, qiuxi1,
	wangbing6



On 27/10/2021 13:31, Lexi Shao wrote:
> On 27/10/2021 18:24, James Clark wrote:
>> On 27/10/2021 10:52, Lexi Shao wrote:
>>> On ARM machine, kernel symbols from modules can be resolved to $a
>>> instead of printing the actual symbol name. Ignore symbols starting with
>>> "$" when building kallsyms rbtree.
>>>
>>> A sample stacktrace is shown as follows:
>>>
>>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>>> bf4a66d8 $a+0x78 ([test_module])
>>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>>>
>>> On ARM machine, $a/$d symbols are used by the compiler to mark the
>>> beginning of code/data part in code section. These symbols are filtered
>>> out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
>>> are left on modules. So there are $a symbols in /proc/kallsyms which
>>> share the same addresses with the actual module symbols and confuses perf
>>> when resolving symbols.
>>
>> Hi Lexi,
>>
>> Is it worth using or re-implementing the entire is_ignored_symbol() function
>>from scripts/kallsyms.c? It seems like this change only fixes one occurrence,
>> but is_ignored_symbol() has a big list of other cases.
>>
>> Unless those cases are different?
>>
>> Thanks
>> James
> 
> Hi James,
> 
> I don't think it's necessary to cover all the cases listed in
> is_ignored_symbol(). As long as the symbols are unique and not overlapping
> with each other, they should't cause problems in resolving symbols. So most
> of the cases in is_ignored_symbol() should be irrelevant.

Is it possible to do the filtering of the module symbols somewhere else like
in kernel/kallsyms.c? I'm not that familiar with it but it seems like
at one point when populating kallsyms '$...' functions are filtered out, but
when modules are loaded the symbols are not filtered because another code path is
used?

If kallsyms has some duplicate addresses in there then isn't the bug in kallsyms
rather than perf? And another tool could get confused too.

James

> 
> Lexi
> 
>>
>>>
>>> After this patch, the module symbol name is printed:
>>>
>>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>>> bf4a66d8 test_func+0x78 ([test_module])
>>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>>>
>>> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
>>> ---
>>>  tools/perf/util/symbol.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>>> index 0fc9a5410739..35116aed74eb 100644
>>> --- a/tools/perf/util/symbol.c
>>> +++ b/tools/perf/util/symbol.c
>>> @@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
>>>  	if (!symbol_type__filter(type))
>>>  		return 0;
>>>  
>>> +	/* Ignore local symbols for ARM modules */
>>> +	if (name[0] == '$')
>>> +		return 0;
>>> +
>>>  	/*
>>>  	 * module symbols are not sorted so we add all
>>>  	 * symbols, setting length to 0, and rely on
>>>
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules
  2021-10-27 15:10     ` James Clark
@ 2021-10-28  2:05       ` Lexi Shao
  2021-10-28  8:42         ` James Clark
  0 siblings, 1 reply; 12+ messages in thread
From: Lexi Shao @ 2021-10-28  2:05 UTC (permalink / raw)
  To: james.clark
  Cc: acme, alexander.shishkin, jolsa, linux-kernel, linux-perf-users,
	mark.rutland, mingo, namhyung, nixiaoming, peterz, qiuxi1,
	shaolexi, wangbing6

On 27/10/2021 23:10, James Clark wrote:
>On 27/10/2021 13:31, Lexi Shao wrote:
>> On 27/10/2021 18:24, James Clark wrote:
>>> On 27/10/2021 10:52, Lexi Shao wrote:
>>>> On ARM machine, kernel symbols from modules can be resolved to $a
>>>> instead of printing the actual symbol name. Ignore symbols starting with
>>>> "$" when building kallsyms rbtree.
>>>>
>>>> A sample stacktrace is shown as follows:
>>>>
>>>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>>>> bf4a66d8 $a+0x78 ([test_module])
>>>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>>>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>>>>
>>>> On ARM machine, $a/$d symbols are used by the compiler to mark the
>>>> beginning of code/data part in code section. These symbols are filtered
>>>> out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
>>>> are left on modules. So there are $a symbols in /proc/kallsyms which
>>>> share the same addresses with the actual module symbols and confuses perf
>>>> when resolving symbols.
>>>
>>> Hi Lexi,
>>>
>>> Is it worth using or re-implementing the entire is_ignored_symbol() function
>>>from scripts/kallsyms.c? It seems like this change only fixes one occurrence,
>>> but is_ignored_symbol() has a big list of other cases.
>>>
>>> Unless those cases are different?
>>>
>>> Thanks
>>> James
>> 
>> Hi James,
>> 
>> I don't think it's necessary to cover all the cases listed in
>> is_ignored_symbol(). As long as the symbols are unique and not overlapping
>> with each other, they should't cause problems in resolving symbols. So most
>> of the cases in is_ignored_symbol() should be irrelevant.
>
>Is it possible to do the filtering of the module symbols somewhere else like
>in kernel/kallsyms.c? I'm not that familiar with it but it seems like
>at one point when populating kallsyms '$...' functions are filtered out, but
>when modules are loaded the symbols are not filtered because another code path is
>used?
>
>If kallsyms has some duplicate addresses in there then isn't the bug in kallsyms
>rather than perf? And another tool could get confused too.
>
>James

Yes we can filter these symbols out when adding module symbols to the kallsyms
list. I will send another patch that modifies module.c to ignore '$' symbols
when loading module.

I think it's worth applying this patch in the sense that perf userspace tool
can be used on all 5.x kernels, and people don't have to update the kernel
to fix it.

Lexi

>
>> 
>> Lexi
>> 
>>>
>>>>
>>>> After this patch, the module symbol name is printed:
>>>>
>>>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>>>> bf4a66d8 test_func+0x78 ([test_module])
>>>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>>>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>>>>
>>>> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
>>>> ---
>>>>  tools/perf/util/symbol.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>>>> index 0fc9a5410739..35116aed74eb 100644
>>>> --- a/tools/perf/util/symbol.c
>>>> +++ b/tools/perf/util/symbol.c
>>>> @@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
>>>>  	if (!symbol_type__filter(type))
>>>>  		return 0;
>>>>  
>>>> +	/* Ignore local symbols for ARM modules */
>>>> +	if (name[0] == '$')
>>>> +		return 0;
>>>> +
>>>>  	/*
>>>>  	 * module symbols are not sorted so we add all
>>>>  	 * symbols, setting length to 0, and rely on
>>>>
>>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules
  2021-10-28  2:05       ` Lexi Shao
@ 2021-10-28  8:42         ` James Clark
  2021-10-29  6:50           ` [PATCH v2 0/2] kallsyms: Ignore $a/$d symbols in kallsyms for ARM Lexi Shao
  0 siblings, 1 reply; 12+ messages in thread
From: James Clark @ 2021-10-28  8:42 UTC (permalink / raw)
  To: Lexi Shao
  Cc: acme, alexander.shishkin, jolsa, linux-kernel, linux-perf-users,
	mark.rutland, mingo, namhyung, nixiaoming, peterz, qiuxi1,
	wangbing6



On 28/10/2021 03:05, Lexi Shao wrote:
> On 27/10/2021 23:10, James Clark wrote:
>> On 27/10/2021 13:31, Lexi Shao wrote:
>>> On 27/10/2021 18:24, James Clark wrote:
>>>> On 27/10/2021 10:52, Lexi Shao wrote:
>>>>> On ARM machine, kernel symbols from modules can be resolved to $a
>>>>> instead of printing the actual symbol name. Ignore symbols starting with
>>>>> "$" when building kallsyms rbtree.
>>>>>
>>>>> A sample stacktrace is shown as follows:
>>>>>
>>>>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>>>>> bf4a66d8 $a+0x78 ([test_module])
>>>>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>>>>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>>>>>
>>>>> On ARM machine, $a/$d symbols are used by the compiler to mark the
>>>>> beginning of code/data part in code section. These symbols are filtered
>>>>> out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
>>>>> are left on modules. So there are $a symbols in /proc/kallsyms which
>>>>> share the same addresses with the actual module symbols and confuses perf
>>>>> when resolving symbols.
>>>>
>>>> Hi Lexi,
>>>>
>>>> Is it worth using or re-implementing the entire is_ignored_symbol() function
>>> >from scripts/kallsyms.c? It seems like this change only fixes one occurrence,
>>>> but is_ignored_symbol() has a big list of other cases.
>>>>
>>>> Unless those cases are different?
>>>>
>>>> Thanks
>>>> James
>>>
>>> Hi James,
>>>
>>> I don't think it's necessary to cover all the cases listed in
>>> is_ignored_symbol(). As long as the symbols are unique and not overlapping
>>> with each other, they should't cause problems in resolving symbols. So most
>>> of the cases in is_ignored_symbol() should be irrelevant.
>>
>> Is it possible to do the filtering of the module symbols somewhere else like
>> in kernel/kallsyms.c? I'm not that familiar with it but it seems like
>> at one point when populating kallsyms '$...' functions are filtered out, but
>> when modules are loaded the symbols are not filtered because another code path is
>> used?
>>
>> If kallsyms has some duplicate addresses in there then isn't the bug in kallsyms
>> rather than perf? And another tool could get confused too.
>>
>> James
> 
> Yes we can filter these symbols out when adding module symbols to the kallsyms
> list. I will send another patch that modifies module.c to ignore '$' symbols
> when loading module.
> 

That sounds great thanks.

> I think it's worth applying this patch in the sense that perf userspace tool
> can be used on all 5.x kernels, and people don't have to update the kernel
> to fix it.

Yes you are right. In that case I will add a review tag to this change.

Thanks
James

> 
> Lexi
> 
>>
>>>
>>> Lexi
>>>
>>>>
>>>>>
>>>>> After this patch, the module symbol name is printed:
>>>>>
>>>>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>>>>> bf4a66d8 test_func+0x78 ([test_module])
>>>>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>>>>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>>>>>
>>>>> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
>>>>> ---
>>>>>  tools/perf/util/symbol.c | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>>>>> index 0fc9a5410739..35116aed74eb 100644
>>>>> --- a/tools/perf/util/symbol.c
>>>>> +++ b/tools/perf/util/symbol.c
>>>>> @@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
>>>>>  	if (!symbol_type__filter(type))
>>>>>  		return 0;
>>>>>  
>>>>> +	/* Ignore local symbols for ARM modules */
>>>>> +	if (name[0] == '$')
>>>>> +		return 0;
>>>>> +
>>>>>  	/*
>>>>>  	 * module symbols are not sorted so we add all
>>>>>  	 * symbols, setting length to 0, and rely on
>>>>>
>>>>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules
  2021-10-27  9:52 [PATCH] perf symbol: ignore $a/$d symbols for ARM modules Lexi Shao
  2021-10-27 10:23 ` James Clark
@ 2021-10-28  8:44 ` James Clark
  2021-11-06 19:48   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 12+ messages in thread
From: James Clark @ 2021-10-28  8:44 UTC (permalink / raw)
  To: Lexi Shao, linux-perf-users, linux-kernel
  Cc: acme, mark.rutland, peterz, mingo, alexander.shishkin, jolsa,
	namhyung, qiuxi1, nixiaoming, wangbing6



On 27/10/2021 10:52, Lexi Shao wrote:
> On ARM machine, kernel symbols from modules can be resolved to $a
> instead of printing the actual symbol name. Ignore symbols starting with
> "$" when building kallsyms rbtree.
> 
> A sample stacktrace is shown as follows:
> 
> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
> bf4a66d8 $a+0x78 ([test_module])
> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
> 
> On ARM machine, $a/$d symbols are used by the compiler to mark the
> beginning of code/data part in code section. These symbols are filtered
> out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
> are left on modules. So there are $a symbols in /proc/kallsyms which
> share the same addresses with the actual module symbols and confuses perf
> when resolving symbols.
> 
> After this patch, the module symbol name is printed:
> 
> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
> bf4a66d8 test_func+0x78 ([test_module])
> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
> 
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>

Reviewed-by: James Clark <james.clark@arm.com>

> ---
>  tools/perf/util/symbol.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 0fc9a5410739..35116aed74eb 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
>  	if (!symbol_type__filter(type))
>  		return 0;
>  
> +	/* Ignore local symbols for ARM modules */
> +	if (name[0] == '$')
> +		return 0;
> +
>  	/*
>  	 * module symbols are not sorted so we add all
>  	 * symbols, setting length to 0, and rely on
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 0/2] kallsyms: Ignore $a/$d symbols in kallsyms for ARM
  2021-10-28  8:42         ` James Clark
@ 2021-10-29  6:50           ` Lexi Shao
  2021-10-29  6:50             ` [PATCH v2 1/2] perf symbol: ignore $a/$d symbols for ARM modules Lexi Shao
  2021-10-29  6:50             ` [PATCH v2 2/2] kallsyms: ignore arm mapping symbols when loading module Lexi Shao
  0 siblings, 2 replies; 12+ messages in thread
From: Lexi Shao @ 2021-10-29  6:50 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users
  Cc: james.clark, acme, alexander.shishkin, jolsa, mark.rutland,
	mingo, namhyung, nixiaoming, peterz, qiuxi1, shaolexi, wangbing6,
	jeyu, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, natechancellor, ndesaulniers, bpf,
	clang-built-linux

On ARM machine, $a/$d symbols are used by the compiler to mark the beginning
of code/data part in code section. These symbols are filtered out when
linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but are left on
modules. So there are $a symbols in /proc/kallsyms whose address overlap
with the actual module symbols and can confuse tools such as perf when
resolving kernel symbols.

A sample stacktrace shown by perf script:
c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
bf4a66d8 $a+0x78 ([test_module]) // $a is shown instead of actual sym name
c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])

This patch set contains 2 patches to fix such problem:
The 1st patch modifies perf userspace tools to ignore $a/$d symbols from
/proc/kallsyms. So people can use new perf tool to get correct kernel symbol
on arm machines instead of updating kernel image.

The 2nd patch modifies the logic of loading modules to ignore arm mapping
symbols in the first place. Being left out in vmlinux and kernelspace API
(e.g. module_kallsyms_on_each_symbol) means these symbols are not used
anywhere, so it should be safe to remove them from module kallsyms list.

v2:
 - Add 2nd patch as discussed with James Clark, see:
   https://lore.kernel.org/all/c7dfbd17-85fd-b914-b90f-082abc64c9d1@arm.com/

Lexi Shao (2):
  perf symbol: ignore $a/$d symbols for ARM modules
  kallsyms: ignore arm mapping symbols when loading module

 kernel/module.c          | 19 +++++++++++--------
 tools/perf/util/symbol.c |  4 ++++
 2 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.12.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/2] perf symbol: ignore $a/$d symbols for ARM modules
  2021-10-29  6:50           ` [PATCH v2 0/2] kallsyms: Ignore $a/$d symbols in kallsyms for ARM Lexi Shao
@ 2021-10-29  6:50             ` Lexi Shao
  2021-10-29  6:50             ` [PATCH v2 2/2] kallsyms: ignore arm mapping symbols when loading module Lexi Shao
  1 sibling, 0 replies; 12+ messages in thread
From: Lexi Shao @ 2021-10-29  6:50 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users
  Cc: james.clark, acme, alexander.shishkin, jolsa, mark.rutland,
	mingo, namhyung, nixiaoming, peterz, qiuxi1, shaolexi, wangbing6,
	jeyu, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, natechancellor, ndesaulniers, bpf,
	clang-built-linux

On ARM machine, kernel symbols from modules can be resolved to $a
instead of printing the actual symbol name. Ignore symbols starting with
"$" when building kallsyms rbtree.

A sample stacktrace is shown as follows:

c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
bf4a66d8 $a+0x78 ([test_module])
c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])

On ARM machine, $a/$d symbols are used by the compiler to mark the
beginning of code/data part in code section. These symbols are filtered
out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
are left on modules. So there are $a symbols in /proc/kallsyms which
share the same addresses with the actual module symbols and confuses perf
when resolving symbols.

After this patch, the module symbol name is printed:

c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
bf4a66d8 test_func+0x78 ([test_module])
c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])

Signed-off-by: Lexi Shao <shaolexi@huawei.com>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/symbol.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0fc9a5410739..35116aed74eb 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
 	if (!symbol_type__filter(type))
 		return 0;
 
+	/* Ignore local symbols for ARM modules */
+	if (name[0] == '$')
+		return 0;
+
 	/*
 	 * module symbols are not sorted so we add all
 	 * symbols, setting length to 0, and rely on
-- 
2.12.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] kallsyms: ignore arm mapping symbols when loading module
  2021-10-29  6:50           ` [PATCH v2 0/2] kallsyms: Ignore $a/$d symbols in kallsyms for ARM Lexi Shao
  2021-10-29  6:50             ` [PATCH v2 1/2] perf symbol: ignore $a/$d symbols for ARM modules Lexi Shao
@ 2021-10-29  6:50             ` Lexi Shao
  2021-11-02  9:43               ` James Clark
  1 sibling, 1 reply; 12+ messages in thread
From: Lexi Shao @ 2021-10-29  6:50 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users
  Cc: james.clark, acme, alexander.shishkin, jolsa, mark.rutland,
	mingo, namhyung, nixiaoming, peterz, qiuxi1, shaolexi, wangbing6,
	jeyu, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, natechancellor, ndesaulniers, bpf,
	clang-built-linux

Arm modules contains mapping symbols(e.g. $a $d) which are ignored in
module_kallsyms_on_each_symbol. However, these symbols are still
displayed when catting /proc/kallsyms. This confuses tools(e.g. perf)
that resolves kernel symbols with address using information from
/proc/kallsyms. See discussion in Link:
https://lore.kernel.org/all/c7dfbd17-85fd-b914-b90f-082abc64c9d1@arm.com/

Being left out in vmlinux(see scripts/kallsyms.c is_ignored_symbol) and
kernelspace API implies that these symbols are not used in any cases.
So we can ignore them in the first place by not adding them to module
kallsyms.

Signed-off-by: Lexi Shao <shaolexi@huawei.com>
---
 kernel/module.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 5c26a76e800b..b30cbbe144c7 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2662,16 +2662,22 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
 	return '?';
 }
 
-static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
-			unsigned int shnum, unsigned int pcpundx)
+static inline int is_arm_mapping_symbol(const char *str);
+static bool is_core_symbol(const Elf_Sym *src, const struct load_info *info)
 {
 	const Elf_Shdr *sec;
+	const Elf_Shdr *sechdrs = info->sechdrs;
+	unsigned int shnum = info->hdr->e_shnum;
+	unsigned int pcpundx = info->index.pcpu;
 
 	if (src->st_shndx == SHN_UNDEF
 	    || src->st_shndx >= shnum
 	    || !src->st_name)
 		return false;
 
+	if (is_arm_mapping_symbol(&info->strtab[src->st_name]))
+		return false;
+
 #ifdef CONFIG_KALLSYMS_ALL
 	if (src->st_shndx == pcpundx)
 		return true;
@@ -2714,8 +2720,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
 	/* Compute total space required for the core symbols' strtab. */
 	for (ndst = i = 0; i < nsrc; i++) {
 		if (i == 0 || is_livepatch_module(mod) ||
-		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
-				   info->index.pcpu)) {
+		    is_core_symbol(src+i, info)) {
 			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
 			ndst++;
 		}
@@ -2778,8 +2783,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
 	for (ndst = i = 0; i < mod->kallsyms->num_symtab; i++) {
 		mod->kallsyms->typetab[i] = elf_type(src + i, info);
 		if (i == 0 || is_livepatch_module(mod) ||
-		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
-				   info->index.pcpu)) {
+		    is_core_symbol(src+i, info)) {
 			mod->core_kallsyms.typetab[ndst] =
 			    mod->kallsyms->typetab[i];
 			dst[ndst] = src[i];
@@ -4246,8 +4250,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
 		 * We ignore unnamed symbols: they're uninformative
 		 * and inserted at a whim.
 		 */
-		if (*kallsyms_symbol_name(kallsyms, i) == '\0'
-		    || is_arm_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
+		if (*kallsyms_symbol_name(kallsyms, i) == '\0')
 			continue;
 
 		if (thisval <= addr && thisval > bestval) {
-- 
2.12.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] kallsyms: ignore arm mapping symbols when loading module
  2021-10-29  6:50             ` [PATCH v2 2/2] kallsyms: ignore arm mapping symbols when loading module Lexi Shao
@ 2021-11-02  9:43               ` James Clark
  0 siblings, 0 replies; 12+ messages in thread
From: James Clark @ 2021-11-02  9:43 UTC (permalink / raw)
  To: Lexi Shao, linux-kernel, linux-perf-users
  Cc: acme, alexander.shishkin, jolsa, mark.rutland, mingo, namhyung,
	nixiaoming, peterz, qiuxi1, wangbing6, jeyu, ast, daniel, andrii,
	kafai, songliubraving, yhs, john.fastabend, kpsingh,
	natechancellor, ndesaulniers, bpf, clang-built-linux



On 29/10/2021 07:50, Lexi Shao wrote:
> Arm modules contains mapping symbols(e.g. $a $d) which are ignored in
> module_kallsyms_on_each_symbol. However, these symbols are still
> displayed when catting /proc/kallsyms. This confuses tools(e.g. perf)
> that resolves kernel symbols with address using information from
> /proc/kallsyms. See discussion in Link:
> https://lore.kernel.org/all/c7dfbd17-85fd-b914-b90f-082abc64c9d1@arm.com/
> 
> Being left out in vmlinux(see scripts/kallsyms.c is_ignored_symbol) and
> kernelspace API implies that these symbols are not used in any cases.
> So we can ignore them in the first place by not adding them to module
> kallsyms.
> 
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>


I tested this and it has removed the $ symbols from kallsyms where I saw
them before.

Reviewed-by: James Clark <james.clark@arm.com>

> ---
>  kernel/module.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 5c26a76e800b..b30cbbe144c7 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2662,16 +2662,22 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
>  	return '?';
>  }
>  
> -static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
> -			unsigned int shnum, unsigned int pcpundx)
> +static inline int is_arm_mapping_symbol(const char *str);
> +static bool is_core_symbol(const Elf_Sym *src, const struct load_info *info)
>  {
>  	const Elf_Shdr *sec;
> +	const Elf_Shdr *sechdrs = info->sechdrs;
> +	unsigned int shnum = info->hdr->e_shnum;
> +	unsigned int pcpundx = info->index.pcpu;
>  
>  	if (src->st_shndx == SHN_UNDEF
>  	    || src->st_shndx >= shnum
>  	    || !src->st_name)
>  		return false;
>  
> +	if (is_arm_mapping_symbol(&info->strtab[src->st_name]))
> +		return false;
> +
>  #ifdef CONFIG_KALLSYMS_ALL
>  	if (src->st_shndx == pcpundx)
>  		return true;
> @@ -2714,8 +2720,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
>  	/* Compute total space required for the core symbols' strtab. */
>  	for (ndst = i = 0; i < nsrc; i++) {
>  		if (i == 0 || is_livepatch_module(mod) ||
> -		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
> -				   info->index.pcpu)) {
> +		    is_core_symbol(src+i, info)) {
>  			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
>  			ndst++;
>  		}
> @@ -2778,8 +2783,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
>  	for (ndst = i = 0; i < mod->kallsyms->num_symtab; i++) {
>  		mod->kallsyms->typetab[i] = elf_type(src + i, info);
>  		if (i == 0 || is_livepatch_module(mod) ||
> -		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
> -				   info->index.pcpu)) {
> +		    is_core_symbol(src+i, info)) {
>  			mod->core_kallsyms.typetab[ndst] =
>  			    mod->kallsyms->typetab[i];
>  			dst[ndst] = src[i];
> @@ -4246,8 +4250,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
>  		 * We ignore unnamed symbols: they're uninformative
>  		 * and inserted at a whim.
>  		 */
> -		if (*kallsyms_symbol_name(kallsyms, i) == '\0'
> -		    || is_arm_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
> +		if (*kallsyms_symbol_name(kallsyms, i) == '\0')
>  			continue;
>  
>  		if (thisval <= addr && thisval > bestval) {
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules
  2021-10-28  8:44 ` [PATCH] perf symbol: ignore $a/$d symbols for ARM modules James Clark
@ 2021-11-06 19:48   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-06 19:48 UTC (permalink / raw)
  To: James Clark
  Cc: Lexi Shao, linux-perf-users, linux-kernel, mark.rutland, peterz,
	mingo, alexander.shishkin, jolsa, namhyung, qiuxi1, nixiaoming,
	wangbing6

Em Thu, Oct 28, 2021 at 09:44:26AM +0100, James Clark escreveu:
> 
> 
> On 27/10/2021 10:52, Lexi Shao wrote:
> > On ARM machine, kernel symbols from modules can be resolved to $a
> > instead of printing the actual symbol name. Ignore symbols starting with
> > "$" when building kallsyms rbtree.
> > 
> > A sample stacktrace is shown as follows:
> > 
> > c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
> > bf4a66d8 $a+0x78 ([test_module])
> > c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
> > c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
> > 
> > On ARM machine, $a/$d symbols are used by the compiler to mark the
> > beginning of code/data part in code section. These symbols are filtered
> > out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
> > are left on modules. So there are $a symbols in /proc/kallsyms which
> > share the same addresses with the actual module symbols and confuses perf
> > when resolving symbols.
> > 
> > After this patch, the module symbol name is printed:
> > 
> > c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
> > bf4a66d8 test_func+0x78 ([test_module])
> > c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
> > c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
> > 
> > Signed-off-by: Lexi Shao <shaolexi@huawei.com>
> 
> Reviewed-by: James Clark <james.clark@arm.com>

Thanks, applied.

- Arnaldo


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-11-06 19:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-27  9:52 [PATCH] perf symbol: ignore $a/$d symbols for ARM modules Lexi Shao
2021-10-27 10:23 ` James Clark
2021-10-27 12:31   ` Lexi Shao
2021-10-27 15:10     ` James Clark
2021-10-28  2:05       ` Lexi Shao
2021-10-28  8:42         ` James Clark
2021-10-29  6:50           ` [PATCH v2 0/2] kallsyms: Ignore $a/$d symbols in kallsyms for ARM Lexi Shao
2021-10-29  6:50             ` [PATCH v2 1/2] perf symbol: ignore $a/$d symbols for ARM modules Lexi Shao
2021-10-29  6:50             ` [PATCH v2 2/2] kallsyms: ignore arm mapping symbols when loading module Lexi Shao
2021-11-02  9:43               ` James Clark
2021-10-28  8:44 ` [PATCH] perf symbol: ignore $a/$d symbols for ARM modules James Clark
2021-11-06 19:48   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.