* [PATCH 1/1] tty: serial: owl: Initialize lock before registering port
@ 2020-05-29 11:06 Cristian Ciocaltea
2020-05-29 11:34 ` Greg Kroah-Hartman
2020-05-29 11:35 ` Andy Shevchenko
0 siblings, 2 replies; 11+ messages in thread
From: Cristian Ciocaltea @ 2020-05-29 11:06 UTC (permalink / raw)
To: Andreas Färber, Manivannan Sadhasivam, Greg Kroah-Hartman,
Jiri Slaby, Andy Shevchenko
Cc: linux-serial, linux-arm-kernel, linux-kernel, linux-actions
Running a lockdep-enabled kernel leads to the following splat when
probing the owl-uart driver:
[ 1.271784] b0124000.serial: ttyOWL2 at MMIO 0xb0124000 (irq = 22, base_baud = 1500000) is a owl-uart
[ 1.281226] INFO: trying to register non-static key.
[ 1.286179] the code is fine but needs lockdep annotation.
[ 1.291648] turning off the locking correctness validator.
[ 1.297125] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc7+ #70
[ 1.303462] Hardware name: Generic DT based system
[ 1.308268] [<80111d3c>] (unwind_backtrace) from [<8010c9b8>] (show_stack+0x10/0x14)
[ 1.316003] [<8010c9b8>] (show_stack) from [<805016b4>] (dump_stack+0xb4/0xe0)
[ 1.323218] [<805016b4>] (dump_stack) from [<80182dec>] (register_lock_class+0x25c/0x8f4)
[ 1.331391] [<80182dec>] (register_lock_class) from [<8017ee34>] (__lock_acquire+0xb4/0x2ee4)
[ 1.339901] [<8017ee34>] (__lock_acquire) from [<8018291c>] (lock_acquire+0x424/0x4c8)
[ 1.347813] [<8018291c>] (lock_acquire) from [<808597b0>] (_raw_spin_lock_irqsave+0x54/0x68)
[ 1.356242] [<808597b0>] (_raw_spin_lock_irqsave) from [<80582e94>] (uart_add_one_port+0x384/0x510)
[ 1.365276] [<80582e94>] (uart_add_one_port) from [<8058b4d0>] (owl_uart_probe+0x1bc/0x248)
[ 1.373621] [<8058b4d0>] (owl_uart_probe) from [<8059c0e4>] (platform_drv_probe+0x48/0x94)
[ 1.381874] [<8059c0e4>] (platform_drv_probe) from [<805997c4>] (really_probe+0x200/0x470)
[ 1.390123] [<805997c4>] (really_probe) from [<80599c80>] (driver_probe_device+0x16c/0x1bc)
[ 1.398461] [<80599c80>] (driver_probe_device) from [<80599f18>] (device_driver_attach+0x44/0x60)
[ 1.407317] [<80599f18>] (device_driver_attach) from [<8059a078>] (__driver_attach+0x144/0x14c)
[ 1.416000] [<8059a078>] (__driver_attach) from [<805978ac>] (bus_for_each_dev+0x5c/0xb4)
[ 1.424162] [<805978ac>] (bus_for_each_dev) from [<8059896c>] (bus_add_driver+0x118/0x204)
[ 1.432410] [<8059896c>] (bus_add_driver) from [<8059ae6c>] (driver_register+0xbc/0xf8)
[ 1.440405] [<8059ae6c>] (driver_register) from [<80c1fd24>] (owl_uart_init+0x20/0x40)
[ 1.448313] [<80c1fd24>] (owl_uart_init) from [<801021d8>] (do_one_initcall+0x184/0x3a4)
[ 1.456399] [<801021d8>] (do_one_initcall) from [<80c01100>] (kernel_init_freeable+0x264/0x2e4)
[ 1.465085] [<80c01100>] (kernel_init_freeable) from [<80850a88>] (kernel_init+0x8/0x110)
[ 1.473249] [<80850a88>] (kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20)
[ 1.480800] Exception stack(0xee8bdfb0 to 0xee8bdff8)
[ 1.485841] dfa0: 00000000 00000000 00000000 00000000
[ 1.494002] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 1.502162] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 1.508914] printk: console [ttyOWL2] enabled
The locking issue occurs in uart_configure_port() when trying to
guard the call to set_mctrl().
uart_add_one_port() should normally initialize the spinlock via
uart_port_spin_lock_init(), but it never happens because the port is
detected as a console and, as a consequence, the spinlock is expected
to be already initialized.
The commit a3cb39d258ef
("serial: core: Allow detach and attach serial device for console")
changed the lock initialization logic to assume the spinlock is
initialized even if the console is not enabled.
Therefore, initialize the lock explicitly in owl_uart_probe(), before
attempting to invoke uart_add_one_port().
Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
---
drivers/tty/serial/owl-uart.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c
index c149f8c30007..c2fa2f15d50a 100644
--- a/drivers/tty/serial/owl-uart.c
+++ b/drivers/tty/serial/owl-uart.c
@@ -705,6 +705,8 @@ static int owl_uart_probe(struct platform_device *pdev)
owl_uart_ports[pdev->id] = owl_port;
platform_set_drvdata(pdev, owl_port);
+ spin_lock_init(&owl_port->port.lock);
+
ret = uart_add_one_port(&owl_uart_driver, &owl_port->port);
if (ret)
owl_uart_ports[pdev->id] = NULL;
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-05-29 11:06 [PATCH 1/1] tty: serial: owl: Initialize lock before registering port Cristian Ciocaltea @ 2020-05-29 11:34 ` Greg Kroah-Hartman 2020-05-29 11:53 ` Andreas Färber 2020-07-10 12:05 ` Cristian Ciocaltea 2020-05-29 11:35 ` Andy Shevchenko 1 sibling, 2 replies; 11+ messages in thread From: Greg Kroah-Hartman @ 2020-05-29 11:34 UTC (permalink / raw) To: Cristian Ciocaltea Cc: Andreas Färber, Manivannan Sadhasivam, Jiri Slaby, Andy Shevchenko, linux-serial, linux-arm-kernel, linux-kernel, linux-actions On Fri, May 29, 2020 at 02:06:47PM +0300, Cristian Ciocaltea wrote: > Running a lockdep-enabled kernel leads to the following splat when > probing the owl-uart driver: > > [ 1.271784] b0124000.serial: ttyOWL2 at MMIO 0xb0124000 (irq = 22, base_baud = 1500000) is a owl-uart > [ 1.281226] INFO: trying to register non-static key. > [ 1.286179] the code is fine but needs lockdep annotation. > [ 1.291648] turning off the locking correctness validator. > [ 1.297125] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc7+ #70 > [ 1.303462] Hardware name: Generic DT based system > [ 1.308268] [<80111d3c>] (unwind_backtrace) from [<8010c9b8>] (show_stack+0x10/0x14) > [ 1.316003] [<8010c9b8>] (show_stack) from [<805016b4>] (dump_stack+0xb4/0xe0) > [ 1.323218] [<805016b4>] (dump_stack) from [<80182dec>] (register_lock_class+0x25c/0x8f4) > [ 1.331391] [<80182dec>] (register_lock_class) from [<8017ee34>] (__lock_acquire+0xb4/0x2ee4) > [ 1.339901] [<8017ee34>] (__lock_acquire) from [<8018291c>] (lock_acquire+0x424/0x4c8) > [ 1.347813] [<8018291c>] (lock_acquire) from [<808597b0>] (_raw_spin_lock_irqsave+0x54/0x68) > [ 1.356242] [<808597b0>] (_raw_spin_lock_irqsave) from [<80582e94>] (uart_add_one_port+0x384/0x510) > [ 1.365276] [<80582e94>] (uart_add_one_port) from [<8058b4d0>] (owl_uart_probe+0x1bc/0x248) > [ 1.373621] [<8058b4d0>] (owl_uart_probe) from [<8059c0e4>] (platform_drv_probe+0x48/0x94) > [ 1.381874] [<8059c0e4>] (platform_drv_probe) from [<805997c4>] (really_probe+0x200/0x470) > [ 1.390123] [<805997c4>] (really_probe) from [<80599c80>] (driver_probe_device+0x16c/0x1bc) > [ 1.398461] [<80599c80>] (driver_probe_device) from [<80599f18>] (device_driver_attach+0x44/0x60) > [ 1.407317] [<80599f18>] (device_driver_attach) from [<8059a078>] (__driver_attach+0x144/0x14c) > [ 1.416000] [<8059a078>] (__driver_attach) from [<805978ac>] (bus_for_each_dev+0x5c/0xb4) > [ 1.424162] [<805978ac>] (bus_for_each_dev) from [<8059896c>] (bus_add_driver+0x118/0x204) > [ 1.432410] [<8059896c>] (bus_add_driver) from [<8059ae6c>] (driver_register+0xbc/0xf8) > [ 1.440405] [<8059ae6c>] (driver_register) from [<80c1fd24>] (owl_uart_init+0x20/0x40) > [ 1.448313] [<80c1fd24>] (owl_uart_init) from [<801021d8>] (do_one_initcall+0x184/0x3a4) > [ 1.456399] [<801021d8>] (do_one_initcall) from [<80c01100>] (kernel_init_freeable+0x264/0x2e4) > [ 1.465085] [<80c01100>] (kernel_init_freeable) from [<80850a88>] (kernel_init+0x8/0x110) > [ 1.473249] [<80850a88>] (kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20) > [ 1.480800] Exception stack(0xee8bdfb0 to 0xee8bdff8) > [ 1.485841] dfa0: 00000000 00000000 00000000 00000000 > [ 1.494002] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > [ 1.502162] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > [ 1.508914] printk: console [ttyOWL2] enabled > > The locking issue occurs in uart_configure_port() when trying to > guard the call to set_mctrl(). > > uart_add_one_port() should normally initialize the spinlock via > uart_port_spin_lock_init(), but it never happens because the port is > detected as a console and, as a consequence, the spinlock is expected > to be already initialized. > > The commit a3cb39d258ef > ("serial: core: Allow detach and attach serial device for console") > changed the lock initialization logic to assume the spinlock is > initialized even if the console is not enabled. > > Therefore, initialize the lock explicitly in owl_uart_probe(), before > attempting to invoke uart_add_one_port(). > > Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > --- > drivers/tty/serial/owl-uart.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c > index c149f8c30007..c2fa2f15d50a 100644 > --- a/drivers/tty/serial/owl-uart.c > +++ b/drivers/tty/serial/owl-uart.c > @@ -705,6 +705,8 @@ static int owl_uart_probe(struct platform_device *pdev) > owl_uart_ports[pdev->id] = owl_port; > platform_set_drvdata(pdev, owl_port); > > + spin_lock_init(&owl_port->port.lock); > + > ret = uart_add_one_port(&owl_uart_driver, &owl_port->port); > if (ret) > owl_uart_ports[pdev->id] = NULL; Ugh, another one :( Thanks for this, will queue this up now. greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-05-29 11:34 ` Greg Kroah-Hartman @ 2020-05-29 11:53 ` Andreas Färber 2020-05-29 12:03 ` Andy Shevchenko 2020-07-10 12:05 ` Cristian Ciocaltea 1 sibling, 1 reply; 11+ messages in thread From: Andreas Färber @ 2020-05-29 11:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Cristian Ciocaltea Cc: Manivannan Sadhasivam, Jiri Slaby, Andy Shevchenko, linux-serial, linux-arm-kernel, linux-kernel, linux-actions Am 29.05.20 um 13:34 schrieb Greg Kroah-Hartman: > On Fri, May 29, 2020 at 02:06:47PM +0300, Cristian Ciocaltea wrote: >> Running a lockdep-enabled kernel leads to the following splat when >> probing the owl-uart driver: >> >> [ 1.271784] b0124000.serial: ttyOWL2 at MMIO 0xb0124000 (irq = 22, base_baud = 1500000) is a owl-uart >> [ 1.281226] INFO: trying to register non-static key. >> [ 1.286179] the code is fine but needs lockdep annotation. >> [ 1.291648] turning off the locking correctness validator. >> [ 1.297125] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc7+ #70 >> [ 1.303462] Hardware name: Generic DT based system >> [ 1.308268] [<80111d3c>] (unwind_backtrace) from [<8010c9b8>] (show_stack+0x10/0x14) >> [ 1.316003] [<8010c9b8>] (show_stack) from [<805016b4>] (dump_stack+0xb4/0xe0) >> [ 1.323218] [<805016b4>] (dump_stack) from [<80182dec>] (register_lock_class+0x25c/0x8f4) >> [ 1.331391] [<80182dec>] (register_lock_class) from [<8017ee34>] (__lock_acquire+0xb4/0x2ee4) >> [ 1.339901] [<8017ee34>] (__lock_acquire) from [<8018291c>] (lock_acquire+0x424/0x4c8) >> [ 1.347813] [<8018291c>] (lock_acquire) from [<808597b0>] (_raw_spin_lock_irqsave+0x54/0x68) >> [ 1.356242] [<808597b0>] (_raw_spin_lock_irqsave) from [<80582e94>] (uart_add_one_port+0x384/0x510) >> [ 1.365276] [<80582e94>] (uart_add_one_port) from [<8058b4d0>] (owl_uart_probe+0x1bc/0x248) >> [ 1.373621] [<8058b4d0>] (owl_uart_probe) from [<8059c0e4>] (platform_drv_probe+0x48/0x94) >> [ 1.381874] [<8059c0e4>] (platform_drv_probe) from [<805997c4>] (really_probe+0x200/0x470) >> [ 1.390123] [<805997c4>] (really_probe) from [<80599c80>] (driver_probe_device+0x16c/0x1bc) >> [ 1.398461] [<80599c80>] (driver_probe_device) from [<80599f18>] (device_driver_attach+0x44/0x60) >> [ 1.407317] [<80599f18>] (device_driver_attach) from [<8059a078>] (__driver_attach+0x144/0x14c) >> [ 1.416000] [<8059a078>] (__driver_attach) from [<805978ac>] (bus_for_each_dev+0x5c/0xb4) >> [ 1.424162] [<805978ac>] (bus_for_each_dev) from [<8059896c>] (bus_add_driver+0x118/0x204) >> [ 1.432410] [<8059896c>] (bus_add_driver) from [<8059ae6c>] (driver_register+0xbc/0xf8) >> [ 1.440405] [<8059ae6c>] (driver_register) from [<80c1fd24>] (owl_uart_init+0x20/0x40) >> [ 1.448313] [<80c1fd24>] (owl_uart_init) from [<801021d8>] (do_one_initcall+0x184/0x3a4) >> [ 1.456399] [<801021d8>] (do_one_initcall) from [<80c01100>] (kernel_init_freeable+0x264/0x2e4) >> [ 1.465085] [<80c01100>] (kernel_init_freeable) from [<80850a88>] (kernel_init+0x8/0x110) >> [ 1.473249] [<80850a88>] (kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20) >> [ 1.480800] Exception stack(0xee8bdfb0 to 0xee8bdff8) >> [ 1.485841] dfa0: 00000000 00000000 00000000 00000000 >> [ 1.494002] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >> [ 1.502162] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 >> [ 1.508914] printk: console [ttyOWL2] enabled >> >> The locking issue occurs in uart_configure_port() when trying to >> guard the call to set_mctrl(). >> >> uart_add_one_port() should normally initialize the spinlock via >> uart_port_spin_lock_init(), but it never happens because the port is >> detected as a console and, as a consequence, the spinlock is expected >> to be already initialized. >> >> The commit a3cb39d258ef >> ("serial: core: Allow detach and attach serial device for console") >> changed the lock initialization logic to assume the spinlock is >> initialized even if the console is not enabled. >> >> Therefore, initialize the lock explicitly in owl_uart_probe(), before >> attempting to invoke uart_add_one_port(). >> >> Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> >> --- >> drivers/tty/serial/owl-uart.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c >> index c149f8c30007..c2fa2f15d50a 100644 >> --- a/drivers/tty/serial/owl-uart.c >> +++ b/drivers/tty/serial/owl-uart.c >> @@ -705,6 +705,8 @@ static int owl_uart_probe(struct platform_device *pdev) >> owl_uart_ports[pdev->id] = owl_port; >> platform_set_drvdata(pdev, owl_port); >> >> + spin_lock_init(&owl_port->port.lock); >> + >> ret = uart_add_one_port(&owl_uart_driver, &owl_port->port); >> if (ret) >> owl_uart_ports[pdev->id] = NULL; > > Ugh, another one :( > > Thanks for this, will queue this up now. Thanks. If this is the expected pattern now, I'll also have to update in-flight patches, such as Sunplus. Regards, Andreas -- SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-05-29 11:53 ` Andreas Färber @ 2020-05-29 12:03 ` Andy Shevchenko 0 siblings, 0 replies; 11+ messages in thread From: Andy Shevchenko @ 2020-05-29 12:03 UTC (permalink / raw) To: Andreas Färber Cc: Greg Kroah-Hartman, Cristian Ciocaltea, Manivannan Sadhasivam, Jiri Slaby, Andy Shevchenko, open list:SERIAL DRIVERS, linux-arm Mailing List, Linux Kernel Mailing List, linux-actions On Fri, May 29, 2020 at 2:56 PM Andreas Färber <afaerber@suse.de> wrote: > > Am 29.05.20 um 13:34 schrieb Greg Kroah-Hartman: > > On Fri, May 29, 2020 at 02:06:47PM +0300, Cristian Ciocaltea wrote: > >> Running a lockdep-enabled kernel leads to the following splat when > >> probing the owl-uart driver: ... > >> Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") > >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > >> + spin_lock_init(&owl_port->port.lock); > >> + > >> ret = uart_add_one_port(&owl_uart_driver, &owl_port->port); > >> if (ret) > >> owl_uart_ports[pdev->id] = NULL; > > > > Ugh, another one :( > > > > Thanks for this, will queue this up now. > > Thanks. If this is the expected pattern now, I'll also have to update > in-flight patches, such as Sunplus. Expected is to register properly console via register_console() call. If you do so, you also need to initialize spin lock before coming to uart_add_one_port(). It seems drivers (ab)used that feature. $ git grep -n -w register_console -- drivers/tty/serial/ | cut -f1 -d: | sort -u | wc -l 37 $ git grep -n _CONSOLE -- drivers/tty/serial/ | cut -f1 -d: | sort -u | wc -l 77 But as a quick fix this initialization is okay. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-05-29 11:34 ` Greg Kroah-Hartman 2020-05-29 11:53 ` Andreas Färber @ 2020-07-10 12:05 ` Cristian Ciocaltea 2020-07-10 13:35 ` Greg Kroah-Hartman 1 sibling, 1 reply; 11+ messages in thread From: Cristian Ciocaltea @ 2020-07-10 12:05 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Andreas Färber, Manivannan Sadhasivam, Jiri Slaby, Andy Shevchenko, linux-serial, linux-arm-kernel, linux-kernel, linux-actions On Fri, May 29, 2020 at 01:34:19PM +0200, Greg Kroah-Hartman wrote: > On Fri, May 29, 2020 at 02:06:47PM +0300, Cristian Ciocaltea wrote: > > Running a lockdep-enabled kernel leads to the following splat when > > probing the owl-uart driver: > > > > [ 1.271784] b0124000.serial: ttyOWL2 at MMIO 0xb0124000 (irq = 22, base_baud = 1500000) is a owl-uart > > [ 1.281226] INFO: trying to register non-static key. > > [ 1.286179] the code is fine but needs lockdep annotation. > > [ 1.291648] turning off the locking correctness validator. > > [ 1.297125] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc7+ #70 > > [ 1.303462] Hardware name: Generic DT based system > > [ 1.308268] [<80111d3c>] (unwind_backtrace) from [<8010c9b8>] (show_stack+0x10/0x14) > > [ 1.316003] [<8010c9b8>] (show_stack) from [<805016b4>] (dump_stack+0xb4/0xe0) > > [ 1.323218] [<805016b4>] (dump_stack) from [<80182dec>] (register_lock_class+0x25c/0x8f4) > > [ 1.331391] [<80182dec>] (register_lock_class) from [<8017ee34>] (__lock_acquire+0xb4/0x2ee4) > > [ 1.339901] [<8017ee34>] (__lock_acquire) from [<8018291c>] (lock_acquire+0x424/0x4c8) > > [ 1.347813] [<8018291c>] (lock_acquire) from [<808597b0>] (_raw_spin_lock_irqsave+0x54/0x68) > > [ 1.356242] [<808597b0>] (_raw_spin_lock_irqsave) from [<80582e94>] (uart_add_one_port+0x384/0x510) > > [ 1.365276] [<80582e94>] (uart_add_one_port) from [<8058b4d0>] (owl_uart_probe+0x1bc/0x248) > > [ 1.373621] [<8058b4d0>] (owl_uart_probe) from [<8059c0e4>] (platform_drv_probe+0x48/0x94) > > [ 1.381874] [<8059c0e4>] (platform_drv_probe) from [<805997c4>] (really_probe+0x200/0x470) > > [ 1.390123] [<805997c4>] (really_probe) from [<80599c80>] (driver_probe_device+0x16c/0x1bc) > > [ 1.398461] [<80599c80>] (driver_probe_device) from [<80599f18>] (device_driver_attach+0x44/0x60) > > [ 1.407317] [<80599f18>] (device_driver_attach) from [<8059a078>] (__driver_attach+0x144/0x14c) > > [ 1.416000] [<8059a078>] (__driver_attach) from [<805978ac>] (bus_for_each_dev+0x5c/0xb4) > > [ 1.424162] [<805978ac>] (bus_for_each_dev) from [<8059896c>] (bus_add_driver+0x118/0x204) > > [ 1.432410] [<8059896c>] (bus_add_driver) from [<8059ae6c>] (driver_register+0xbc/0xf8) > > [ 1.440405] [<8059ae6c>] (driver_register) from [<80c1fd24>] (owl_uart_init+0x20/0x40) > > [ 1.448313] [<80c1fd24>] (owl_uart_init) from [<801021d8>] (do_one_initcall+0x184/0x3a4) > > [ 1.456399] [<801021d8>] (do_one_initcall) from [<80c01100>] (kernel_init_freeable+0x264/0x2e4) > > [ 1.465085] [<80c01100>] (kernel_init_freeable) from [<80850a88>] (kernel_init+0x8/0x110) > > [ 1.473249] [<80850a88>] (kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20) > > [ 1.480800] Exception stack(0xee8bdfb0 to 0xee8bdff8) > > [ 1.485841] dfa0: 00000000 00000000 00000000 00000000 > > [ 1.494002] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > > [ 1.502162] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > > [ 1.508914] printk: console [ttyOWL2] enabled > > > > The locking issue occurs in uart_configure_port() when trying to > > guard the call to set_mctrl(). > > > > uart_add_one_port() should normally initialize the spinlock via > > uart_port_spin_lock_init(), but it never happens because the port is > > detected as a console and, as a consequence, the spinlock is expected > > to be already initialized. > > > > The commit a3cb39d258ef > > ("serial: core: Allow detach and attach serial device for console") > > changed the lock initialization logic to assume the spinlock is > > initialized even if the console is not enabled. > > > > Therefore, initialize the lock explicitly in owl_uart_probe(), before > > attempting to invoke uart_add_one_port(). > > > > Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > --- > > drivers/tty/serial/owl-uart.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c > > index c149f8c30007..c2fa2f15d50a 100644 > > --- a/drivers/tty/serial/owl-uart.c > > +++ b/drivers/tty/serial/owl-uart.c > > @@ -705,6 +705,8 @@ static int owl_uart_probe(struct platform_device *pdev) > > owl_uart_ports[pdev->id] = owl_port; > > platform_set_drvdata(pdev, owl_port); > > > > + spin_lock_init(&owl_port->port.lock); > > + > > ret = uart_add_one_port(&owl_uart_driver, &owl_port->port); > > if (ret) > > owl_uart_ports[pdev->id] = NULL; > > Ugh, another one :( > > Thanks for this, will queue this up now. > > greg k-h Hi Greg, I think this was mistakenly omitted from the inclusion in v5.8, or has it been postponed for v5.9? Thanks, Cristi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-07-10 12:05 ` Cristian Ciocaltea @ 2020-07-10 13:35 ` Greg Kroah-Hartman 2020-07-10 15:58 ` Cristian Ciocaltea 0 siblings, 1 reply; 11+ messages in thread From: Greg Kroah-Hartman @ 2020-07-10 13:35 UTC (permalink / raw) To: Cristian Ciocaltea Cc: Andreas Färber, Manivannan Sadhasivam, Jiri Slaby, Andy Shevchenko, linux-serial, linux-arm-kernel, linux-kernel, linux-actions On Fri, Jul 10, 2020 at 03:05:49PM +0300, Cristian Ciocaltea wrote: > On Fri, May 29, 2020 at 01:34:19PM +0200, Greg Kroah-Hartman wrote: > > On Fri, May 29, 2020 at 02:06:47PM +0300, Cristian Ciocaltea wrote: > > > Running a lockdep-enabled kernel leads to the following splat when > > > probing the owl-uart driver: > > > > > > [ 1.271784] b0124000.serial: ttyOWL2 at MMIO 0xb0124000 (irq = 22, base_baud = 1500000) is a owl-uart > > > [ 1.281226] INFO: trying to register non-static key. > > > [ 1.286179] the code is fine but needs lockdep annotation. > > > [ 1.291648] turning off the locking correctness validator. > > > [ 1.297125] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc7+ #70 > > > [ 1.303462] Hardware name: Generic DT based system > > > [ 1.308268] [<80111d3c>] (unwind_backtrace) from [<8010c9b8>] (show_stack+0x10/0x14) > > > [ 1.316003] [<8010c9b8>] (show_stack) from [<805016b4>] (dump_stack+0xb4/0xe0) > > > [ 1.323218] [<805016b4>] (dump_stack) from [<80182dec>] (register_lock_class+0x25c/0x8f4) > > > [ 1.331391] [<80182dec>] (register_lock_class) from [<8017ee34>] (__lock_acquire+0xb4/0x2ee4) > > > [ 1.339901] [<8017ee34>] (__lock_acquire) from [<8018291c>] (lock_acquire+0x424/0x4c8) > > > [ 1.347813] [<8018291c>] (lock_acquire) from [<808597b0>] (_raw_spin_lock_irqsave+0x54/0x68) > > > [ 1.356242] [<808597b0>] (_raw_spin_lock_irqsave) from [<80582e94>] (uart_add_one_port+0x384/0x510) > > > [ 1.365276] [<80582e94>] (uart_add_one_port) from [<8058b4d0>] (owl_uart_probe+0x1bc/0x248) > > > [ 1.373621] [<8058b4d0>] (owl_uart_probe) from [<8059c0e4>] (platform_drv_probe+0x48/0x94) > > > [ 1.381874] [<8059c0e4>] (platform_drv_probe) from [<805997c4>] (really_probe+0x200/0x470) > > > [ 1.390123] [<805997c4>] (really_probe) from [<80599c80>] (driver_probe_device+0x16c/0x1bc) > > > [ 1.398461] [<80599c80>] (driver_probe_device) from [<80599f18>] (device_driver_attach+0x44/0x60) > > > [ 1.407317] [<80599f18>] (device_driver_attach) from [<8059a078>] (__driver_attach+0x144/0x14c) > > > [ 1.416000] [<8059a078>] (__driver_attach) from [<805978ac>] (bus_for_each_dev+0x5c/0xb4) > > > [ 1.424162] [<805978ac>] (bus_for_each_dev) from [<8059896c>] (bus_add_driver+0x118/0x204) > > > [ 1.432410] [<8059896c>] (bus_add_driver) from [<8059ae6c>] (driver_register+0xbc/0xf8) > > > [ 1.440405] [<8059ae6c>] (driver_register) from [<80c1fd24>] (owl_uart_init+0x20/0x40) > > > [ 1.448313] [<80c1fd24>] (owl_uart_init) from [<801021d8>] (do_one_initcall+0x184/0x3a4) > > > [ 1.456399] [<801021d8>] (do_one_initcall) from [<80c01100>] (kernel_init_freeable+0x264/0x2e4) > > > [ 1.465085] [<80c01100>] (kernel_init_freeable) from [<80850a88>] (kernel_init+0x8/0x110) > > > [ 1.473249] [<80850a88>] (kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20) > > > [ 1.480800] Exception stack(0xee8bdfb0 to 0xee8bdff8) > > > [ 1.485841] dfa0: 00000000 00000000 00000000 00000000 > > > [ 1.494002] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > > > [ 1.502162] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > > > [ 1.508914] printk: console [ttyOWL2] enabled > > > > > > The locking issue occurs in uart_configure_port() when trying to > > > guard the call to set_mctrl(). > > > > > > uart_add_one_port() should normally initialize the spinlock via > > > uart_port_spin_lock_init(), but it never happens because the port is > > > detected as a console and, as a consequence, the spinlock is expected > > > to be already initialized. > > > > > > The commit a3cb39d258ef > > > ("serial: core: Allow detach and attach serial device for console") > > > changed the lock initialization logic to assume the spinlock is > > > initialized even if the console is not enabled. > > > > > > Therefore, initialize the lock explicitly in owl_uart_probe(), before > > > attempting to invoke uart_add_one_port(). > > > > > > Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > > --- > > > drivers/tty/serial/owl-uart.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c > > > index c149f8c30007..c2fa2f15d50a 100644 > > > --- a/drivers/tty/serial/owl-uart.c > > > +++ b/drivers/tty/serial/owl-uart.c > > > @@ -705,6 +705,8 @@ static int owl_uart_probe(struct platform_device *pdev) > > > owl_uart_ports[pdev->id] = owl_port; > > > platform_set_drvdata(pdev, owl_port); > > > > > > + spin_lock_init(&owl_port->port.lock); > > > + > > > ret = uart_add_one_port(&owl_uart_driver, &owl_port->port); > > > if (ret) > > > owl_uart_ports[pdev->id] = NULL; > > > > Ugh, another one :( > > > > Thanks for this, will queue this up now. > > > > greg k-h > > Hi Greg, > > I think this was mistakenly omitted from the inclusion in v5.8, or > has it been postponed for v5.9? I can't find it anywhere :( But I think we now have a core fix for this, no need to do this in any individual driver, right? Can you test my tty-linus branch to see if your change is still needed or not? thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-07-10 13:35 ` Greg Kroah-Hartman @ 2020-07-10 15:58 ` Cristian Ciocaltea 2020-07-10 16:30 ` Andy Shevchenko 0 siblings, 1 reply; 11+ messages in thread From: Cristian Ciocaltea @ 2020-07-10 15:58 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Andreas Färber, Manivannan Sadhasivam, Jiri Slaby, Andy Shevchenko, linux-serial, linux-arm-kernel, linux-kernel, linux-actions On Fri, Jul 10, 2020 at 03:35:42PM +0200, Greg Kroah-Hartman wrote: > On Fri, Jul 10, 2020 at 03:05:49PM +0300, Cristian Ciocaltea wrote: > > On Fri, May 29, 2020 at 01:34:19PM +0200, Greg Kroah-Hartman wrote: > > > On Fri, May 29, 2020 at 02:06:47PM +0300, Cristian Ciocaltea wrote: > > > > Running a lockdep-enabled kernel leads to the following splat when > > > > probing the owl-uart driver: > > > > > > > > [ 1.271784] b0124000.serial: ttyOWL2 at MMIO 0xb0124000 (irq = 22, base_baud = 1500000) is a owl-uart > > > > [ 1.281226] INFO: trying to register non-static key. > > > > [ 1.286179] the code is fine but needs lockdep annotation. > > > > [ 1.291648] turning off the locking correctness validator. > > > > [ 1.297125] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc7+ #70 > > > > [ 1.303462] Hardware name: Generic DT based system > > > > [ 1.308268] [<80111d3c>] (unwind_backtrace) from [<8010c9b8>] (show_stack+0x10/0x14) > > > > [ 1.316003] [<8010c9b8>] (show_stack) from [<805016b4>] (dump_stack+0xb4/0xe0) > > > > [ 1.323218] [<805016b4>] (dump_stack) from [<80182dec>] (register_lock_class+0x25c/0x8f4) > > > > [ 1.331391] [<80182dec>] (register_lock_class) from [<8017ee34>] (__lock_acquire+0xb4/0x2ee4) > > > > [ 1.339901] [<8017ee34>] (__lock_acquire) from [<8018291c>] (lock_acquire+0x424/0x4c8) > > > > [ 1.347813] [<8018291c>] (lock_acquire) from [<808597b0>] (_raw_spin_lock_irqsave+0x54/0x68) > > > > [ 1.356242] [<808597b0>] (_raw_spin_lock_irqsave) from [<80582e94>] (uart_add_one_port+0x384/0x510) > > > > [ 1.365276] [<80582e94>] (uart_add_one_port) from [<8058b4d0>] (owl_uart_probe+0x1bc/0x248) > > > > [ 1.373621] [<8058b4d0>] (owl_uart_probe) from [<8059c0e4>] (platform_drv_probe+0x48/0x94) > > > > [ 1.381874] [<8059c0e4>] (platform_drv_probe) from [<805997c4>] (really_probe+0x200/0x470) > > > > [ 1.390123] [<805997c4>] (really_probe) from [<80599c80>] (driver_probe_device+0x16c/0x1bc) > > > > [ 1.398461] [<80599c80>] (driver_probe_device) from [<80599f18>] (device_driver_attach+0x44/0x60) > > > > [ 1.407317] [<80599f18>] (device_driver_attach) from [<8059a078>] (__driver_attach+0x144/0x14c) > > > > [ 1.416000] [<8059a078>] (__driver_attach) from [<805978ac>] (bus_for_each_dev+0x5c/0xb4) > > > > [ 1.424162] [<805978ac>] (bus_for_each_dev) from [<8059896c>] (bus_add_driver+0x118/0x204) > > > > [ 1.432410] [<8059896c>] (bus_add_driver) from [<8059ae6c>] (driver_register+0xbc/0xf8) > > > > [ 1.440405] [<8059ae6c>] (driver_register) from [<80c1fd24>] (owl_uart_init+0x20/0x40) > > > > [ 1.448313] [<80c1fd24>] (owl_uart_init) from [<801021d8>] (do_one_initcall+0x184/0x3a4) > > > > [ 1.456399] [<801021d8>] (do_one_initcall) from [<80c01100>] (kernel_init_freeable+0x264/0x2e4) > > > > [ 1.465085] [<80c01100>] (kernel_init_freeable) from [<80850a88>] (kernel_init+0x8/0x110) > > > > [ 1.473249] [<80850a88>] (kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20) > > > > [ 1.480800] Exception stack(0xee8bdfb0 to 0xee8bdff8) > > > > [ 1.485841] dfa0: 00000000 00000000 00000000 00000000 > > > > [ 1.494002] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > > > > [ 1.502162] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > > > > [ 1.508914] printk: console [ttyOWL2] enabled > > > > > > > > The locking issue occurs in uart_configure_port() when trying to > > > > guard the call to set_mctrl(). > > > > > > > > uart_add_one_port() should normally initialize the spinlock via > > > > uart_port_spin_lock_init(), but it never happens because the port is > > > > detected as a console and, as a consequence, the spinlock is expected > > > > to be already initialized. > > > > > > > > The commit a3cb39d258ef > > > > ("serial: core: Allow detach and attach serial device for console") > > > > changed the lock initialization logic to assume the spinlock is > > > > initialized even if the console is not enabled. > > > > > > > > Therefore, initialize the lock explicitly in owl_uart_probe(), before > > > > attempting to invoke uart_add_one_port(). > > > > > > > > Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") > > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > > > > --- > > > > drivers/tty/serial/owl-uart.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c > > > > index c149f8c30007..c2fa2f15d50a 100644 > > > > --- a/drivers/tty/serial/owl-uart.c > > > > +++ b/drivers/tty/serial/owl-uart.c > > > > @@ -705,6 +705,8 @@ static int owl_uart_probe(struct platform_device *pdev) > > > > owl_uart_ports[pdev->id] = owl_port; > > > > platform_set_drvdata(pdev, owl_port); > > > > > > > > + spin_lock_init(&owl_port->port.lock); > > > > + > > > > ret = uart_add_one_port(&owl_uart_driver, &owl_port->port); > > > > if (ret) > > > > owl_uart_ports[pdev->id] = NULL; > > > > > > Ugh, another one :( > > > > > > Thanks for this, will queue this up now. > > > > > > greg k-h > > > > Hi Greg, > > > > I think this was mistakenly omitted from the inclusion in v5.8, or > > has it been postponed for v5.9? > > I can't find it anywhere :( > > But I think we now have a core fix for this, no need to do this in any > individual driver, right? Can you test my tty-linus branch to see if > your change is still needed or not? I was not aware of the alternative approach, at that time Andy confirmed the expected behaviour is to have a proper initialization of the spinlock in the driver: https://lore.kernel.org/lkml/CAHp75Vcz0a87LSnb6Ubt5_bSF3wUcs21GbP119trXER5KBDxbQ@mail.gmail.com/ https://lore.kernel.org/lkml/CAHp75Vf8xJ2yX-11JsTDnRvZQOK+16aePcB1AUzq=5oO-mFCGQ@mail.gmail.com/ I have just checked the tty-linus tree and the only related change seems to be f743061a85f5 ("serial: core: Initialise spin lock before use in uart_configure_port()") I tested it on top of 5.8.0-rc4 and I confirm it fixes the splat, however I'm a bit confused now regarding the recommended approach since there is at least one more commit in the tty-linux tree that still handles a similar issue in the driver specific code: f38278e9b810 ("serial: sh-sci: Initialize spinlock for uart console") Thanks, Cristi > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-07-10 15:58 ` Cristian Ciocaltea @ 2020-07-10 16:30 ` Andy Shevchenko 2020-07-11 6:30 ` Greg Kroah-Hartman 0 siblings, 1 reply; 11+ messages in thread From: Andy Shevchenko @ 2020-07-10 16:30 UTC (permalink / raw) To: Cristian Ciocaltea Cc: Greg Kroah-Hartman, Andreas Färber, Manivannan Sadhasivam, Jiri Slaby, linux-serial, linux-arm-kernel, linux-kernel, linux-actions On Fri, Jul 10, 2020 at 06:58:06PM +0300, Cristian Ciocaltea wrote: > On Fri, Jul 10, 2020 at 03:35:42PM +0200, Greg Kroah-Hartman wrote: > > On Fri, Jul 10, 2020 at 03:05:49PM +0300, Cristian Ciocaltea wrote: ... > > But I think we now have a core fix for this, no need to do this in any > > individual driver, right? Can you test my tty-linus branch to see if > > your change is still needed or not? > > I was not aware of the alternative approach, at that time Andy > confirmed the expected behaviour is to have a proper initialization > of the spinlock in the driver: > https://lore.kernel.org/lkml/CAHp75Vcz0a87LSnb6Ubt5_bSF3wUcs21GbP119trXER5KBDxbQ@mail.gmail.com/ > https://lore.kernel.org/lkml/CAHp75Vf8xJ2yX-11JsTDnRvZQOK+16aePcB1AUzq=5oO-mFCGQ@mail.gmail.com/ > > I have just checked the tty-linus tree and the only related change > seems to be f743061a85f5 > ("serial: core: Initialise spin lock before use in uart_configure_port()") > > I tested it on top of 5.8.0-rc4 and I confirm it fixes the splat, Thank you for the testing! > however I'm a bit confused now regarding the recommended approach since > there is at least one more commit in the tty-linux tree that still > handles a similar issue in the driver specific code: > f38278e9b810 ("serial: sh-sci: Initialize spinlock for uart console") They now should be reverted. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-07-10 16:30 ` Andy Shevchenko @ 2020-07-11 6:30 ` Greg Kroah-Hartman 2020-07-11 13:55 ` Andy Shevchenko 0 siblings, 1 reply; 11+ messages in thread From: Greg Kroah-Hartman @ 2020-07-11 6:30 UTC (permalink / raw) To: Andy Shevchenko Cc: Cristian Ciocaltea, Andreas Färber, Manivannan Sadhasivam, Jiri Slaby, linux-serial, linux-arm-kernel, linux-kernel, linux-actions On Fri, Jul 10, 2020 at 07:30:22PM +0300, Andy Shevchenko wrote: > On Fri, Jul 10, 2020 at 06:58:06PM +0300, Cristian Ciocaltea wrote: > > On Fri, Jul 10, 2020 at 03:35:42PM +0200, Greg Kroah-Hartman wrote: > > > On Fri, Jul 10, 2020 at 03:05:49PM +0300, Cristian Ciocaltea wrote: > > ... > > > > But I think we now have a core fix for this, no need to do this in any > > > individual driver, right? Can you test my tty-linus branch to see if > > > your change is still needed or not? > > > > I was not aware of the alternative approach, at that time Andy > > confirmed the expected behaviour is to have a proper initialization > > of the spinlock in the driver: > > https://lore.kernel.org/lkml/CAHp75Vcz0a87LSnb6Ubt5_bSF3wUcs21GbP119trXER5KBDxbQ@mail.gmail.com/ > > https://lore.kernel.org/lkml/CAHp75Vf8xJ2yX-11JsTDnRvZQOK+16aePcB1AUzq=5oO-mFCGQ@mail.gmail.com/ > > > > I have just checked the tty-linus tree and the only related change > > seems to be f743061a85f5 > > ("serial: core: Initialise spin lock before use in uart_configure_port()") > > > > I tested it on top of 5.8.0-rc4 and I confirm it fixes the splat, > > Thank you for the testing! > > > however I'm a bit confused now regarding the recommended approach since > > there is at least one more commit in the tty-linux tree that still > > handles a similar issue in the driver specific code: > > f38278e9b810 ("serial: sh-sci: Initialize spinlock for uart console") > > They now should be reverted. Please send patches for that :) thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-07-11 6:30 ` Greg Kroah-Hartman @ 2020-07-11 13:55 ` Andy Shevchenko 0 siblings, 0 replies; 11+ messages in thread From: Andy Shevchenko @ 2020-07-11 13:55 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Cristian Ciocaltea, Andreas Färber, Manivannan Sadhasivam, Jiri Slaby, linux-serial, linux-arm-kernel, linux-kernel, linux-actions On Sat, Jul 11, 2020 at 08:30:41AM +0200, Greg Kroah-Hartman wrote: > On Fri, Jul 10, 2020 at 07:30:22PM +0300, Andy Shevchenko wrote: > > On Fri, Jul 10, 2020 at 06:58:06PM +0300, Cristian Ciocaltea wrote: > > > On Fri, Jul 10, 2020 at 03:35:42PM +0200, Greg Kroah-Hartman wrote: ... > > > however I'm a bit confused now regarding the recommended approach since > > > there is at least one more commit in the tty-linux tree that still > > > handles a similar issue in the driver specific code: > > > f38278e9b810 ("serial: sh-sci: Initialize spinlock for uart console") > > > > They now should be reverted. > > Please send patches for that :) Just sent. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] tty: serial: owl: Initialize lock before registering port 2020-05-29 11:06 [PATCH 1/1] tty: serial: owl: Initialize lock before registering port Cristian Ciocaltea 2020-05-29 11:34 ` Greg Kroah-Hartman @ 2020-05-29 11:35 ` Andy Shevchenko 1 sibling, 0 replies; 11+ messages in thread From: Andy Shevchenko @ 2020-05-29 11:35 UTC (permalink / raw) To: Cristian Ciocaltea Cc: Andreas Färber, Manivannan Sadhasivam, Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, open list:SERIAL DRIVERS, linux-arm Mailing List, Linux Kernel Mailing List, linux-actions On Fri, May 29, 2020 at 2:09 PM Cristian Ciocaltea <cristian.ciocaltea@gmail.com> wrote: > > Running a lockdep-enabled kernel leads to the following splat when > probing the owl-uart driver: > > [ 1.271784] b0124000.serial: ttyOWL2 at MMIO 0xb0124000 (irq = 22, base_baud = 1500000) is a owl-uart > [ 1.281226] INFO: trying to register non-static key. > [ 1.286179] the code is fine but needs lockdep annotation. > [ 1.291648] turning off the locking correctness validator. > [ 1.297125] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc7+ #70 > [ 1.303462] Hardware name: Generic DT based system > [ 1.308268] [<80111d3c>] (unwind_backtrace) from [<8010c9b8>] (show_stack+0x10/0x14) > [ 1.316003] [<8010c9b8>] (show_stack) from [<805016b4>] (dump_stack+0xb4/0xe0) > [ 1.323218] [<805016b4>] (dump_stack) from [<80182dec>] (register_lock_class+0x25c/0x8f4) > [ 1.331391] [<80182dec>] (register_lock_class) from [<8017ee34>] (__lock_acquire+0xb4/0x2ee4) > [ 1.339901] [<8017ee34>] (__lock_acquire) from [<8018291c>] (lock_acquire+0x424/0x4c8) > [ 1.347813] [<8018291c>] (lock_acquire) from [<808597b0>] (_raw_spin_lock_irqsave+0x54/0x68) > [ 1.356242] [<808597b0>] (_raw_spin_lock_irqsave) from [<80582e94>] (uart_add_one_port+0x384/0x510) > [ 1.365276] [<80582e94>] (uart_add_one_port) from [<8058b4d0>] (owl_uart_probe+0x1bc/0x248) > [ 1.373621] [<8058b4d0>] (owl_uart_probe) from [<8059c0e4>] (platform_drv_probe+0x48/0x94) > [ 1.381874] [<8059c0e4>] (platform_drv_probe) from [<805997c4>] (really_probe+0x200/0x470) > [ 1.390123] [<805997c4>] (really_probe) from [<80599c80>] (driver_probe_device+0x16c/0x1bc) > [ 1.398461] [<80599c80>] (driver_probe_device) from [<80599f18>] (device_driver_attach+0x44/0x60) > [ 1.407317] [<80599f18>] (device_driver_attach) from [<8059a078>] (__driver_attach+0x144/0x14c) > [ 1.416000] [<8059a078>] (__driver_attach) from [<805978ac>] (bus_for_each_dev+0x5c/0xb4) > [ 1.424162] [<805978ac>] (bus_for_each_dev) from [<8059896c>] (bus_add_driver+0x118/0x204) > [ 1.432410] [<8059896c>] (bus_add_driver) from [<8059ae6c>] (driver_register+0xbc/0xf8) > [ 1.440405] [<8059ae6c>] (driver_register) from [<80c1fd24>] (owl_uart_init+0x20/0x40) > [ 1.448313] [<80c1fd24>] (owl_uart_init) from [<801021d8>] (do_one_initcall+0x184/0x3a4) > [ 1.456399] [<801021d8>] (do_one_initcall) from [<80c01100>] (kernel_init_freeable+0x264/0x2e4) > [ 1.465085] [<80c01100>] (kernel_init_freeable) from [<80850a88>] (kernel_init+0x8/0x110) > [ 1.473249] [<80850a88>] (kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20) > [ 1.480800] Exception stack(0xee8bdfb0 to 0xee8bdff8) > [ 1.485841] dfa0: 00000000 00000000 00000000 00000000 > [ 1.494002] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > [ 1.502162] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > [ 1.508914] printk: console [ttyOWL2] enabled > > The locking issue occurs in uart_configure_port() when trying to > guard the call to set_mctrl(). > > uart_add_one_port() should normally initialize the spinlock via > uart_port_spin_lock_init(), but it never happens because the port is > detected as a console and, as a consequence, the spinlock is expected > to be already initialized. > > The commit a3cb39d258ef > ("serial: core: Allow detach and attach serial device for console") > changed the lock initialization logic to assume the spinlock is > initialized even if the console is not enabled. > > Therefore, initialize the lock explicitly in owl_uart_probe(), before > attempting to invoke uart_add_one_port(). Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> You are faster than me, thanks! > Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com> > --- > drivers/tty/serial/owl-uart.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c > index c149f8c30007..c2fa2f15d50a 100644 > --- a/drivers/tty/serial/owl-uart.c > +++ b/drivers/tty/serial/owl-uart.c > @@ -705,6 +705,8 @@ static int owl_uart_probe(struct platform_device *pdev) > owl_uart_ports[pdev->id] = owl_port; > platform_set_drvdata(pdev, owl_port); > > + spin_lock_init(&owl_port->port.lock); > + > ret = uart_add_one_port(&owl_uart_driver, &owl_port->port); > if (ret) > owl_uart_ports[pdev->id] = NULL; > -- > 2.26.2 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-07-11 13:55 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-29 11:06 [PATCH 1/1] tty: serial: owl: Initialize lock before registering port Cristian Ciocaltea 2020-05-29 11:34 ` Greg Kroah-Hartman 2020-05-29 11:53 ` Andreas Färber 2020-05-29 12:03 ` Andy Shevchenko 2020-07-10 12:05 ` Cristian Ciocaltea 2020-07-10 13:35 ` Greg Kroah-Hartman 2020-07-10 15:58 ` Cristian Ciocaltea 2020-07-10 16:30 ` Andy Shevchenko 2020-07-11 6:30 ` Greg Kroah-Hartman 2020-07-11 13:55 ` Andy Shevchenko 2020-05-29 11:35 ` Andy Shevchenko
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).