enable dumpstack() to printk log level

PATCH In review he, bo 3 participants 7 messages Mar 23, 2012 – Apr 6, 2012 1 message not in the archive

Subsystems (5)

x86 Library code Scheduler Printk +1 more subsystems

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): 1332493027.2359.5.camel@hebo

he, bo

author

2012-03-23 09:01 UTC #1 lore raw

PATCH 2/2 enable dumpstack() to printk log level

From: he bo <[email protected]>

Function dump_stack calls printk with default log level.
At some scenarios, we need dump the logs at special log level. For example, Android aplog saves log at a low level to reduce log storage. With __might_sleep checking, it just prints out the warning source line with KERNE_ERR and Android saves it to aplog, but later dump_stack log is not saved. We couldn’t see the important information when developers check it.
the patch adds function dump_stack_log_lvl to support this capability.

Signed-off-by he, bo <[email protected]>

Reviewed-by zhang, yanmin <[email protected]>

---
 arch/x86/kernel/dumpstack.c |   19 ++++++++++++++-----
 include/linux/printk.h      |    1 +
 kernel/sched/core.c         |    2 +-
 lib/dump_stack.c            |    7 ++++++-
 4 files changed, 22 insertions(+), 7 deletions(-)

arch/x86/kernel/dumpstack.c +14 -5

index 4025fe4..7b2160c 100644
@@ -177,20 +177,29 @@ void show_stack(struct task_struct *task, unsigned long *sp)
 }
 
 /*
- * The architecture-independent dump_stack generator
+ * The architecture-independent dump_stack_log_lvl generator
  */
-void dump_stack(void)
+void dump_stack_log_lvl(char *log_lvl)
 {
 	unsigned long bp;
 	unsigned long stack;
 
 	bp = stack_frame(current, NULL);
-	printk("Pid: %d, comm: %.20s %s %s %.*s\n",
-		current->pid, current->comm, print_tainted(),
+	printk("%sPid: %d, comm: %.20s %s %s %.*s\n",
+		log_lvl, current->pid, current->comm, print_tainted(),
 		init_utsname()->release,
 		(int)strcspn(init_utsname()->version, " "),
 		init_utsname()->version);
-	show_trace(NULL, NULL, &stack, bp);
+	show_trace_log_lvl(NULL, NULL, &stack, bp, log_lvl);
+}
+EXPORT_SYMBOL(dump_stack_log_lvl);
+
+/*
+ * The architecture-independent dump_stack generator
+ */
+void dump_stack(void)
+{
+	dump_stack_log_lvl("");
 }
 EXPORT_SYMBOL(dump_stack);

include/linux/printk.h +1 -0

index 0525927..2a13dd8 100644
@@ -157,6 +157,7 @@ static inline void setup_log_buf(int early)
 #endif
 
 extern void dump_stack(void) __cold;
+extern void dump_stack_log_lvl(char *log_lvl) __cold;
 
 #ifndef pr_fmt
 #define pr_fmt(fmt) fmt

kernel/sched/core.c +1 -1

index 503d642..ab0dcdf 100644
@@ -7060,7 +7060,7 @@ void __might_sleep(const char *file, int line, int preempt_offset)
 	debug_show_held_locks(current);
 	if (irqs_disabled())
 		print_irqtrace_events(current);
-	dump_stack();
+	dump_stack_log_lvl(KERN_ERR);
 }
 EXPORT_SYMBOL(__might_sleep);
 #endif

lib/dump_stack.c +6 -1

index 53bff4c..d44ca5e 100644
@@ -11,5 +11,10 @@ void dump_stack(void)
 	printk(KERN_NOTICE
 		"This architecture does not implement dump_stack()\n");
 }
-
 EXPORT_SYMBOL(dump_stack);
+
+void dump_stack_log_lvl(char *log_lvl)
+{
+	dump_stack();
+}
+EXPORT_SYMBOL(dump_stack_log_lvl);

Signature · 1 line

1.7.1

Ingo Molnar

