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

Reply via email to