Re: [PATCH] riscv: Adjust dependencies of HAVE_DYNAMIC_FTRACE selection

From: Conor Dooley <conor@kernel.org>
To: Nathan Chancellor <nathan@kernel.org>
Cc: paul.walmsley@sifive.com, palmer@dabbelt.com,
	aou@eecs.berkeley.edu, ndesaulniers@google.com, trix@redhat.com,
	linux-riscv@lists.infradead.org, llvm@lists.linux.dev,
	patches@lists.linux.dev
Subject: Re: [PATCH] riscv: Adjust dependencies of HAVE_DYNAMIC_FTRACE selection
Date: Wed, 5 Apr 2023 18:56:36 +0100	[thread overview]
Message-ID: <20230405-dejected-unending-28e29d89f206@spud> (raw)
In-Reply-To: <20230404-riscv-dynamic-ftrace-checks-clang-v1-1-0ce296b7d423@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

On Tue, Apr 04, 2023 at 01:34:21PM -0700, Nathan Chancellor wrote:
> When building allmodconfig with clang and its integrated assembler and
> linking with a version of GNU ld prior to 2.36, the following link error
> occurs:
> 
>   riscv64-linux-gnu-ld: .init.data has both ordered [`__patchable_function_entries' in init/main.o] and unordered [`.init_array.0' in kernel/trace/trace_benchmark.o] sections
>   riscv64-linux-gnu-ld: final link failed: bad value
> 
> This is the same error addressed by commit 45bd8951806e ("arm64: Improve
> HAVE_DYNAMIC_FTRACE_WITH_REGS selection for clang") for arm64. See that
> changelog for a full description of why this error occurs with this
> combination of tools.
> 
> In a similar manner as that change, restrict the
> CONFIG_HAVE_DYNAMIC_FTRACE selection to combinations of tools known to
> work so that there are no errors.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1817
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  arch/riscv/Kconfig | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index eb7f29a412f8..4b82838ac870 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -136,12 +136,23 @@ config RISCV
>  	select TRACE_IRQFLAGS_SUPPORT
>  	select UACCESS_MEMCPY if !MMU
>  	select ZONE_DMA32 if 64BIT
> -	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> +	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE)
>  	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>  	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
>  	select HAVE_FUNCTION_GRAPH_TRACER
>  	select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
>  
> +config CLANG_SUPPORTS_DYNAMIC_FTRACE
> +	def_bool CC_IS_CLANG
> +	# https://github.com/llvm/llvm-project/commit/6ab8927931851bb42b2c93a00801dc499d7d9b1e
> +	depends on CLANG_VERSION >= 130000
> +	# https://github.com/ClangBuiltLinux/linux/issues/1817
> +	depends on AS_IS_GNU || (AS_IS_LLVM && (LD_IS_LLD || LD_VERSION >= 23600))
> +
> +config GCC_SUPPORTS_DYNAMIC_FTRACE
> +	def_bool CC_IS_GCC
> +	depends on $(cc-option,-fpatchable-function-entry=8)

I thought about merging these, but I think you just end up with
something that is difficult to grok. Explanation in the arm64 is pretty
good & the change follows.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]