[RFC PATCH 0/2] Introduce ftrace_disabled

[RFC PATCH 0/2] Introduce ftrace_disabled

[Tiezhu Yang]

Tiezhu Yang (2):
  ftrace: Introduce cmdline argument ftrace_disabled
  docs: kernel-parameters: Add ftrace_disabled

 Documentation/admin-guide/kernel-parameters.txt | 4 ++++
 kernel/trace/ftrace.c                           | 8 ++++++++
 2 files changed, 12 insertions(+)

-- 
2.1.0

[RFC PATCH 1/2] ftrace: Introduce cmdline argument ftrace_disabled

[Tiezhu Yang]

If set CONFIG_FUNCTION_TRACER=y, we can use the following command to
disable ftrace after boot up:
echo 0 > /proc/sys/kernel/ftrace_enabled

ftrace_disabled is much stronger than ftrace_enabled, introduce a new
cmdline argument ftrace_disabled for user to control whether to disable
ftrace when boot up.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 kernel/trace/ftrace.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 72ef4dc..a015699 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5517,6 +5517,14 @@ static char ftrace_filter_buf[FTRACE_FILTER_SIZE] __initdata;
 /* Used by function selftest to not test if filter is set */
 bool ftrace_filter_param __initdata;
 
+static int __init set_ftrace_disabled(char *str)
+{
+	pr_info("Set ftrace_disabled to disable ftrace\n");
+	ftrace_disabled = 1;
+	return 1;
+}
+__setup("ftrace_disabled", set_ftrace_disabled);
+
 static int __init set_ftrace_notrace(char *str)
 {
 	ftrace_filter_param = true;
-- 
2.1.0

[RFC PATCH 2/2] docs: kernel-parameters: Add ftrace_disabled

[Tiezhu Yang]

Explain the cmdline argument ftrace_disabled in the file
Documentation/admin-guide/kernel-parameters.txt

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 Documentation/admin-guide/kernel-parameters.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index cb89dbd..f255aff 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1392,6 +1392,10 @@
 			dump only the buffer of the CPU that triggered the
 			oops.
 
+	ftrace_disabled
+			Set ftrace_disabled in cmdline to disable ftrace when
+			boot up.
+
 	ftrace_filter=[function-list]
 			[FTRACE] Limit the functions traced by the function
 			tracer at boot up. function-list is a comma-separated
-- 
2.1.0

Re: [RFC PATCH 1/2] ftrace: Introduce cmdline argument ftrace_disabled

[Steven Rostedt]

On Sat, 19 Jun 2021 14:29:19 +0800
Tiezhu Yang <yangtiezhu@loongson.cn> wrote:

> If set CONFIG_FUNCTION_TRACER=y, we can use the following command to
> disable ftrace after boot up:
> echo 0 > /proc/sys/kernel/ftrace_enabled

I'd love to just remove that. It's original purpose was to stop function
tracing when the latency tracers were active. That's now done via a tracefs
option. The "ftrace_enabled" really has no use anymore.

> 
> ftrace_disabled is much stronger than ftrace_enabled, introduce a new
> cmdline argument ftrace_disabled for user to control whether to disable
> ftrace when boot up.

"ftrace_disabled" is triggered when an anomaly is detected, and for the
safety of the system, ftrace shuts down. It was never meant to be a user
triggered event.

You have no rationale for this change. What's the purpose of this?

-- Steve


> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  kernel/trace/ftrace.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 72ef4dc..a015699 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5517,6 +5517,14 @@ static char ftrace_filter_buf[FTRACE_FILTER_SIZE] __initdata;
>  /* Used by function selftest to not test if filter is set */
>  bool ftrace_filter_param __initdata;
>  
> +static int __init set_ftrace_disabled(char *str)
> +{
> +	pr_info("Set ftrace_disabled to disable ftrace\n");
> +	ftrace_disabled = 1;
> +	return 1;
> +}
> +__setup("ftrace_disabled", set_ftrace_disabled);
> +
>  static int __init set_ftrace_notrace(char *str)
>  {
>  	ftrace_filter_param = true;

Re: [RFC PATCH 1/2] ftrace: Introduce cmdline argument ftrace_disabled

[Tiezhu Yang]

On 06/19/2021 11:22 PM, Steven Rostedt wrote:
> On Sat, 19 Jun 2021 14:29:19 +0800
> Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
>> If set CONFIG_FUNCTION_TRACER=y, we can use the following command to
>> disable ftrace after boot up:
>> echo 0 > /proc/sys/kernel/ftrace_enabled
> I'd love to just remove that. It's original purpose was to stop function
> tracing when the latency tracers were active. That's now done via a tracefs
> option. The "ftrace_enabled" really has no use anymore.
>
>> ftrace_disabled is much stronger than ftrace_enabled, introduce a new
>> cmdline argument ftrace_disabled for user to control whether to disable
>> ftrace when boot up.
> "ftrace_disabled" is triggered when an anomaly is detected, and for the
> safety of the system, ftrace shuts down. It was never meant to be a user
> triggered event.

Thanks for your explanation.

>
> You have no rationale for this change. What's the purpose of this?

The "System Benchmarks Index Score" of UnixBench under FUNCTION_TRACER
is lower than !FUNCTION_TRACER, I want to use this new cmdline argument
ftrace_disabled to test it, this is the original intention.

I see the following help info of "config FUNCTION_TRACER":

[If it's runtime disabled (the bootup default), then the overhead of the
instructions is very small and not measurable even in micro-benchmarks.]

I am not quite understand the above description, could you tell me how to
avoid the runtime performance overhead under FUNCTION_TRACER?

Thanks,
Tiezhu

>
> -- Steve
>
>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>   kernel/trace/ftrace.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index 72ef4dc..a015699 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -5517,6 +5517,14 @@ static char ftrace_filter_buf[FTRACE_FILTER_SIZE] __initdata;
>>   /* Used by function selftest to not test if filter is set */
>>   bool ftrace_filter_param __initdata;
>>   
>> +static int __init set_ftrace_disabled(char *str)
>> +{
>> +	pr_info("Set ftrace_disabled to disable ftrace\n");
>> +	ftrace_disabled = 1;
>> +	return 1;
>> +}
>> +__setup("ftrace_disabled", set_ftrace_disabled);
>> +
>>   static int __init set_ftrace_notrace(char *str)
>>   {
>>   	ftrace_filter_param = true;

Re: [RFC PATCH 1/2] ftrace: Introduce cmdline argument ftrace_disabled

[Steven Rostedt]

On Mon, 21 Jun 2021 09:21:31 +0800
Tiezhu Yang <yangtiezhu@loongson.cn> wrote:

> >
> > You have no rationale for this change. What's the purpose of this?  
> 
> The "System Benchmarks Index Score" of UnixBench under FUNCTION_TRACER
> is lower than !FUNCTION_TRACER, I want to use this new cmdline argument
> ftrace_disabled to test it, this is the original intention.
> 
> I see the following help info of "config FUNCTION_TRACER":
> 
> [If it's runtime disabled (the bootup default), then the overhead of the
> instructions is very small and not measurable even in micro-benchmarks.]

Those benchmarks were done a long time ago, and they may be measurable today :-/

> 
> I am not quite understand the above description, could you tell me how to
> avoid the runtime performance overhead under FUNCTION_TRACER?

Anyway, your patch wont do anything to change the benchmarks.

When CONFIG_FUNCTION_TRACER is enabled, on x86_64, most functions will
start with a call to fentry. At boot up, these functions will be
converted over to become a nop. And thinking about it, "ftrace_disable"
stops all conversions, so if you add that to the kernel command line,
those calls to fentry, wont be converted to nops, and you'll make
things much worse!

Now, some versions of gcc (and perhaps clang) can do the conversion to
nops at compile time (in which case, your patch would keep the nops and
not the calls to fentry).

The overhead that FUNCTION_TRACER adds is the 5 byte nop at the start
of most functions. This causes a slight hit to instruction cache, and a
minuscule amount of time in the instruction pipeline of the CPU. This
is the "overhead" that is talked about. Your patch doesn't do anything
to address it. The only way to remove that overhead is to compile the
kernel without CONFIG_FUNCTION_TRACER.

-- Steve

Re: [RFC PATCH 1/2] ftrace: Introduce cmdline argument ftrace_disabled

[Tiezhu Yang]

On 06/21/2021 10:00 PM, Steven Rostedt wrote:
> On Mon, 21 Jun 2021 09:21:31 +0800
> Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
>>> You have no rationale for this change. What's the purpose of this?
>> The "System Benchmarks Index Score" of UnixBench under FUNCTION_TRACER
>> is lower than !FUNCTION_TRACER, I want to use this new cmdline argument
>> ftrace_disabled to test it, this is the original intention.
>>
>> I see the following help info of "config FUNCTION_TRACER":
>>
>> [If it's runtime disabled (the bootup default), then the overhead of the
>> instructions is very small and not measurable even in micro-benchmarks.]
> Those benchmarks were done a long time ago, and they may be measurable today :-/
>
>> I am not quite understand the above description, could you tell me how to
>> avoid the runtime performance overhead under FUNCTION_TRACER?
> Anyway, your patch wont do anything to change the benchmarks.
>
> When CONFIG_FUNCTION_TRACER is enabled, on x86_64, most functions will
> start with a call to fentry. At boot up, these functions will be
> converted over to become a nop. And thinking about it, "ftrace_disable"
> stops all conversions, so if you add that to the kernel command line,
> those calls to fentry, wont be converted to nops, and you'll make
> things much worse!
>
> Now, some versions of gcc (and perhaps clang) can do the conversion to
> nops at compile time (in which case, your patch would keep the nops and
> not the calls to fentry).
>
> The overhead that FUNCTION_TRACER adds is the 5 byte nop at the start
> of most functions. This causes a slight hit to instruction cache, and a
> minuscule amount of time in the instruction pipeline of the CPU. This
> is the "overhead" that is talked about. Your patch doesn't do anything
> to address it. The only way to remove that overhead is to compile the
> kernel without CONFIG_FUNCTION_TRACER.
>
> -- Steve

OK, I see, thank you very much.

Thanks,
Tiezhu