diff mbox series

ftrace: Fix function name for trampoline

Message ID 20241010130300.2083-1-tatsuya.s2862@gmail.com (mailing list archive)
State Superseded
Headers show
Series ftrace: Fix function name for trampoline | expand

Commit Message

Tatsuya S Oct. 10, 2024, 1:02 p.m. UTC
The issue that unrelated function name is shown on stack trace like
following even though it should be trampoline code address is caused by
the creation of trampoline code in the area where .init.text section
of module was freed after module is loaded.

bash-1344    [002] .....    43.644608: <stack trace>
  => (MODULE INIT FUNCTION)
  => vfs_write
  => ksys_write
  => do_syscall_64
  => entry_SYSCALL_64_after_hwframe

To resolve this, when function address of stack trace entry is in
trampoline, output without looking up symbol name.

Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
---
 kernel/trace/trace_output.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Steven Rostedt Oct. 10, 2024, 3:02 p.m. UTC | #1
On Thu, 10 Oct 2024 22:02:59 +0900
Tatsuya S <tatsuya.s2862@gmail.com> wrote:

> The issue that unrelated function name is shown on stack trace like
> following even though it should be trampoline code address is caused by
> the creation of trampoline code in the area where .init.text section
> of module was freed after module is loaded.
> 
> bash-1344    [002] .....    43.644608: <stack trace>
>   => (MODULE INIT FUNCTION)
>   => vfs_write
>   => ksys_write
>   => do_syscall_64
>   => entry_SYSCALL_64_after_hwframe  
> 
> To resolve this, when function address of stack trace entry is in
> trampoline, output without looking up symbol name.
> 
> Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
> ---
>  kernel/trace/trace_output.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 868f2f912f28..32a0858373e2 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
>  			break;
>  
>  		trace_seq_puts(s, " => ");
> +		if (is_ftrace_trampoline((*p) + delta)) {

This is not reliable. The output is called when the user reads the trace
file and the ops may no longer exist.

The only way to test this is if you call it during the trace. Yes it may
slow things down a little, but it will be accurate.

-- Steve


> +			trace_seq_printf(s, "0x%08lx", (*p) + delta);
> +			trace_seq_putc(s, '\n');
> +			continue;
> +		}
>  		seq_print_ip_sym(s, (*p) + delta, flags);
>  		trace_seq_putc(s, '\n');
>  	}
Masami Hiramatsu (Google) Oct. 10, 2024, 3:03 p.m. UTC | #2
On Thu, 10 Oct 2024 22:02:59 +0900
Tatsuya S <tatsuya.s2862@gmail.com> wrote:

> The issue that unrelated function name is shown on stack trace like
> following even though it should be trampoline code address is caused by
> the creation of trampoline code in the area where .init.text section
> of module was freed after module is loaded.
> 
> bash-1344    [002] .....    43.644608: <stack trace>
>   => (MODULE INIT FUNCTION)
>   => vfs_write
>   => ksys_write
>   => do_syscall_64
>   => entry_SYSCALL_64_after_hwframe
> 
> To resolve this, when function address of stack trace entry is in
> trampoline, output without looking up symbol name.
> 
> Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
> ---
>  kernel/trace/trace_output.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 868f2f912f28..32a0858373e2 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
>  			break;
>  
>  		trace_seq_puts(s, " => ");
> +		if (is_ftrace_trampoline((*p) + delta)) {
> +			trace_seq_printf(s, "0x%08lx", (*p) + delta);

If we know that address is the ftrace trampoline, we'd better show something
like "[FTRACE TRAMPOLINE]"

> +			trace_seq_putc(s, '\n');

And this is not needed. So for example,

			trace_seq_puts(s, "[FTRACE TRAMPOLINE]\n");

is enough.

> +			continue;
> +		}
>  		seq_print_ip_sym(s, (*p) + delta, flags);
>  		trace_seq_putc(s, '\n');
>  	}

Thank you,

