2022-03-24 21:27:24

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 16/22] powerpc/ftrace: Minimise number of #ifdefs

A lot of #ifdefs can be replaced by IS_ENABLED()

Do so.

This requires to have kernel_toc_addr() defined at all time
and PPC_INST_LD_TOC as well.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/code-patching.h | 2 -
arch/powerpc/include/asm/module.h | 2 -
arch/powerpc/include/asm/sections.h | 24 +--
arch/powerpc/kernel/trace/ftrace.c | 201 ++++++++++++-----------
4 files changed, 113 insertions(+), 116 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index 4260e89f62b1..071fcbec31c5 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -217,7 +217,6 @@ static inline unsigned long ppc_kallsyms_lookup_name(const char *name)
return addr;
}

-#ifdef CONFIG_PPC64
/*
* Some instruction encodings commonly used in dynamic ftracing
* and function live patching.
@@ -234,6 +233,5 @@ static inline unsigned long ppc_kallsyms_lookup_name(const char *name)

/* usually preceded by a mflr r0 */
#define PPC_INST_STD_LR PPC_RAW_STD(_R0, _R1, PPC_LR_STKOFF)
-#endif /* CONFIG_PPC64 */

#endif /* _ASM_POWERPC_CODE_PATCHING_H */
diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h
index e6f5963fd96e..700d7ecd9012 100644
--- a/arch/powerpc/include/asm/module.h
+++ b/arch/powerpc/include/asm/module.h
@@ -41,9 +41,7 @@ struct mod_arch_specific {

#ifdef CONFIG_FUNCTION_TRACER
unsigned long tramp;
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
unsigned long tramp_regs;
-#endif
#endif

/* List of BUG addresses, source line numbers and filenames */
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 8be2c491c733..6980eaeb16fe 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -29,18 +29,6 @@ extern char start_virt_trampolines[];
extern char end_virt_trampolines[];
#endif

-/*
- * This assumes the kernel is never compiled -mcmodel=small or
- * the total .toc is always less than 64k.
- */
-static inline unsigned long kernel_toc_addr(void)
-{
- unsigned long toc_ptr;
-
- asm volatile("mr %0, 2" : "=r" (toc_ptr));
- return toc_ptr;
-}
-
static inline int overlaps_interrupt_vector_text(unsigned long start,
unsigned long end)
{
@@ -60,5 +48,17 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)

#endif

+/*
+ * This assumes the kernel is never compiled -mcmodel=small or
+ * the total .toc is always less than 64k.
+ */
+static inline unsigned long kernel_toc_addr(void)
+{
+ unsigned long toc_ptr;
+
+ asm volatile("mr %0, 2" : "=r" (toc_ptr));
+ return toc_ptr;
+}
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_SECTIONS_H */
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index ffedf8c82ea8..4dd30e396026 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -150,55 +150,55 @@ __ftrace_make_nop(struct module *mod,
return -EINVAL;
}

- /* When using -mkernel_profile or PPC32 there is no load to jump over */
- pop = ppc_inst(PPC_RAW_NOP());
+ if (IS_ENABLED(CONFIG_MPROFILE_KERNEL)) {
+ /* When using -mkernel_profile or PPC32 there is no load to jump over */
+ pop = ppc_inst(PPC_RAW_NOP());

-#ifdef CONFIG_PPC64
-#ifdef CONFIG_MPROFILE_KERNEL
- if (copy_inst_from_kernel_nofault(&op, (void *)(ip - 4))) {
- pr_err("Fetching instruction at %lx failed.\n", ip - 4);
- return -EFAULT;
- }
+ if (copy_inst_from_kernel_nofault(&op, (void *)(ip - 4))) {
+ pr_err("Fetching instruction at %lx failed.\n", ip - 4);
+ return -EFAULT;
+ }

- /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
- if (!ppc_inst_equal(op, ppc_inst(PPC_RAW_MFLR(_R0))) &&
- !ppc_inst_equal(op, ppc_inst(PPC_INST_STD_LR))) {
- pr_err("Unexpected instruction %s around bl _mcount\n",
- ppc_inst_as_str(op));
- return -EINVAL;
- }
-#else
- /*
- * Our original call site looks like:
- *
- * bl <tramp>
- * ld r2,XX(r1)
- *
- * Milton Miller pointed out that we can not simply nop the branch.
- * If a task was preempted when calling a trace function, the nops
- * will remove the way to restore the TOC in r2 and the r2 TOC will
- * get corrupted.
- *
- * Use a b +8 to jump over the load.
- */
+ /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
+ if (!ppc_inst_equal(op, ppc_inst(PPC_RAW_MFLR(_R0))) &&
+ !ppc_inst_equal(op, ppc_inst(PPC_INST_STD_LR))) {
+ pr_err("Unexpected instruction %s around bl _mcount\n",
+ ppc_inst_as_str(op));
+ return -EINVAL;
+ }
+ } else if (IS_ENABLED(CONFIG_PPC64)) {
+ /*
+ * Our original call site looks like:
+ *
+ * bl <tramp>
+ * ld r2,XX(r1)
+ *
+ * Milton Miller pointed out that we can not simply nop the branch.
+ * If a task was preempted when calling a trace function, the nops
+ * will remove the way to restore the TOC in r2 and the r2 TOC will
+ * get corrupted.
+ *
+ * Use a b +8 to jump over the load.
+ */

- pop = ppc_inst(PPC_RAW_BRANCH(8)); /* b +8 */
+ pop = ppc_inst(PPC_RAW_BRANCH(8)); /* b +8 */

- /*
- * Check what is in the next instruction. We can see ld r2,40(r1), but
- * on first pass after boot we will see mflr r0.
- */
- if (copy_inst_from_kernel_nofault(&op, (void *)(ip + 4))) {
- pr_err("Fetching op failed.\n");
- return -EFAULT;
- }
+ /*
+ * Check what is in the next instruction. We can see ld r2,40(r1), but
+ * on first pass after boot we will see mflr r0.
+ */
+ if (copy_inst_from_kernel_nofault(&op, (void *)(ip + 4))) {
+ pr_err("Fetching op failed.\n");
+ return -EFAULT;
+ }

- if (!ppc_inst_equal(op, ppc_inst(PPC_INST_LD_TOC))) {
- pr_err("Expected %08lx found %s\n", PPC_INST_LD_TOC, ppc_inst_as_str(op));
- return -EINVAL;
+ if (!ppc_inst_equal(op, ppc_inst(PPC_INST_LD_TOC))) {
+ pr_err("Expected %08lx found %s\n", PPC_INST_LD_TOC, ppc_inst_as_str(op));
+ return -EINVAL;
+ }
+ } else {
+ pop = ppc_inst(PPC_RAW_NOP());
}
-#endif /* CONFIG_MPROFILE_KERNEL */
-#endif /* PPC64 */

if (patch_instruction((u32 *)ip, pop)) {
pr_err("Patching NOP failed.\n");
@@ -207,6 +207,11 @@ __ftrace_make_nop(struct module *mod,

return 0;
}
+#else
+static int __ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr)
+{
+ return 0;
+}
#endif /* CONFIG_MODULES */

static unsigned long find_ftrace_tramp(unsigned long ip)
@@ -279,11 +284,11 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
}

/* Let's re-write the tramp to go to ftrace_[regs_]caller */
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
- ptr = ppc_global_function_entry((void *)ftrace_regs_caller);
-#else
- ptr = ppc_global_function_entry((void *)ftrace_caller);
-#endif
+ if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
+ ptr = ppc_global_function_entry((void *)ftrace_regs_caller);
+ else
+ ptr = ppc_global_function_entry((void *)ftrace_caller);
+
if (patch_branch((u32 *)tramp, ptr, 0)) {
pr_debug("REL24 out of range!\n");
return -1;
@@ -352,10 +357,12 @@ int ftrace_make_nop(struct module *mod,
old = ftrace_call_replace(ip, addr, 1);
new = ppc_inst(PPC_RAW_NOP());
return ftrace_modify_code(ip, old, new);
- } else if (core_kernel_text(ip))
+ } else if (core_kernel_text(ip)) {
return __ftrace_make_nop_kernel(rec, addr);
+ } else if (!IS_ENABLED(CONFIG_MODULES)) {
+ return -EINVAL;
+ }

-#ifdef CONFIG_MODULES
/*
* Out of range jumps are called from modules.
* We should either already have a pointer to the module
@@ -378,10 +385,6 @@ int ftrace_make_nop(struct module *mod,
mod = rec->arch.mod;

return __ftrace_make_nop(mod, rec, addr);
-#else
- /* We should not get here without modules */
- return -EINVAL;
-#endif /* CONFIG_MODULES */
}

