Messages in this thread | | | Date | Tue, 12 Nov 2019 10:49:17 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH v2] xtensa: improve stack dumping |
| |
On Thu 2019-11-07 16:44:48, Max Filippov wrote: > Calculate printable stack size and use print_hex_dump instead of > opencoding it. > Make size of stack dump configurable. > Drop extra newline output in show_trace as its output format does not > depend on CONFIG_KALLSYMS.
> diff --git a/arch/xtensa/Kconfig.debug b/arch/xtensa/Kconfig.debug > index 39de98e20018..83cc8d12fa0e 100644 > --- a/arch/xtensa/Kconfig.debug > +++ b/arch/xtensa/Kconfig.debug > @@ -31,3 +31,10 @@ config S32C1I_SELFTEST > It is easy to make wrong hardware configuration, this test should catch it early. > > Say 'N' on stable hardware. > + > +config PRINT_STACK_DEPTH > + int "Stack depth to print" if DEBUG_KERNEL > + default 64 > + help > + This option allows you to set the stack depth that the kernel > + prints in stack traces.
I would split this into separate patch.
> diff --git a/arch/xtensa/kernel/traps.c b/arch/xtensa/kernel/traps.c > index 4a6c495ce9b6..fe090ab1cab8 100644 > --- a/arch/xtensa/kernel/traps.c > +++ b/arch/xtensa/kernel/traps.c > @@ -491,32 +491,24 @@ void show_trace(struct task_struct *task, unsigned long *sp) > > pr_info("Call Trace:\n"); > walk_stackframe(sp, show_trace_cb, NULL); > -#ifndef CONFIG_KALLSYMS > - pr_cont("\n"); > -#endif > } > > -static int kstack_depth_to_print = 24; > +static int kstack_depth_to_print = CONFIG_PRINT_STACK_DEPTH; > > void show_stack(struct task_struct *task, unsigned long *sp) > { > - int i = 0; > - unsigned long *stack; > + size_t len; > > if (!sp) > sp = stack_pointer(task); > - stack = sp; > > - pr_info("Stack:\n"); > + len = min((-(unsigned long)sp) & (THREAD_SIZE - 4), > + kstack_depth_to_print * 4ul);
I would replace the hardcoded 4 with sizeof(void *).
> > - for (i = 0; i < kstack_depth_to_print; i++) { > - if (kstack_end(sp)) > - break; > - pr_cont(" %08lx", *sp++); > - if (i % 8 == 7) > - pr_cont("\n"); > - } > - show_trace(task, stack); > + pr_info("Stack:\n"); > + print_hex_dump(KERN_INFO, " ", DUMP_PREFIX_NONE, 32, 4, > + sp, len, false); > + show_trace(task, sp); > }
The conversion looks fine to me. It is up to you (as a maintainer) what you do with the above proposal for two cosmetic changes ;-) Either way, feel free to use:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards, Petr
| |