[PATCH V2] riscv: ftrace: Fix the logic issue in DYNAMIC_FTRACE selection
Alexandre Ghiti
alex at ghiti.fr
Wed Jul 16 04:07:08 PDT 2025
On 7/15/25 14:17, Alexandre Ghiti wrote:
> On 7/6/25 17:18, ChenMiao wrote:
>> From: chenmiao <chenmiao.ku at gmail.com>
>>
>> V1: The first version mainly involves modifications to the
>> configuration of the dynamic ftracer.
>>
>> Link
>> https://lore.kernel.org/all/f7e12c6d-892e-4ca3-9ef0-fbb524d04a48@ghiti.fr/
>>
>> V2: After a series of discussions, Steven concluded that only
>> supporting the dynamic ftracer would suffice. Alex also pointed
>> out that if only the dynamic ftracer needs to be supported, the
>> code related to the static ftracer would become obsolete and
>> turn into dead code. He had already done some preliminary work on this.
>>
>> Based on this, the modifications to the configuration were made,
>> and the dead code generated by the ftracer
>> (originally related to the static ftracer) was also removed.
>>
>> Link
>> https://lore.kernel.org/all/20250703115222.2d7c8cd5@batman.local.home/
>> Link
>> https://github.com/linux-riscv/linux/pull/556/commits/0481092a5bec3818658981c11f629e06e66382b3
>>
>> Signed-off-by: chenmiao <chenmiao.ku at gmail.com>
>> ---
>> arch/riscv/Kconfig | 3 +-
>> arch/riscv/include/asm/ftrace.h | 3 --
>> arch/riscv/kernel/ftrace.c | 11 +-----
>> arch/riscv/kernel/mcount.S | 64 ---------------------------------
>> kernel/trace/Kconfig | 2 +-
>> 5 files changed, 4 insertions(+), 79 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 36061f4732b7..95d24b19c466 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -97,8 +97,9 @@ config RISCV
>> select CLONE_BACKWARDS
>> select COMMON_CLK
>> select CPU_PM if CPU_IDLE || HIBERNATION || SUSPEND
>> + select DYNAMIC_FTRACE if FUNCTION_TRACER
>> select EDAC_SUPPORT
>> - select FRAME_POINTER if PERF_EVENTS || (FUNCTION_TRACER &&
>> !DYNAMIC_FTRACE)
>> + select FRAME_POINTER if PERF_EVENTS
>> select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY if
>> DYNAMIC_FTRACE
>> select FUNCTION_ALIGNMENT_8B if DYNAMIC_FTRACE_WITH_CALL_OPS
>> select GENERIC_ARCH_TOPOLOGY
>> diff --git a/arch/riscv/include/asm/ftrace.h
>> b/arch/riscv/include/asm/ftrace.h
>> index 22ebea3c2b26..77ddb6dce0a8 100644
>> --- a/arch/riscv/include/asm/ftrace.h
>> +++ b/arch/riscv/include/asm/ftrace.h
>> @@ -49,7 +49,6 @@ struct dyn_arch_ftrace {
>> };
>> #endif
>> -#ifdef CONFIG_DYNAMIC_FTRACE
>> /*
>> * A general call in RISC-V is a pair of insts:
>> * 1) auipc: setting high-20 pc-related bits to ra register
>> @@ -237,6 +236,4 @@ static inline void
>> arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsi
>> #endif /* __ASSEMBLY__ */
>> -#endif /* CONFIG_DYNAMIC_FTRACE */
>> -
>> #endif /* _ASM_RISCV_FTRACE_H */
>> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
>> index 4c6c24380cfd..93bad646d233 100644
>> --- a/arch/riscv/kernel/ftrace.c
>> +++ b/arch/riscv/kernel/ftrace.c
>> @@ -13,7 +13,6 @@
>> #include <asm/cacheflush.h>
>> #include <asm/text-patching.h>
>> -#ifdef CONFIG_DYNAMIC_FTRACE
>> unsigned long ftrace_call_adjust(unsigned long addr)
>> {
>> if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS))
>> @@ -191,13 +190,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
>> return 0;
>> }
>> -#else /* CONFIG_DYNAMIC_FTRACE */
>> -unsigned long ftrace_call_adjust(unsigned long addr)
>> -{
>> - return addr;
>> -}
>> -#endif /* CONFIG_DYNAMIC_FTRACE */
>> -
>> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>> int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
>> unsigned long addr)
>> @@ -236,7 +228,6 @@ void prepare_ftrace_return(unsigned long *parent,
>> unsigned long self_addr,
>> *parent = return_hooker;
>> }
>> -#ifdef CONFIG_DYNAMIC_FTRACE
>> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>> struct ftrace_ops *op, struct ftrace_regs *fregs)
>> {
>> @@ -257,5 +248,5 @@ void ftrace_graph_func(unsigned long ip, unsigned
>> long parent_ip,
>> if (!function_graph_enter_regs(old, ip, frame_pointer, parent,
>> fregs))
>> *parent = return_hooker;
>> }
>> -#endif /* CONFIG_DYNAMIC_FTRACE */
>> +
>> #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
>> diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
>> index da4a4000e57e..bb550c15f1c9 100644
>> --- a/arch/riscv/kernel/mcount.S
>> +++ b/arch/riscv/kernel/mcount.S
>> @@ -14,13 +14,6 @@
>> .text
>> - .macro SAVE_ABI_STATE
>> - addi sp, sp, -16
>> - REG_S s0, 0*SZREG(sp)
>> - REG_S ra, 1*SZREG(sp)
>> - addi s0, sp, 16
>> - .endm
>> -
>> /*
>> * The call to ftrace_return_to_handler would overwrite the return
>> * register if a0 was not saved.
>> @@ -34,12 +27,6 @@
>> addi s0, sp, FREGS_SIZE_ON_STACK
>> .endm
>> - .macro RESTORE_ABI_STATE
>> - REG_L ra, 1*SZREG(sp)
>> - REG_L s0, 0*SZREG(sp)
>> - addi sp, sp, 16
>> - .endm
>> -
>> .macro RESTORE_RET_ABI_STATE
>> REG_L ra, FREGS_RA(sp)
>> REG_L s0, FREGS_S0(sp)
>> @@ -49,10 +36,8 @@
>> .endm
>> SYM_TYPED_FUNC_START(ftrace_stub)
>> -#ifdef CONFIG_DYNAMIC_FTRACE
>> .global _mcount
>> .set _mcount, ftrace_stub
>> -#endif
>> ret
>> SYM_FUNC_END(ftrace_stub)
>> @@ -79,53 +64,4 @@ SYM_FUNC_START(return_to_handler)
>> SYM_FUNC_END(return_to_handler)
>> #endif
>> -#ifndef CONFIG_DYNAMIC_FTRACE
>> -SYM_FUNC_START(_mcount)
>> - la t4, ftrace_stub
>> -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
>> - la t0, ftrace_graph_return
>> - REG_L t1, 0(t0)
>> - bne t1, t4, .Ldo_ftrace_graph_caller
>> -
>> - la t3, ftrace_graph_entry
>> - REG_L t2, 0(t3)
>> - la t6, ftrace_graph_entry_stub
>> - bne t2, t6, .Ldo_ftrace_graph_caller
>> -#endif
>> - la t3, ftrace_trace_function
>> - REG_L t5, 0(t3)
>> - bne t5, t4, .Ldo_trace
>> - ret
>> -
>> -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
>> -/*
>> - * A pseudo representation for the function graph tracer:
>> - * prepare_to_return(&ra_to_caller_of_caller, ra_to_caller)
>> - */
>> -.Ldo_ftrace_graph_caller:
>> - addi a0, s0, -SZREG
>> - mv a1, ra
>> -#ifdef HAVE_FUNCTION_GRAPH_FP_TEST
>> - REG_L a2, -2*SZREG(s0)
>> -#endif
>> - SAVE_ABI_STATE
>> - call prepare_ftrace_return
>> - RESTORE_ABI_STATE
>> - ret
>> -#endif
>> -
>> -/*
>> - * A pseudo representation for the function tracer:
>> - * (*ftrace_trace_function)(ra_to_caller, ra_to_caller_of_caller)
>> - */
>> -.Ldo_trace:
>> - REG_L a1, -SZREG(s0)
>> - mv a0, ra
>> -
>> - SAVE_ABI_STATE
>> - jalr t5
>> - RESTORE_ABI_STATE
>> - ret
>> -SYM_FUNC_END(_mcount)
>> -#endif
>> EXPORT_SYMBOL(_mcount)
>> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
>> index a3f35c7d83b6..28afc6941e7a 100644
>> --- a/kernel/trace/Kconfig
>> +++ b/kernel/trace/Kconfig
>> @@ -275,7 +275,7 @@ config FUNCTION_TRACE_ARGS
>> funcgraph-args (for the function graph tracer)
>> config DYNAMIC_FTRACE
>> - bool "enable/disable function tracing dynamically"
>> + bool
>> depends on FUNCTION_TRACER
>> depends on HAVE_DYNAMIC_FTRACE
>> default y
>>
>> base-commit: fda589c286040d9ba2d72a0eaf0a13945fc48026
>
>
> I have just given it a try but I'm getting the following errors:
>
> ../arch/riscv/kernel/ftrace.c: In function 'arch_ftrace_update_code':
> ../arch/riscv/kernel/ftrace.c:44:9: error: implicit declaration of
> function 'ftrace_modify_all_code' [-Werror=implicit-function-declaration]
> 44 | ftrace_modify_all_code(command);
> | ^~~~~~~~~~~~~~~~~~~~~~
> ../arch/riscv/kernel/ftrace.c: At top level:
> ../arch/riscv/kernel/ftrace.c:116:5: warning: no previous prototype
> for 'ftrace_make_call' [-Wmissing-prototypes]
> 116 | int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> | ^~~~~~~~~~~~~~~~
> ../arch/riscv/kernel/ftrace.c: In function 'ftrace_make_call':
> ../arch/riscv/kernel/ftrace.c:118:52: error: invalid use of undefined
> type 'struct dyn_ftrace'
> 118 | unsigned long distance, orig_addr, pc = rec->ip -
> MCOUNT_AUIPC_SIZE;
> | ^~
> ../arch/riscv/kernel/ftrace.c:125:37: error: 'ftrace_caller'
> undeclared (first use in this function); did you mean 'ftrace_kill'?
> 125 | orig_addr = (unsigned long)&ftrace_caller;
> | ^~~~~~~~~~~~~
> | ftrace_kill
> ../arch/riscv/kernel/ftrace.c:125:37: note: each undeclared identifier
> is reported only once for each function it appears in
> ../arch/riscv/kernel/ftrace.c:128:24: error: 'FTRACE_ADDR' undeclared
> (first use in this function)
> 128 | addr = FTRACE_ADDR;
> | ^~~~~~~~~~~
> ../arch/riscv/kernel/ftrace.c: At top level:
> ../arch/riscv/kernel/ftrace.c:133:5: warning: no previous prototype
> for 'ftrace_make_nop' [-Wmissing-prototypes]
> 133 | int ftrace_make_nop(struct module *mod, struct dyn_ftrace
> *rec, unsigned long addr)
> | ^~~~~~~~~~~~~~~
> ../arch/riscv/kernel/ftrace.c: In function 'ftrace_make_nop':
> ../arch/riscv/kernel/ftrace.c:142:41: error: invalid use of undefined
> type 'struct dyn_ftrace'
> 142 | if (patch_insn_write((void *)rec->ip, &nop4,
> MCOUNT_NOP4_SIZE))
> | ^~
> ../arch/riscv/kernel/ftrace.c: In function 'ftrace_init_nop':
> ../arch/riscv/kernel/ftrace.c:157:31: error: invalid use of undefined
> type 'struct dyn_ftrace'
> 157 | unsigned long pc = rec->ip - MCOUNT_AUIPC_SIZE;
> | ^~
> CC drivers/nvme/target/io-cmd-bdev.o
> ../arch/riscv/kernel/ftrace.c:167:35: error: 'ftrace_caller'
> undeclared (first use in this function); did you mean 'ftrace_kill'?
> 167 | offset = (unsigned long) &ftrace_caller - pc;
> | ^~~~~~~~~~~~~
> | ftrace_kill
> ../arch/riscv/kernel/ftrace.c: At top level:
> ../arch/riscv/kernel/ftrace.c:177:5: warning: no previous prototype
> for 'ftrace_update_ftrace_func' [-Wmissing-prototypes]
> 177 | int ftrace_update_ftrace_func(ftrace_func_t func)
I'll send a simple version this afternoon to fix the build error so that
we can merge it in 6.16. That will be a simple version that does not
remove the dead code.
I'll add your SoB, let me know if you want to work on removing the dead
code in a follow-up patch targeting 6.17.
Thanks,
Alex
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
More information about the linux-riscv
mailing list