2012-03-23 12:22 UTC #2 lore raw

arch/x86/kernel/dumpstack.c void show_stack(struct task_struct *task, unsigned long *sp) · line 180

Quoted 8 lines · he, bo

arch/x86/kernel/dumpstack.c +1 -1

@@ -177,20 +177,29 @@ void show_stack(struct task_struct *task, unsigned long *sp)
 }
 
 /*
- * The architecture-independent dump_stack generator
+ * The architecture-independent dump_stack_log_lvl generator

I guess it wants to say "architecture-dependent"?

arch/x86/kernel/dumpstack.c void show_stack(struct task_struct *task, unsigned long *sp) · line 195

Quoted 3 lines

arch/x86/kernel/dumpstack.c +3 -0

+	show_trace_log_lvl(NULL, NULL, &stack, bp, log_lvl);
+}
+EXPORT_SYMBOL(dump_stack_log_lvl);

this should be a _GPL export.

arch/x86/kernel/dumpstack.c void show_stack(struct task_struct *task, unsigned long *sp) · line 185

Quoted 2 lines

arch/x86/kernel/dumpstack.c +2 -0
+}
+EXPORT_SYMBOL(dump_stack_log_lvl);

Ditto.

Looks useful otherwise.

Thanks,

Ingo

he, bo

author

2012-03-26 00:59 UTC #3 lore raw

Quoted 6 lines · Ingo Molnar

arch/x86/kernel/dumpstack.c void show_stack(struct task_struct *task, unsigned long *sp) · line 177
Quoted 4 lines · he, bo
arch/x86/kernel/dumpstack.c +0 -0
@@ -177,20 +177,29 @@ void show_stack(struct task_struct *task, unsigned long *sp)
 }

Ingo,

Thanks for your kind comments. It's the 1st time for me to send patches to LKML. There are 2 patches. I sent the 1st one to a wrong address. I will resend the 2 patches.

Quoted 6 lines

arch/x86/kernel/dumpstack.c void show_stack(struct task_struct *task, unsigned long *sp) · line 180
Quoted 4 lines
arch/x86/kernel/dumpstack.c +1 -1
 /*
- * The architecture-independent dump_stack generator
+ * The architecture-independent dump_stack_log_lvl generator
I guess it wants to say "architecture-dependent"?

Right. I will change it to architecture-dependent.

Quoted 6 lines

arch/x86/kernel/dumpstack.c void show_stack(struct task_struct *task, unsigned long *sp) · line 195
Quoted 4 lines
arch/x86/kernel/dumpstack.c +3 -0
+	show_trace_log_lvl(NULL, NULL, &stack, bp, log_lvl);
+}
+EXPORT_SYMBOL(dump_stack_log_lvl);
this should be a _GPL export.

Yes. I will change it.

Quoted 3 lines

arch/x86/kernel/dumpstack.c void show_stack(struct task_struct *task, unsigned long *sp) · line 184
Quoted 3 lines
arch/x86/kernel/dumpstack.c +2 -0
+}
+EXPORT_SYMBOL(dump_stack_log_lvl);

Yes. I will change it.

he, bo

author

2012-03-26 01:36 UTC #4 lore raw

PATCH 1/2 fix the bug that printk can't support printk(KERN_LEVEL)

In reply to Ingo Molnar

From: he bo <[email protected]>

Usually, there is a special scenario that developer wants to call printk to just set up a log level (might be transferred here as a parameter from upper level), then, later calling of printk prints out real string with the same log level continuously.

Current function vprintk has an issue to support this capability. When the whole string in one calling to printk is just a log level, it ignores it.

Signed-off-by he, bo <[email protected]>

Reviewed-by Zhang, Yanmin <[email protected]>

---
 kernel/printk.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

kernel/printk.c +4 -3

index b663c2c..473afdb 100644
@@ -909,7 +909,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 	 * Copy the output into log_buf. If the caller didn't provide
 	 * the appropriate log prefix, we insert them here
 	 */
