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

[Michal Simek <monstr@monstr.eu>]

[-- Attachment #1.1: Type: text/plain, Size: 6844 bytes --]

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@monstr.eu>
> [1]: https://lore.kernel.org/lkml/20190528002412.1625-1-dima@arista.com/T/#u
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
>  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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]