> -- 
> 2.46.2
>
Tatsuya S Oct. 11, 2024, 6:34 a.m. UTC | #3
On 10/11/24 12:03 AM, Masami Hiramatsu (Google) wrote:
> On Thu, 10 Oct 2024 22:02:59 +0900
> Tatsuya S <tatsuya.s2862@gmail.com> wrote:
> 
>> The issue that unrelated function name is shown on stack trace like
>> following even though it should be trampoline code address is caused by
>> the creation of trampoline code in the area where .init.text section
>> of module was freed after module is loaded.
>>
>> bash-1344    [002] .....    43.644608: <stack trace>
>>    => (MODULE INIT FUNCTION)
>>    => vfs_write
>>    => ksys_write
>>    => do_syscall_64
>>    => entry_SYSCALL_64_after_hwframe
>>
>> To resolve this, when function address of stack trace entry is in
>> trampoline, output without looking up symbol name.
>>
>> Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
>> ---
>>   kernel/trace/trace_output.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
>> index 868f2f912f28..32a0858373e2 100644
>> --- a/kernel/trace/trace_output.c
>> +++ b/kernel/trace/trace_output.c
>> @@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
>>   			break;
>>   
>>   		trace_seq_puts(s, " => ");
>> +		if (is_ftrace_trampoline((*p) + delta)) {
>> +			trace_seq_printf(s, "0x%08lx", (*p) + delta);
> 
> If we know that address is the ftrace trampoline, we'd better show something
> like "[FTRACE TRAMPOLINE]"
> 
>> +			trace_seq_putc(s, '\n');
> 
> And this is not needed. So for example,
> 
> 			trace_seq_puts(s, "[FTRACE TRAMPOLINE]\n");
> 
> is enough.

Thanks for review. OK.

> 
>> +			continue;
>> +		}
>>   		seq_print_ip_sym(s, (*p) + delta, flags);
>>   		trace_seq_putc(s, '\n');
>>   	}
> 
> Thank you,
> 
>> -- 
>> 2.46.2
>>
> 
>
Tatsuya S Oct. 11, 2024, 7:34 a.m. UTC | #4
On 10/11/24 12:02 AM, Steven Rostedt wrote:
> On Thu, 10 Oct 2024 22:02:59 +0900
> Tatsuya S <tatsuya.s2862@gmail.com> wrote:
> 
>> The issue that unrelated function name is shown on stack trace like
>> following even though it should be trampoline code address is caused by
>> the creation of trampoline code in the area where .init.text section
>> of module was freed after module is loaded.
>>
>> bash-1344    [002] .....    43.644608: <stack trace>
>>    => (MODULE INIT FUNCTION)
>>    => vfs_write
>>    => ksys_write
>>    => do_syscall_64
>>    => entry_SYSCALL_64_after_hwframe
>>
>> To resolve this, when function address of stack trace entry is in
>> trampoline, output without looking up symbol name.
>>
>> Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
>> ---
>>   kernel/trace/trace_output.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
>> index 868f2f912f28..32a0858373e2 100644
>> --- a/kernel/trace/trace_output.c
>> +++ b/kernel/trace/trace_output.c
>> @@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
>>   			break;
>>   
>>   		trace_seq_puts(s, " => ");
>> +		if (is_ftrace_trampoline((*p) + delta)) {

Thank you for review.

> 
> This is not reliable. The output is called when the user reads the trace
> file and the ops may no longer exist.
> 
> The only way to test this is if you call it during the trace. Yes it may
> slow things down a little, but it will be accurate.

OK, I will do it.

> 
> -- Steve
> 
> 
>> +			trace_seq_printf(s, "0x%08lx", (*p) + delta);
>> +			trace_seq_putc(s, '\n');
>> +			continue;
>> +		}
>>   		seq_print_ip_sym(s, (*p) + delta, flags);
>>   		trace_seq_putc(s, '\n');
>>   	}
> 

Thanks
diff mbox series

Patch

diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 868f2f912f28..32a0858373e2 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1246,6 +1246,11 @@  static enum print_line_t trace_stack_print(struct trace_iterator *iter,
 			break;
 
 		trace_seq_puts(s, " => ");
+		if (is_ftrace_trampoline((*p) + delta)) {
+			trace_seq_printf(s, "0x%08lx", (*p) + delta);
+			trace_seq_putc(s, '\n');
+			continue;
+		}
 		seq_print_ip_sym(s, (*p) + delta, flags);
 		trace_seq_putc(s, '\n');
 	}