* [PATCH tty/serial 1/1] tty: serial: imx: fix sysrq lockdep issue
@ 2019-11-22 10:00 Andy Duan
2019-11-22 10:33 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Andy Duan @ 2019-11-22 10:00 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, jslaby, Andy Duan, u.kleine-koenig, festevam
From: Fugang Duan <fugang.duan@nxp.com>
commit dbdda842fe96 ("printk: Add console owner and waiter logic to
load balance console writes") introduces the lockdep issue for imx
serial driver in sysrq case:
CPU0 CPU1
---- ----
lock(&port_lock_key);
lock(console_owner);
lock(&port_lock_key);
lock(console_owner);
It should unlock port_lock_key in handle_sysrq().
lockdep splats:
======================================================
WARNING: possible circular locking dependency detected
------------------------------------------------------
swapper/0/0 is trying to acquire lock:
c141e540 (console_owner){-...}, at: console_unlock+0x180/0x5e4
but task is already holding lock:
d864e450 (&port_lock_key){-.-.}, at: imx_uart_rxint+0x18/0x294
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&port_lock_key){-.-.}:
imx_uart_console_write+0x1b4/0x1dc
console_unlock+0x2a4/0x5e4
register_console+0x1ec/0x3b4
uart_add_one_port+0x480/0x4e8
platform_drv_probe+0x48/0x98
really_probe+0x1d8/0x34c
driver_probe_device+0x5c/0x168
device_driver_attach+0x58/0x60
__driver_attach+0x58/0xd0
bus_for_each_dev+0x74/0xbc
bus_add_driver+0x150/0x1dc
driver_register+0x74/0x108
imx_uart_init+0x20/0x40
do_one_initcall+0x84/0x348
kernel_init_freeable+0x11c/0x1e0
kernel_init+0x8/0x110
ret_from_fork+0x14/0x20
0x0
-> #0 (console_owner){-...}:
lock_acquire+0xcc/0x204
console_unlock+0x208/0x5e4
vprintk_emit+0x108/0x2c8
vprintk_default+0x20/0x28
printk+0x2c/0x58
__handle_sysrq+0x178/0x1f0
imx_uart_rxint+0x234/0x294
imx_uart_int+0x210/0x2f0
__handle_irq_event_percpu+0x48/0x34c
handle_irq_event_percpu+0x2c/0x88
handle_irq_event+0x38/0x5c
handle_fasteoi_irq+0xc8/0x180
generic_handle_irq+0x20/0x34
__handle_domain_irq+0x64/0xdc
gic_handle_irq+0x48/0x9c
__irq_svc+0x70/0x98
cpuidle_enter_state+0x16c/0x520
cpuidle_enter_state+0x16c/0x520
cpuidle_enter+0x28/0x38
do_idle+0x1dc/0x2b0
cpu_startup_entry+0x18/0x1c
start_kernel+0x404/0x4cc
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&port_lock_key);
lock(console_owner);
lock(&port_lock_key);
lock(console_owner);
*** DEADLOCK ***
3 locks held by swapper/0/0:
#0: d864e450 (&port_lock_key){-.-.}, at: imx_uart_rxint+0x18/0x294
#1: c141eb88 (rcu_read_lock){....}, at: __handle_sysrq+0x0/0x1f0
#2: c141e4c0 (console_lock){+.+.}, at: vprintk_emit+0xfc/0x2c8
stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[<c0112e30>] (unwind_backtrace) from [<c010cdc0>] (show_stack+0x10/0x14)
[<c010cdc0>] (show_stack) from [<c0d2a73c>] (dump_stack+0xe0/0x114)
[<c0d2a73c>] (dump_stack) from [<c018f744>] (check_noncircular+0x138/0x1ec)
[<c018f744>] (check_noncircular) from [<c0191730>] (__lock_acquire+0xe00/0x2418)
[<c0191730>] (__lock_acquire) from [<c01935c0>] (lock_acquire+0xcc/0x204)
[<c01935c0>] (lock_acquire) from [<c019e41c>] (console_unlock+0x208/0x5e4)
[<c019e41c>] (console_unlock) from [<c019fd38>] (vprintk_emit+0x108/0x2c8)
[<c019fd38>] (vprintk_emit) from [<c019ff18>] (vprintk_default+0x20/0x28)
[<c019ff18>] (vprintk_default) from [<c01a04e0>] (printk+0x2c/0x58)
[<c01a04e0>] (printk) from [<c061e990>] (__handle_sysrq+0x178/0x1f0)
[<c061e990>] (__handle_sysrq) from [<c0635cb0>] (imx_uart_rxint+0x234/0x294)
[<c0635cb0>] (imx_uart_rxint) from [<c06376a8>] (imx_uart_int+0x210/0x2f0)
[<c06376a8>] (imx_uart_int) from [<c01a1cdc>] (__handle_irq_event_percpu+0x48/0x34c)
[<c01a1cdc>] (__handle_irq_event_percpu) from [<c01a200c>] (handle_irq_event_percpu+0x2c/0x88)
[<c01a200c>] (handle_irq_event_percpu) from [<c01a20a0>] (handle_irq_event+0x38/0x5c)
[<c01a20a0>] (handle_irq_event) from [<c01a6930>] (handle_fasteoi_irq+0xc8/0x180)
[<c01a6930>] (handle_fasteoi_irq) from [<c01a0e10>] (generic_handle_irq+0x20/0x34)
[<c01a0e10>] (generic_handle_irq) from [<c01a1418>] (__handle_domain_irq+0x64/0xdc)
[<c01a1418>] (__handle_domain_irq) from [<c054bcd0>] (gic_handle_irq+0x48/0x9c)
[<c054bcd0>] (gic_handle_irq) from [<c0101a70>] (__irq_svc+0x70/0x98)
Exception stack(0xc1401ed8 to 0xc1401f20)
1ec0: 00000001 00000006
1ee0: 00000000 c140c600 00000001 5b0881e8 c140f934 db6eadb8 c1504668 0000098b
1f00: 0000098b 5c28eb3d 00000000 c1401f28 c0194520 c0938fcc 20070013 ffffffff
[<c0101a70>] (__irq_svc) from [<c0938fcc>] (cpuidle_enter_state+0x16c/0x520)
[<c0938fcc>] (cpuidle_enter_state) from [<c09393bc>] (cpuidle_enter+0x28/0x38)
[<c09393bc>] (cpuidle_enter) from [<c016f6a4>] (do_idle+0x1dc/0x2b0)
[<c016f6a4>] (do_idle) from [<c016fb2c>] (cpu_startup_entry+0x18/0x1c)
[<c016fb2c>] (cpu_startup_entry) from [<c1300e20>] (start_kernel+0x404/0x4cc)
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
drivers/tty/serial/imx.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index a9e20e6..a665439 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -736,6 +736,7 @@ static irqreturn_t imx_uart_rxint(int irq, void *dev_id)
while (imx_uart_readl(sport, USR2) & USR2_RDR) {
u32 usr2;
+ int sysrq;
flg = TTY_NORMAL;
sport->port.icount.rx++;
@@ -749,7 +750,11 @@ static irqreturn_t imx_uart_rxint(int irq, void *dev_id)
continue;
}
- if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx))
+ spin_unlock(&sport->port.lock);
+ sysrq = uart_handle_sysrq_char(&sport->port, (unsigned char)rx);
+ spin_lock(&sport->port.lock);
+
+ if (sysrq)
continue;
if (unlikely(rx & URXD_ERR)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH tty/serial 1/1] tty: serial: imx: fix sysrq lockdep issue
2019-11-22 10:00 [PATCH tty/serial 1/1] tty: serial: imx: fix sysrq lockdep issue Andy Duan
@ 2019-11-22 10:33 ` Uwe Kleine-König
2019-11-25 1:23 ` [EXT] " Andy Duan
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2019-11-22 10:33 UTC (permalink / raw)
To: Andy Duan
Cc: gregkh, linux-serial, jslaby, festevam, kernel,
Sergey Senozhatsky, Steven Rostedt, Petr Mladek
Hello,
On Fri, Nov 22, 2019 at 10:00:53AM +0000, Andy Duan wrote:
> From: Fugang Duan <fugang.duan@nxp.com>
>
> commit dbdda842fe96 ("printk: Add console owner and waiter logic to
> load balance console writes") introduces the lockdep issue for imx
> serial driver in sysrq case:
> CPU0 CPU1
> ---- ----
> lock(&port_lock_key);
> lock(console_owner);
> lock(&port_lock_key);
> lock(console_owner);
>
> It should unlock port_lock_key in handle_sysrq().
I already discussed this problem some time ago but then failed to
complete the topic. You might want to look at the old discussion, see
https://www.spinics.net/lists/kernel/msg3266353.html.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [EXT] Re: [PATCH tty/serial 1/1] tty: serial: imx: fix sysrq lockdep issue
2019-11-22 10:33 ` Uwe Kleine-König
@ 2019-11-25 1:23 ` Andy Duan
2019-11-26 9:53 ` Petr Mladek
0 siblings, 1 reply; 4+ messages in thread
From: Andy Duan @ 2019-11-25 1:23 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: gregkh, linux-serial, jslaby, festevam, kernel,
Sergey Senozhatsky, Steven Rostedt, Petr Mladek
From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Sent: Friday, November 22, 2019 6:34 PM
> Hello,
>
> On Fri, Nov 22, 2019 at 10:00:53AM +0000, Andy Duan wrote:
> > From: Fugang Duan <fugang.duan@nxp.com>
> >
> > commit dbdda842fe96 ("printk: Add console owner and waiter logic to
> > load balance console writes") introduces the lockdep issue for imx
> > serial driver in sysrq case:
> > CPU0 CPU1
> > ---- ----
> > lock(&port_lock_key);
> > lock(console_owner);
> > lock(&port_lock_key);
> > lock(console_owner);
> >
> > It should unlock port_lock_key in handle_sysrq().
>
> I already discussed this problem some time ago but then failed to complete
> the topic. You might want to look at the old discussion, see
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.s
> pinics.net%2Flists%2Fkernel%2Fmsg3266353.html&data=02%7C01%7Cf
> ugang.duan%40nxp.com%7C88047af87afa448bddaf08d76f377e8b%7C686ea
> 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637100156446083651&
> sdata=mLk%2BLEyiJjIuRlLs0STJpWJ8K7Q2uPa2fL44bcf2mgY%3D&reserv
> ed=0.
>
> Best regards
> Uwe
Thanks for point out the old discussion.
The issue seems exist all most of serial drivers. It is better to fix it on common code.
How about the next step, we hope to the lockdep issue is fixed ASAP.
Thanks!
Regards,
Fugang
>
> --
> Pengutronix e.K. | Uwe Kleine-König
> |
> Industrial Linux Solutions |
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> pengutronix.de%2F&data=02%7C01%7Cfugang.duan%40nxp.com%7C88
> 047af87afa448bddaf08d76f377e8b%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C0%7C637100156446093642&sdata=cCymbZXjFr6YgarayXHbn
> %2F2i8aI%2BLHOLM14A7ETCu28%3D&reserved=0 |
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [EXT] Re: [PATCH tty/serial 1/1] tty: serial: imx: fix sysrq lockdep issue
2019-11-25 1:23 ` [EXT] " Andy Duan
@ 2019-11-26 9:53 ` Petr Mladek
0 siblings, 0 replies; 4+ messages in thread
From: Petr Mladek @ 2019-11-26 9:53 UTC (permalink / raw)
To: Andy Duan
Cc: Uwe Kleine-König, gregkh, linux-serial, jslaby, festevam,
kernel, Sergey Senozhatsky, Steven Rostedt
On Mon 2019-11-25 01:23:14, Andy Duan wrote:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Sent: Friday, November 22, 2019 6:34 PM
> > Hello,
> >
> > On Fri, Nov 22, 2019 at 10:00:53AM +0000, Andy Duan wrote:
> > > From: Fugang Duan <fugang.duan@nxp.com>
> > >
> > > commit dbdda842fe96 ("printk: Add console owner and waiter logic to
> > > load balance console writes") introduces the lockdep issue for imx
> > > serial driver in sysrq case:
> > > CPU0 CPU1
> > > ---- ----
> > > lock(&port_lock_key);
> > > lock(console_owner);
> > > lock(&port_lock_key);
> > > lock(console_owner);
> > >
> > > It should unlock port_lock_key in handle_sysrq().
> >
> > I already discussed this problem some time ago but then failed to complete
> > the topic. You might want to look at the old discussion, see
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.s
> > pinics.net%2Flists%2Fkernel%2Fmsg3266353.html&data=02%7C01%7Cf
> > ugang.duan%40nxp.com%7C88047af87afa448bddaf08d76f377e8b%7C686ea
> > 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637100156446083651&
> > sdata=mLk%2BLEyiJjIuRlLs0STJpWJ8K7Q2uPa2fL44bcf2mgY%3D&reserv
> > ed=0.
> >
> > Best regards
> > Uwe
>
> Thanks for point out the old discussion.
> The issue seems exist all most of serial drivers. It is better to fix it on common code.
Do you have an idea how to fix this in a common code, please?
I am afraid that all affected drivers need to be fixed separately
as discussed in the above mentioned thread.
> How about the next step, we hope to the lockdep issue is fixed ASAP.
> Thanks!
I would prefer if
+ uart_prepare_sysrq_char() gets renamed to uart_store_sysrq_char()
+ uart_unlock_and_check_sysrq() gets replaced with:
sysrq_ch = uart_get_sysrq_char(port);
spin_unlock(&port->lock);
if (sysrq_ch)
handle_sysrq(sysrq_ch);
as metnioned in
https://lore.kernel.org/lkml/20190926085855.debu7t46s7kgb26p@pathway.suse.cz/
Would you like to prepare the patchset, please?
Best Regards,
Petr
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-26 9:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-22 10:00 [PATCH tty/serial 1/1] tty: serial: imx: fix sysrq lockdep issue Andy Duan
2019-11-22 10:33 ` Uwe Kleine-König
2019-11-25 1:23 ` [EXT] " Andy Duan
2019-11-26 9:53 ` Petr Mladek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).