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

[Chengming Zhou]

On 2022/2/24 9:17 上午, Steven Rostedt wrote:
> On Wed, 23 Feb 2022 17:01:31 +0800
> Chengming Zhou <zhouchengming at 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