diff mbox series

tools/perf: Fix powerpc gap between kernel end and module start

Message ID 1609208054-1566-1-git-send-email-atrajeev@linux.vnet.ibm.com (mailing list archive)
State Not Applicable
Headers show
Series tools/perf: Fix powerpc gap between kernel end and module start | expand

Commit Message

Athira Rajeev Dec. 29, 2020, 2:14 a.m. UTC 
Running "perf mem report" in TUI mode fails with ENOMEM message
in powerpc:

failed to process sample

Running with debug and verbose options points that issue is while
allocating memory for sample histograms.

The error path is:
symbol__inc_addr_samples -> __symbol__inc_addr_samples
-> annotated_source__histogram

symbol__inc_addr_samples calls annotated_source__alloc_histograms
to allocate memory for sample histograms using calloc. Here calloc fails
since the size of symbol is huge. The size of a symbol is calculated as
difference between its start and end address.

Example histogram allocation that fails is:
sym->name is _end, sym->start is 0xc0000000027a0000, sym->end is
0xc008000003890000, symbol__size(sym) is 0x80000010f0000

In above case, difference between sym->start (0xc0000000027a0000)
and sym->end (0xc008000003890000) is huge.

This is same problem as in s390 and arm64 which are fixed in commits:
'commit b9c0a64901d5 ("perf annotate: Fix s390 gap between kernel end
and module start")'
'commit 78886f3ed37e ("perf symbols: Fix arm64 gap between kernel start
and module end")'

When this symbol was read first, its start and end address was set to
address which matches with data from /proc/kallsyms.

After symbol__new:
symbol__new: _end 0xc0000000027a0000-0xc0000000027a0000

From /proc/kallsyms:
...
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.

Signed-off-by: Athira Rajeev<atrajeev@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/Build     |  1 +
 tools/perf/arch/powerpc/util/machine.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 tools/perf/arch/powerpc/util/machine.c

Comments

Jiri Olsa Jan. 12, 2021, 9:38 a.m. UTC | #1 
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.
> 
> 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

> ---
>  tools/perf/arch/powerpc/util/Build     |  1 +
>  tools/perf/arch/powerpc/util/machine.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 tools/perf/arch/powerpc/util/machine.c
> 
> diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
> index e86e210bf514..b7945e5a543b 100644
> --- a/tools/perf/arch/powerpc/util/Build
> +++ b/tools/perf/arch/powerpc/util/Build
> @@ -1,4 +1,5 @@
>  perf-y += header.o
> +perf-y += machine.o
>  perf-y += kvm-stat.o
>  perf-y += perf_regs.o
>  perf-y += mem-events.o
> diff --git a/tools/perf/arch/powerpc/util/machine.c b/tools/perf/arch/powerpc/util/machine.c
> new file mode 100644
> index 000000000000..c30e5cc88c16
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/machine.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <internal/lib.h> // page_size
> +#include "debug.h"
> +#include "symbol.h"
> +
> +/* On powerpc kernel text segment start at memory addresses, 0xc000000000000000
> + * whereas the modules are located at very high memory addresses,
> + * for example 0xc00800000xxxxxxx. The gap between end of kernel text segment
> + * and beginning of first module's text segment is very high.
> + * Therefore do not fill this gap and do not assign it to the kernel dso map.
> + */
> +
> +void arch__symbols__fixup_end(struct symbol *p, struct symbol *c)
> +{
> +	if (strchr(p->name, '[') == NULL && strchr(c->name, '['))
> +		/* Limit the range of last kernel symbol */
> +		p->end += page_size;
> +	else
> +		p->end = c->start;
> +	pr_debug4("%s sym:%s end:%#lx\n", __func__, p->name, p->end);
> +}
> -- 
> 1.8.3.1
>
Athira Rajeev Jan. 13, 2021, 6:44 a.m. UTC | #2 
> On 12-Jan-2021, at 3:08 PM, Jiri Olsa <jolsa@redhat.com> 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.
>> 
>> 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

Thanks Jiri for reviewing the patch,

