Messages in this thread | | | From | Christophe Leroy <> | Subject | Re: [PATCH v1 10/22] powerpc/ftrace: Use CONFIG_FUNCTION_TRACER instead of CONFIG_DYNAMIC_FTRACE | Date | Fri, 6 May 2022 11:41:02 +0000 |
| |
Le 18/04/2022 à 09:00, Naveen N. Rao a écrit : > Christophe Leroy wrote: >> Since commit 0c0c52306f47 ("powerpc: Only support DYNAMIC_FTRACE not >> static"), CONFIG_DYNAMIC_FTRACE is always selected when >> CONFIG_FUNCTION_TRACER is selected. >> >> To avoid confusion and have the reader wonder what's happen when >> CONFIG_FUNCTION_TRACER is selected and CONFIG_DYNAMIC_FTRACE is not, >> use CONFIG_FUNCTION_TRACER in ifdefs instead of CONFIG_DYNAMIC_FTRACE. >> >> As CONFIG_FUNCTION_GRAPH_TRACER depends on CONFIG_FUNCTION_TRACER, >> ftrace.o doesn't need to appear for both symbols in Makefile. >> >> Then as ftrace.o is built only when CONFIG_FUNCTION_TRACER is selected > > and since it implies CONFIG_DYNAMIC_FTRACE, CONFIG_DYNAMIC_FTRACE is not > needed in ftrace.c
Ok, added to the commit message
> >> ifdef CONFIG_FUNCTION_TRACER is not needed in ftrace.c >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +- >> arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +- >> arch/powerpc/include/asm/module.h | 4 ++-- >> arch/powerpc/include/asm/nohash/pgtable.h | 2 +- >> arch/powerpc/kernel/module_32.c | 4 ++-- >> arch/powerpc/kernel/module_64.c | 6 +++--- >> arch/powerpc/kernel/trace/Makefile | 4 +--- >> arch/powerpc/kernel/trace/ftrace.c | 4 ---- >> 8 files changed, 11 insertions(+), 17 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h >> b/arch/powerpc/include/asm/book3s/32/pgtable.h >> index 772e00dc4ef1..992aed626eb4 100644 >> --- a/arch/powerpc/include/asm/book3s/32/pgtable.h >> +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h >> @@ -124,7 +124,7 @@ static inline bool pte_user(pte_t pte) >> * on platforms where such control is possible. >> */ >> #if defined(CONFIG_KGDB) || defined(CONFIG_XMON) || >> defined(CONFIG_BDI_SWITCH) ||\ >> - defined(CONFIG_KPROBES) || defined(CONFIG_DYNAMIC_FTRACE) >> + defined(CONFIG_KPROBES) || defined(CONFIG_FUNCTION_TRACER) >> #define PAGE_KERNEL_TEXT PAGE_KERNEL_X >> #else >> #define PAGE_KERNEL_TEXT PAGE_KERNEL_ROX >> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h >> b/arch/powerpc/include/asm/book3s/64/pgtable.h >> index 875730d5af40..cf01b609572f 100644 >> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h >> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h >> @@ -169,7 +169,7 @@ >> * on platforms where such control is possible. >> */ >> #if defined(CONFIG_KGDB) || defined(CONFIG_XMON) || >> defined(CONFIG_BDI_SWITCH) || \ >> - defined(CONFIG_KPROBES) || defined(CONFIG_DYNAMIC_FTRACE) >> + defined(CONFIG_KPROBES) || defined(CONFIG_FUNCTION_TRACER) >> #define PAGE_KERNEL_TEXT PAGE_KERNEL_X >> #else >> #define PAGE_KERNEL_TEXT PAGE_KERNEL_ROX >> diff --git a/arch/powerpc/include/asm/module.h >> b/arch/powerpc/include/asm/module.h >> index 857d9ff24295..e6f5963fd96e 100644 >> --- a/arch/powerpc/include/asm/module.h >> +++ b/arch/powerpc/include/asm/module.h >> @@ -39,7 +39,7 @@ struct mod_arch_specific { >> unsigned int init_plt_section; >> #endif /* powerpc64 */ >> >> -#ifdef CONFIG_DYNAMIC_FTRACE >> +#ifdef CONFIG_FUNCTION_TRACER >> unsigned long tramp; >> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS >> unsigned long tramp_regs; >> @@ -68,7 +68,7 @@ struct mod_arch_specific { >> # endif /* MODULE */ >> #endif >> >> -#ifdef CONFIG_DYNAMIC_FTRACE >> +#ifdef CONFIG_FUNCTION_TRACER >> # ifdef MODULE >> asm(".section .ftrace.tramp,\"ax\",@nobits; .align 3; .previous"); >> # endif /* MODULE */ >> diff --git a/arch/powerpc/include/asm/nohash/pgtable.h >> b/arch/powerpc/include/asm/nohash/pgtable.h >> index ac75f4ab0dba..2e8cf217a191 100644 >> --- a/arch/powerpc/include/asm/nohash/pgtable.h >> +++ b/arch/powerpc/include/asm/nohash/pgtable.h >> @@ -23,7 +23,7 @@ >> * on platforms where such control is possible. >> */ >> #if defined(CONFIG_KGDB) || defined(CONFIG_XMON) || >> defined(CONFIG_BDI_SWITCH) ||\ >> - defined(CONFIG_KPROBES) || defined(CONFIG_DYNAMIC_FTRACE) >> + defined(CONFIG_KPROBES) || defined(CONFIG_FUNCTION_TRACER) >> #define PAGE_KERNEL_TEXT PAGE_KERNEL_X >> #else >> #define PAGE_KERNEL_TEXT PAGE_KERNEL_ROX >> diff --git a/arch/powerpc/kernel/module_32.c >> b/arch/powerpc/kernel/module_32.c >> index a0432ef46967..2aa368ce21c9 100644 >> --- a/arch/powerpc/kernel/module_32.c >> +++ b/arch/powerpc/kernel/module_32.c >> @@ -39,7 +39,7 @@ static unsigned int count_relocs(const Elf32_Rela >> *rela, unsigned int num) >> r_addend = rela[i].r_addend; >> } >> >> -#ifdef CONFIG_DYNAMIC_FTRACE >> +#ifdef CONFIG_FUNCTION_TRACER >> _count_relocs++; /* add one for ftrace_caller */ >> #endif >> return _count_relocs; >> @@ -288,7 +288,7 @@ int apply_relocate_add(Elf32_Shdr *sechdrs, >> return 0; >> } >> >> -#ifdef CONFIG_DYNAMIC_FTRACE >> +#ifdef CONFIG_FUNCTION_TRACER >> int module_trampoline_target(struct module *mod, unsigned long addr, >> unsigned long *target) >> { >> diff --git a/arch/powerpc/kernel/module_64.c >> b/arch/powerpc/kernel/module_64.c >> index 794720530442..b13a72665eee 100644 >> --- a/arch/powerpc/kernel/module_64.c >> +++ b/arch/powerpc/kernel/module_64.c >> @@ -207,7 +207,7 @@ static unsigned long get_stubs_size(const >> Elf64_Ehdr *hdr, >> } >> } >> >> -#ifdef CONFIG_DYNAMIC_FTRACE >> +#ifdef CONFIG_FUNCTION_TRACER >> /* make the trampoline to the ftrace_caller */ >> relocs++; >> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS >> @@ -372,7 +372,7 @@ static bool is_mprofile_ftrace_call(const char *name) >> { >> if (!strcmp("_mcount", name)) >> return true; >> -#ifdef CONFIG_DYNAMIC_FTRACE >> +#ifdef CONFIG_FUNCTION_TRACER >> if (!strcmp("ftrace_caller", name)) >> return true; >> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS >> @@ -740,7 +740,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, >> return 0; >> } >> >> -#ifdef CONFIG_DYNAMIC_FTRACE >> +#ifdef CONFIG_FUNCTION_TRACER >> int module_trampoline_target(struct module *mod, unsigned long addr, >> unsigned long *target) >> { > > The below two changes to trace/Makefile and trace/ftrace.c make sense, > but I'm not sure the above changes are necessary. > > For generic code (outside arch/powerpc/kernel/trace), I think it is good > to retain the actual dependency which is DYNAMIC_FTRACE. In my view, > gating some of the above code on FUNCTION_TRACER is also confusing. The > primary implication of always selecting DYNAMIC_FTRACE is within the > powerpc trace code, and it is good to keep it that way.
Ok I keep only the changes to trace directory for now.
> >> diff --git a/arch/powerpc/kernel/trace/Makefile >> b/arch/powerpc/kernel/trace/Makefile >> index fc32ec30b297..af8527538fe4 100644 >> --- a/arch/powerpc/kernel/trace/Makefile >> +++ b/arch/powerpc/kernel/trace/Makefile >> @@ -14,9 +14,7 @@ obj64-$(CONFIG_FUNCTION_TRACER) += >> ftrace_mprofile.o >> else >> obj64-$(CONFIG_FUNCTION_TRACER) += ftrace_64_pg.o >> endif >> -obj-$(CONFIG_FUNCTION_TRACER) += ftrace_low.o >> -obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o >> -obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o >> +obj-$(CONFIG_FUNCTION_TRACER) += ftrace_low.o ftrace.o >> obj-$(CONFIG_TRACING) += trace_clock.o >> >> obj-$(CONFIG_PPC64) += $(obj64-y) >> diff --git a/arch/powerpc/kernel/trace/ftrace.c >> b/arch/powerpc/kernel/trace/ftrace.c >> index 2c7e42e439bb..188f59f4ee4a 100644 >> --- a/arch/powerpc/kernel/trace/ftrace.c >> +++ b/arch/powerpc/kernel/trace/ftrace.c >> @@ -28,9 +28,6 @@ >> #include <asm/syscall.h> >> #include <asm/inst.h> >> >> - >> -#ifdef CONFIG_DYNAMIC_FTRACE >> - >> /* >> * We generally only have a single long_branch tramp and at most 2 or >> 3 plt >> * tramps generated. But, we don't use the plt tramps currently. We >> also allot >> @@ -783,7 +780,6 @@ int __init ftrace_dyn_arch_init(void) >> return 0; >> } >> #endif >> -#endif /* CONFIG_DYNAMIC_FTRACE */ >> >> #ifdef CONFIG_FUNCTION_GRAPH_TRACER >> >> -- >> 2.35.1 >> > > - Naveen
| |