#ifdef CONFIG_MODULES
@@ -411,10 +414,9 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
if (copy_inst_from_kernel_nofault(op, ip))
return -EFAULT;

-#ifndef CONFIG_DYNAMIC_FTRACE_WITH_REGS
- if (copy_inst_from_kernel_nofault(op + 1, ip + 4))
+ if (!IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) &&
+ copy_inst_from_kernel_nofault(op + 1, ip + 4))
return -EFAULT;
-#endif

if (!expected_nop_sequence(ip, op[0], op[1])) {
pr_err("Unexpected call sequence at %p: %s %s\n",
@@ -423,20 +425,15 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
}

/* If we never set up ftrace trampoline(s), then bail */
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
- if (!mod->arch.tramp || !mod->arch.tramp_regs) {
-#else
- if (!mod->arch.tramp) {
-#endif
+ if (!mod->arch.tramp ||
+ (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) && !mod->arch.tramp_regs)) {
pr_err("No ftrace trampoline\n");
return -EINVAL;
}

-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
- if (rec->flags & FTRACE_FL_REGS)
+ if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) && rec->flags & FTRACE_FL_REGS)
tramp = mod->arch.tramp_regs;
else
-#endif
tramp = mod->arch.tramp;

if (module_trampoline_target(mod, tramp, &ptr)) {
@@ -460,6 +457,11 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)

