Re: [PATCH] tools/perf: Fix powerpc gap between kernel end and module start

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Feb 03, 2021 at 12:31:48PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Feb 02, 2021 at 04:02:36PM +0530, Athira Rajeev escreveu:
> > 
> > 
> >     On 18-Jan-2021, at 3:51 PM, kajoljain <kjain@linux.ibm.com> wrote:
> > 
> > 
> > 
> >     On 1/12/21 3:08 PM, Jiri Olsa wrote:
> > 
> >         On Mon, Dec 28, 2020 at 09:14:14PM -0500, Athira Rajeev wrote:
> > 
> >         SNIP
> > 
> > 
> >             c000000002799370 b backtrace_flag
> >             c000000002799378 B radix_tree_node_cachep
> >             c000000002799380 B __bss_stop
> >             c0000000027a0000 B _end
> >             c008000003890000 t icmp_checkentry      [ip_tables]
> >             c008000003890038 t ipt_alloc_initial_table      [ip_tables]
> >             c008000003890468 T ipt_do_table [ip_tables]
> >             c008000003890de8 T ipt_unregister_table_pre_exit        [ip_tables]
> >             ...
> > 
> >             Perf calls function symbols__fixup_end() which sets the end of
> >             symbol
> >             to 0xc008000003890000, which is the next address and this is the
> >             start
> >             address of first module (icmp_checkentry in above) which will make
> >             the
> >             huge symbol size of 0x80000010f0000.
> > 
> >             After symbols__fixup_end:
> >             symbols__fixup_end: sym->name: _end, sym->start:
> >             0xc0000000027a0000,
> >             sym->end: 0xc008000003890000
> > 
> >             On powerpc, kernel text segment is located at 0xc000000000000000
> >             whereas the modules are located at very high memory addresses,
> >             0xc00800000xxxxxxx. Since the gap between end of kernel text
> >             segment
> >             and beginning of first module's address is high, histogram
> >             allocation
> >             using calloc fails.
> > 
> >             Fix this by detecting the kernel's last symbol and limiting
> >             the range of last kernel symbol to pagesize.
> > 
> > 
> >     Patch looks good to me.
> > 
> >     Tested-By: Kajol Jain<kjain@linux.ibm.com>
> > 
> >     Thanks,
> >     Kajol Jain
> > 
> > 
> >             Signed-off-by: Athira Rajeev<atrajeev@linux.vnet.ibm.com>
> > 
> > 
> >         I can't test, but since the same approach works for arm and s390,
> >         this also looks ok
> > 
> >         Acked-by: Jiri Olsa <jolsa@redhat.com>
> > 
> >         thanks,
> >         jirka
> > 
> > 
> > Hi Arnaldo,
> > 
> > Can you please help review this patch and merge if this looks good..
> 
> Thanks, collected the Tested-by from Kajol and the Acked-by from Jiri
> and applied to my local tree for testing, then up to my perf/core
> branch.

Had to apply this on top.

- Arnaldo

commit 0f000f9c89182950cd3500226729977251529364
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Tue Feb 9 09:41:21 2021 -0300

    perf powerpc: Fix printf conversion specifier for IP addresses
    
    We need to use "%#" PRIx64 for u64 values, not "%lx", fixing this build
    problem on powerpc 32-bit:
    
      72    13.69 ubuntu:18.04-x-powerpc        : FAIL powerpc-linux-gnu-gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
        arch/powerpc/util/machine.c: In function 'arch__symbols__fixup_end':
        arch/powerpc/util/machine.c:23:12: error: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type 'u64 {aka long long unsigned int}' [-Werror=format=]
          pr_debug4("%s sym:%s end:%#lx\n", __func__, p->name, p->end);
                    ^
        /git/linux/tools/perf/util/debug.h:18:21: note: in definition of macro 'pr_fmt'
         #define pr_fmt(fmt) fmt
                             ^~~
        /git/linux/tools/perf/util/debug.h:33:29: note: in expansion of macro 'pr_debugN'
         #define pr_debug4(fmt, ...) pr_debugN(4, pr_fmt(fmt), ##__VA_ARGS__)
                                     ^~~~~~~~~
        /git/linux/tools/perf/util/debug.h:33:42: note: in expansion of macro 'pr_fmt'
         #define pr_debug4(fmt, ...) pr_debugN(4, pr_fmt(fmt), ##__VA_ARGS__)
                                                  ^~~~~~
        arch/powerpc/util/machine.c:23:2: note: in expansion of macro 'pr_debug4'
          pr_debug4("%s sym:%s end:%#lx\n", __func__, p->name, p->end);
          ^~~~~~~~~
        cc1: all warnings being treated as errors
        /git/linux/tools/build/Makefile.build:139: recipe for target 'util' failed
        make[5]: *** [util] Error 2
        /git/linux/tools/build/Makefile.build:139: recipe for target 'powerpc' failed
        make[4]: *** [powerpc] Error 2
        /git/linux/tools/build/Makefile.build:139: recipe for target 'arch' failed
        make[3]: *** [arch] Error 2
      73    30.47 ubuntu:18.04-x-powerpc64      : Ok   powerpc64-linux-gnu-gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
    
    Fixes: 557c3eadb7712741 ("perf powerpc: Fix gap between kernel end and module start")
    Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Kajol Jain <kjain@linux.ibm.com>
    Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/arch/powerpc/util/machine.c b/tools/perf/arch/powerpc/util/machine.c
index c30e5cc88c1673d6..e652a1aa8132274f 100644
--- a/tools/perf/arch/powerpc/util/machine.c
+++ b/tools/perf/arch/powerpc/util/machine.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <inttypes.h>
 #include <stdio.h>
 #include <string.h>
 #include <internal/lib.h> // page_size
@@ -20,5 +21,5 @@ void arch__symbols__fixup_end(struct symbol *p, struct symbol *c)
 		p->end += page_size;
 	else
 		p->end = c->start;
-	pr_debug4("%s sym:%s end:%#lx\n", __func__, p->name, p->end);
+	pr_debug4("%s sym:%s end:%#" PRIx64 "\n", __func__, p->name, p->end);
 }