-	for (; *p; p++) {
+	for (; plen || *p; p++) {
 		if (new_text_line) {
 			new_text_line = 0;

@@ -920,6 +920,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 				for (i = 0; i < plen; i++)
 					emit_log_char(printk_buf[i]);
 				printed_len += plen;
+				plen = 0;
 			} else {
 				/* Add log prefix */
 				emit_log_char('<');

@@ -946,10 +947,10 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 				printed_len += tlen;
 			}
 
-			if (!*p)
-				break;
 		}
 
+		if (!*p)
+			break;
 		emit_log_char(*p);
 		if (*p == '\n')
 			new_text_line = 1;

Signature · 1 line

1.7.1

Yanmin Zhang

2012-03-28 03:29 UTC #5 lore raw

Re: [PATCH_V2 1/2] fix the bug that printk can't support printk(KERN_LEVEL)

Quoted 7 lines · bo

From: he bo <[email protected]>
Usually, there is a special scenario that developer wants to call printk to just set up a log level (might be transferred here as a parameter from upper level), then, later calling of printk prints out real string with the same log level continuously.

The patch fixes a real issue. See the function.

static void print_trace_address(void *data, unsigned long addr, int reliable)

{
        touch_nmi_watchdog();
        printk(data);		<============= Here is to print the log level only
        printk_address(addr, reliable);
}

Without the patch, printk(data) doesn't emit the log level, so printk_address would use the default log level.

Besides above real issue, developer might write below codes to debug issues. printk("%s", log_level);

for (i = 0; i < fields; i++) {
	printk("one field");
}

I did so years ago and felt weird why the log level was incorrect. :)

Quoted 45 lines

Current function vprintk has an issue to support this capability. When the whole string in one calling to printk is just a log level, it ignores it.

Signed-off-by he, bo <[email protected]>

Reviewed-by Zhang, Yanmin <[email protected]>

---
 kernel/printk.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

kernel/printk.c +4 -3

index b663c2c..473afdb 100644
@@ -909,7 +909,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 	 * Copy the output into log_buf. If the caller didn't provide
 	 * the appropriate log prefix, we insert them here
 	 */
