* [PATCH 1/2] x86: fix output of show_stack_log_lvl()
2015-02-19 22:43 [PATCH 0/2] x86: fix output of show_stack_log_lvl Adrien Schildknecht
@ 2015-02-19 22:43 ` Adrien Schildknecht
2015-02-20 0:06 ` Borislav Petkov
2015-02-19 22:43 ` [PATCH 2/2] x86: fix output of show_trace_log_lvl() Adrien Schildknecht
2015-02-20 2:34 ` [PATCH v2] x86: fix output of show_stack_log_lvl() Adrien Schildknecht
2 siblings, 1 reply; 16+ messages in thread
From: Adrien Schildknecht @ 2015-02-19 22:43 UTC (permalink / raw)
To: tglx, mingo, hpa, x86, rostedt, heukelum, luto, adech.fo,
masami.hiramatsu.pt, akpm, a.ryabinin, fruggeri
Cc: linux-kernel, Adrien Schildknecht
Prepend the log level at the message following a newline.
It is not possible to use pr_cont after a newline, the log level will be
reseted.
Signed-off-by: Adrien Schildknecht <adrien+dev@schischi.me>
---
arch/x86/kernel/dumpstack_32.c | 9 ++++++---
arch/x86/kernel/dumpstack_64.c | 9 ++++++---
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 5abd4cd..efff5ed 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -108,9 +108,12 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
for (i = 0; i < kstack_depth_to_print; i++) {
if (kstack_end(stack))
break;
- if (i && ((i % STACKSLOTS_PER_LINE) == 0))
- pr_cont("\n");
- pr_cont(" %08lx", *stack++);
+ if ((i % STACKSLOTS_PER_LINE) == 0) {
+ if (i != 0)
+ pr_cont("\n");
+ printk("%s %08lx", log_lvl, *stack++);
+ } else
+ pr_cont(" %08lx", *stack++);
touch_nmi_watchdog();
}
pr_cont("\n");
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index ff86f19..553573b 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -283,9 +283,12 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
if (((long) stack & (THREAD_SIZE-1)) == 0)
break;
}
- if (i && ((i % STACKSLOTS_PER_LINE) == 0))
- pr_cont("\n");
- pr_cont(" %016lx", *stack++);
+ if ((i % STACKSLOTS_PER_LINE) == 0) {
+ if (i != 0)
+ pr_cont("\n");
+ printk("%s %016lx", log_lvl, *stack++);
+ } else
+ pr_cont(" %016lx", *stack++);
touch_nmi_watchdog();
}
preempt_enable();
--
2.2.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] x86: fix output of show_stack_log_lvl()
2015-02-19 22:43 ` [PATCH 1/2] x86: fix output of show_stack_log_lvl() Adrien Schildknecht
@ 2015-02-20 0:06 ` Borislav Petkov
0 siblings, 0 replies; 16+ messages in thread
From: Borislav Petkov @ 2015-02-20 0:06 UTC (permalink / raw)
To: Adrien Schildknecht
Cc: tglx, mingo, hpa, x86, rostedt, heukelum, luto, adech.fo,
masami.hiramatsu.pt, akpm, a.ryabinin, fruggeri, linux-kernel
On Thu, Feb 19, 2015 at 11:43:15PM +0100, Adrien Schildknecht wrote:
> Prepend the log level at the message following a newline.
> It is not possible to use pr_cont after a newline, the log level will be
> reseted.
>
> Signed-off-by: Adrien Schildknecht <adrien+dev@schischi.me>
> ---
> arch/x86/kernel/dumpstack_32.c | 9 ++++++---
> arch/x86/kernel/dumpstack_64.c | 9 ++++++---
> 2 files changed, 12 insertions(+), 6 deletions(-)
So those two patches belong into one as they logically are one fix.
Please merge them.
Then, please change your commit message to the format:
"Problem is A.
We need to do B.
This patch does it or this patch does C in order fix it."
Something like that in free form. Being verbose is a good thing when
explaning why this fix is needed and for people looking at those commit
messages months and years from now.
Thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2] x86: fix output of show_trace_log_lvl()
2015-02-19 22:43 [PATCH 0/2] x86: fix output of show_stack_log_lvl Adrien Schildknecht
2015-02-19 22:43 ` [PATCH 1/2] x86: fix output of show_stack_log_lvl() Adrien Schildknecht
@ 2015-02-19 22:43 ` Adrien Schildknecht
2015-02-20 2:34 ` [PATCH v2] x86: fix output of show_stack_log_lvl() Adrien Schildknecht
2 siblings, 0 replies; 16+ messages in thread
From: Adrien Schildknecht @ 2015-02-19 22:43 UTC (permalink / raw)
To: tglx, mingo, hpa, x86, rostedt, heukelum, luto, adech.fo,
masami.hiramatsu.pt, akpm, a.ryabinin, fruggeri
Cc: linux-kernel, Adrien Schildknecht
Prepend the log level in printk_stack_address.
Using printk with just a log level is ignored and thus has no effect on
the next pr_cont.
Signed-off-by: Adrien Schildknecht <adrien+dev@schischi.me>
---
arch/x86/kernel/dumpstack.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index cf3df1d..da1ab6a 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -25,10 +25,12 @@ unsigned int code_bytes = 64;
int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE;
static int die_counter;
-static void printk_stack_address(unsigned long address, int reliable)
+static void printk_stack_address(unsigned long address, int reliable,
+ void *data)
{
- pr_cont(" [<%p>] %s%pB\n",
- (void *)address, reliable ? "" : "? ", (void *)address);
+ pr_cont("%s [<%p>] %s%pB\n",
+ (char *)data, (void *)address, reliable ? "" : "? ",
+ (void *)address);
}
void printk_address(unsigned long address)
@@ -155,8 +157,7 @@ static int print_trace_stack(void *data, char *name)
static void print_trace_address(void *data, unsigned long addr, int reliable)
{
touch_nmi_watchdog();
- printk(data);
- printk_stack_address(addr, reliable);
+ printk_stack_address(addr, reliable, data);
}
static const struct stacktrace_ops print_trace_ops = {
--
2.2.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2] x86: fix output of show_stack_log_lvl()
2015-02-19 22:43 [PATCH 0/2] x86: fix output of show_stack_log_lvl Adrien Schildknecht
2015-02-19 22:43 ` [PATCH 1/2] x86: fix output of show_stack_log_lvl() Adrien Schildknecht
2015-02-19 22:43 ` [PATCH 2/2] x86: fix output of show_trace_log_lvl() Adrien Schildknecht
@ 2015-02-20 2:34 ` Adrien Schildknecht
2015-02-20 4:45 ` Steven Rostedt
` (2 more replies)
2 siblings, 3 replies; 16+ messages in thread
From: Adrien Schildknecht @ 2015-02-20 2:34 UTC (permalink / raw)
To: tglx, mingo, hpa, x86, rostedt, heukelum, luto, adech.fo,
masami.hiramatsu.pt, akpm, a.ryabinin, fruggeri, bp
Cc: linux-kernel, Adrien Schildknecht
show_stack_log_lvl() does not set the log level after a new line,
the following messages printed with pr_cont are thus assigned to the
default log level.
This patch prepends the log level to the next message following a new
line.
print_trace_address() uses printk(log_lvl). Using printk with just
a log level is ignored and thus has no effect on the next pr_cont.
We need to prepend the log level directly into the message.
Signed-off-by: Adrien Schildknecht <adrien+dev@schischi.me>
---
arch/x86/kernel/dumpstack.c | 11 ++++++-----
arch/x86/kernel/dumpstack_32.c | 9 ++++++---
arch/x86/kernel/dumpstack_64.c | 9 ++++++---
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index cf3df1d..81b3932 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -25,10 +25,12 @@ unsigned int code_bytes = 64;
int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE;
static int die_counter;
-static void printk_stack_address(unsigned long address, int reliable)
+static void printk_stack_address(unsigned long address, int reliable,
+ void *data)
{
- pr_cont(" [<%p>] %s%pB\n",
- (void *)address, reliable ? "" : "? ", (void *)address);
+ printk("%s [<%p>] %s%pB\n",
+ (char *)data, (void *)address, reliable ? "" : "? ",
+ (void *)address);
}
void printk_address(unsigned long address)
@@ -155,8 +157,7 @@ static int print_trace_stack(void *data, char *name)
static void print_trace_address(void *data, unsigned long addr, int reliable)
{
touch_nmi_watchdog();
- printk(data);
- printk_stack_address(addr, reliable);
+ printk_stack_address(addr, reliable, data);
}
static const struct stacktrace_ops print_trace_ops = {
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 5abd4cd..efff5ed 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -108,9 +108,12 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
for (i = 0; i < kstack_depth_to_print; i++) {
if (kstack_end(stack))
break;
- if (i && ((i % STACKSLOTS_PER_LINE) == 0))
- pr_cont("\n");
- pr_cont(" %08lx", *stack++);
+ if ((i % STACKSLOTS_PER_LINE) == 0) {
+ if (i != 0)
+ pr_cont("\n");
+ printk("%s %08lx", log_lvl, *stack++);
+ } else
+ pr_cont(" %08lx", *stack++);
touch_nmi_watchdog();
}
pr_cont("\n");
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index ff86f19..553573b 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -283,9 +283,12 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
if (((long) stack & (THREAD_SIZE-1)) == 0)
break;
}
- if (i && ((i % STACKSLOTS_PER_LINE) == 0))
- pr_cont("\n");
- pr_cont(" %016lx", *stack++);
+ if ((i % STACKSLOTS_PER_LINE) == 0) {
+ if (i != 0)
+ pr_cont("\n");
+ printk("%s %016lx", log_lvl, *stack++);
+ } else
+ pr_cont(" %016lx", *stack++);
touch_nmi_watchdog();
}
preempt_enable();
--
2.2.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2] x86: fix output of show_stack_log_lvl()
2015-02-20 2:34 ` [PATCH v2] x86: fix output of show_stack_log_lvl() Adrien Schildknecht
@ 2015-02-20 4:45 ` Steven Rostedt
[not found] ` <CA+55aFzDt_7Rs9=4XFKo0LP4iBnV4qmJWUDACtBDbV4eRE-X9A@mail.gmail.com>
2015-02-20 8:10 ` Ingo Molnar
2015-03-03 11:28 ` [tip:x86/debug] x86/kernel: Fix " tip-bot for Adrien Schildknecht
2 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2015-02-20 4:45 UTC (permalink / raw)
To: Adrien Schildknecht
Cc: tglx, mingo, hpa, x86, heukelum, luto, adech.fo,
masami.hiramatsu.pt, akpm, a.ryabinin, fruggeri, bp, linux-kernel
On Fri, 20 Feb 2015 03:34:21 +0100
Adrien Schildknecht <adrien+dev@schischi.me> wrote:
> show_stack_log_lvl() does not set the log level after a new line,
> the following messages printed with pr_cont are thus assigned to the
> default log level.
This looks like a bug in printk(). Why doesn't pr_cont() continue? It
shouldn't care if there's a newline or not. pr_cont() is supposed to
continue whatever the last printk log level was.
If this is broken here, it's probably broken elsewhere. The fix is to
fix printk, not to hunt and peck for the places with work arounds that
are broken by it.
-- Steve
> This patch prepends the log level to the next message following a new
> line.
>
> print_trace_address() uses printk(log_lvl). Using printk with just
> a log level is ignored and thus has no effect on the next pr_cont.
> We need to prepend the log level directly into the message.
>
> Signed-off-by: Adrien Schildknecht <adrien+dev@schischi.me>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] x86: fix output of show_stack_log_lvl()
2015-02-20 2:34 ` [PATCH v2] x86: fix output of show_stack_log_lvl() Adrien Schildknecht
2015-02-20 4:45 ` Steven Rostedt
@ 2015-02-20 8:10 ` Ingo Molnar
2015-02-20 10:38 ` Borislav Petkov
2015-03-03 11:28 ` [tip:x86/debug] x86/kernel: Fix " tip-bot for Adrien Schildknecht
2 siblings, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2015-02-20 8:10 UTC (permalink / raw)
To: Adrien Schildknecht
Cc: tglx, mingo, hpa, x86, rostedt, heukelum, luto, adech.fo,
masami.hiramatsu.pt, akpm, a.ryabinin, fruggeri, bp, linux-kernel
* Adrien Schildknecht <adrien+dev@schischi.me> wrote:
> show_stack_log_lvl() does not set the log level after a new line,
> the following messages printed with pr_cont are thus assigned to the
> default log level.
> This patch prepends the log level to the next message following a new
> line.
>
> print_trace_address() uses printk(log_lvl). Using printk with just
> a log level is ignored and thus has no effect on the next pr_cont.
> We need to prepend the log level directly into the message.
This approach looks good to me, we want to print multi-line
messages with the same consistent loglevel.
Totally unrelated observation:
> +++ b/arch/x86/kernel/dumpstack.c
> touch_nmi_watchdog();
> + printk_stack_address(addr, reliable, data);
The whole code is sprinkled with touch_nmi_watchdog()
calls. Shouldn't we simply make it a rule that if a printk
message makes it to a real console (as opposed to the log
buffer only), it should imply a touch_nmi_watchdog()?
Then all of those crappy touch_nmi_watchdog() calls could
be removed here and in other places where long printk
streams may happen.
Totally unrelated observation #2:
> if (kstack_end(stack))
> break;
> - if (i && ((i % STACKSLOTS_PER_LINE) == 0))
> - pr_cont("\n");
> +++ b/arch/x86/kernel/dumpstack_64.c
> if (((long) stack & (THREAD_SIZE-1)) == 0)
> break;
> }
> - if (i && ((i % STACKSLOTS_PER_LINE) == 0))
> - pr_cont("\n");
Looks like kstack_end() could be defined on 64-bit as well,
unifying the stack printing logic some more?
( I'd no go so far as to unify the two functions, but the
closer to each other the better it is to make changes
that affect both of them. )
Thanks,
Ingo
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] x86: fix output of show_stack_log_lvl()
2015-02-20 8:10 ` Ingo Molnar
@ 2015-02-20 10:38 ` Borislav Petkov
2015-02-20 16:39 ` Adrien Schildknecht
0 siblings, 1 reply; 16+ messages in thread
From: Borislav Petkov @ 2015-02-20 10:38 UTC (permalink / raw)
To: Ingo Molnar
Cc: Adrien Schildknecht, tglx, mingo, hpa, x86, rostedt, heukelum,
luto, adech.fo, masami.hiramatsu.pt, akpm, a.ryabinin, fruggeri,
linux-kernel
On Fri, Feb 20, 2015 at 09:10:03AM +0100, Ingo Molnar wrote:
> This approach looks good to me, we want to print multi-line
> messages with the same consistent loglevel.
Right, I'll pick this one up for now as it is obviously correct and
whatever we end up doing to pr_cont() won't influence it.
> Totally unrelated observation #2:
>
> > if (kstack_end(stack))
> > break;
> > - if (i && ((i % STACKSLOTS_PER_LINE) == 0))
> > - pr_cont("\n");
>
> > +++ b/arch/x86/kernel/dumpstack_64.c
>
> > if (((long) stack & (THREAD_SIZE-1)) == 0)
> > break;
> > }
> > - if (i && ((i % STACKSLOTS_PER_LINE) == 0))
> > - pr_cont("\n");
>
> Looks like kstack_end() could be defined on 64-bit as well,
> unifying the stack printing logic some more?
>
> ( I'd no go so far as to unify the two functions, but the
> closer to each other the better it is to make changes
> that affect both of them. )
Adrien, want to take care of that one too?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] x86: fix output of show_stack_log_lvl()
2015-02-20 10:38 ` Borislav Petkov
@ 2015-02-20 16:39 ` Adrien Schildknecht
0 siblings, 0 replies; 16+ messages in thread
From: Adrien Schildknecht @ 2015-02-20 16:39 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, tglx, mingo, hpa, x86, rostedt, heukelum, luto,
adech.fo, masami.hiramatsu.pt, akpm, a.ryabinin, fruggeri,
linux-kernel
> > Looks like kstack_end() could be defined on 64-bit as well,
> > unifying the stack printing logic some more?
> >
> > ( I'd no go so far as to unify the two functions, but the
> > closer to each other the better it is to make changes
> > that affect both of them. )
>
> Adrien, want to take care of that one too?
Sure, I can do that.
--
Adrien Schildknecht
^ permalink raw reply [flat|nested] 16+ messages in thread
* [tip:x86/debug] x86/kernel: Fix output of show_stack_log_lvl()
2015-02-20 2:34 ` [PATCH v2] x86: fix output of show_stack_log_lvl() Adrien Schildknecht
2015-02-20 4:45 ` Steven Rostedt
2015-02-20 8:10 ` Ingo Molnar
@ 2015-03-03 11:28 ` tip-bot for Adrien Schildknecht
2 siblings, 0 replies; 16+ messages in thread
From: tip-bot for Adrien Schildknecht @ 2015-03-03 11:28 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, adrien+dev, bp, torvalds, mingo, hpa, tglx, rostedt
Commit-ID: 1fc7f61c3e604f6bf778b5c6afc2715d79ab7f36
Gitweb: http://git.kernel.org/tip/1fc7f61c3e604f6bf778b5c6afc2715d79ab7f36
Author: Adrien Schildknecht <adrien+dev@schischi.me>
AuthorDate: Fri, 20 Feb 2015 03:34:21 +0100
Committer: Borislav Petkov <bp@suse.de>
CommitDate: Mon, 23 Feb 2015 18:34:42 +0100
x86/kernel: Fix output of show_stack_log_lvl()
show_stack_log_lvl() does not set the log level after a new line, the
following messages printed with pr_cont() are thus assigned to the
default log level.
This patch prepends the log level to the next message following a new
line.
print_trace_address() uses printk(log_lvl). Using printk() with just
a log level is ignored and thus has no effect on the next pr_cont().
We need to prepend the log level directly into the message.
Signed-off-by: Adrien Schildknecht <adrien+dev@schischi.me>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1424399661-20327-1-git-send-email-adrien+dev@schischi.me
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/kernel/dumpstack.c | 11 ++++++-----
arch/x86/kernel/dumpstack_32.c | 9 ++++++---
arch/x86/kernel/dumpstack_64.c | 9 ++++++---
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index cf3df1d..81b3932 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -25,10 +25,12 @@ unsigned int code_bytes = 64;
int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE;
static int die_counter;
-static void printk_stack_address(unsigned long address, int reliable)
+static void printk_stack_address(unsigned long address, int reliable,
+ void *data)
{
- pr_cont(" [<%p>] %s%pB\n",
- (void *)address, reliable ? "" : "? ", (void *)address);
+ printk("%s [<%p>] %s%pB\n",
+ (char *)data, (void *)address, reliable ? "" : "? ",
+ (void *)address);
}
void printk_address(unsigned long address)
@@ -155,8 +157,7 @@ static int print_trace_stack(void *data, char *name)
static void print_trace_address(void *data, unsigned long addr, int reliable)
{
touch_nmi_watchdog();
- printk(data);
- printk_stack_address(addr, reliable);
+ printk_stack_address(addr, reliable, data);
}
static const struct stacktrace_ops print_trace_ops = {
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 5abd4cd..efff5ed 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -108,9 +108,12 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
for (i = 0; i < kstack_depth_to_print; i++) {
if (kstack_end(stack))
break;
- if (i && ((i % STACKSLOTS_PER_LINE) == 0))
- pr_cont("\n");
- pr_cont(" %08lx", *stack++);
+ if ((i % STACKSLOTS_PER_LINE) == 0) {
+ if (i != 0)
+ pr_cont("\n");
+ printk("%s %08lx", log_lvl, *stack++);
+ } else
+ pr_cont(" %08lx", *stack++);
touch_nmi_watchdog();
}
pr_cont("\n");
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index ff86f19..553573b 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -283,9 +283,12 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
if (((long) stack & (THREAD_SIZE-1)) == 0)
break;
}
- if (i && ((i % STACKSLOTS_PER_LINE) == 0))
- pr_cont("\n");
- pr_cont(" %016lx", *stack++);
+ if ((i % STACKSLOTS_PER_LINE) == 0) {
+ if (i != 0)
+ pr_cont("\n");
+ printk("%s %016lx", log_lvl, *stack++);
+ } else
+ pr_cont(" %016lx", *stack++);
touch_nmi_watchdog();
}
preempt_enable();
^ permalink raw reply related [flat|nested] 16+ messages in thread