return 0;
}
+#else
+static int __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
+{
+ return 0;
+}
#endif /* CONFIG_MODULES */

static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
@@ -472,16 +474,12 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
entry = ppc_global_function_entry((void *)ftrace_caller);
ptr = ppc_global_function_entry((void *)addr);

- if (ptr != entry) {
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ if (ptr != entry && IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
entry = ppc_global_function_entry((void *)ftrace_regs_caller);
- if (ptr != entry) {
-#endif
- pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr);
- return -EINVAL;
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
- }
-#endif
+
+ if (ptr != entry) {
+ pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr);
+ return -EINVAL;
}

/* Make sure we have a nop */
@@ -524,10 +522,13 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
old = ppc_inst(PPC_RAW_NOP());
new = ftrace_call_replace(ip, addr, 1);
return ftrace_modify_code(ip, old, new);
- } else if (core_kernel_text(ip))
+ } else if (core_kernel_text(ip)) {
return __ftrace_make_call_kernel(rec, addr);
+ } else if (!IS_ENABLED(CONFIG_MODULES)) {
+ /* We should not get here without modules */
+ return -EINVAL;
+ }

-#ifdef CONFIG_MODULES
/*
* Out of range jumps are called from modules.
* Being that we are converting from nop, it had better
@@ -539,10 +540,6 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
}

return __ftrace_make_call(rec, addr);
-#else
- /* We should not get here without modules */
- return -EINVAL;
-#endif /* CONFIG_MODULES */
}

#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
@@ -633,6 +630,11 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,

return 0;
}
+#else
+static int __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr)
+{
+ return 0;
+}
#endif

