Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1408094 > unrolled thread

[PATCH RT] kernel/printk: Don't try to print from IRQ/NMI region

Back to article view | Back to linux.kernel

Contents

  [PATCH RT] kernel/printk: Don't try to print from IRQ/NMI region Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-05-27 16:00 +0200
    Re: [PATCH RT] kernel/printk: Don't try to print from IRQ/NMI  region Steven Rostedt <rostedt@goodmis.org> - 2016-05-27 16:20 +0200
      Re: [PATCH RT] kernel/printk: Don't try to print from IRQ/NMI region Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-05-27 17:00 +0200
        Re: [PATCH RT] kernel/printk: Don't try to print from IRQ/NMI  region Steven Rostedt <rostedt@goodmis.org> - 2016-05-27 17:10 +0200
          Re: [PATCH RT] kernel/printk: Don't try to print from IRQ/NMI region Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-05-27 18:20 +0200

#1408094 — [PATCH RT] kernel/printk: Don't try to print from IRQ/NMI region

On -RT we try to acquire sleeping locks which might lead to warnings
from lockdep or a warn_on() from spin_try_lock() (which is a rtmutex on
RT).
We don't print in general from a IRQ off region so we should not try
this via console_unblank() / bust_spinlocks() as well.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/printk/printk.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1502,6 +1502,11 @@ static void call_console_drivers(int lev
 	if (!console_drivers)
 		return;
 
+	if (IS_ENABLED(CONFIG_PREEMPT_RT_BASE)) {
+		if (in_irq() || in_nmi())
+			return;
+	}
+
 	migrate_disable();
 	for_each_console(con) {
 		if (exclusive_console && con != exclusive_console)
@@ -2434,6 +2439,11 @@ void console_unblank(void)
 {
 	struct console *c;
 
+	if (IS_ENABLED(CONFIG_PREEMPT_RT_BASE)) {
+		if (in_irq() || in_nmi())
+			return;
+	}
+
 	/*
 	 * console_unblank can no longer be called in interrupt context unless
 	 * oops_in_progress is set to 1..

[toc] | [next] | [standalone]

#1408103 — Re: [PATCH RT] kernel/printk: Don't try to print from IRQ/NMI region

On Fri, 27 May 2016 15:58:12 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> On -RT we try to acquire sleeping locks which might lead to warnings
> from lockdep or a warn_on() from spin_try_lock() (which is a rtmutex on
> RT).
> We don't print in general from a IRQ off region so we should not try
> this via console_unblank() / bust_spinlocks() as well.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  kernel/printk/printk.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1502,6 +1502,11 @@ static void call_console_drivers(int lev
>  	if (!console_drivers)
>  		return;
>  
> +	if (IS_ENABLED(CONFIG_PREEMPT_RT_BASE)) {
> +		if (in_irq() || in_nmi())
> +			return;
> +	}
> +

We use to have a patch where a console could flag itself as atomic.
That is, that it doesn't call any sleeping locks. What happened to that.

IIRC, the video console was one such console. Otherwise, we lose out on
backtraces in irq context.

-- Steve

>  	migrate_disable();
>  	for_each_console(con) {
>  		if (exclusive_console && con != exclusive_console)

[toc] | [prev] | [next] | [standalone]

#1408121

* Steven Rostedt | 2016-05-27 10:12:51 [-0400]:

>On Fri, 27 May 2016 15:58:12 +0200
>Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>
>> On -RT we try to acquire sleeping locks which might lead to warnings
>> from lockdep or a warn_on() from spin_try_lock() (which is a rtmutex on
>> RT).
>> We don't print in general from a IRQ off region so we should not try
>> this via console_unblank() / bust_spinlocks() as well.
>> 
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> ---
>>  kernel/printk/printk.c |   10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1502,6 +1502,11 @@ static void call_console_drivers(int lev
>>  	if (!console_drivers)
>>  		return;
>>  
>> +	if (IS_ENABLED(CONFIG_PREEMPT_RT_BASE)) {
>> +		if (in_irq() || in_nmi())
>> +			return;
>> +	}
>> +
>
>We use to have a patch where a console could flag itself as atomic.
>That is, that it doesn't call any sleeping locks. What happened to that.
>
>IIRC, the video console was one such console. Otherwise, we lose out on
>backtraces in irq context.

video, like VT?
The call_console_drivers() does not check such a flag and I don't
remember about removing something like that. I was actually thinking
about adding such a flag… I remember you had something in your tree to
print from IRQ off regions via UART.

We don't print from a context with interrupts disabled or even a preempt
disabled region. In such cases we just wake_up_klogd() and print it
later.
The reason for the patch was the HW/SW lockup detector which comes with
interrupts disabled or NMI context and gets to the console(s). Usually
we defer everything until later except in this case. And I got busted
later in the VT code (and the problem was that one sleeping while atomic
warning which led to more stuff to be printed).

Sebastian

[toc] | [prev] | [next] | [standalone]

#1408122 — Re: [PATCH RT] kernel/printk: Don't try to print from IRQ/NMI region

On Fri, 27 May 2016 16:56:01 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> >We use to have a patch where a console could flag itself as atomic.
> >That is, that it doesn't call any sleeping locks. What happened to that.
> >
> >IIRC, the video console was one such console. Otherwise, we lose out on
> >backtraces in irq context.  
> 
> video, like VT?
> The call_console_drivers() does not check such a flag and I don't
> remember about removing something like that. I was actually thinking

You probably didn't remove it. It may have been removed a while ago
when Thomas did his big rewrite to get things working again.

> about adding such a flag… I remember you had something in your tree to
> print from IRQ off regions via UART.

I had a few hacks for a while, but they were nothing more than hacks.

Perhaps we should make a better early_printk() or simple console for RT
that can handle printk in atomic locations.

I have a hack patch that gives early_printk() a new "spin lock", where
it only takes the lock if the owner isn't on the CPU. Otherwise it
allows lock to continue (and wont release it). That hack was required
to get legible output from early_printk() when I was getting a bunch of
crashes on all the CPUs, because early_printk() has no locks, and the
console is just a mess when all CPUs print at once.

> 
> We don't print from a context with interrupts disabled or even a preempt
> disabled region. In such cases we just wake_up_klogd() and print it
> later.

Yeah, but that doesn't help if the "later" never comes.

> The reason for the patch was the HW/SW lockup detector which comes with
> interrupts disabled or NMI context and gets to the console(s). Usually
> we defer everything until later except in this case. And I got busted
> later in the VT code (and the problem was that one sleeping while atomic
> warning which led to more stuff to be printed).

Yeah, it's a pain where our output needs to be in schedulable context.

-- Steve

[toc] | [prev] | [next] | [standalone]

#1408166

* Steven Rostedt | 2016-05-27 11:06:58 [-0400]:

>You probably didn't remove it. It may have been removed a while ago
>when Thomas did his big rewrite to get things working again.

okay. As long as I don't lose patches :)

>> about adding such a flag… I remember you had something in your tree to
>> print from IRQ off regions via UART.
>
>I had a few hacks for a while, but they were nothing more than hacks.
>
>Perhaps we should make a better early_printk() or simple console for RT
>that can handle printk in atomic locations.
>
>I have a hack patch that gives early_printk() a new "spin lock", where
>it only takes the lock if the owner isn't on the CPU. Otherwise it
>allows lock to continue (and wont release it). That hack was required
>to get legible output from early_printk() when I was getting a bunch of
>crashes on all the CPUs, because early_printk() has no locks, and the
>console is just a mess when all CPUs print at once.

An "atomic" console would be nice debugging wise. pstore with rawlocks
would be available after a reboot if you can use pstore. And its
overhead is probably not that huge. But its availability is limited.

Another thing might be a dedicated 8250 UART console which would print
in IRQ-off regions (and increase latency) but you would have the output
which you wouldn't have otherwise (say from panic() handler). Anyway,
nothing for production.

Sebastian

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web