[PATCH 0/5] dump_stack: allow specifying printk log level

[PATCH 0/5] dump_stack: allow specifying printk log level

[aris]

This patchset lays the foundation work to allow using dump_stack() with a
specified printk log level. Currently each architecture uses a different
log level in show_stack() and it's not possible to control it without
calling directly architecture specific functions.

The motivation behind this work is to limit the amount of kernel messages
printed in the console when a process is killed by the OOM killer. In some
scenarios (lots of containers running different customers' workloads) OOMs
are way more common and don't require the console to be flooded by stack
traces when the OOM killer probably did the right choice. During a recent
discussion it was determined that a knob to control when dump_stack() is
called is a bad idea and instead we should tune the log level in dump_stack()
which prompted this work.

This patchset introduces two new functions:
	dump_stack_lvl(char *log_lvl)
	show_stack_lvl(struct task_struct *task, unsigned long *sp, char *log_lvl)

and both can be reimplemented by each architecture but only the second is
expected. The idea is to initially implement show_stack_lvl() in all
architectures then simply have show_stack() to require log_lvl as parameter.
While that happens, dump_stack() uses can be changed to dump_stack_lvl() and
once everything is in place, dump_stack() will require the log_level as well.

I have a draft patch for every architecture but for this patchset I'm only
including x86 to get some feedback while I try to get a cross compiler working
for each one of them (which is being harder than I thought).

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Greg Thelen <gthelen@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Aristeu Rozanski <aris@redhat.com>

-- 
Aristeu

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/5] dump_stack: pass log level to dump_stack_print_info()

[aris]

[-- Attachment #1: 0001-dump_stack-pass-log-level-to-dump_stack_print_info.patch --]
[-- Type: text/plain, Size: 1754 bytes --]

This is in preparation for dump_stack_lvl() which will allow the log level to
be passed.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Greg Thelen <gthelen@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Aristeu Rozanski <aris@redhat.com>

---
 lib/dump_stack.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

--- linux-2.6.orig/lib/dump_stack.c	2015-11-05 13:56:08.387048346 -0500
+++ linux-2.6/lib/dump_stack.c	2015-11-05 13:56:34.900748897 -0500
@@ -9,9 +9,9 @@
 #include <linux/smp.h>
 #include <linux/atomic.h>
 
-static void __dump_stack(void)
+static void __dump_stack(char *log_lvl)
 {
-	dump_stack_print_info(KERN_DEFAULT);
+	dump_stack_print_info(log_lvl);
 	show_stack(NULL, NULL);
 }
 
@@ -23,7 +23,7 @@ static void __dump_stack(void)
 #ifdef CONFIG_SMP
 static atomic_t dump_lock = ATOMIC_INIT(-1);
 
-asmlinkage __visible void dump_stack(void)
+static void _dump_stack(char *log_lvl)
 {
 	int was_locked;
 	int old;
@@ -47,17 +47,23 @@ was_locked = 0;
 		goto retry;
 	}
 
-	__dump_stack();
+	__dump_stack(log_lvl);
 
 	if (!was_locked)
 		atomic_set(&dump_lock, -1);
 
 	preempt_enable();
 }
+
+asmlinkage __visible void dump_stack(void)
+{
+	_dump_stack(KERN_DEFAULT);
+}
+
 #else
 asmlinkage __visible void dump_stack(void)
 {
-	__dump_stack();
+	__dump_stack(KERN_DEFAULT);
 }
 #endif
 EXPORT_SYMBOL(dump_stack);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/5] dump_stack: introduce dump_stack_lvl

[aris]

[-- Attachment #1: introduce_dump_stack_lvl.patch --]
[-- Type: text/plain, Size: 1736 bytes --]

dump_stack_lvl() allows passing the log level down to the architecture specific
code.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Greg Thelen <gthelen@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Aristeu Rozanski <aris@redhat.com>

---
 include/linux/printk.h |    1 +
 lib/dump_stack.c       |   11 +++++++++++
 2 files changed, 12 insertions(+)

--- linux-2.6.orig/lib/dump_stack.c	2015-11-05 10:56:52.529526114 -0500
+++ linux-2.6/lib/dump_stack.c	2015-11-05 13:16:15.057078858 -0500
@@ -55,6 +55,11 @@ was_locked = 0;
 	preempt_enable();
 }
 