int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
@@ -657,9 +659,11 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
* variant, so there is nothing to do here
*/
return 0;
+ } else if (!IS_ENABLED(CONFIG_MODULES)) {
+ /* We should not get here without modules */
+ return -EINVAL;
}

-#ifdef CONFIG_MODULES
/*
* Out of range jumps are called from modules.
*/
@@ -669,10 +673,6 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
}

return __ftrace_modify_call(rec, old_addr, addr);
-#else
- /* We should not get here without modules */
- return -EINVAL;
-#endif /* CONFIG_MODULES */
}
#endif

@@ -686,15 +686,13 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
new = ftrace_call_replace(ip, (unsigned long)func, 1);
ret = ftrace_modify_code(ip, old, new);

-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
/* Also update the regs callback function */
- if (!ret) {
+ if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) && !ret) {
ip = (unsigned long)(&ftrace_regs_call);
old = ppc_inst_read((u32 *)&ftrace_regs_call);
new = ftrace_call_replace(ip, (unsigned long)func, 1);
ret = ftrace_modify_code(ip, old, new);
}
-#endif

return ret;
}
@@ -710,6 +708,9 @@ void arch_ftrace_update_code(int command)

#ifdef CONFIG_PPC64
#define PACATOC offsetof(struct paca_struct, kernel_toc)
+#else
+#define PACATOC 0
+#endif

extern unsigned int ftrace_tramp_text[], ftrace_tramp_init[];

@@ -724,12 +725,18 @@ int __init ftrace_dyn_arch_init(void)
PPC_RAW_MTCTR(_R12),
PPC_RAW_BCTR()
};
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
- unsigned long addr = ppc_global_function_entry((void *)ftrace_regs_caller);
-#else
- unsigned long addr = ppc_global_function_entry((void *)ftrace_caller);
-#endif
- long reladdr = addr - kernel_toc_addr();
+ unsigned long addr;
+ long reladdr;
+
+ if (IS_ENABLED(CONFIG_PPC32))
+ return 0;
+
+ if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS))
+ addr = ppc_global_function_entry((void *)ftrace_regs_caller);
+ else
+ addr = ppc_global_function_entry((void *)ftrace_caller);
+
+ reladdr = addr - kernel_toc_addr();

