| Message ID | 20210907100524.1454928-1-o451686892@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] ftrace: Cleanup ftrace_dyn_arch_init() | expand |
> -----Message d'origine----- > De : Linuxppc-dev <linuxppc-dev- > bounces+christophe.leroy=csgroup.eu@lists.ozlabs.org> De la part de Weizhao > Ouyang > > Most of ARCHs use empty ftrace_dyn_arch_init(), introduce a weak common > ftrace_dyn_arch_init() to cleanup them. > > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> > Acked-by: Heiko Carstens <hca@linux.ibm.com> (s390) > Acked-by: Helge Deller <deller@gmx.de> (parisc) > > --- > Changes in v3: > -- fix unrecognized opcode on PowerPC > > Changes in v2: > -- correct CONFIG_DYNAMIC_FTRACE on PowerPC > -- add Acked-by tag > > --- > arch/arm/kernel/ftrace.c | 5 ----- > arch/arm64/kernel/ftrace.c | 5 ----- > arch/csky/kernel/ftrace.c | 5 ----- > arch/ia64/kernel/ftrace.c | 6 ------ > arch/microblaze/kernel/ftrace.c | 5 ----- > arch/mips/include/asm/ftrace.h | 2 ++ > arch/nds32/kernel/ftrace.c | 5 ----- > arch/parisc/kernel/ftrace.c | 5 ----- > arch/powerpc/include/asm/ftrace.h | 4 ++++ > arch/riscv/kernel/ftrace.c | 5 ----- > arch/s390/kernel/ftrace.c | 5 ----- > arch/sh/kernel/ftrace.c | 5 ----- > arch/sparc/kernel/ftrace.c | 5 ----- > arch/x86/kernel/ftrace.c | 5 ----- > include/linux/ftrace.h | 1 - > kernel/trace/ftrace.c | 5 +++++ > 16 files changed, 11 insertions(+), 62 deletions(-) > > diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h > index b463f2aa5a61..ed013e767390 100644 > --- a/arch/mips/include/asm/ftrace.h > +++ b/arch/mips/include/asm/ftrace.h > @@ -76,6 +76,8 @@ do { \ > > > #ifdef CONFIG_DYNAMIC_FTRACE > +int __init ftrace_dyn_arch_init(void); > + Why ? > static inline unsigned long ftrace_call_adjust(unsigned long addr) > { > return addr; > diff --git a/arch/powerpc/include/asm/ftrace.h > b/arch/powerpc/include/asm/ftrace.h > index debe8c4f7062..b05c43f13a4d 100644 > --- a/arch/powerpc/include/asm/ftrace.h > +++ b/arch/powerpc/include/asm/ftrace.h > @@ -126,6 +126,10 @@ static inline void this_cpu_enable_ftrace(void) { } > static inline void this_cpu_set_ftrace_enabled(u8 ftrace_enabled) { } > static inline u8 this_cpu_get_ftrace_enabled(void) { return 1; } > #endif /* CONFIG_PPC64 */ > + > +#ifdef CONFIG_DYNAMIC_FTRACE > +int __init ftrace_dyn_arch_init(void); > +#endif /* CONFIG_DYNAMIC_FTRACE */ Why ? > #endif /* !__ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_FTRACE */ > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 832e65f06754..f1eca123d89d 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -573,7 +573,6 @@ ftrace_set_early_filter(struct ftrace_ops *ops, char > *buf, int enable); > > /* defined in arch */ > extern int ftrace_ip_converted(unsigned long ip); > -extern int ftrace_dyn_arch_init(void); Why removing that ? Have you tried to build kernel/trace/ftrace.o with C=2 ? It will likely tell you that the function is not declared and that it should be static We could eventually consider that in the past, this generic declaration was unrelevant because the definitions where in the arch specific sections. Now that you are implementing a generic weak version of this function, it would make sense to have a generic declaration as well. I really don't see the point in duplicating the declaration of the function in the arch specific headers. > extern void ftrace_replace_code(int enable); > extern int ftrace_update_ftrace_func(ftrace_func_t func); > extern void ftrace_caller(void); Christophe CS Group - Document Interne
Thanks for reply. On 2021/9/7 23:55, LEROY Christophe wrote: > >> -----Message d'origine----- >> De : Linuxppc-dev <linuxppc-dev- >> bounces+christophe.leroy=csgroup.eu@lists.ozlabs.org> De la part de Weizhao >> Ouyang >> >> Most of ARCHs use empty ftrace_dyn_arch_init(), introduce a weak common >> ftrace_dyn_arch_init() to cleanup them. >> >> Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> >> Acked-by: Heiko Carstens <hca@linux.ibm.com> (s390) >> Acked-by: Helge Deller <deller@gmx.de> (parisc) >> >> --- >> Changes in v3: >> -- fix unrecognized opcode on PowerPC >> >> Changes in v2: >> -- correct CONFIG_DYNAMIC_FTRACE on PowerPC >> -- add Acked-by tag >> >> --- >> arch/arm/kernel/ftrace.c | 5 ----- >> arch/arm64/kernel/ftrace.c | 5 ----- >> arch/csky/kernel/ftrace.c | 5 ----- >> arch/ia64/kernel/ftrace.c | 6 ------ >> arch/microblaze/kernel/ftrace.c | 5 ----- >> arch/mips/include/asm/ftrace.h | 2 ++ >> arch/nds32/kernel/ftrace.c | 5 ----- >> arch/parisc/kernel/ftrace.c | 5 ----- >> arch/powerpc/include/asm/ftrace.h | 4 ++++ >> arch/riscv/kernel/ftrace.c | 5 ----- >> arch/s390/kernel/ftrace.c | 5 ----- >> arch/sh/kernel/ftrace.c | 5 ----- >> arch/sparc/kernel/ftrace.c | 5 ----- >> arch/x86/kernel/ftrace.c | 5 ----- >> include/linux/ftrace.h | 1 - >> kernel/trace/ftrace.c | 5 +++++ >> 16 files changed, 11 insertions(+), 62 deletions(-) >> >> diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h >> index b463f2aa5a61..ed013e767390 100644 >> --- a/arch/mips/include/asm/ftrace.h >> +++ b/arch/mips/include/asm/ftrace.h >> @@ -76,6 +76,8 @@ do { \ >> >> >> #ifdef CONFIG_DYNAMIC_FTRACE >> +int __init ftrace_dyn_arch_init(void); >> + > Why ? > > >> static inline unsigned long ftrace_call_adjust(unsigned long addr) >> { >> return addr; >> diff --git a/arch/powerpc/include/asm/ftrace.h >> b/arch/powerpc/include/asm/ftrace.h >> index debe8c4f7062..b05c43f13a4d 100644 >> --- a/arch/powerpc/include/asm/ftrace.h >> +++ b/arch/powerpc/include/asm/ftrace.h >> @@ -126,6 +126,10 @@ static inline void this_cpu_enable_ftrace(void) { } >> static inline void this_cpu_set_ftrace_enabled(u8 ftrace_enabled) { } >> static inline u8 this_cpu_get_ftrace_enabled(void) { return 1; } >> #endif /* CONFIG_PPC64 */ >> + >> +#ifdef CONFIG_DYNAMIC_FTRACE >> +int __init ftrace_dyn_arch_init(void); >> +#endif /* CONFIG_DYNAMIC_FTRACE */ > Why ? > >> #endif /* !__ASSEMBLY__ */ >> >> #endif /* _ASM_POWERPC_FTRACE */ >> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h >> index 832e65f06754..f1eca123d89d 100644 >> --- a/include/linux/ftrace.h >> +++ b/include/linux/ftrace.h >> @@ -573,7 +573,6 @@ ftrace_set_early_filter(struct ftrace_ops *ops, char >> *buf, int enable); >> >> /* defined in arch */ >> extern int ftrace_ip_converted(unsigned long ip); >> -extern int ftrace_dyn_arch_init(void); > Why removing that ? > > Have you tried to build kernel/trace/ftrace.o with C=2 ? It will likely tell you that the function is not declared and that it should be static Yes I missed this check. Under the situation, the function should be static. > We could eventually consider that in the past, this generic declaration was unrelevant because the definitions where in the arch specific sections. > Now that you are implementing a generic weak version of this function, it would make sense to have a generic declaration as well. > > I really don't see the point in duplicating the declaration of the function in the arch specific headers. I use declaration in arch specific headers in tend to clarify the arch has implement ftrace_dyn_arch_init(). Anyway, it maybe pointless, a generic declaration is enough. Will update it later. >> extern void ftrace_replace_code(int enable); >> extern int ftrace_update_ftrace_func(ftrace_func_t func); >> extern void ftrace_caller(void); > Christophe > > CS Group - Document Interne
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c index 3c83b5d29697..a006585e1c09 100644 --- a/arch/arm/kernel/ftrace.c +++ b/arch/arm/kernel/ftrace.c @@ -193,11 +193,6 @@ int ftrace_make_nop(struct module *mod, return ret; } - -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} #endif /* CONFIG_DYNAMIC_FTRACE */ #ifdef CONFIG_FUNCTION_GRAPH_TRACER diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index 7f467bd9db7a..fc62dfe73f93 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -236,11 +236,6 @@ void arch_ftrace_update_code(int command) command |= FTRACE_MAY_SLEEP; ftrace_modify_all_code(command); } - -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} #endif /* CONFIG_DYNAMIC_FTRACE */ #ifdef CONFIG_FUNCTION_GRAPH_TRACER diff --git a/arch/csky/kernel/ftrace.c b/arch/csky/kernel/ftrace.c index b4a7ec1517ff..50bfcf129078 100644 --- a/arch/csky/kernel/ftrace.c +++ b/arch/csky/kernel/ftrace.c @@ -133,11 +133,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func) (unsigned long)func, true, true); return ret; } - -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} #endif /* CONFIG_DYNAMIC_FTRACE */ #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS diff --git a/arch/ia64/kernel/ftrace.c b/arch/ia64/kernel/ftrace.c index b2ab2d58fb30..d6360fd404ab 100644 --- a/arch/ia64/kernel/ftrace.c +++ b/arch/ia64/kernel/ftrace.c @@ -194,9 +194,3 @@ int ftrace_update_ftrace_func(ftrace_func_t func) flush_icache_range(addr, addr + 16); return 0; } - -/* run from kstop_machine */ -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} diff --git a/arch/microblaze/kernel/ftrace.c b/arch/microblaze/kernel/ftrace.c index 224eea40e1ee..188749d62709 100644 --- a/arch/microblaze/kernel/ftrace.c +++ b/arch/microblaze/kernel/ftrace.c @@ -163,11 +163,6 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) return ret; } -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} - int ftrace_update_ftrace_func(ftrace_func_t func) { unsigned long ip = (unsigned long)(&ftrace_call); diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h index b463f2aa5a61..ed013e767390 100644 --- a/arch/mips/include/asm/ftrace.h +++ b/arch/mips/include/asm/ftrace.h @@ -76,6 +76,8 @@ do { \ #ifdef CONFIG_DYNAMIC_FTRACE +int __init ftrace_dyn_arch_init(void); + static inline unsigned long ftrace_call_adjust(unsigned long addr) { return addr; diff --git a/arch/nds32/kernel/ftrace.c b/arch/nds32/kernel/ftrace.c index 0e23e3a8df6b..f0ef4842d191 100644 --- a/arch/nds32/kernel/ftrace.c +++ b/arch/nds32/kernel/ftrace.c @@ -84,11 +84,6 @@ void _ftrace_caller(unsigned long parent_ip) /* restore all state needed by the compiler epilogue */ } -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} - static unsigned long gen_sethi_insn(unsigned long addr) { unsigned long opcode = 0x46000000; diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c index 0a1e75af5382..01581f715737 100644 --- a/arch/parisc/kernel/ftrace.c +++ b/arch/parisc/kernel/ftrace.c @@ -94,11 +94,6 @@ int ftrace_disable_ftrace_graph_caller(void) #endif #ifdef CONFIG_DYNAMIC_FTRACE - -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} int ftrace_update_ftrace_func(ftrace_func_t func) { return 0; diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h index debe8c4f7062..b05c43f13a4d 100644 --- a/arch/powerpc/include/asm/ftrace.h +++ b/arch/powerpc/include/asm/ftrace.h @@ -126,6 +126,10 @@ static inline void this_cpu_enable_ftrace(void) { } static inline void this_cpu_set_ftrace_enabled(u8 ftrace_enabled) { } static inline u8 this_cpu_get_ftrace_enabled(void) { return 1; } #endif /* CONFIG_PPC64 */ + +#ifdef CONFIG_DYNAMIC_FTRACE +int __init ftrace_dyn_arch_init(void); +#endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* !__ASSEMBLY__ */ #endif /* _ASM_POWERPC_FTRACE */ diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c index 7f1e5203de88..4716f4cdc038 100644 --- a/arch/riscv/kernel/ftrace.c +++ b/arch/riscv/kernel/ftrace.c @@ -154,11 +154,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func) return ret; } - -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} #endif #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c index 0a464d328467..3fd80397ff52 100644 --- a/arch/s390/kernel/ftrace.c +++ b/arch/s390/kernel/ftrace.c @@ -262,11 +262,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func) return 0; } -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} - void arch_ftrace_update_code(int command) { if (ftrace_shared_hotpatch_trampoline(NULL)) diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c index 295c43315bbe..930001bb8c6a 100644 --- a/arch/sh/kernel/ftrace.c +++ b/arch/sh/kernel/ftrace.c @@ -252,11 +252,6 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) return ftrace_modify_code(rec->ip, old, new); } - -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} #endif /* CONFIG_DYNAMIC_FTRACE */ #ifdef CONFIG_FUNCTION_GRAPH_TRACER diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c index 684b84ce397f..eaead3da8e03 100644 --- a/arch/sparc/kernel/ftrace.c +++ b/arch/sparc/kernel/ftrace.c @@ -82,11 +82,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func) new = ftrace_call_replace(ip, (unsigned long)func); return ftrace_modify_code(ip, old, new); } - -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} #endif #ifdef CONFIG_FUNCTION_GRAPH_TRACER diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 1b3ce3b4a2a2..23d221a9a3cd 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -252,11 +252,6 @@ void arch_ftrace_update_code(int command) ftrace_modify_all_code(command); } -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} - /* Currently only x86_64 supports dynamic trampolines */ #ifdef CONFIG_X86_64 diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 832e65f06754..f1eca123d89d 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -573,7 +573,6 @@ ftrace_set_early_filter(struct ftrace_ops *ops, char *buf, int enable); /* defined in arch */ extern int ftrace_ip_converted(unsigned long ip); -extern int ftrace_dyn_arch_init(void); extern void ftrace_replace_code(int enable); extern int ftrace_update_ftrace_func(ftrace_func_t func); extern void ftrace_caller(void); diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 7efbc8aaf7f6..4c090323198d 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -6846,6 +6846,11 @@ void __init ftrace_free_init_mem(void) ftrace_free_mem(NULL, start, end); } +int __init __weak ftrace_dyn_arch_init(void) +{ + return 0; +} + void __init ftrace_init(void) { extern unsigned long __start_mcount_loc[];