* [PATCH v2] tracing: Function stack size and its name mismatch in arm64
@ 2019-08-01 8:33 Jiping Ma
2019-08-01 9:41 ` Will Deacon
2019-08-01 13:15 ` Steven Rostedt
0 siblings, 2 replies; 5+ messages in thread
From: Jiping Ma @ 2019-08-01 8:33 UTC (permalink / raw)
To: rostedt, mingo, catalin.marinas, will.deacon, joel
Cc: linux-kernel, linux-arm-kernel, jiping.ma2
In arm64, the PC of the frame is matched to the last frame function,
rather than the function of his frame. For the following example, the
stack size of occupy_stack_init function should be 3376, rather than 176.
Wrong info:
Depth Size Location (16 entries)
----- ---- --------
0) 5400 16 __update_load_avg_se.isra.2+0x28/0x220
1) 5384 96 put_prev_entity+0x250/0x338
2) 5288 80 pick_next_task_fair+0x4c4/0x508
3) 5208 72 __schedule+0x100/0x600
4) 5136 184 preempt_schedule_common+0x28/0x48
5) 4952 32 preempt_schedule+0x28/0x30
6) 4920 16 vprintk_emit+0x170/0x1f8
7) 4904 128 vprintk_default+0x48/0x58
8) 4776 64 vprintk_func+0xf8/0x1c8
9) 4712 112 printk+0x70/0x90
10) 4600 176 occupy_stack_init+0x64/0xc0 [kernel_stack]
11) 4424 3376 do_one_initcall+0x68/0x248
12) 1048 144 do_init_module+0x60/0x1f0
13) 904 48 load_module+0x1d50/0x2340
14) 856 352 sys_finit_module+0xd0/0xe8
15) 504 504 el0_svc_naked+0x30/0x34
Correct info:
Depth Size Location (18 entries)
----- ---- --------
0) 5464 48 cgroup_rstat_updated+0x20/0x100
1) 5416 32 cgroup_base_stat_cputime_account_end.isra.0+0x30/0x60
2) 5384 32 __cgroup_account_cputime+0x3c/0x48
3) 5352 64 update_curr+0xc4/0x1d0
4) 5288 72 pick_next_task_fair+0x444/0x508
5) 5216 184 __schedule+0x100/0x600
6) 5032 32 preempt_schedule_common+0x28/0x48
7) 5000 16 preempt_schedule+0x28/0x30
8) 4984 128 vprintk_emit+0x170/0x1f8
9) 4856 64 vprintk_default+0x48/0x58
10) 4792 112 vprintk_func+0xf8/0x1c8
11) 4680 176 printk+0x70/0x90
12) 4504 80 func_test+0x7c/0xb8 [kernel_stack]
13) 4424 3376 occupy_stack_init+0x7c/0xc0 [kernel_stack]
14) 1048 144 do_one_initcall+0x68/0x248
15) 904 48 do_init_module+0x60/0x1f0
16) 856 352 load_module+0x1d50/0x2340
17) 504 504 sys_finit_module+0xd0/0xe8
Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
---
kernel/trace/trace_stack.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index 5d16f73898db..ed80b95abf06 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -40,16 +40,28 @@ static void print_max_stack(void)
pr_emerg(" Depth Size Location (%d entries)\n"
" ----- ---- --------\n",
+#ifdef CONFIG_ARM64
+ stack_trace_nr_entries - 1);
+#else
stack_trace_nr_entries);
-
+#endif
+#ifdef CONFIG_ARM64
+ for (i = 1; i < stack_trace_nr_entries; i++) {
+#else
for (i = 0; i < stack_trace_nr_entries; i++) {
+#endif
if (i + 1 == stack_trace_nr_entries)
size = stack_trace_index[i];
else
size = stack_trace_index[i] - stack_trace_index[i+1];
+#ifdef CONFIG_ARM64
+ pr_emerg("%3ld) %8d %5d %pS\n", i-1, stack_trace_index[i],
+ size, (void *)stack_dump_trace[i-1]);
+#else
pr_emerg("%3ld) %8d %5d %pS\n", i, stack_trace_index[i],
size, (void *)stack_dump_trace[i]);
+#endif
}
}
@@ -324,8 +336,11 @@ static int t_show(struct seq_file *m, void *v)
seq_printf(m, " Depth Size Location"
" (%d entries)\n"
" ----- ---- --------\n",
+#ifdef CONFIG_ARM64
+ stack_trace_nr_entries - 1);
+#else
stack_trace_nr_entries);
-
+#endif
if (!stack_tracer_enabled && !stack_trace_max_size)
print_disabled(m);
@@ -334,6 +349,10 @@ static int t_show(struct seq_file *m, void *v)
i = *(long *)v;
+#ifdef CONFIG_ARM64
+ if (i == 0)
+ return 0;
+#endif
if (i >= stack_trace_nr_entries)
return 0;
@@ -342,9 +361,14 @@ static int t_show(struct seq_file *m, void *v)
else
size = stack_trace_index[i] - stack_trace_index[i+1];
+#ifdef CONFIG_ARM64
+ seq_printf(m, "%3ld) %8d %5d ", i-1, stack_trace_index[i], size);
+ trace_lookup_stack(m, i-1);
+#else
seq_printf(m, "%3ld) %8d %5d ", i, stack_trace_index[i], size);
trace_lookup_stack(m, i);
+#endif
return 0;
}
--
2.18.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] tracing: Function stack size and its name mismatch in arm64
2019-08-01 8:33 [PATCH v2] tracing: Function stack size and its name mismatch in arm64 Jiping Ma
@ 2019-08-01 9:41 ` Will Deacon
2019-08-02 2:43 ` Jiping Ma
2019-08-01 13:15 ` Steven Rostedt
1 sibling, 1 reply; 5+ messages in thread
From: Will Deacon @ 2019-08-01 9:41 UTC (permalink / raw)
To: Jiping Ma
Cc: rostedt, mingo, catalin.marinas, will.deacon, joel, linux-kernel,
linux-arm-kernel
On Thu, Aug 01, 2019 at 04:33:40PM +0800, Jiping Ma wrote:
> In arm64, the PC of the frame is matched to the last frame function,
> rather than the function of his frame. For the following example, the
> stack size of occupy_stack_init function should be 3376, rather than 176.
>
> Wrong info:
> Depth Size Location (16 entries)
> ----- ---- --------
> 0) 5400 16 __update_load_avg_se.isra.2+0x28/0x220
> 1) 5384 96 put_prev_entity+0x250/0x338
> 2) 5288 80 pick_next_task_fair+0x4c4/0x508
> 3) 5208 72 __schedule+0x100/0x600
> 4) 5136 184 preempt_schedule_common+0x28/0x48
> 5) 4952 32 preempt_schedule+0x28/0x30
> 6) 4920 16 vprintk_emit+0x170/0x1f8
> 7) 4904 128 vprintk_default+0x48/0x58
> 8) 4776 64 vprintk_func+0xf8/0x1c8
> 9) 4712 112 printk+0x70/0x90
> 10) 4600 176 occupy_stack_init+0x64/0xc0 [kernel_stack]
> 11) 4424 3376 do_one_initcall+0x68/0x248
> 12) 1048 144 do_init_module+0x60/0x1f0
> 13) 904 48 load_module+0x1d50/0x2340
> 14) 856 352 sys_finit_module+0xd0/0xe8
> 15) 504 504 el0_svc_naked+0x30/0x34
>
> Correct info:
> Depth Size Location (18 entries)
> ----- ---- --------
> 0) 5464 48 cgroup_rstat_updated+0x20/0x100
> 1) 5416 32 cgroup_base_stat_cputime_account_end.isra.0+0x30/0x60
> 2) 5384 32 __cgroup_account_cputime+0x3c/0x48
> 3) 5352 64 update_curr+0xc4/0x1d0
> 4) 5288 72 pick_next_task_fair+0x444/0x508
> 5) 5216 184 __schedule+0x100/0x600
> 6) 5032 32 preempt_schedule_common+0x28/0x48
> 7) 5000 16 preempt_schedule+0x28/0x30
> 8) 4984 128 vprintk_emit+0x170/0x1f8
> 9) 4856 64 vprintk_default+0x48/0x58
> 10) 4792 112 vprintk_func+0xf8/0x1c8
> 11) 4680 176 printk+0x70/0x90
> 12) 4504 80 func_test+0x7c/0xb8 [kernel_stack]
> 13) 4424 3376 occupy_stack_init+0x7c/0xc0 [kernel_stack]
> 14) 1048 144 do_one_initcall+0x68/0x248
> 15) 904 48 do_init_module+0x60/0x1f0
> 16) 856 352 load_module+0x1d50/0x2340
> 17) 504 504 sys_finit_module+0xd0/0xe8
>
> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
> ---
> kernel/trace/trace_stack.c | 28 ++++++++++++++++++++++++++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> index 5d16f73898db..ed80b95abf06 100644
> --- a/kernel/trace/trace_stack.c
> +++ b/kernel/trace/trace_stack.c
> @@ -40,16 +40,28 @@ static void print_max_stack(void)
>
> pr_emerg(" Depth Size Location (%d entries)\n"
> " ----- ---- --------\n",
> +#ifdef CONFIG_ARM64
> + stack_trace_nr_entries - 1);
> +#else
> stack_trace_nr_entries);
Sorry, but I have no idea what the problem is here. All I know is that the
solution looks highly dubious and I find it very hard to believe that the
arm64 backtracing code is uniquely special as to deserve being called out
like this. I suspect there's a bug lurking somewhere, but you really need
to do a better job of explaining the issue rather than simply providing a
couple of backtraces with no context.
*Why* does the frame appear to be off-by-one?
Will
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] tracing: Function stack size and its name mismatch in arm64
2019-08-01 9:41 ` Will Deacon
@ 2019-08-02 2:43 ` Jiping Ma
2019-08-02 13:03 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Jiping Ma @ 2019-08-02 2:43 UTC (permalink / raw)
To: Will Deacon
Cc: rostedt, mingo, catalin.marinas, will.deacon, joel, linux-kernel,
linux-arm-kernel
On 2019年08月01日 17:41, Will Deacon wrote:
> On Thu, Aug 01, 2019 at 04:33:40PM +0800, Jiping Ma wrote:
>> In arm64, the PC of the frame is matched to the last frame function,
>> rather than the function of his frame. For the following example, the
>> stack size of occupy_stack_init function should be 3376, rather than 176.
>>
>> Wrong info:
>> Depth Size Location (16 entries)
>> ----- ---- --------
>> 0) 5400 16 __update_load_avg_se.isra.2+0x28/0x220
>> 1) 5384 96 put_prev_entity+0x250/0x338
>> 2) 5288 80 pick_next_task_fair+0x4c4/0x508
>> 3) 5208 72 __schedule+0x100/0x600
>> 4) 5136 184 preempt_schedule_common+0x28/0x48
>> 5) 4952 32 preempt_schedule+0x28/0x30
>> 6) 4920 16 vprintk_emit+0x170/0x1f8
>> 7) 4904 128 vprintk_default+0x48/0x58
>> 8) 4776 64 vprintk_func+0xf8/0x1c8
>> 9) 4712 112 printk+0x70/0x90
>> 10) 4600 176 occupy_stack_init+0x64/0xc0 [kernel_stack]
>> 11) 4424 3376 do_one_initcall+0x68/0x248
>> 12) 1048 144 do_init_module+0x60/0x1f0
>> 13) 904 48 load_module+0x1d50/0x2340
>> 14) 856 352 sys_finit_module+0xd0/0xe8
>> 15) 504 504 el0_svc_naked+0x30/0x34
>>
>> Correct info:
>> Depth Size Location (18 entries)
>> ----- ---- --------
>> 0) 5464 48 cgroup_rstat_updated+0x20/0x100
>> 1) 5416 32 cgroup_base_stat_cputime_account_end.isra.0+0x30/0x60
>> 2) 5384 32 __cgroup_account_cputime+0x3c/0x48
>> 3) 5352 64 update_curr+0xc4/0x1d0
>> 4) 5288 72 pick_next_task_fair+0x444/0x508
>> 5) 5216 184 __schedule+0x100/0x600
>> 6) 5032 32 preempt_schedule_common+0x28/0x48
>> 7) 5000 16 preempt_schedule+0x28/0x30
>> 8) 4984 128 vprintk_emit+0x170/0x1f8
>> 9) 4856 64 vprintk_default+0x48/0x58
>> 10) 4792 112 vprintk_func+0xf8/0x1c8
>> 11) 4680 176 printk+0x70/0x90
>> 12) 4504 80 func_test+0x7c/0xb8 [kernel_stack]
>> 13) 4424 3376 occupy_stack_init+0x7c/0xc0 [kernel_stack]
>> 14) 1048 144 do_one_initcall+0x68/0x248
>> 15) 904 48 do_init_module+0x60/0x1f0
>> 16) 856 352 load_module+0x1d50/0x2340
>> 17) 504 504 sys_finit_module+0xd0/0xe8
>>
>> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
>> ---
>> kernel/trace/trace_stack.c | 28 ++++++++++++++++++++++++++--
>> 1 file changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
>> index 5d16f73898db..ed80b95abf06 100644
>> --- a/kernel/trace/trace_stack.c
>> +++ b/kernel/trace/trace_stack.c
>> @@ -40,16 +40,28 @@ static void print_max_stack(void)
>>
>> pr_emerg(" Depth Size Location (%d entries)\n"
>> " ----- ---- --------\n",
>> +#ifdef CONFIG_ARM64
>> + stack_trace_nr_entries - 1);
>> +#else
>> stack_trace_nr_entries);
> Sorry, but I have no idea what the problem is here. All I know is that the
> solution looks highly dubious and I find it very hard to believe that the
> arm64 backtracing code is uniquely special as to deserve being called out
> like this. I suspect there's a bug lurking somewhere, but you really need
> to do a better job of explaining the issue rather than simply providing a
> couple of backtraces with no context.
>
> *Why* does the frame appear to be off-by-one?
Because the PC is LR in ARM64 stack actually. Following is ARM64 stack
layout. Please refer to the figure 3 in
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
LR
FP
......
LR
FP
Jiping
>
> Will
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] tracing: Function stack size and its name mismatch in arm64
2019-08-01 8:33 [PATCH v2] tracing: Function stack size and its name mismatch in arm64 Jiping Ma
2019-08-01 9:41 ` Will Deacon
@ 2019-08-01 13:15 ` Steven Rostedt
1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2019-08-01 13:15 UTC (permalink / raw)
To: Jiping Ma
Cc: mingo, catalin.marinas, will.deacon, joel, linux-kernel,
linux-arm-kernel
On Thu, 1 Aug 2019 16:33:40 +0800
Jiping Ma <jiping.ma2@windriver.com> wrote:
> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> index 5d16f73898db..ed80b95abf06 100644
> --- a/kernel/trace/trace_stack.c
> +++ b/kernel/trace/trace_stack.c
> @@ -40,16 +40,28 @@ static void print_max_stack(void)
>
> pr_emerg(" Depth Size Location (%d entries)\n"
> " ----- ---- --------\n",
> +#ifdef CONFIG_ARM64
> + stack_trace_nr_entries - 1);
> +#else
> stack_trace_nr_entries);
> -
NACK! I do not allow arch specific code in generic code like this.
The stack saving should be the same across all archs. If it is not, it
is a bug with the arch that is different.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-02 13:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 8:33 [PATCH v2] tracing: Function stack size and its name mismatch in arm64 Jiping Ma
2019-08-01 9:41 ` Will Deacon
2019-08-02 2:43 ` Jiping Ma
2019-08-02 13:03 ` Steven Rostedt
2019-08-01 13:15 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).