Re: [External] Re: [PATCH v2 1/2] ftrace: cleanup ftrace_graph_caller enable and disable

Chengming Zhou
2022-02-24T02:00:23Z

On 2022/2/24 9:17 上午, Steven Rostedt wrote:
> On Wed, 23 Feb 2022 17:01:31 +0800
> Chengming Zhou <zhouchengming@bytedance.com> wrote:
>
>> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
>> index 7cc540e6de0c..c119ef7a9295 100644
>> --- a/arch/x86/kernel/ftrace.c
>> +++ b/arch/x86/kernel/ftrace.c
>> @@ -615,18 +615,8 @@ int ftrace_disable_ftrace_graph_caller(void)
>>
>> return ftrace_mod_jmp(ip, &ftrace_stub);
>> }
>> -#else /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
>> -int ftrace_enable_ftrace_graph_caller(void)
>> -{
>> - return 0;
>> -}
>> -
>> -int ftrace_disable_ftrace_graph_caller(void)
>> -{
>> - return 0;
>> -}
>> -#endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
>> -#endif /* !CONFIG_DYNAMIC_FTRACE */
>> +#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
>> +#endif /* CONFIG_DYNAMIC_FTRACE */
>
> Since you are cleaning this up, and the above starts with:
>
> #ifdef CONFIG_DYNAMIC_FTRACE
>
> #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
>
>
> They were separate because of the #else that you just removed. I would
> recommend consolidate them into a single #ifdef:
>
> #if defined(CONFIG_DYNAMIC_FTRACE) && !defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS)
>
> [..]
>
> #endif /* CONFIG_DYNAMIC_FTRACE && !CONFIG_HAVE_DYNAMI_FTRACE_WITH_ARGS */
>

Good suggestion, this is better.

Thanks.

>
> -- Steve
>
>
>>
>> /*
>> * Hook the return address and push it in the stack of return addrs
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index f9feb197b2da..60ae009e6684 100644