Athira
> 
>> ---
>> tools/perf/arch/powerpc/util/Build     |  1 +
>> tools/perf/arch/powerpc/util/machine.c | 24 ++++++++++++++++++++++++
>> 2 files changed, 25 insertions(+)
>> create mode 100644 tools/perf/arch/powerpc/util/machine.c
>> 
>> diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
>> index e86e210bf514..b7945e5a543b 100644
>> --- a/tools/perf/arch/powerpc/util/Build
>> +++ b/tools/perf/arch/powerpc/util/Build
>> @@ -1,4 +1,5 @@
>> perf-y += header.o
>> +perf-y += machine.o
>> perf-y += kvm-stat.o
>> perf-y += perf_regs.o
>> perf-y += mem-events.o
>> diff --git a/tools/perf/arch/powerpc/util/machine.c b/tools/perf/arch/powerpc/util/machine.c
>> new file mode 100644
>> index 000000000000..c30e5cc88c16
>> --- /dev/null
>> +++ b/tools/perf/arch/powerpc/util/machine.c
>> @@ -0,0 +1,24 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <internal/lib.h> // page_size
>> +#include "debug.h"
>> +#include "symbol.h"
>> +
>> +/* On powerpc kernel text segment start at memory addresses, 0xc000000000000000
>> + * whereas the modules are located at very high memory addresses,
>> + * for example 0xc00800000xxxxxxx. The gap between end of kernel text segment
>> + * and beginning of first module's text segment is very high.
>> + * Therefore do not fill this gap and do not assign it to the kernel dso map.
>> + */
>> +
>> +void arch__symbols__fixup_end(struct symbol *p, struct symbol *c)
>> +{
>> +	if (strchr(p->name, '[') == NULL && strchr(c->name, '['))
>> +		/* Limit the range of last kernel symbol */
>> +		p->end += page_size;
>> +	else
>> +		p->end = c->start;
>> +	pr_debug4("%s sym:%s end:%#lx\n", __func__, p->name, p->end);
>> +}
>> -- 
>> 1.8.3.1
Kajol Jain Jan. 18, 2021, 10:21 a.m. UTC | #3 
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
> 
>> ---
>>  tools/perf/arch/powerpc/util/Build     |  1 +
>>  tools/perf/arch/powerpc/util/machine.c | 24 ++++++++++++++++++++++++
>>  2 files changed, 25 insertions(+)
>>  create mode 100644 tools/perf/arch/powerpc/util/machine.c
>>
>> diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
>> index e86e210bf514..b7945e5a543b 100644
>> --- a/tools/perf/arch/powerpc/util/Build
>> +++ b/tools/perf/arch/powerpc/util/Build
>> @@ -1,4 +1,5 @@
>>  perf-y += header.o
>> +perf-y += machine.o
>>  perf-y += kvm-stat.o
>>  perf-y += perf_regs.o
>>  perf-y += mem-events.o
>> diff --git a/tools/perf/arch/powerpc/util/machine.c b/tools/perf/arch/powerpc/util/machine.c
>> new file mode 100644
>> index 000000000000..c30e5cc88c16
>> --- /dev/null
>> +++ b/tools/perf/arch/powerpc/util/machine.c
>> @@ -0,0 +1,24 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <internal/lib.h> // page_size
>> +#include "debug.h"
>> +#include "symbol.h"
>> +
>> +/* On powerpc kernel text segment start at memory addresses, 0xc000000000000000
>> + * whereas the modules are located at very high memory addresses,
>> + * for example 0xc00800000xxxxxxx. The gap between end of kernel text segment
>> + * and beginning of first module's text segment is very high.
>> + * Therefore do not fill this gap and do not assign it to the kernel dso map.
>> + */
>> +
>> +void arch__symbols__fixup_end(struct symbol *p, struct symbol *c)
>> +{
>> +	if (strchr(p->name, '[') == NULL && strchr(c->name, '['))
>> +		/* Limit the range of last kernel symbol */
>> +		p->end += page_size;
>> +	else
>> +		p->end = c->start;
>> +	pr_debug4("%s sym:%s end:%#lx\n", __func__, p->name, p->end);
>> +}
>> -- 
>> 1.8.3.1
>>
>
Arnaldo Carvalho de Melo Feb. 3, 2021, 3:31 p.m. UTC | #4 
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.

- Arnaldo
Athira Rajeev Feb. 4, 2021, 12:11 p.m. UTC | #5 
> On 03-Feb-2021, at 9:01 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> 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.
> 
> - Arnaldo

Thanks Arnaldo for taking this fix.
Arnaldo Carvalho de Melo Feb. 9, 2021, 12:47 p.m. UTC | #6 
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);
 }
Athira Rajeev Feb. 11, 2021, 12:19 p.m. UTC | #7 
> On 09-Feb-2021, at 6:17 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> 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>

Thanks Arnaldo for the fix.

Athira.
> 
> 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);
> }
diff mbox series

Patch

diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
index e86e210bf514..b7945e5a543b 100644
--- a/tools/perf/arch/powerpc/util/Build
+++ b/tools/perf/arch/powerpc/util/Build
@@ -1,4 +1,5 @@ 
 perf-y += header.o
+perf-y += machine.o
 perf-y += kvm-stat.o
 perf-y += perf_regs.o
 perf-y += mem-events.o
diff --git a/tools/perf/arch/powerpc/util/machine.c b/tools/perf/arch/powerpc/util/machine.c
new file mode 100644
index 000000000000..c30e5cc88c16
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/machine.c
@@ -0,0 +1,24 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <string.h>
+#include <internal/lib.h> // page_size
+#include "debug.h"
+#include "symbol.h"
+
+/* On powerpc kernel text segment start at memory addresses, 0xc000000000000000
+ * whereas the modules are located at very high memory addresses,
+ * for example 0xc00800000xxxxxxx. The gap between end of kernel text segment
+ * and beginning of first module's text segment is very high.
+ * Therefore do not fill this gap and do not assign it to the kernel dso map.
+ */
+
+void arch__symbols__fixup_end(struct symbol *p, struct symbol *c)
+{
+	if (strchr(p->name, '[') == NULL && strchr(c->name, '['))
+		/* Limit the range of last kernel symbol */
+		p->end += page_size;
+	else
+		p->end = c->start;
+	pr_debug4("%s sym:%s end:%#lx\n", __func__, p->name, p->end);
+}