+asmlinkage __visible void dump_stack_lvl(char *log_lvl)
+{
+	_dump_stack(log_lvl);
+}
+
 asmlinkage __visible void dump_stack(void)
 {
 	_dump_stack(KERN_DEFAULT);
@@ -65,5 +70,11 @@ asmlinkage __visible void dump_stack(voi
 {
 	__dump_stack(KERN_DEFAULT);
 }
+
+asmlinkage __visible void dump_stack_lvl(char *log_lvl)
+{
+	__dump_stack(log_lvl);
+}
 #endif
 EXPORT_SYMBOL(dump_stack);
+EXPORT_SYMBOL(dump_stack_lvl);
--- linux-2.6.orig/include/linux/printk.h	2015-11-05 10:56:13.163970713 -0500
+++ linux-2.6/include/linux/printk.h	2015-11-05 13:16:15.057078858 -0500
@@ -231,6 +231,7 @@ static inline void show_regs_print_info(
 #endif
 
 extern asmlinkage void dump_stack(void) __cold;
+extern asmlinkage void dump_stack_lvl(char *log_lvl);
 
 #ifndef pr_fmt
 #define pr_fmt(fmt) fmt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/5] dump_stack: introduce generic show_stack_lvl()

[aris]

[-- Attachment #1: introduce_generic_show_stack.patch --]
[-- Type: text/plain, Size: 1897 bytes --]

show_stack_lvl() works like show_stack() but allows also passing the log level.
The default implementation that should be overrided by architecture specific
code simply will call the existing show_stack().

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Greg Thelen <gthelen@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Aristeu Rozanski <aris@redhat.com>

---
 include/linux/sched.h |    2 ++
 lib/dump_stack.c      |    8 +++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

--- linux-2.6.orig/lib/dump_stack.c	2015-11-05 14:31:44.581921915 -0500
+++ linux-2.6/lib/dump_stack.c	2015-11-05 14:32:47.508211219 -0500
@@ -9,10 +9,16 @@
 #include <linux/smp.h>
 #include <linux/atomic.h>
 
+void __weak show_stack_lvl(struct task_struct *task, unsigned long *sp,
+			       char *log_lvl)
+{
+	return show_stack(task, sp);
+}
+
 static void __dump_stack(char *log_lvl)
 {
 	dump_stack_print_info(log_lvl);
-	show_stack(NULL, NULL);
+	show_stack_lvl(NULL, NULL, log_lvl);
 }
 
 /**
--- linux-2.6.orig/include/linux/sched.h	2015-11-05 14:31:44.581921915 -0500
+++ linux-2.6/include/linux/sched.h	2015-11-05 14:31:52.426833314 -0500
@@ -368,6 +368,8 @@ extern void show_regs(struct pt_regs *);
  * trace (or NULL if the entire call-chain of the task should be shown).
  */
 extern void show_stack(struct task_struct *task, unsigned long *sp);
+extern void show_stack_lvl(struct task_struct *task, unsigned long *sp,
+			   char *log_lvl);
 
 extern void cpu_init (void);
 extern void trap_init(void);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 4/5] x86: dumpstack - implement show_stack_lvl()

[aris]

[-- Attachment #1: arch-x86.patch --]
[-- Type: text/plain, Size: 1556 bytes --]

show_stack_lvl() allows passing the log level and is used by dump_stack_lvl().

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Greg Thelen <gthelen@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Aristeu Rozanski <aris@redhat.com>

---
 arch/x86/kernel/dumpstack.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

--- linux-2.6.orig/arch/x86/kernel/dumpstack.c	2015-11-05 13:33:30.994378877 -0500
+++ linux-2.6/arch/x86/kernel/dumpstack.c	2015-11-05 13:44:37.014856773 -0500
@@ -180,7 +180,7 @@ void show_trace(struct task_struct *task
 	show_trace_log_lvl(task, regs, stack, bp, "");
 }
 
-void show_stack(struct task_struct *task, unsigned long *sp)
+void show_stack_lvl(struct task_struct *task, unsigned long *sp, char *log_lvl)
 {
 	unsigned long bp = 0;
 	unsigned long stack;
@@ -194,7 +194,12 @@ unsigned long bp = 0;
 		bp = stack_frame(current, NULL);
 	}
 
-	show_stack_log_lvl(task, NULL, sp, bp, "");
+	show_stack_log_lvl(task, NULL, sp, bp, log_lvl);
+}
+
+void show_stack(struct task_struct *task, unsigned long *sp)
+{
+	show_stack_lvl(task, sp, KERN_DEFAULT);
 }
 
 static arch_spinlock_t die_lock = __ARCH_SPIN_LOCK_UNLOCKED;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 5/5] mm: use KERN_DEBUG for dump_stack() during an OOM

[aris]

[-- Attachment #1: mm-reduce_priority_in_oom_dump_stack.patch --]
[-- Type: text/plain, Size: 1176 bytes --]

dump_stack() isn't always useful and in some scenarios OOMs can be quite
common and there's no need to flood the console with dump_stack()'s output.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Greg Thelen <gthelen@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Aristeu Rozanski <aris@redhat.com>

---
 mm/oom_kill.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.orig/mm/oom_kill.c	2015-10-27 09:24:01.014413690 -0400
+++ linux-2.6/mm/oom_kill.c	2015-11-05 14:51:31.091521337 -0500
@@ -384,7 +384,7 @@ pr_warning("%s invoked oom-killer: gfp_m
 		current->signal->oom_score_adj);
 	cpuset_print_task_mems_allowed(current);
 	task_unlock(current);
-	dump_stack();
+	dump_stack_lvl(KERN_DEBUG);
 	if (memcg)
 		mem_cgroup_print_oom_info(memcg, p);
 	else

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] dump_stack: allow specifying printk log level

[Michal Hocko]

On Thu 05-11-15 17:30:14, aris@redhat.com wrote:
> This patchset lays the foundation work to allow using dump_stack() with a
> specified printk log level. Currently each architecture uses a different
> log level in show_stack() and it's not possible to control it without
> calling directly architecture specific functions.
> 
> The motivation behind this work is to limit the amount of kernel messages
> printed in the console when a process is killed by the OOM killer. In some
> scenarios (lots of containers running different customers' workloads) OOMs
> are way more common and don't require the console to be flooded by stack
> traces when the OOM killer probably did the right choice. During a recent
> discussion it was determined that a knob to control when dump_stack() is
> called is a bad idea and instead we should tune the log level in dump_stack()
> which prompted this work.
> 
> This patchset introduces two new functions:
> 	dump_stack_lvl(char *log_lvl)
> 	show_stack_lvl(struct task_struct *task, unsigned long *sp, char *log_lvl)
> 
> and both can be reimplemented by each architecture but only the second is
> expected. The idea is to initially implement show_stack_lvl() in all
> architectures then simply have show_stack() to require log_lvl as parameter.
> While that happens, dump_stack() uses can be changed to dump_stack_lvl() and
> once everything is in place, dump_stack() will require the log_level as well.

This looks good to me FWIW.
 
> I have a draft patch for every architecture but for this patchset I'm only
> including x86 to get some feedback while I try to get a cross compiler working
> for each one of them (which is being harder than I thought).
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: David Rientjes <rientjes@google.com>
> Signed-off-by: Aristeu Rozanski <aris@redhat.com>
> 
> -- 
> Aristeu

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] x86: dumpstack - implement show_stack_lvl()

[Ingo Molnar]

* aris@redhat.com <aris@redhat.com> wrote:

> show_stack_lvl() allows passing the log level and is used by dump_stack_lvl().
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: David Rientjes <rientjes@google.com>
> Signed-off-by: Aristeu Rozanski <aris@redhat.com>
> 
> ---
>  arch/x86/kernel/dumpstack.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> --- linux-2.6.orig/arch/x86/kernel/dumpstack.c	2015-11-05 13:33:30.994378877 -0500
> +++ linux-2.6/arch/x86/kernel/dumpstack.c	2015-11-05 13:44:37.014856773 -0500
> @@ -180,7 +180,7 @@ void show_trace(struct task_struct *task
>  	show_trace_log_lvl(task, regs, stack, bp, "");
>  }
>  
> -void show_stack(struct task_struct *task, unsigned long *sp)
> +void show_stack_lvl(struct task_struct *task, unsigned long *sp, char *log_lvl)
>  {
>  	unsigned long bp = 0;
>  	unsigned long stack;
> @@ -194,7 +194,12 @@ unsigned long bp = 0;
>  		bp = stack_frame(current, NULL);
>  	}
>  
> -	show_stack_log_lvl(task, NULL, sp, bp, "");
> +	show_stack_log_lvl(task, NULL, sp, bp, log_lvl);
> +}
> +
> +void show_stack(struct task_struct *task, unsigned long *sp)
> +{
> +	show_stack_lvl(task, sp, KERN_DEFAULT);
>  }
>  
>  static arch_spinlock_t die_lock = __ARCH_SPIN_LOCK_UNLOCKED;

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] mm: use KERN_DEBUG for dump_stack() during an OOM

