| Message ID | 20241209124732.693834-1-xiaolei.wang@windriver.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | serial: imx: Use uart_port_lock_irqsave() instead of uart_port_lock() | expand |
Am Montag, dem 09.12.2024 um 20:47 +0800 schrieb Xiaolei Wang: > When executing 'ehco mem > /sys/power/state', the following > deadlock occurs. Since there is output during the serial > port entering the suspend process, the suspend will be > interrupted, resulting in the nesting of locks. Therefore, > use uart_port_lock_irqsave instead of uart_port_unlock. > > WARNING: inconsistent lock state > 6.12.0-rc2-00002-g3c199ed5bd64-dirty #23 Not tainted > -------------------------------- > inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage. > sh/494 [HC0[0]:SC0[0]:HE1:SE1] takes: > c4db5850 (&port_lock_key){?.-.}-{3:3}, at: imx_uart_enable_wakeup+0x14/0x254 > {IN-HARDIRQ-W} state was registered at: > lock_acquire+0x104/0x348 > _raw_spin_lock+0x48/0x84 > imx_uart_int+0x14/0x4dc > __handle_irq_event_percpu+0xac/0x2fc > handle_irq_event_percpu+0xc/0x40 > handle_irq_event+0x38/0x8c > handle_fasteoi_irq+0xb4/0x1b8 > handle_irq_desc+0x1c/0x2c > gic_handle_irq+0x6c/0xa0 > generic_handle_arch_irq+0x2c/0x64 > call_with_stack+0x18/0x20 > __irq_svc+0x9c/0xbc > _raw_spin_unlock_irqrestore+0x2c/0x48 > uart_write+0xd8/0x3a0 > do_output_char+0x1a8/0x1e4 > n_tty_write+0x224/0x440 > file_tty_write.constprop.0+0x124/0x250 > do_iter_readv_writev+0x100/0x1e0 > vfs_writev+0xc4/0x448 > do_writev+0x68/0xf8 > ret_fast_syscall+0x0/0x1c > irq event stamp: 31593 > hardirqs last enabled at (31593): [<c1150e48>] _raw_spin_unlock_irqrestore+0x44/0x48 > hardirqs last disabled at (31592): [<c07f32f0>] clk_enable_lock+0x60/0x120 > softirqs last enabled at (30334): [<c012d1d4>] handle_softirqs+0x2cc/0x478 > softirqs last disabled at (30325): [<c012d510>] __irq_exit_rcu+0x120/0x15c > > other info that might help us debug this: > Possible unsafe locking scenario: > > CPU0 > ---- > lock(&port_lock_key); > <Interrupt> > lock(&port_lock_key); > > Fixes: 3c199ed5bd64 ("serial: imx: Grab port lock in imx_uart_enable_wakeup()") > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com> > --- > drivers/tty/serial/imx.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 17f70e4bee43..4470bcb3ef19 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -2691,8 +2691,9 @@ static void imx_uart_save_context(struct imx_port *sport) > static void imx_uart_enable_wakeup(struct imx_port *sport, bool on) > { > u32 ucr3; > + unsigned long flags; > > - uart_port_lock(&sport->port); > + uart_port_lock_irqsave(&sport->port, &flags); This function is only called from process context, so it should be fine to use uart_port_lock_irq(). Regards, Lucas > > ucr3 = imx_uart_readl(sport, UCR3); > if (on) { > @@ -2714,7 +2715,7 @@ static void imx_uart_enable_wakeup(struct imx_port *sport, bool on) > imx_uart_writel(sport, ucr1, UCR1); > } > > - uart_port_unlock(&sport->port); > + uart_port_unlock_irqrestore(&sport->port, flags); > } > > static int imx_uart_suspend_noirq(struct device *dev)
On 12/10/24 1:37 AM, Lucas Stach wrote: > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and know the content is safe. > > Am Montag, dem 09.12.2024 um 20:47 +0800 schrieb Xiaolei Wang: >> When executing 'ehco mem > /sys/power/state', the following >> deadlock occurs. Since there is output during the serial >> port entering the suspend process, the suspend will be >> interrupted, resulting in the nesting of locks. Therefore, >> use uart_port_lock_irqsave instead of uart_port_unlock. >> >> WARNING: inconsistent lock state >> 6.12.0-rc2-00002-g3c199ed5bd64-dirty #23 Not tainted >> -------------------------------- >> inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage. >> sh/494 [HC0[0]:SC0[0]:HE1:SE1] takes: >> c4db5850 (&port_lock_key){?.-.}-{3:3}, at: imx_uart_enable_wakeup+0x14/0x254 >> {IN-HARDIRQ-W} state was registered at: >> lock_acquire+0x104/0x348 >> _raw_spin_lock+0x48/0x84 >> imx_uart_int+0x14/0x4dc >> __handle_irq_event_percpu+0xac/0x2fc >> handle_irq_event_percpu+0xc/0x40 >> handle_irq_event+0x38/0x8c >> handle_fasteoi_irq+0xb4/0x1b8 >> handle_irq_desc+0x1c/0x2c >> gic_handle_irq+0x6c/0xa0 >> generic_handle_arch_irq+0x2c/0x64 >> call_with_stack+0x18/0x20 >> __irq_svc+0x9c/0xbc >> _raw_spin_unlock_irqrestore+0x2c/0x48 >> uart_write+0xd8/0x3a0 >> do_output_char+0x1a8/0x1e4 >> n_tty_write+0x224/0x440 >> file_tty_write.constprop.0+0x124/0x250 >> do_iter_readv_writev+0x100/0x1e0 >> vfs_writev+0xc4/0x448 >> do_writev+0x68/0xf8 >> ret_fast_syscall+0x0/0x1c >> irq event stamp: 31593 >> hardirqs last enabled at (31593): [<c1150e48>] _raw_spin_unlock_irqrestore+0x44/0x48 >> hardirqs last disabled at (31592): [<c07f32f0>] clk_enable_lock+0x60/0x120 >> softirqs last enabled at (30334): [<c012d1d4>] handle_softirqs+0x2cc/0x478 >> softirqs last disabled at (30325): [<c012d510>] __irq_exit_rcu+0x120/0x15c >> >> other info that might help us debug this: >> Possible unsafe locking scenario: >> >> CPU0 >> ---- >> lock(&port_lock_key); >> <Interrupt> >> lock(&port_lock_key); >> >> Fixes: 3c199ed5bd64 ("serial: imx: Grab port lock in imx_uart_enable_wakeup()") >> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com> >> --- >> drivers/tty/serial/imx.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c >> index 17f70e4bee43..4470bcb3ef19 100644 >> --- a/drivers/tty/serial/imx.c >> +++ b/drivers/tty/serial/imx.c >> @@ -2691,8 +2691,9 @@ static void imx_uart_save_context(struct imx_port *sport) >> static void imx_uart_enable_wakeup(struct imx_port *sport, bool on) >> { >> u32 ucr3; >> + unsigned long flags; >> >> - uart_port_lock(&sport->port); >> + uart_port_lock_irqsave(&sport->port, &flags); > This function is only called from process context, so it should be fine > to use uart_port_lock_irq(). Thank you for your reminder, Lucas, I will send a new version thanks xiaolei > > Regards, > Lucas > >> ucr3 = imx_uart_readl(sport, UCR3); >> if (on) { >> @@ -2714,7 +2715,7 @@ static void imx_uart_enable_wakeup(struct imx_port *sport, bool on) >> imx_uart_writel(sport, ucr1, UCR1); >> } >> >> - uart_port_unlock(&sport->port); >> + uart_port_unlock_irqrestore(&sport->port, flags); >> } >> >> static int imx_uart_suspend_noirq(struct device *dev)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 17f70e4bee43..4470bcb3ef19 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2691,8 +2691,9 @@ static void imx_uart_save_context(struct imx_port *sport) static void imx_uart_enable_wakeup(struct imx_port *sport, bool on) { u32 ucr3; + unsigned long flags; - uart_port_lock(&sport->port); + uart_port_lock_irqsave(&sport->port, &flags); ucr3 = imx_uart_readl(sport, UCR3); if (on) { @@ -2714,7 +2715,7 @@ static void imx_uart_enable_wakeup(struct imx_port *sport, bool on) imx_uart_writel(sport, ucr1, UCR1); } - uart_port_unlock(&sport->port); + uart_port_unlock_irqrestore(&sport->port, flags); } static int imx_uart_suspend_noirq(struct device *dev)
When executing 'ehco mem > /sys/power/state', the following deadlock occurs. Since there is output during the serial port entering the suspend process, the suspend will be interrupted, resulting in the nesting of locks. Therefore, use uart_port_lock_irqsave instead of uart_port_unlock. WARNING: inconsistent lock state 6.12.0-rc2-00002-g3c199ed5bd64-dirty #23 Not tainted -------------------------------- inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage. sh/494 [HC0[0]:SC0[0]:HE1:SE1] takes: c4db5850 (&port_lock_key){?.-.}-{3:3}, at: imx_uart_enable_wakeup+0x14/0x254 {IN-HARDIRQ-W} state was registered at: lock_acquire+0x104/0x348 _raw_spin_lock+0x48/0x84 imx_uart_int+0x14/0x4dc __handle_irq_event_percpu+0xac/0x2fc handle_irq_event_percpu+0xc/0x40 handle_irq_event+0x38/0x8c handle_fasteoi_irq+0xb4/0x1b8 handle_irq_desc+0x1c/0x2c gic_handle_irq+0x6c/0xa0 generic_handle_arch_irq+0x2c/0x64 call_with_stack+0x18/0x20 __irq_svc+0x9c/0xbc _raw_spin_unlock_irqrestore+0x2c/0x48 uart_write+0xd8/0x3a0 do_output_char+0x1a8/0x1e4 n_tty_write+0x224/0x440 file_tty_write.constprop.0+0x124/0x250 do_iter_readv_writev+0x100/0x1e0 vfs_writev+0xc4/0x448 do_writev+0x68/0xf8 ret_fast_syscall+0x0/0x1c irq event stamp: 31593 hardirqs last enabled at (31593): [<c1150e48>] _raw_spin_unlock_irqrestore+0x44/0x48 hardirqs last disabled at (31592): [<c07f32f0>] clk_enable_lock+0x60/0x120 softirqs last enabled at (30334): [<c012d1d4>] handle_softirqs+0x2cc/0x478 softirqs last disabled at (30325): [<c012d510>] __irq_exit_rcu+0x120/0x15c other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&port_lock_key); <Interrupt> lock(&port_lock_key); Fixes: 3c199ed5bd64 ("serial: imx: Grab port lock in imx_uart_enable_wakeup()") Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com> --- drivers/tty/serial/imx.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)