Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sun, 14 Apr 2019 14:34:58 +0100
From:   Russell King - ARM Linux admin <linux@armlinux.org.uk>
To:     Masami Hiramatsu <mhiramat@kernel.org>
Cc:     Steven Rostedt <rostedt@goodmis.org>,
        Stefan Agner <stefan@agner.ch>, Arnd Bergmann <arnd@arndb.de>,
        Masahiro Yamada <yamada.masahiro@socionext.com>,
        linux-arm-kernel@lists.infradead.org, naresh.kamboju@linaro.org,
        linux-kernel@vger.kernel.org
Subject: Re: Kconfig dependency issue on function-graph tracer and frame
 pointer on arm
Message-ID: <20190414133458.zxdekgx6qwd562qa@shell.armlinux.org.uk>
References: <20190414194705.2e10802aca2df36c8f27f349@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190414194705.2e10802aca2df36c8f27f349@kernel.org>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, Apr 14, 2019 at 07:47:05PM +0900, Masami Hiramatsu wrote:
> Hello,
> 
> Recently, Naresh reported that the function-graph tracer on the latest
> kernel crashes on arm. I could reproduce it and bisected. I finally found
> the commit f9b58e8c7d03 ("ARM: 8800/1: use choice for kernel unwinders")
> was the first bad commit.

I don't think that littering the rest of the kernel Kconfig with ARM
specific stuff is really a viable solution to this.

If we examine the current situation, we have:

- THUMB2_KERNEL selecting ARM_UNWIND when enabled.
- UNWINDER_FRAME_POINTER disabled if THUMB2_KERNEL is enabled, provided
  we're not using Clang.  This leaves UNWINDER_ARM as the only choice,
  which also selects ARM_UNWIND.
- The default choice is dependent on the settings of AEABI and
  FUNCTION_GRAPH_TRACER.
- HAVE_FUNCTION_GRAPH_TRACER is disabled if THUMB2_KERNEL is enabled.

which seems to be _way_ too messy.

Looking back before this commit, the function graph tracer never had a
strong dependence on frame pointers being enabled in the kernel, but it
seems the code relies upon them (see ftrace_return_to_handler() in
kernel/trace/ and return_to_handler in arch/arm/kernel/entry-frace.S).
There is also the __ftrace_graph_caller macro which seems to rely on it.

Since Clang does not support frame pointers, we shouldn't even offer
the function graph tracer for Clang compilers, so let's do that with
the first hunk of the patch below.

The subsequent hunks remove the defaulting of the choice according to
the function graph tracer - this is not a "hint" where the user can
still choose either option irrespective of the state of the function
graph tracer.  They should only be able to select the frame pointer
option in that case.

Another way forward would be for someone to put the work in to making
the function graph tracer work without frame pointers.

So, how about this:

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 850b4805e2d1..9aed25a6019b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -73,7 +73,7 @@ config ARM
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
 	select HAVE_EXIT_THREAD
 	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
-	select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL
+	select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
 	select HAVE_FUNCTION_TRACER if !XIP_KERNEL
 	select HAVE_GCC_PLUGINS
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7)
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 6d6e0330930b..e388af4594a6 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -47,8 +47,8 @@ config DEBUG_WX
 
 choice
 	prompt "Choose kernel unwinder"
-	default UNWINDER_ARM if AEABI && !FUNCTION_GRAPH_TRACER
-	default UNWINDER_FRAME_POINTER if !AEABI || FUNCTION_GRAPH_TRACER
+	default UNWINDER_ARM if AEABI
+	default UNWINDER_FRAME_POINTER if !AEABI
 	help
 	  This determines which method will be used for unwinding kernel stack
 	  traces for panics, oopses, bugs, warnings, perf, /proc/<pid>/stack,
@@ -65,7 +65,7 @@ config UNWINDER_FRAME_POINTER
 
 config UNWINDER_ARM
 	bool "ARM EABI stack unwinder"
-	depends on AEABI
+	depends on AEABI && !FUNCTION_GRAPH_TRACER
 	select ARM_UNWIND
 	help
 	  This option enables stack unwinding support in the kernel

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up