From: Daniel Wagner daniel.wagner@siemens.com
[ Upstream commit 8afb1d2c12163f77777f84616a8e9444d0050ebe ]
Commit 40f70c03e33a ("serial: sh-sci: add locking to console write function to avoid SMP lockup") copied the strategy to avoid locking problems in conjuncture with the console from the UART8250 driver. Instead using directly spin_{try}lock_irqsave(), local_irq_save() followed by spin_{try}lock() was used. While this is correct on mainline, for -rt it is a problem. spin_{try}lock() will check if it is running in a valid context. Since the local_irq_save() has already been executed, the context has changed and spin_{try}lock() will complain. The reason why spin_{try}lock() complains is that on -rt the spin locks are turned into mutexes and therefore can sleep. Sleeping with interrupts disabled is not valid.
BUG: sleeping function called from invalid context at /home/wagi/work/rt/v4.4-cip-rt/kernel/locking/rtmutex.c:995 in_atomic(): 0, irqs_disabled(): 128, pid: 778, name: irq/76-eth0 CPU: 0 PID: 778 Comm: irq/76-eth0 Not tainted 4.4.126-test-cip22-rt14-00403-gcd03665c8318 #12 Hardware name: Generic RZ/G1 (Flattened Device Tree) Backtrace: [<c00140a0>] (dump_backtrace) from [<c001424c>] (show_stack+0x18/0x1c) r7:c06b01f0 r6:60010193 r5:00000000 r4:c06b01f0 [<c0014234>] (show_stack) from [<c01d3c94>] (dump_stack+0x78/0x94) [<c01d3c1c>] (dump_stack) from [<c004c134>] (___might_sleep+0x134/0x194) r7:60010113 r6:c06d3559 r5:00000000 r4:ffffe000 [<c004c000>] (___might_sleep) from [<c04ded60>] (rt_spin_lock+0x20/0x74) r5:c06f4d60 r4:c06f4d60 [<c04ded40>] (rt_spin_lock) from [<c02577e4>] (serial_console_write+0x100/0x118) r5:c06f4d60 r4:c06f4d60 [<c02576e4>] (serial_console_write) from [<c0061060>] (call_console_drivers.constprop.15+0x10c/0x124) r10:c06d2894 r9:c04e18b0 r8:00000028 r7:00000000 r6:c06d3559 r5:c06d2798 r4:c06b9914 r3:c02576e4 [<c0060f54>] (call_console_drivers.constprop.15) from [<c0062984>] (console_unlock+0x32c/0x430) r10:c06d30d8 r9:00000028 r8:c06dd518 r7:00000005 r6:00000000 r5:c06d2798 r4:c06d2798 r3:00000028 [<c0062658>] (console_unlock) from [<c0062e1c>] (vprintk_emit+0x394/0x4f0) r10:c06d2798 r9:c06d30ee r8:00000006 r7:00000005 r6:c06a78fc r5:00000027 r4:00000003 [<c0062a88>] (vprintk_emit) from [<c0062fa0>] (vprintk+0x28/0x30) r10:c060bd46 r9:00001000 r8:c06b9a90 r7:c06b9a90 r6:c06b994c r5:c06b9a3c r4:c0062fa8 [<c0062f78>] (vprintk) from [<c0062fb8>] (vprintk_default+0x10/0x14) [<c0062fa8>] (vprintk_default) from [<c009cd30>] (printk+0x78/0x84) [<c009ccbc>] (printk) from [<c025afdc>] (credit_entropy_bits+0x17c/0x2cc) r3:00000001 r2:decade60 r1:c061a5ee r0:c061a523 r4:00000006 [<c025ae60>] (credit_entropy_bits) from [<c025bf74>] (add_interrupt_randomness+0x160/0x178) r10:466e7196 r9:1f536000 r8:fffeef74 r7:00000000 r6:c06b9a60 r5:c06b9a3c r4:dfbcf680 [<c025be14>] (add_interrupt_randomness) from [<c006536c>] (irq_thread+0x1e8/0x248) r10:c006537c r9:c06cdf21 r8:c0064fcc r7:df791c24 r6:df791c00 r5:ffffe000 r4:df525180 [<c0065184>] (irq_thread) from [<c003fba4>] (kthread+0x108/0x11c) r10:00000000 r9:00000000 r8:c0065184 r7:df791c00 r6:00000000 r5:df791d00 r4:decac000 [<c003fa9c>] (kthread) from [<c00101b8>] (ret_from_fork+0x14/0x3c) r8:00000000 r7:00000000 r6:00000000 r5:c003fa9c r4:df791d00
Cc: Sebastian Andrzej Siewior bigeasy@linutronix.de Signed-off-by: Daniel Wagner daniel.wagner@siemens.com Reviewed-by: Geert Uytterhoeven geert+renesas@glider.be Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [dw: Backported to 4.4.] Signed-off-by: Daniel Wagner daniel.wagner@siemens.com --- drivers/tty/serial/sh-sci.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index 8dd822feb972..b63920481b1d 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -2419,13 +2419,12 @@ static void serial_console_write(struct console *co, const char *s, unsigned long flags; int locked = 1;
- local_irq_save(flags); if (port->sysrq) locked = 0; else if (oops_in_progress) - locked = spin_trylock(&port->lock); + locked = spin_trylock_irqsave(&port->lock, flags); else - spin_lock(&port->lock); + spin_lock_irqsave(&port->lock, flags);
/* first save the SCSCR then disable the interrupts */ ctrl = serial_port_in(port, SCSCR); @@ -2442,8 +2441,7 @@ static void serial_console_write(struct console *co, const char *s, serial_port_out(port, SCSCR, ctrl);
if (locked) - spin_unlock(&port->lock); - local_irq_restore(flags); + spin_unlock_irqrestore(&port->lock, flags); }
static int serial_console_setup(struct console *co, char *options)
From: Daniel Wagner daniel.wagner@siemens.com
[ Upstream commit 8afb1d2c12163f77777f84616a8e9444d0050ebe ]
Commit 40f70c03e33a ("serial: sh-sci: add locking to console write function to avoid SMP lockup") copied the strategy to avoid locking problems in conjuncture with the console from the UART8250 driver. Instead using directly spin_{try}lock_irqsave(), local_irq_save() followed by spin_{try}lock() was used. While this is correct on mainline, for -rt it is a problem. spin_{try}lock() will check if it is running in a valid context. Since the local_irq_save() has already been executed, the context has changed and spin_{try}lock() will complain. The reason why spin_{try}lock() complains is that on -rt the spin locks are turned into mutexes and therefore can sleep. Sleeping with interrupts disabled is not valid.
BUG: sleeping function called from invalid context at /home/wagi/work/rt/v4.4-cip-rt/kernel/locking/rtmutex.c:995 in_atomic(): 0, irqs_disabled(): 128, pid: 778, name: irq/76-eth0 CPU: 0 PID: 778 Comm: irq/76-eth0 Not tainted 4.4.126-test-cip22-rt14-00403-gcd03665c8318 #12 Hardware name: Generic RZ/G1 (Flattened Device Tree) Backtrace: [<c00140a0>] (dump_backtrace) from [<c001424c>] (show_stack+0x18/0x1c) r7:c06b01f0 r6:60010193 r5:00000000 r4:c06b01f0 [<c0014234>] (show_stack) from [<c01d3c94>] (dump_stack+0x78/0x94) [<c01d3c1c>] (dump_stack) from [<c004c134>] (___might_sleep+0x134/0x194) r7:60010113 r6:c06d3559 r5:00000000 r4:ffffe000 [<c004c000>] (___might_sleep) from [<c04ded60>] (rt_spin_lock+0x20/0x74) r5:c06f4d60 r4:c06f4d60 [<c04ded40>] (rt_spin_lock) from [<c02577e4>] (serial_console_write+0x100/0x118) r5:c06f4d60 r4:c06f4d60 [<c02576e4>] (serial_console_write) from [<c0061060>] (call_console_drivers.constprop.15+0x10c/0x124) r10:c06d2894 r9:c04e18b0 r8:00000028 r7:00000000 r6:c06d3559 r5:c06d2798 r4:c06b9914 r3:c02576e4 [<c0060f54>] (call_console_drivers.constprop.15) from [<c0062984>] (console_unlock+0x32c/0x430) r10:c06d30d8 r9:00000028 r8:c06dd518 r7:00000005 r6:00000000 r5:c06d2798 r4:c06d2798 r3:00000028 [<c0062658>] (console_unlock) from [<c0062e1c>] (vprintk_emit+0x394/0x4f0) r10:c06d2798 r9:c06d30ee r8:00000006 r7:00000005 r6:c06a78fc r5:00000027 r4:00000003 [<c0062a88>] (vprintk_emit) from [<c0062fa0>] (vprintk+0x28/0x30) r10:c060bd46 r9:00001000 r8:c06b9a90 r7:c06b9a90 r6:c06b994c r5:c06b9a3c r4:c0062fa8 [<c0062f78>] (vprintk) from [<c0062fb8>] (vprintk_default+0x10/0x14) [<c0062fa8>] (vprintk_default) from [<c009cd30>] (printk+0x78/0x84) [<c009ccbc>] (printk) from [<c025afdc>] (credit_entropy_bits+0x17c/0x2cc) r3:00000001 r2:decade60 r1:c061a5ee r0:c061a523 r4:00000006 [<c025ae60>] (credit_entropy_bits) from [<c025bf74>] (add_interrupt_randomness+0x160/0x178) r10:466e7196 r9:1f536000 r8:fffeef74 r7:00000000 r6:c06b9a60 r5:c06b9a3c r4:dfbcf680 [<c025be14>] (add_interrupt_randomness) from [<c006536c>] (irq_thread+0x1e8/0x248) r10:c006537c r9:c06cdf21 r8:c0064fcc r7:df791c24 r6:df791c00 r5:ffffe000 r4:df525180 [<c0065184>] (irq_thread) from [<c003fba4>] (kthread+0x108/0x11c) r10:00000000 r9:00000000 r8:c0065184 r7:df791c00 r6:00000000 r5:df791d00 r4:decac000 [<c003fa9c>] (kthread) from [<c00101b8>] (ret_from_fork+0x14/0x3c) r8:00000000 r7:00000000 r6:00000000 r5:c003fa9c r4:df791d00
Cc: Sebastian Andrzej Siewior bigeasy@linutronix.de Signed-off-by: Daniel Wagner daniel.wagner@siemens.com Reviewed-by: Geert Uytterhoeven geert+renesas@glider.be Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [dw: Backported] Signed-off-by: Daniel Wagner daniel.wagner@siemens.com --- drivers/tty/serial/sh-sci.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index da46f0fba5da..6ff53b604ff6 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -2807,16 +2807,15 @@ static void serial_console_write(struct console *co, const char *s, unsigned long flags; int locked = 1;
- local_irq_save(flags); #if defined(SUPPORT_SYSRQ) if (port->sysrq) locked = 0; else #endif if (oops_in_progress) - locked = spin_trylock(&port->lock); + locked = spin_trylock_irqsave(&port->lock, flags); else - spin_lock(&port->lock); + spin_lock_irqsave(&port->lock, flags);
/* first save SCSCR then disable interrupts, keep clock source */ ctrl = serial_port_in(port, SCSCR); @@ -2835,8 +2834,7 @@ static void serial_console_write(struct console *co, const char *s, serial_port_out(port, SCSCR, ctrl);
if (locked) - spin_unlock(&port->lock); - local_irq_restore(flags); + spin_unlock_irqrestore(&port->lock, flags); }
static int serial_console_setup(struct console *co, char *options)
On 2018-06-22 15:56:42 [+0200], Daniel Wagner wrote:
From: Daniel Wagner daniel.wagner@siemens.com
[ Upstream commit 8afb1d2c12163f77777f84616a8e9444d0050ebe ]
Commit 40f70c03e33a ("serial: sh-sci: add locking to console write function to avoid SMP lockup") copied the strategy to avoid locking problems in conjuncture with the console from the UART8250 driver. Instead using directly spin_{try}lock_irqsave(), local_irq_save() followed by spin_{try}lock() was used. While this is correct on mainline, for -rt it is a problem. spin_{try}lock() will check if it is running in a valid context. Since the local_irq_save() has already been executed, the context has changed and spin_{try}lock() will complain. The reason why spin_{try}lock() complains is that on -rt the spin locks are turned into mutexes and therefore can sleep. Sleeping with interrupts disabled is not valid.
This change only effects RT. Usually I don't bother Greg/stable team which changes which went upstream and help/fix RT but don't affect (as in fix) non-RT code. I usually add them to the RT queue and let RT-stable team pick it up. So I would suggest I pick it up for v4.16 and it gets then picked up the RT-stable team. Any objections?
Sebastian
On 06/22/2018 04:15 PM, Sebastian Andrzej Siewior wrote:
On 2018-06-22 15:56:42 [+0200], Daniel Wagner wrote:
From: Daniel Wagner daniel.wagner@siemens.com
[ Upstream commit 8afb1d2c12163f77777f84616a8e9444d0050ebe ]
Commit 40f70c03e33a ("serial: sh-sci: add locking to console write function to avoid SMP lockup") copied the strategy to avoid locking problems in conjuncture with the console from the UART8250 driver. Instead using directly spin_{try}lock_irqsave(), local_irq_save() followed by spin_{try}lock() was used. While this is correct on mainline, for -rt it is a problem. spin_{try}lock() will check if it is running in a valid context. Since the local_irq_save() has already been executed, the context has changed and spin_{try}lock() will complain. The reason why spin_{try}lock() complains is that on -rt the spin locks are turned into mutexes and therefore can sleep. Sleeping with interrupts disabled is not valid.
This change only effects RT. Usually I don't bother Greg/stable team which changes which went upstream and help/fix RT but don't affect (as in fix) non-RT code. I usually add them to the RT queue and let RT-stable team pick it up. So I would suggest I pick it up for v4.16 and it gets then picked up the RT-stable team. Any objections?
I don't mind for all non 4.4 versions. The 4.4 is special for me. Ben will merge the stable into the cip tree and I merge the -rt patches on top the cip-rt tree. That's why I would like to get the patch via the stable tree.
On 2018-06-22 16:26:48 [+0200], Daniel Wagner wrote:
On 06/22/2018 04:15 PM, Sebastian Andrzej Siewior wrote:
On 2018-06-22 15:56:42 [+0200], Daniel Wagner wrote:
From: Daniel Wagner daniel.wagner@siemens.com
[ Upstream commit 8afb1d2c12163f77777f84616a8e9444d0050ebe ]
Commit 40f70c03e33a ("serial: sh-sci: add locking to console write function to avoid SMP lockup") copied the strategy to avoid locking problems in conjuncture with the console from the UART8250 driver. Instead using directly spin_{try}lock_irqsave(), local_irq_save() followed by spin_{try}lock() was used. While this is correct on mainline, for -rt it is a problem. spin_{try}lock() will check if it is running in a valid context. Since the local_irq_save() has already been executed, the context has changed and spin_{try}lock() will complain. The reason why spin_{try}lock() complains is that on -rt the spin locks are turned into mutexes and therefore can sleep. Sleeping with interrupts disabled is not valid.
This change only effects RT. Usually I don't bother Greg/stable team which changes which went upstream and help/fix RT but don't affect (as in fix) non-RT code. I usually add them to the RT queue and let RT-stable team pick it up. So I would suggest I pick it up for v4.16 and it gets then picked up the RT-stable team. Any objections?
I don't mind for all non 4.4 versions. The 4.4 is special for me. Ben will merge the stable into the cip tree and I merge the -rt patches on top the cip-rt tree. That's why I would like to get the patch via the stable tree.
Does Ben need this patch in CIP?
Sebastian
Does Ben need this patch in CIP?
Renesas ported a lot of features back for the iwg20m board to the CIP tree. They have an strong interest to run the -rt spin on the CIP kernel too. I don't think Ben needs it but he also prefers the patch flow via the stable tree.
That said, I don't mind adding the patch only to my cip-rt tree if that's the correct way for -rt specific fixes.
On Fri, Jun 22, 2018 at 04:51:47PM +0200, Daniel Wagner wrote:
Does Ben need this patch in CIP?
Renesas ported a lot of features back for the iwg20m board to the CIP tree. They have an strong interest to run the -rt spin on the CIP kernel too. I don't think Ben needs it but he also prefers the patch flow via the stable tree.
That said, I don't mind adding the patch only to my cip-rt tree if that's the correct way for -rt specific fixes.
For any -rt specific fixes that are needed for those trees, I'll be glad to take them in the stable tree if it makes their life easier. No need for everyone to have to take the same fixes in lots of different trees, might as well share the load. For a patch like this, I don't object to taking it in my trees for that reason.
thanks,
greg k-h
On Fri, 22 Jun 2018 16:15:26 +0200 Sebastian Andrzej Siewior bigeasy@linutronix.de wrote:
This change only effects RT. Usually I don't bother Greg/stable team which changes which went upstream and help/fix RT but don't affect (as in fix) non-RT code. I usually add them to the RT queue and let RT-stable team pick it up. So I would suggest I pick it up for v4.16 and it gets then picked up the RT-stable team. Any objections?
I agree. This should be an RT stable only issue.
-- Steve
linux-stable-mirror@lists.linaro.org
-
Daniel Wagner -
Greg KH -
Sebastian Andrzej Siewior -
Steven Rostedt