[Michal Hocko]

[Hmm, this got stuck in my outgoing queue for some reason - sending
again]

On Thu 05-11-15 17:30:19, aris@redhat.com wrote:
> dump_stack() isn't always useful and in some scenarios OOMs can be quite
> common and there's no need to flood the console with dump_stack()'s output.

I think we want to revisit loglevel of other parts of the oom report as
well but this is a good start.

> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: David Rientjes <rientjes@google.com>
> Signed-off-by: Aristeu Rozanski <aris@redhat.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> 
> ---
>  mm/oom_kill.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-2.6.orig/mm/oom_kill.c	2015-10-27 09:24:01.014413690 -0400
> +++ linux-2.6/mm/oom_kill.c	2015-11-05 14:51:31.091521337 -0500
> @@ -384,7 +384,7 @@ pr_warning("%s invoked oom-killer: gfp_m
>  		current->signal->oom_score_adj);
>  	cpuset_print_task_mems_allowed(current);
>  	task_unlock(current);
> -	dump_stack();
> +	dump_stack_lvl(KERN_DEBUG);
>  	if (memcg)
>  		mem_cgroup_print_oom_info(memcg, p);
>  	else

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] dump_stack: allow specifying printk log level

[Andrew Morton]

On Mon, 9 Nov 2015 17:21:25 +0100 Michal Hocko <mhocko@kernel.org> wrote:

