Messages in this thread | | | Date | Wed, 31 Jul 2019 06:57:55 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH] Function stack size and its name mismatch in arm64 |
| |
On Wed, 31 Jul 2019 17:04:37 +0800 Jiping Ma <jiping.ma2@windriver.com> wrote:
Hi Jiping,
Note, the subject is not properly written, as it is missing the subsystem. In this case, it should start with "tracing: "
> The PC of one the frame is matched to the next frame function, rather > than the function of his frame.
The above change log doesn't make sense. I have no idea what the actual problem is here. Why is this different for arm64 and no one else? Seems the bug is with the stack logic code in arm64 not here.
> > 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
We do not allow arch specific defines in generic code. Otherwise this would blow up and become unmaintainable. Not to mention it makes the code ugly and hard to follow.
Please explain the problem better. I'm sure there's much better ways to solve this than this patch.
Thanks,
-- Steve
> + 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; > }
| |