Perf doesn't display kernel symbols anymore (bisected commit 659ad3492b91 ("perf maps: Switch from rbtree to lazily sorted array for addresses"))

[Christophe Leroy]

Le 03/01/2025 à 17:26, Arnaldo Carvalho de Melo a écrit :
> On Fri, Jan 03, 2025 at 01:40:24PM +0100, Christophe Leroy wrote:
>> Le 03/01/2025 à 02:08, Arnaldo Carvalho de Melo a écrit :
>>>>      PerfTop:     524 irqs/sec  kernel:51.1%  exact:  0.0% lost: 0/0 drop: 0/0
>>>> [4000Hz cpu-clock:ppp],  (all, 1 CPU)
>>>> -------------------------------------------------------------------------------
> 
>>>>       17.18%  [unknown]      [k] 0xc0891c14
>>>>        7.63%  [unknown]      [k] 0xc005f11c
> 
>>> I think I hadn't notice this '[unknown]' one bit before :-\ the [k] is
>>> there, so having unknown is odd
>   
>> Problem found, it's in maps__find_next_entry(), this leads to both
>> map->start and map->end of kernel map being set to 0xc0000000, which leads
>> to the failure of bsearch() in maps__find().
> 
> Right, and since you don't have any module (CONFIG_MODULES not set),
> and most machines do, that is when the buggy function is called:
> 
> machine__create_kernel_maps()
> 	if (!machine__get_running_kernel_start(machine, &name, &start, &end))
> <SNIP>
>          if (end == ~0ULL) {
>                  /* update end address of the kernel map using adjacent module address */
>                  struct map *next = maps__find_next_entry(machine__kernel_maps(machine),
>                                                           machine__kernel_map(machine));
> 
>                  if (next) {
>                          machine__set_kernel_mmap(machine, start, map__start(next));
>                          map__put(next);
>                  }
>          }
> <SNIP>
> 
> So machine__get_running_kernel_start() doesn't manage to fill end with
> either because it doesn't find the ref_reloc_sym, one of:
> 
> const char *ref_reloc_sym_names[] = {"_text", "_stext", NULL}
> 
> And returns -1, so that first if block fails, and then start also
> doesn't get set and remains 0, which doesn't seem to be the case, as you
> get 0xc0000000 in it, or this fails:
> 
>          err = kallsyms__get_symbol_start(filename, "_edata", &addr);
>          if (err)
>                  err = kallsyms__get_function_start(filename, "_etext", &addr);
>          if (!err)
>                  *end = addr;
> 

Indeed yes that one fails, because:

~# grep -e _stext -e _etext -e _edata /proc/kallsyms
c0000000 T _stext
c08b8000 D _etext

So there is no _edata and _etext is not text

$ ppc-linux-objdump -x vmlinux | grep -e _stext -e _etext -e _edata
c0000000 g       .head.text	00000000 _stext
c08b8000 g       .rodata	00000000 _etext
c1378000 g       .sbss	00000000 _edata

Changing

	kallsyms__get_function_start(filename, "_etext", &addr);

to

	kallsyms__get_symbol_start(filename, "_etext", &addr);

works.


The following change works as well:

diff --git a/arch/powerpc/kernel/vmlinux.lds.S 
b/arch/powerpc/kernel/vmlinux.lds.S
index b4c9decc7a75..b7b2cd7e2a20 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -123,10 +123,11 @@ SECTIONS
  		 */
  		*(.sfpr);
  		*(.text.asan.* .text.tsan.*)
+
+		. = ALIGN(PAGE_SIZE);
+		_etext = .;
  	} :text

-	. = ALIGN(PAGE_SIZE);
-	_etext = .;
  	PROVIDE32 (etext = .);

  	/* Read-only data */

As it leads to:

~# grep -e _stext -e _etext -e _edata /proc/kallsyms
c0000000 T _stext
c08b8000 T _etext

$ ppc-linux-objdump -x vmlinux | grep -e _stext -e _etext -e _edata
c0000000 g       .head.text	00000000 _stext
c08b8000 g       .text	00000000 _etext
c1378000 g       .sbss	00000000 _edata

So what is the most correct fix ? Change architectures link script or 
make perf _etext lookup more flexible, allowing non-text ?

Looking at vmlinux.lds.S from various architectures, I have the feeling 
several of them are affected.

Now, regarding _edata, what I see is:

~# tail -2 /proc/kallsyms
c136a000 D __start___bug_table
c1377c14 D __stop___bug_table

And in System.map I have:

c136a000 D __start___bug_table
c1377c14 D __stop___bug_table
c1378000 B __bss_start
c1378000 B _edata
c1378000 B initcall_debug
c1378004 B reset_devices

Should perf try to locate the very last symbol when it doesn't find 
_edata ? Or should architecture's link script be modified ? Otherwise 
commit 69a87a32f5cd ("perf machine: Include data symbols in the kernel 
map") is just pointless.

Christophe