> > This patchset introduces two new functions:
> > 	dump_stack_lvl(char *log_lvl)
> > 	show_stack_lvl(struct task_struct *task, unsigned long *sp, char *log_lvl)
> > 
> > and both can be reimplemented by each architecture but only the second is
> > expected. The idea is to initially implement show_stack_lvl() in all
> > architectures then simply have show_stack() to require log_lvl as parameter.
> > While that happens, dump_stack() uses can be changed to dump_stack_lvl() and
> > once everything is in place, dump_stack() will require the log_level as well.
> 
> This looks good to me FWIW.

Seems reasonable to me as well and yes, there will be extra fill-in
work to do.

The "lvl" thing stands out - kernel code doesn't do this arbitrary
vowelicide to make identifiers shorter.  s/lvl/level/g?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] dump_stack: allow specifying printk log level

[Aristeu Rozanski]

On Tue, Dec 01, 2015 at 03:48:20PM -0800, Andrew Morton wrote:
> Seems reasonable to me as well and yes, there will be extra fill-in
> work to do.

I have some of the archs already converted, will wait until this initial
patchset is in to submit.

> The "lvl" thing stands out - kernel code doesn't do this arbitrary
> vowelicide to make identifiers shorter.  s/lvl/level/g?

There are show_regs_log_lvl(), show_trace_log_lvl(),
show_stack_log_lvl() (avr32, x86) and I just wanted to keep the naming
consistent. If you're strong about this I can change it and resubmit.

-- 
Aristeu

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>