linux-hexagon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 00/50] Add log level to show_stack()
@ 2020-03-16 14:38 Dmitry Safonov
  2020-03-16 14:38 ` [PATCHv2 14/50] hexagon: Add show_stack_loglvl() Dmitry Safonov
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Safonov @ 2020-03-16 14:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Juri Lelli, linux-sh, Catalin Marinas, Ben Segall, Guo Ren,
	Pavel Machek, Vincent Guittot, Paul Burton, Dmitry Safonov,
	Michael Ellerman, Geert Uytterhoeven, Mel Gorman, Jiri Slaby,
	Matt Turner, uclinux-h8-devel, Len Brown, linux-pm,
	Heiko Carstens, linux-um, Thomas Gleixner, Dietmar Eggemann,
	Richard Henderson, Greg Kroah-Hartman, Rafael J. Wysocki, Ralf

Changes to v2:
- Removed excessive pr_cont("\n") (nits by Senozhatsky)
- Leave backtrace debugging messages with pr_debug()
  (noted by Russell King and Will Deacon)
- Correct microblaze_unwind_inner() declaration
  (Thanks to Michal Simek and kbuild test robot)
- Fix copy'n'paste typo in show_stack_loglvl() for sparc
  (kbuild robot)
- Fix backtrace output on xtensa
  (Thanks Max Filippov)
- Add loglevel to show_stack() on s390 (kbuild robot)
- Collected all Reviewed-by and Acked-by (thanks!)

Add log level argument to show_stack().
Done in three stages:
1. Introducing show_stack_loglvl() for every architecture
2. Migrating old users with an explicit log level
3. Renaming show_stack_loglvl() into show_stack()

Justification:
o It's a design mistake to move a business-logic decision
  into platform realization detail.
o I have currently two patches sets that would benefit from this work:
  Removing console_loglevel jumps in sysrq driver [1]
  Hung task warning before panic [2] - suggested by Tetsuo (but he
  probably didn't realise what it would involve).
o While doing (1), (2) the backtraces were adjusted to headers
  and other messages for each situation - so there won't be a situation
  when the backtrace is printed, but the headers are missing because
  they have lesser log level (or the reverse).

The least important for upstream, but maybe still worth to note that
every company I've worked in so far had an off-list patch to print
backtrace with the needed log level (but only for the architecture they
cared about).
If you have other ideas how you will benefit from show_stack() with
a log level - please, reply to this cover letter.

See also discussion on v1:
https://lore.kernel.org/linux-riscv/20191106083538.z5nlpuf64cigxigh@pathway.suse.cz/

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Thanks,
Dmitry

[1]: https://lore.kernel.org/lkml/20190528002412.1625-1-dima@arista.com/T/#u
[2]: https://lkml.kernel.org/r/41fd7652-df1f-26f6-aba0-b87ebae07db6@i-love.sakura.ne.jp

Dmitry Safonov (50):
  kallsyms/printk: Add loglvl to print_ip_sym()
  alpha: Add show_stack_loglvl()
  arc: Add show_stack_loglvl()
  arm/asm: Add loglvl to c_backtrace()
  arm: Add loglvl to unwind_backtrace()
  arm: Add loglvl to dump_backtrace()
  arm: Wire up dump_backtrace_{entry,stm}
  arm: Add show_stack_loglvl()
  arm64: Add loglvl to dump_backtrace()
  arm64: Add show_stack_loglvl()
  c6x: Add show_stack_loglvl()
  csky: Add show_stack_loglvl()
  h8300: Add show_stack_loglvl()
  hexagon: Add show_stack_loglvl()
  ia64: Pass log level as arg into ia64_do_show_stack()
  ia64: Add show_stack_loglvl()
  m68k: Add show_stack_loglvl()
  microblaze: Add loglvl to microblaze_unwind_inner()
  microblaze: Add loglvl to microblaze_unwind()
  microblaze: Add show_stack_loglvl()
  mips: Add show_stack_loglvl()
  nds32: Add show_stack_loglvl()
  nios2: Add show_stack_loglvl()
  openrisc: Add show_stack_loglvl()
  parisc: Add show_stack_loglvl()
  powerpc: Add show_stack_loglvl()
  riscv: Add show_stack_loglvl()
  s390: Add show_stack_loglvl()
  sh: Add loglvl to dump_mem()
  sh: Remove needless printk()
  sh: Add loglvl to printk_address()
  sh: Add loglvl to show_trace()
  sh: Add show_stack_loglvl()
  sparc: Add show_stack_loglvl()
  um/sysrq: Remove needless variable sp
  um: Add show_stack_loglvl()
  unicore32: Remove unused pmode argument in c_backtrace()
  unicore32: Add loglvl to c_backtrace()
  unicore32: Add show_stack_loglvl()
  x86: Add missing const qualifiers for log_lvl
  x86: Add show_stack_loglvl()
  xtensa: Add loglvl to show_trace()
  xtensa: Add show_stack_loglvl()
  sysrq: Use show_stack_loglvl()
  x86/amd_gart: Print stacktrace for a leak with KERN_ERR
  power: Use show_stack_loglvl()
  kdb: Don't play with console_loglevel
  sched: Print stack trace with KERN_INFO
  kernel: Use show_stack_loglvl()
  kernel: Rename show_stack_loglvl() => show_stack()

 arch/alpha/kernel/traps.c            | 22 +++++++--------
 arch/arc/include/asm/bug.h           |  3 ++-
 arch/arc/kernel/stacktrace.c         | 17 +++++++-----
 arch/arc/kernel/troubleshoot.c       |  2 +-
 arch/arm/include/asm/bug.h           |  3 ++-
 arch/arm/include/asm/traps.h         |  3 ++-
 arch/arm/include/asm/unwind.h        |  3 ++-
 arch/arm/kernel/traps.c              | 40 ++++++++++++++++------------
 arch/arm/kernel/unwind.c             |  7 ++---
 arch/arm/lib/backtrace-clang.S       |  9 +++++--
 arch/arm/lib/backtrace.S             | 14 +++++++---
 arch/arm64/include/asm/stacktrace.h  |  3 ++-
 arch/arm64/kernel/process.c          |  2 +-
 arch/arm64/kernel/traps.c            | 19 ++++++-------
 arch/c6x/kernel/traps.c              | 18 +++++++------
 arch/csky/kernel/dumpstack.c         |  9 ++++---
 arch/csky/kernel/ptrace.c            |  4 +--
 arch/h8300/kernel/traps.c            | 12 ++++-----
 arch/hexagon/kernel/traps.c          | 25 ++++++++---------
 arch/ia64/include/asm/ptrace.h       |  1 -
 arch/ia64/kernel/mca.c               |  2 +-
 arch/ia64/kernel/process.c           | 17 ++++++------
 arch/m68k/kernel/traps.c             | 13 ++++-----
 arch/microblaze/include/asm/unwind.h |  3 ++-
 arch/microblaze/kernel/stacktrace.c  |  4 +--
 arch/microblaze/kernel/traps.c       | 12 ++++-----
 arch/microblaze/kernel/unwind.c      | 37 ++++++++++++++-----------
 arch/mips/kernel/traps.c             | 35 ++++++++++++------------
 arch/nds32/kernel/traps.c            | 15 ++++++-----
 arch/nios2/kernel/traps.c            | 17 ++++++------
 arch/openrisc/kernel/traps.c         | 12 +++++----
 arch/parisc/kernel/traps.c           | 24 ++++++++---------
 arch/powerpc/kernel/process.c        | 15 ++++++-----
 arch/powerpc/kernel/stacktrace.c     |  2 +-
 arch/riscv/kernel/stacktrace.c       |  9 ++++---
 arch/s390/kernel/dumpstack.c         | 11 ++++----
 arch/sh/include/asm/kdebug.h         |  6 +++--
 arch/sh/include/asm/processor_32.h   |  2 +-
 arch/sh/kernel/dumpstack.c           | 36 ++++++++++++-------------
 arch/sh/kernel/process_32.c          |  2 +-
 arch/sh/kernel/process_64.c          |  3 +--
 arch/sh/kernel/traps.c               |  4 +--
 arch/sh/mm/fault.c                   |  2 +-
 arch/sparc/kernel/process_32.c       | 10 +++----
 arch/sparc/kernel/process_64.c       |  2 +-
 arch/um/drivers/mconsole_kern.c      |  2 +-
 arch/um/kernel/sysrq.c               | 23 ++++++++--------
 arch/unicore32/kernel/setup.h        |  2 +-
 arch/unicore32/kernel/traps.c        | 34 +++++++++++------------
 arch/unicore32/lib/backtrace.S       | 24 +++++++++++------
 arch/x86/include/asm/stacktrace.h    |  2 +-
 arch/x86/kernel/amd_gart_64.c        |  2 +-
 arch/x86/kernel/dumpstack.c          |  9 ++++---
 arch/xtensa/kernel/traps.c           | 26 ++++++++++--------
 drivers/base/power/main.c            |  2 +-
 drivers/tty/sysrq.c                  |  2 +-
 include/linux/kallsyms.h             |  4 +--
 include/linux/sched/debug.h          |  3 ++-
 kernel/debug/kdb/kdb_bt.c            | 11 +++-----
 kernel/locking/lockdep.c             |  4 +--
 kernel/locking/rtmutex-debug.c       |  2 +-
 kernel/sched/core.c                  |  6 ++---
 kernel/trace/ftrace.c                |  8 +++---
 lib/dump_stack.c                     |  2 +-
 64 files changed, 368 insertions(+), 311 deletions(-)

-- 
2.23.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCHv2 14/50] hexagon: Add show_stack_loglvl()
  2020-03-16 14:38 [PATCHv2 00/50] Add log level to show_stack() Dmitry Safonov
@ 2020-03-16 14:38 ` Dmitry Safonov
  2020-03-17 15:34   ` Brian Cain
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Safonov @ 2020-03-16 14:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Dmitry Safonov, Andrew Morton, Greg Kroah-Hartman,
	Ingo Molnar, Jiri Slaby, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Tetsuo Handa, Brian Cain, linux-hexagon

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.

Introduce show_stack_loglvl(), that eventually will substitute
show_stack().

As a good side-effect die() now prints the stacktrace with KERN_EMERG
aligned with other messages.

Cc: Brian Cain <bcain@codeaurora.org>
Cc: linux-hexagon@vger.kernel.org
[1]: https://lore.kernel.org/lkml/20190528002412.1625-1-dima@arista.com/T/#u
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 arch/hexagon/kernel/traps.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/arch/hexagon/kernel/traps.c b/arch/hexagon/kernel/traps.c
index 69c623b14ddd..a8a3a210d781 100644
--- a/arch/hexagon/kernel/traps.c
+++ b/arch/hexagon/kernel/traps.c
@@ -79,7 +79,7 @@ static const char *ex_name(int ex)
 }
 
 static void do_show_stack(struct task_struct *task, unsigned long *fp,
-			  unsigned long ip)
+			  unsigned long ip, const char *loglvl)
 {
 	int kstack_depth_to_print = 24;
 	unsigned long offset, size;
@@ -93,9 +93,8 @@ static void do_show_stack(struct task_struct *task, unsigned long *fp,
 	if (task == NULL)
 		task = current;
 
-	printk(KERN_INFO "CPU#%d, %s/%d, Call Trace:\n",
-	       raw_smp_processor_id(), task->comm,
-	       task_pid_nr(task));
+	printk("%sCPU#%d, %s/%d, Call Trace:\n", loglvl, raw_smp_processor_id(),
+		task->comm, task_pid_nr(task));
 
 	if (fp == NULL) {
 		if (task == current) {
@@ -108,7 +107,7 @@ static void do_show_stack(struct task_struct *task, unsigned long *fp,
 	}
 
 	if ((((unsigned long) fp) & 0x3) || ((unsigned long) fp < 0x1000)) {
-		printk(KERN_INFO "-- Corrupt frame pointer %p\n", fp);
+		printk("%s-- Corrupt frame pointer %p\n", loglvl, fp);
 		return;
 	}
 
@@ -125,8 +124,7 @@ static void do_show_stack(struct task_struct *task, unsigned long *fp,
 
 		name = kallsyms_lookup(ip, &size, &offset, &modname, tmpstr);
 
-		printk(KERN_INFO "[%p] 0x%lx: %s + 0x%lx", fp, ip, name,
-			offset);
+		printk("%s[%p] 0x%lx: %s + 0x%lx", loglvl, fp, ip, name, offset);
 		if (((unsigned long) fp < low) || (high < (unsigned long) fp))
 			printk(KERN_CONT " (FP out of bounds!)");
 		if (modname)
@@ -136,8 +134,7 @@ static void do_show_stack(struct task_struct *task, unsigned long *fp,
 		newfp = (unsigned long *) *fp;
 
 		if (((unsigned long) newfp) & 0x3) {
-			printk(KERN_INFO "-- Corrupt frame pointer %p\n",
-				newfp);
+			printk("%s-- Corrupt frame pointer %p\n", loglvl, newfp);
 			break;
 		}
 
@@ -147,7 +144,7 @@ static void do_show_stack(struct task_struct *task, unsigned long *fp,
 						+ 8);
 
 			if (regs->syscall_nr != -1) {
-				printk(KERN_INFO "-- trap0 -- syscall_nr: %ld",
+				printk("%s-- trap0 -- syscall_nr: %ld", loglvl,
 					regs->syscall_nr);
 				printk(KERN_CONT "  psp: %lx  elr: %lx\n",
 					 pt_psp(regs), pt_elr(regs));
@@ -155,7 +152,7 @@ static void do_show_stack(struct task_struct *task, unsigned long *fp,
 			} else {
 				/* really want to see more ... */
 				kstack_depth_to_print += 6;
-				printk(KERN_INFO "-- %s (0x%lx)  badva: %lx\n",
+				printk("%s-- %s (0x%lx)  badva: %lx\n", loglvl,
 					ex_name(pt_cause(regs)), pt_cause(regs),
 					pt_badva(regs));
 			}
@@ -178,10 +175,16 @@ static void do_show_stack(struct task_struct *task, unsigned long *fp,
 	}
 }
 
-void show_stack(struct task_struct *task, unsigned long *fp)
+void show_stack_loglvl(struct task_struct *task, unsigned long *fp,
+		       const char *loglvl)
 {
 	/* Saved link reg is one word above FP */
-	do_show_stack(task, fp, 0);
+	do_show_stack(task, fp, 0, loglvl);
+}
+
+void show_stack(struct task_struct *task, unsigned long *fp)
+{
+	show_stack_loglvl(task, fp, 0, KERN_INFO);
 }
 
 int die(const char *str, struct pt_regs *regs, long err)
@@ -207,7 +210,7 @@ int die(const char *str, struct pt_regs *regs, long err)
 
 	print_modules();
 	show_regs(regs);
-	do_show_stack(current, &regs->r30, pt_elr(regs));
+	do_show_stack(current, &regs->r30, pt_elr(regs), KERN_EMERG);
 
 	bust_spinlocks(0);
 	add_taint(TAINT_DIE, LOCKDEP_NOW_UNRELIABLE);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [PATCHv2 14/50] hexagon: Add show_stack_loglvl()
  2020-03-16 14:38 ` [PATCHv2 14/50] hexagon: Add show_stack_loglvl() Dmitry Safonov