-	for (; *p; p++) {
+	for (; plen || *p; p++) {
 		if (new_text_line) {
 			new_text_line = 0;

@@ -920,6 +920,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 				for (i = 0; i < plen; i++)
 					emit_log_char(printk_buf[i]);
 				printed_len += plen;
+				plen = 0;
 			} else {
 				/* Add log prefix */
 				emit_log_char('<');

@@ -946,10 +947,10 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 				printed_len += tlen;
 			}
 
-			if (!*p)
-				break;
 		}
 
+		if (!*p)
+			break;
 		emit_log_char(*p);
 		if (*p == '\n')
 			new_text_line = 1;

he, bo

author

2012-04-06 00:57 UTC #6 lore raw

Re: [PATCH_V2 1/2] fix the bug that printk can't support printk(KERN_LEVEL)

In reply to he, bo

Quoted 12 lines · bo

From: he bo <[email protected]>
Usually, there is a special scenario that developer wants to call printk to just set up a log level (might be transferred here as a parameter from upper level), then, later calling of printk prints out real string with the same log level continuously.
Current function vprintk has an issue to support this capability. When the whole string in one calling to printk is just a log level, it ignores it.

Quoted 5 lines

Signed-off-by he, bo <[email protected]>
Reviewed-by Zhang, Yanmin <[email protected]>
---
 kernel/printk.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

Ingo,

What's your opinion about the 2 patches? The 1st patch fixes an issue and 2nd patch enhances dump_stack with log level.

https://lkml.org/lkml/2012/3/25/152
https://lkml.org/lkml/2012/3/25/153

Thanks,
Bo

Quoted 35 lines

kernel/printk.c +4 -3

index b663c2c..473afdb 100644
@@ -909,7 +909,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 	 * Copy the output into log_buf. If the caller didn't provide
 	 * the appropriate log prefix, we insert them here
 	 */
-	for (; *p; p++) {
+	for (; plen || *p; p++) {
 		if (new_text_line) {
 			new_text_line = 0;

@@ -920,6 +920,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 				for (i = 0; i < plen; i++)
 					emit_log_char(printk_buf[i]);
 				printed_len += plen;
+				plen = 0;
 			} else {
 				/* Add log prefix */
 				emit_log_char('<');

@@ -946,10 +947,10 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 				printed_len += tlen;
 			}
 
-			if (!*p)
-				break;
 		}
 
+		if (!*p)
+			break;
 		emit_log_char(*p);
 		if (*p == '\n')
 			new_text_line = 1;

he, bo

author

2012-03-26 01:37 UTC #7 lore raw

PATCH 2/2 enable dumpstack() to printk log level

In reply to Ingo Molnar

From: he bo <[email protected]>

Signed-off-by he, bo <[email protected]>

Reviewed-by zhang, yanmin <[email protected]>

---
 arch/x86/kernel/dumpstack.c |   19 ++++++++++++++-----
 include/linux/printk.h      |    1 +
 kernel/sched/core.c         |    2 +-
 lib/dump_stack.c            |    7 ++++++-
 4 files changed, 22 insertions(+), 7 deletions(-)

arch/x86/kernel/dumpstack.c +14 -5

index 4025fe4..2ffa890 100644
@@ -177,20 +177,29 @@ void show_stack(struct task_struct *task, unsigned long *sp)
 }
 
 /*
- * The architecture-independent dump_stack generator
+ * The architecture-dependent dump_stack_log_lvl generator
  */
-void dump_stack(void)
+void dump_stack_log_lvl(char *log_lvl)
 {
 	unsigned long bp;
 	unsigned long stack;
 
 	bp = stack_frame(current, NULL);
-	printk("Pid: %d, comm: %.20s %s %s %.*s\n",
-		current->pid, current->comm, print_tainted(),
+	printk("%sPid: %d, comm: %.20s %s %s %.*s\n",
+		log_lvl, current->pid, current->comm, print_tainted(),
 		init_utsname()->release,
 		(int)strcspn(init_utsname()->version, " "),
 		init_utsname()->version);
-	show_trace(NULL, NULL, &stack, bp);
+	show_trace_log_lvl(NULL, NULL, &stack, bp, log_lvl);
+}
+EXPORT_SYMBOL_GPL(dump_stack_log_lvl);
+
+/*
+ * The architecture-dependent dump_stack generator
+ */
+void dump_stack(void)
+{
+	dump_stack_log_lvl("");
 }
 EXPORT_SYMBOL(dump_stack);

include/linux/printk.h +1 -0

index 0525927..2a13dd8 100644
@@ -157,6 +157,7 @@ static inline void setup_log_buf(int early)
 #endif
 
 extern void dump_stack(void) __cold;
+extern void dump_stack_log_lvl(char *log_lvl) __cold;
 
 #ifndef pr_fmt
 #define pr_fmt(fmt) fmt

kernel/sched/core.c +1 -1

index 503d642..ab0dcdf 100644
@@ -7060,7 +7060,7 @@ void __might_sleep(const char *file, int line, int preempt_offset)
 	debug_show_held_locks(current);
 	if (irqs_disabled())
 		print_irqtrace_events(current);
-	dump_stack();
+	dump_stack_log_lvl(KERN_ERR);
 }
 EXPORT_SYMBOL(__might_sleep);
 #endif

lib/dump_stack.c +6 -1

index 53bff4c..fd14940 100644
@@ -11,5 +11,10 @@ void dump_stack(void)
 	printk(KERN_NOTICE
 		"This architecture does not implement dump_stack()\n");
 }
-
 EXPORT_SYMBOL(dump_stack);
+
+void dump_stack_log_lvl(char *log_lvl)
+{
+	dump_stack();
+}
+EXPORT_SYMBOL_GPL(dump_stack_log_lvl);

Signature · 1 line

1.7.1