linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Mao Han <han_mao@c-sky.com>
To: Nick Kossifidis <mick@ics.forth.gr>
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: Perf-related compilation issues
Date: Thu, 24 Oct 2019 14:01:17 +0800	[thread overview]
Message-ID: <20191024060116.GA786@vmh-VirtualBox> (raw)
In-Reply-To: <1bba622b-1f59-d21b-f396-d9c1a021dc3a@ics.forth.gr>

On Wed, Oct 23, 2019 at 06:15:43PM +0300, Nick Kossifidis wrote:
> Hello all,
> 
> a) Compiling the current fixes branch with a minimal config I get the
> following error:
> 
> riscv64-unknown-linux-gnu-ld: arch/riscv/kernel/perf_callchain.o: in
> function `.L0 ':
> perf_callchain.c:(.text+0x16a): undefined reference to `walk_stackframe'
> make: *** [Makefile:1074: vmlinux] Error 1
> 
> 
> I've removed the static delcaration of walk_stackframe on stackframe.c
> and marked walk_stackframe as extern on perf_callchain.c to fix the
> above issue.
>

Beside the compile problem caused by:
[PATCH v3 5/8] riscv: mark some code and data as file-static
similar issue may happen when CONFIG_FRAME_POINTER is not defined.
I didn't see the CONFIG_FRAME_POINTER in stacktrace.c, and the 
conditional for !CONFIG_FRAME_POINTER looks quite strange, keep
adding the sp and read pc from that?

        ksp = (unsigned long *)sp;
        while (!kstack_end(ksp)) {
                if (__kernel_text_address(pc) && unlikely(fn(pc, arg)))
                        break;
                pc = (*ksp++) - 0x4;
        }

A conditional for perf_callchain_kernel might be properly to fix that.
--- a/arch/riscv/kernel/perf_callchain.c
+++ b/arch/riscv/kernel/perf_callchain.c
+#ifdef CONFIG_FRAME_POINTER
 void notrace walk_stackframe(struct task_struct *task,
        struct pt_regs *regs, bool (*fn)(unsigned long, void *), void *arg);
 void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
@@ -92,3 +93,4 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
 
        walk_stackframe(NULL, regs, fill_callchain, entry);
 }
+#endif


> 
> b) Then If I compile the kernel without CONFIG_RISCV_BASE_PMU I get
> 
> 
> ./arch/riscv/include/asm/perf_event.h:26:2: error: #error "Please
> provide a valid RISCV_MAX_COUNTERS for the PMU."
>  #error "Please provide a valid RISCV_MAX_COUNTERS for the PMU."
> 
> 
> I noticed that the only place where CONFIG_RISCV_BASE_PMU is checked is
> on perf_event.h and only for this parameter that's not defined anywhere
> else. So for now if one tries to compile the kernel without PMU the
> kernel won't compile + I don't see how unsetting this saves code size as
> the config description says.
> 

The content inside perf_event.h mostly relate to HW PMU. Other
architectures normally put them inside perf_event.c or pmu.h, they are
not compiled when CONFIG_PERF_EVENTS is selected and HW PMU is not
selected, user can use software event under this configuration.
Base pmu shouldn't be registed when it is defined in dts.
I think it can be fixed by put the content inside perf_event.h
into a riscv_pmu.h, only compile perf_event.c when CONFIG_RISCV_BASE_PMU
is selected or add conditional inside it:

--- a/arch/riscv/kernel/perf_event.c
+++ b/arch/riscv/kernel/perf_event.c
@@ -474,12 +474,13 @@ int __init init_hw_perf_events(void)
        if (node) {
                of_id = of_match_node(riscv_pmu_of_ids, node);
 
-               if (of_id)
+               if (of_id) {
                        riscv_pmu = of_id->data;
+                       perf_pmu_register(riscv_pmu->pmu, "cpu", PERF_TYPE_RAW);
+               }
                of_node_put(node);
        }
 
-       perf_pmu_register(riscv_pmu->pmu, "cpu", PERF_TYPE_RAW);
        return 0;
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -38,7 +38,7 @@ obj-$(CONFIG_MODULE_SECTIONS) += module-sections.o
 obj-$(CONFIG_FUNCTION_TRACER)  += mcount.o ftrace.o
 obj-$(CONFIG_DYNAMIC_FTRACE)   += mcount-dyn.o
 
-obj-$(CONFIG_PERF_EVENTS)      += perf_event.o
+obj-$(CONFIG_RISCV_BASE_PMU)   += perf_event.o
 obj-$(CONFIG_PERF_EVENTS)      += perf_callchain.o
 obj-$(CONFIG_HAVE_PERF_REGS)   += perf_regs.o

Thanks,
Mao Han

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

      parent reply	other threads:[~2019-10-24  6:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-23 15:15 Perf-related compilation issues Nick Kossifidis
2019-10-23 23:52 ` Alan Kao
2019-10-24  2:23 ` Paul Walmsley
2019-10-24  6:01 ` Mao Han [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191024060116.GA786@vmh-VirtualBox \
    --to=han_mao@c-sky.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mick@ics.forth.gr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).