@ 2020-03-17 15:34   ` Brian Cain
  0 siblings, 0 replies; 3+ messages in thread
From: Brian Cain @ 2020-03-17 15:34 UTC (permalink / raw)
  To: 'Dmitry Safonov', linux-kernel
  Cc: 'Dmitry Safonov', 'Andrew Morton',
	'Greg Kroah-Hartman', 'Ingo Molnar',
	'Jiri Slaby', 'Petr Mladek',
	'Sergey Senozhatsky', 'Steven Rostedt',
	'Tetsuo Handa', linux-hexagon

Acked-by: Brian Cain <bcain@codeaurora.org>

> Cc: Brian Cain <bcain@codeaurora.org>
> Cc: linux-hexagon@vger.kernel.org
> [1]: https://lore.kernel.org/lkml/20190528002412.1625-1-
> dima@arista.com/T/#u
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
>  arch/hexagon/kernel/traps.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/hexagon/kernel/traps.c b/arch/hexagon/kernel/traps.c
> index 69c623b14ddd..a8a3a210d781 100644
> --- a/arch/hexagon/kernel/traps.c
> +++ b/arch/hexagon/kernel/traps.c
> @@ -79,7 +79,7 @@ static const char *ex_name(int ex)  }
> 
>  static void do_show_stack(struct task_struct *task, unsigned long *fp,
> -			  unsigned long ip)
> +			  unsigned long ip, const char *loglvl)
>  {
>  	int kstack_depth_to_print = 24;
>  	unsigned long offset, size;
> @@ -93,9 +93,8 @@ static void do_show_stack(struct task_struct *task,
> unsigned long *fp,
>  	if (task == NULL)
>  		task = current;
> 
> -	printk(KERN_INFO "CPU#%d, %s/%d, Call Trace:\n",
> -	       raw_smp_processor_id(), task->comm,
> -	       task_pid_nr(task));
> +	printk("%sCPU#%d, %s/%d, Call Trace:\n", loglvl,
> raw_smp_processor_id(),
> +		task->comm, task_pid_nr(task));
> 
>  	if (fp == NULL) {
>  		if (task == current) {
> @@ -108,7 +107,7 @@ static void do_show_stack(struct task_struct *task,
> unsigned long *fp,
>  	}
> 
>  	if ((((unsigned long) fp) & 0x3) || ((unsigned long) fp < 0x1000)) {
> -		printk(KERN_INFO "-- Corrupt frame pointer %p\n", fp);
> +		printk("%s-- Corrupt frame pointer %p\n", loglvl, fp);
>  		return;
>  	}
> 
> @@ -125,8 +124,7 @@ static void do_show_stack(struct task_struct *task,
> unsigned long *fp,
> 
>  		name = kallsyms_lookup(ip, &size, &offset, &modname,
> tmpstr);
> 
> -		printk(KERN_INFO "[%p] 0x%lx: %s + 0x%lx", fp, ip, name,
> -			offset);
> +		printk("%s[%p] 0x%lx: %s + 0x%lx", loglvl, fp, ip, name,
> offset);
>  		if (((unsigned long) fp < low) || (high < (unsigned long)
fp))
>  			printk(KERN_CONT " (FP out of bounds!)");
>  		if (modname)
> @@ -136,8 +134,7 @@ static void do_show_stack(struct task_struct *task,
> unsigned long *fp,
>  		newfp = (unsigned long *) *fp;
> 
>  		if (((unsigned long) newfp) & 0x3) {
> -			printk(KERN_INFO "-- Corrupt frame pointer %p\n",
> -				newfp);
> +			printk("%s-- Corrupt frame pointer %p\n", loglvl,
> newfp);
>  			break;
>  		}
> 
> @@ -147,7 +144,7 @@ static void do_show_stack(struct task_struct *task,
> unsigned long *fp,
>  						+ 8);
> 
>  			if (regs->syscall_nr != -1) {
> -				printk(KERN_INFO "-- trap0 -- syscall_nr:
> %ld",
> +				printk("%s-- trap0 -- syscall_nr: %ld",
loglvl,
>  					regs->syscall_nr);
>  				printk(KERN_CONT "  psp: %lx  elr: %lx\n",
>  					 pt_psp(regs), pt_elr(regs));
> @@ -155,7 +152,7 @@ static void do_show_stack(struct task_struct *task,
> unsigned long *fp,
>  			} else {
>  				/* really want to see more ... */
>  				kstack_depth_to_print += 6;
> -				printk(KERN_INFO "-- %s (0x%lx)  badva:
> %lx\n",
> +				printk("%s-- %s (0x%lx)  badva: %lx\n",
loglvl,
>  					ex_name(pt_cause(regs)),
> pt_cause(regs),
>  					pt_badva(regs));
>  			}
> @@ -178,10 +175,16 @@ static void do_show_stack(struct task_struct
> *task, unsigned long *fp,
>  	}
>  }
> 
> -void show_stack(struct task_struct *task, unsigned long *fp)
> +void show_stack_loglvl(struct task_struct *task, unsigned long *fp,
> +		       const char *loglvl)
>  {
>  	/* Saved link reg is one word above FP */
> -	do_show_stack(task, fp, 0);
> +	do_show_stack(task, fp, 0, loglvl);
> +}
> +
> +void show_stack(struct task_struct *task, unsigned long *fp) {
> +	show_stack_loglvl(task, fp, 0, KERN_INFO);
>  }
> 
>  int die(const char *str, struct pt_regs *regs, long err) @@ -207,7 +210,7
> @@ int die(const char *str, struct pt_regs *regs, long err)
> 
>  	print_modules();
>  	show_regs(regs);
> -	do_show_stack(current, &regs->r30, pt_elr(regs));
> +	do_show_stack(current, &regs->r30, pt_elr(regs), KERN_EMERG);
> 
>  	bust_spinlocks(0);
>  	add_taint(TAINT_DIE, LOCKDEP_NOW_UNRELIABLE);
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-03-17 15:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-16 14:38 [PATCHv2 00/50] Add log level to show_stack() Dmitry Safonov
2020-03-16 14:38 ` [PATCHv2 14/50] hexagon: Add show_stack_loglvl() Dmitry Safonov
2020-03-17 15:34   ` Brian Cain

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).