Re: [PATCH 18/50] microblaze: Add loglvl to microblaze_unwind_inner()

From: Michal Simek
Date: Thu Nov 07 2019 - 04:00:06 EST

On 06. 11. 19 4:05, Dmitry Safonov wrote:
> Currently, the log-level of show_stack() depends on a platform
> realization. It creates situations where the headers are printed with
> lower log level or higher than the stacktrace (depending on
> a platform or user).
>
> Furthermore, it forces the logic decision from user to an architecture
> side. In result, some users as sysrq/kdb/etc are doing tricks with
> temporary rising console_loglevel while printing their messages.
> And in result it not only may print unwanted messages from other CPUs,
> but also omit printing at all in the unlucky case where the printk()
> was deferred.
>
> Introducing log-level parameter and KERN_UNSUPPRESSED [1] seems
> an easier approach than introducing more printk buffers.
> Also, it will consolidate printings with headers.
>
> Add log level argument to microblaze_unwind_inner() as a preparation for
> introducing show_stack_loglvl().
>
> Cc: Michal Simek <monstr@xxxxxxxxx>
> [1]: https://lore.kernel.org/lkml/20190528002412.1625-1-dima@xxxxxxxxxx/T/#u
> Signed-off-by: Dmitry Safonov <dima@xxxxxxxxxx>
> ---
> arch/microblaze/kernel/unwind.c | 35 ++++++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/arch/microblaze/kernel/unwind.c b/arch/microblaze/kernel/unwind.c
> index 4241cdd28ee7..9a7343feadf5 100644
> --- a/arch/microblaze/kernel/unwind.c
> +++ b/arch/microblaze/kernel/unwind.c
> @@ -162,16 +162,18 @@ static void microblaze_unwind_inner(struct task_struct *task,
> */
> #ifdef CONFIG_MMU
> static inline void unwind_trap(struct task_struct *task, unsigned long pc,
> - unsigned long fp, struct stack_trace *trace)
> + unsigned long fp, struct stack_trace *trace,
> + const char *loglvl)
> {
> /* To be implemented */
> }
> #else
> static inline void unwind_trap(struct task_struct *task, unsigned long pc,
> - unsigned long fp, struct stack_trace *trace)
> + unsigned long fp, struct stack_trace *trace,
> + const char *loglvl)
> {
> const struct pt_regs *regs = (const struct pt_regs *) fp;
> - microblaze_unwind_inner(task, regs->pc, regs->r1, regs->r15, trace);
> + microblaze_unwind_inner(task, regs->pc, regs->r1, regs->r15, trace, loglvl);
> }
> #endif
>
> @@ -184,11 +186,13 @@ static inline void unwind_trap(struct task_struct *task, unsigned long pc,
> * the caller's return address.
> * @trace : Where to store stack backtrace (PC values).
> * NULL == print backtrace to kernel log
> + * @loglvl : Used for printk log level if (trace == NULL).
> */
> static void microblaze_unwind_inner(struct task_struct *task,
> unsigned long pc, unsigned long fp,
> unsigned long leaf_return,
> - struct stack_trace *trace)
> + struct stack_trace *trace,
> + const char *loglvl)
> {
> int ofs = 0;
>
> @@ -214,11 +218,11 @@ static void microblaze_unwind_inner(struct task_struct *task,
> const struct pt_regs *regs =
> (const struct pt_regs *) fp;
> #endif
> - pr_info("HW EXCEPTION\n");
> + printk("%sHW EXCEPTION\n", loglvl);
> #ifndef CONFIG_MMU
> microblaze_unwind_inner(task, regs->r17 - 4,
> fp + EX_HANDLER_STACK_SIZ,
> - regs->r15, trace);
> + regs->r15, trace, loglvl);
> #endif
> return;
> }
> @@ -228,8 +232,8 @@ static void microblaze_unwind_inner(struct task_struct *task,
> if ((return_to >= handler->start_addr)
> && (return_to <= handler->end_addr)) {
> if (!trace)
> - pr_info("%s\n", handler->trap_name);
> - unwind_trap(task, pc, fp, trace);
> + printk("%s%s\n", loglvl, handler->trap_name);
> + unwind_trap(task, pc, fp, trace, loglvl);
> return;
> }
> }
> @@ -248,13 +252,13 @@ static void microblaze_unwind_inner(struct task_struct *task,
> } else {
> /* Have we reached userland? */
> if (unlikely(pc == task_pt_regs(task)->pc)) {
> - pr_info("[<%p>] PID %lu [%s]\n",
> - (void *) pc,
> + printk("%s[<%p>] PID %lu [%s]\n",
> + loglvl, (void *) pc,
> (unsigned long) task->pid,
> task->comm);
> break;
> } else
> - print_ip_sym(KERN_INFO, pc);
> + print_ip_sym(loglvl, pc);
> }
>
> /* Stop when we reach anything not part of the kernel */
> @@ -285,11 +289,13 @@ static void microblaze_unwind_inner(struct task_struct *task,
> */
> void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
> {
> + const char *loglvl = KERN_INFO;
> +
> if (task) {
> if (task == current) {
> const struct pt_regs *regs = task_pt_regs(task);
> microblaze_unwind_inner(task, regs->pc, regs->r1,
> - regs->r15, trace);
> + regs->r15, trace, loglvl);
> } else {
> struct thread_info *thread_info =
> (struct thread_info *)(task->stack);
> @@ -299,7 +305,8 @@ void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
> microblaze_unwind_inner(task,
> (unsigned long) &_switch_to,
> cpu_context->r1,
> - cpu_context->r15, trace);
> + cpu_context->r15,
> + trace, loglvl);
> }
> } else {
> unsigned long pc, fp;
> @@ -314,7 +321,7 @@ void microblaze_unwind(struct task_struct *task, struct stack_trace *trace)
> );
>
> /* Since we are not a leaf function, use leaf_return = 0 */
> - microblaze_unwind_inner(current, pc, fp, 0, trace);
> + microblaze_unwind_inner(current, pc, fp, 0, trace, loglvl);
> }
> }
>
>

I can't see any reaction from bots but you forget to update function
declaration there which is causing build issue.

here is c&p patch.
Other then this it looks good to me.

diff --git a/arch/microblaze/kernel/unwind.c
b/arch/microblaze/kernel/unwind.c
index 74dded96c10a..778a761af0a7 100644
--- a/arch/microblaze/kernel/unwind.c
+++ b/arch/microblaze/kernel/unwind.c
@@ -154,7 +154,8 @@ static int lookup_prev_stack_frame(unsigned long fp,
unsigned long pc,
static void microblaze_unwind_inner(struct task_struct *task,
unsigned long pc, unsigned long fp,
unsigned long leaf_return,
- struct stack_trace *trace);
+ struct stack_trace *trace,
+ const char *loglvl);

/**
* unwind_trap - Unwind through a system trap, that stored previous state

Thanks,
Michal




--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


Attachment: signature.asc
Description: OpenPGP digital signature