if (reladdr >= SZ_2G || reladdr < -SZ_2G) {
pr_err("Address of %ps out of range of kernel_toc.\n",
@@ -746,12 +753,6 @@ int __init ftrace_dyn_arch_init(void)

return 0;
}
-#else
-int __init ftrace_dyn_arch_init(void)
-{
- return 0;
-}
-#endif

#ifdef CONFIG_FUNCTION_GRAPH_TRACER

--
2.35.1


2022-04-19 06:36:15

by Naveen N. Rao

[permalink] [raw]
Subject: Re: [PATCH v1 16/22] powerpc/ftrace: Minimise number of #ifdefs

Christophe Leroy wrote:
> A lot of #ifdefs can be replaced by IS_ENABLED()
>
> Do so.
>
> This requires to have kernel_toc_addr() defined at all time
> and PPC_INST_LD_TOC as well.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/include/asm/code-patching.h | 2 -
> arch/powerpc/include/asm/module.h | 2 -
> arch/powerpc/include/asm/sections.h | 24 +--
> arch/powerpc/kernel/trace/ftrace.c | 201 ++++++++++++-----------
> 4 files changed, 113 insertions(+), 116 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index 4260e89f62b1..071fcbec31c5 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -217,7 +217,6 @@ static inline unsigned long ppc_kallsyms_lookup_name(const char *name)
> return addr;
> }
>
> -#ifdef CONFIG_PPC64
> /*
> * Some instruction encodings commonly used in dynamic ftracing
> * and function live patching.
> @@ -234,6 +233,5 @@ static inline unsigned long ppc_kallsyms_lookup_name(const char *name)
>
> /* usually preceded by a mflr r0 */
> #define PPC_INST_STD_LR PPC_RAW_STD(_R0, _R1, PPC_LR_STKOFF)
> -#endif /* CONFIG_PPC64 */
>
> #endif /* _ASM_POWERPC_CODE_PATCHING_H */
> diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h
> index e6f5963fd96e..700d7ecd9012 100644
> --- a/arch/powerpc/include/asm/module.h
> +++ b/arch/powerpc/include/asm/module.h
> @@ -41,9 +41,7 @@ struct mod_arch_specific {
>
> #ifdef CONFIG_FUNCTION_TRACER
> unsigned long tramp;
> -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> unsigned long tramp_regs;
> -#endif
> #endif
>
> /* List of BUG addresses, source line numbers and filenames */
> diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
> index 8be2c491c733..6980eaeb16fe 100644
> --- a/arch/powerpc/include/asm/sections.h
> +++ b/arch/powerpc/include/asm/sections.h
> @@ -29,18 +29,6 @@ extern char start_virt_trampolines[];
> extern char end_virt_trampolines[];
> #endif
>
> -/*
> - * This assumes the kernel is never compiled -mcmodel=small or
> - * the total .toc is always less than 64k.
> - */
> -static inline unsigned long kernel_toc_addr(void)
> -{
> - unsigned long toc_ptr;
> -
> - asm volatile("mr %0, 2" : "=r" (toc_ptr));
> - return toc_ptr;
> -}
> -
> static inline int overlaps_interrupt_vector_text(unsigned long start,
> unsigned long end)
> {
> @@ -60,5 +48,17 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)
>
> #endif
>
> +/*
> + * This assumes the kernel is never compiled -mcmodel=small or
> + * the total .toc is always less than 64k.
> + */
> +static inline unsigned long kernel_toc_addr(void)
> +{
> + unsigned long toc_ptr;
> +
> + asm volatile("mr %0, 2" : "=r" (toc_ptr));
> + return toc_ptr;
> +}
> +
> #endif /* __KERNEL__ */
> #endif /* _ASM_POWERPC_SECTIONS_H */
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index ffedf8c82ea8..4dd30e396026 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -150,55 +150,55 @@ __ftrace_make_nop(struct module *mod,
> return -EINVAL;
> }
>
> - /* When using -mkernel_profile or PPC32 there is no load to jump over */
> - pop = ppc_inst(PPC_RAW_NOP());
> + if (IS_ENABLED(CONFIG_MPROFILE_KERNEL)) {
> + /* When using -mkernel_profile or PPC32 there is no load to jump over */
> + pop = ppc_inst(PPC_RAW_NOP());

Why move this inside the if() statement? You can keep this out and drop
the else clause.

- Naveen

2022-05-06 11:14:19

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v1 16/22] powerpc/ftrace: Minimise number of #ifdefs



Le 18/04/2022 à 09:59, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>> A lot of #ifdefs can be replaced by IS_ENABLED()
>>
>> Do so.
>>
>> This requires to have kernel_toc_addr() defined at all time
>> and PPC_INST_LD_TOC as well.
>>
>> Signed-off-by: Christophe Leroy <[email protected]>
>> ---
>>  arch/powerpc/include/asm/code-patching.h |   2 -
>>  arch/powerpc/include/asm/module.h        |   2 -
>>  arch/powerpc/include/asm/sections.h      |  24 +--
>>  arch/powerpc/kernel/trace/ftrace.c       | 201 ++++++++++++-----------
>>  4 files changed, 113 insertions(+), 116 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/code-patching.h
>> b/arch/powerpc/include/asm/code-patching.h
>> index 4260e89f62b1..071fcbec31c5 100644
>> --- a/arch/powerpc/include/asm/code-patching.h
>> +++ b/arch/powerpc/include/asm/code-patching.h
>> @@ -217,7 +217,6 @@ static inline unsigned long
>> ppc_kallsyms_lookup_name(const char *name)
>>      return addr;
>>  }
>>
>> -#ifdef CONFIG_PPC64
>>  /*
>>   * Some instruction encodings commonly used in dynamic ftracing
>>   * and function live patching.
>> @@ -234,6 +233,5 @@ static inline unsigned long
>> ppc_kallsyms_lookup_name(const char *name)
>>
>>  /* usually preceded by a mflr r0 */
>>  #define PPC_INST_STD_LR        PPC_RAW_STD(_R0, _R1, PPC_LR_STKOFF)
>> -#endif /* CONFIG_PPC64 */
>>
>>  #endif /* _ASM_POWERPC_CODE_PATCHING_H */
>> diff --git a/arch/powerpc/include/asm/module.h
>> b/arch/powerpc/include/asm/module.h
>> index e6f5963fd96e..700d7ecd9012 100644
>> --- a/arch/powerpc/include/asm/module.h
>> +++ b/arch/powerpc/include/asm/module.h
>> @@ -41,9 +41,7 @@ struct mod_arch_specific {
>>
>>  #ifdef CONFIG_FUNCTION_TRACER
>>      unsigned long tramp;
>> -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
>>      unsigned long tramp_regs;
>> -#endif
>>  #endif
>>
>>      /* List of BUG addresses, source line numbers and filenames */
>> diff --git a/arch/powerpc/include/asm/sections.h
>> b/arch/powerpc/include/asm/sections.h
>> index 8be2c491c733..6980eaeb16fe 100644
>> --- a/arch/powerpc/include/asm/sections.h
>> +++ b/arch/powerpc/include/asm/sections.h
>> @@ -29,18 +29,6 @@ extern char start_virt_trampolines[];
>>  extern char end_virt_trampolines[];
>>  #endif
>>
>> -/*
>> - * This assumes the kernel is never compiled -mcmodel=small or
>> - * the total .toc is always less than 64k.
>> - */
>> -static inline unsigned long kernel_toc_addr(void)
>> -{
>> -    unsigned long toc_ptr;
>> -
>> -    asm volatile("mr %0, 2" : "=r" (toc_ptr));
>> -    return toc_ptr;
>> -}
>> -
>>  static inline int overlaps_interrupt_vector_text(unsigned long start,
>>                              unsigned long end)
>>  {
>> @@ -60,5 +48,17 @@ static inline int overlaps_kernel_text(unsigned
>> long start, unsigned long end)
>>
>>  #endif
>>
>> +/*
>> + * This assumes the kernel is never compiled -mcmodel=small or
>> + * the total .toc is always less than 64k.
>> + */
>> +static inline unsigned long kernel_toc_addr(void)
>> +{
>> +    unsigned long toc_ptr;
>> +
>> +    asm volatile("mr %0, 2" : "=r" (toc_ptr));
>> +    return toc_ptr;
>> +}
>> +
>>  #endif /* __KERNEL__ */
>>  #endif    /* _ASM_POWERPC_SECTIONS_H */
>> diff --git a/arch/powerpc/kernel/trace/ftrace.c
>> b/arch/powerpc/kernel/trace/ftrace.c
>> index ffedf8c82ea8..4dd30e396026 100644
>> --- a/arch/powerpc/kernel/trace/ftrace.c
>> +++ b/arch/powerpc/kernel/trace/ftrace.c
>> @@ -150,55 +150,55 @@ __ftrace_make_nop(struct module *mod,
>>          return -EINVAL;
>>      }
>>
>> -    /* When using -mkernel_profile or PPC32 there is no load to jump
>> over */
>> -    pop = ppc_inst(PPC_RAW_NOP());
>> +    if (IS_ENABLED(CONFIG_MPROFILE_KERNEL)) {
>> +        /* When using -mkernel_profile or PPC32 there is no load to
>> jump over */
>> +        pop = ppc_inst(PPC_RAW_NOP());
>
> Why move this inside the if() statement? You can keep this out and drop
> the else clause.

I find it less error prone that way.

Putting a bad value and then replace it later with the good one can be
misleading, and if some day someone removes that second set by error, it
will go unnoticed. When you set it only once, GCC will complain in case
that setting disappear by error.

Christophe