pinctrl: single: ensure pcs irq will not be forced threaded
diff mbox

Message ID 1436195617-24003-1-git-send-email-grygorii.strashko@ti.com
State New
Headers show

Commit Message

Grygorii Strashko July 6, 2015, 3:13 p.m. UTC
The PSC IRQ is requested using request_irq() API and as result it can
be forced to be threaded IRQ in RT-Kernel if PCS_QUIRK_HAS_SHARED_IRQ
is enabled for pinctrl domain.

As result, following 'possible irq lock inversion dependency' report
can be seen:

Comments

Tony Lindgren July 6, 2015, 3:29 p.m. UTC | #1
* Grygorii Strashko <grygorii.strashko@ti.com> [150706 08:16]:
> The PSC IRQ is requested using request_irq() API and as result it can
> be forced to be threaded IRQ in RT-Kernel if PCS_QUIRK_HAS_SHARED_IRQ
> is enabled for pinctrl domain.
> 
> As result, following 'possible irq lock inversion dependency' report
> can be seen:
...
 
> To fix it use IRQF_NO_THREAD to ensure that pcs irq will not be forced threaded.
> 
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/pinctrl/pinctrl-single.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index b2de09d..0b8d480 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -1760,7 +1760,8 @@ static int pcs_irq_init_chained_handler(struct pcs_device *pcs,
>  		int res;
>  
>  		res = request_irq(pcs_soc->irq, pcs_irq_handler,
> -				  IRQF_SHARED | IRQF_NO_SUSPEND,
> +				  IRQF_SHARED | IRQF_NO_SUSPEND |
> +				  IRQF_NO_THREAD,
>  				  name, pcs_soc);
>  		if (res) {
>  			pcs_soc->irq = -1;

Looks OK to me. The only case this would be a problem if a system has
a huge number of wake-up events as the list of status registers to
check could grow to the number of GPIO pins in theory. Anyways,
feel free to add:

Acked-by: Tony Lindgren <tony@atomide.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij July 16, 2015, 12:29 p.m. UTC | #2
On Mon, Jul 6, 2015 at 5:13 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> The PSC IRQ is requested using request_irq() API and as result it can
> be forced to be threaded IRQ in RT-Kernel if PCS_QUIRK_HAS_SHARED_IRQ
> is enabled for pinctrl domain.
>
> As result, following 'possible irq lock inversion dependency' report
> can be seen:
> =========================================================
> [ INFO: possible irq lock inversion dependency detected ]
> 3.14.43-rt42-00360-g96ff499-dirty #24 Not tainted
> ---------------------------------------------------------
> irq/369-pinctrl/927 just changed the state of lock:
>  (&pcs->lock){+.....}, at: [<c0375b54>] pcs_irq_handle+0x48/0x9c
> but this lock was taken by another, HARDIRQ-safe lock in the past:
>  (&irq_desc_lock_class){-.....}
>
> and interrupts could create inverse lock ordering between them.
>
> other info that might help us debug this:
>  Possible interrupt unsafe locking scenario:
>
>        CPU0                    CPU1
>        ----                    ----
>   lock(&pcs->lock);
>                                local_irq_disable();
>                                lock(&irq_desc_lock_class);
>                                lock(&pcs->lock);
>   <Interrupt>
>     lock(&irq_desc_lock_class);
>
>  *** DEADLOCK ***
>
> no locks held by irq/369-pinctrl/927.
>
> the shortest dependencies between 2nd lock and 1st lock:
>   -> (&irq_desc_lock_class){-.....} ops: 58724 {
>      IN-HARDIRQ-W at:
>                        [<c0090040>] lock_acquire+0x9c/0x158
>                        [<c07065c8>] _raw_spin_lock+0x48/0x58
>                        [<c009edac>] handle_fasteoi_irq+0x24/0x15c
>                        [<c009abb0>] generic_handle_irq+0x3c/0x4c
>                        [<c000f83c>] handle_IRQ+0x50/0xa0
>                        [<c0008674>] gic_handle_irq+0x3c/0x6c
>                        [<c0707a04>] __irq_svc+0x44/0x8c
>                        [<c000fc44>] arch_cpu_idle+0x40/0x4c
>                        [<c009aadc>] cpu_startup_entry+0x270/0x2e0
>                        [<c06fcbf8>] rest_init+0xd4/0xe4
>                        [<c0a44bfc>] start_kernel+0x3d0/0x3dc
>                        [<80008084>] 0x80008084
>      INITIAL USE at:
>                       [<c0090040>] lock_acquire+0x9c/0x158
>                       [<c070674c>] _raw_spin_lock_irqsave+0x54/0x68
>                       [<c009aff8>] __irq_get_desc_lock+0x64/0xa4
>                       [<c009e38c>] irq_set_chip+0x30/0x78
>                       [<c009ec30>] irq_set_chip_and_handler_name+0x24/0x3c
>                       [<c036ca10>] gic_irq_domain_map+0x48/0xb4
>                       [<c00a0a80>] irq_domain_associate+0x84/0x1d4
>                       [<c00a1154>] irq_create_mapping+0x80/0x11c
>                       [<c00a1270>] irq_create_of_mapping+0x80/0x120
>                       [<c05cdaa8>] irq_of_parse_and_map+0x34/0x3c
>                       [<c0a4ea24>] omap_dm_timer_init_one+0x90/0x30c
>                       [<c0a4eef0>] omap5_realtime_timer_init+0x8c/0x48c
>                       [<c0a486b0>] time_init+0x28/0x38
>                       [<c0a44a6c>] start_kernel+0x240/0x3dc
>                       [<80008084>] 0x80008084
>    }
>    ... key      at: [<c1049ce0>] irq_desc_lock_class+0x0/0x8
>    ... acquired at:
>    [<c07065c8>] _raw_spin_lock+0x48/0x58
>    [<c0375a90>] pcs_irq_unmask+0x58/0xa0
>    [<c009ea48>] irq_enable+0x38/0x48
>    [<c009ead0>] irq_startup+0x78/0x7c
>    [<c009d440>] __setup_irq+0x4a8/0x4f4
>    [<c009d5dc>] request_threaded_irq+0xb8/0x138
>    [<c0415a5c>] omap_8250_startup+0x4c/0x148
>    [<c041276c>] serial8250_startup+0x24/0x30
>    [<c040d0ec>] uart_startup.part.9+0x5c/0x1b4
>    [<c040dbcc>] uart_open+0xf4/0x16c
>    [<c03f0540>] tty_open+0x170/0x61c
>    [<c0157028>] chrdev_open+0xbc/0x1b4
>    [<c0150494>] do_dentry_open+0x1e8/0x2bc
>    [<c0150a84>] finish_open+0x44/0x5c
>    [<c0160d50>] do_last.isra.47+0x710/0xca0
>    [<c01613a4>] path_openat+0xc4/0x640
>    [<c0162904>] do_filp_open+0x3c/0x98
>    [<c0151bdc>] do_sys_open+0x114/0x1d8
>    [<c0151cc8>] SyS_open+0x28/0x2c
>    [<c0a44d70>] kernel_init_freeable+0x168/0x1e4
>    [<c06fcc24>] kernel_init+0x1c/0xf8
>    [<c000eee8>] ret_from_fork+0x14/0x20
>
> -> (&pcs->lock){+.....} ops: 65 {
>    HARDIRQ-ON-W at:
>                     [<c0090040>] lock_acquire+0x9c/0x158
>                     [<c07065c8>] _raw_spin_lock+0x48/0x58
>                     [<c0375b54>] pcs_irq_handle+0x48/0x9c
>                     [<c0375c5c>] pcs_irq_handler+0x1c/0x28
>                     [<c009c458>] irq_forced_thread_fn+0x30/0x74
>                     [<c009c784>] irq_thread+0x158/0x1c4
>                     [<c0063fc4>] kthread+0xd4/0xe8
>                     [<c000eee8>] ret_from_fork+0x14/0x20
>    INITIAL USE at:
>                    [<c0090040>] lock_acquire+0x9c/0x158
>                    [<c070674c>] _raw_spin_lock_irqsave+0x54/0x68
>                    [<c0375344>] pcs_enable+0x7c/0xe8
>                    [<c0372a44>] pinmux_enable_setting+0x178/0x220
>                    [<c036fecc>] pinctrl_select_state+0x110/0x194
>                    [<c04732dc>] pinctrl_bind_pins+0x7c/0x108
>                    [<c045853c>] driver_probe_device+0x70/0x254
>                    [<c0458810>] __driver_attach+0x9c/0xa0
>                    [<c045674c>] bus_for_each_dev+0x78/0xac
>                    [<c0458030>] driver_attach+0x2c/0x30
>                    [<c0457c78>] bus_add_driver+0x15c/0x204
>                    [<c0458ee0>] driver_register+0x88/0x108
>                    [<c045a168>] __platform_driver_register+0x64/0x6c
>                    [<c0a8170c>] omap_hsmmc_driver_init+0x1c/0x20
>                    [<c0008a94>] do_one_initcall+0x110/0x170
>                    [<c0a44d48>] kernel_init_freeable+0x140/0x1e4
>                    [<c06fcc24>] kernel_init+0x1c/0xf8
>                    [<c000eee8>] ret_from_fork+0x14/0x20
>  }
>  ... key      at: [<c1088a8c>] __key.18572+0x0/0x8
>  ... acquired at:
>    [<c008cdd4>] mark_lock+0x388/0x76c
>    [<c008df40>] __lock_acquire+0x6d0/0x1f98
>    [<c0090040>] lock_acquire+0x9c/0x158
>    [<c07065c8>] _raw_spin_lock+0x48/0x58
>    [<c0375b54>] pcs_irq_handle+0x48/0x9c
>    [<c0375c5c>] pcs_irq_handler+0x1c/0x28
>    [<c009c458>] irq_forced_thread_fn+0x30/0x74
>    [<c009c784>] irq_thread+0x158/0x1c4
>    [<c0063fc4>] kthread+0xd4/0xe8
>    [<c000eee8>] ret_from_fork+0x14/0x20
>
> stack backtrace:
> CPU: 1 PID: 927 Comm: irq/369-pinctrl Not tainted 3.14.43-rt42-00360-g96ff499-dirty #24
> [<c00177e0>] (unwind_backtrace) from [<c00130b0>] (show_stack+0x20/0x24)
> [<c00130b0>] (show_stack) from [<c0702958>] (dump_stack+0x84/0xd0)
> [<c0702958>] (dump_stack) from [<c008bcfc>] (print_irq_inversion_bug+0x1d0/0x21c)
> [<c008bcfc>] (print_irq_inversion_bug) from [<c008bf18>] (check_usage_backwards+0xb4/0x11c)
> [<c008bf18>] (check_usage_backwards) from [<c008cdd4>] (mark_lock+0x388/0x76c)
> [<c008cdd4>] (mark_lock) from [<c008df40>] (__lock_acquire+0x6d0/0x1f98)
> [<c008df40>] (__lock_acquire) from [<c0090040>] (lock_acquire+0x9c/0x158)
> [<c0090040>] (lock_acquire) from [<c07065c8>] (_raw_spin_lock+0x48/0x58)
> [<c07065c8>] (_raw_spin_lock) from [<c0375b54>] (pcs_irq_handle+0x48/0x9c)
> [<c0375b54>] (pcs_irq_handle) from [<c0375c5c>] (pcs_irq_handler+0x1c/0x28)
> [<c0375c5c>] (pcs_irq_handler) from [<c009c458>] (irq_forced_thread_fn+0x30/0x74)
> [<c009c458>] (irq_forced_thread_fn) from [<c009c784>] (irq_thread+0x158/0x1c4)
> [<c009c784>] (irq_thread) from [<c0063fc4>] (kthread+0xd4/0xe8)
> [<c0063fc4>] (kthread) from [<c000eee8>] (ret_from_fork+0x14/0x20)
>
> To fix it use IRQF_NO_THREAD to ensure that pcs irq will not be forced threaded.
>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

OK queued for fixes with Tony's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

=========================================================
[ INFO: possible irq lock inversion dependency detected ]
3.14.43-rt42-00360-g96ff499-dirty #24 Not tainted
---------------------------------------------------------
irq/369-pinctrl/927 just changed the state of lock:
 (&pcs->lock){+.....}, at: [<c0375b54>] pcs_irq_handle+0x48/0x9c
but this lock was taken by another, HARDIRQ-safe lock in the past:
 (&irq_desc_lock_class){-.....}

and interrupts could create inverse lock ordering between them.

other info that might help us debug this:
 Possible interrupt unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&pcs->lock);
                               local_irq_disable();
                               lock(&irq_desc_lock_class);
                               lock(&pcs->lock);
  <Interrupt>
    lock(&irq_desc_lock_class);

 *** DEADLOCK ***

no locks held by irq/369-pinctrl/927.

the shortest dependencies between 2nd lock and 1st lock:
  -> (&irq_desc_lock_class){-.....} ops: 58724 {
     IN-HARDIRQ-W at:
                       [<c0090040>] lock_acquire+0x9c/0x158
                       [<c07065c8>] _raw_spin_lock+0x48/0x58
                       [<c009edac>] handle_fasteoi_irq+0x24/0x15c
                       [<c009abb0>] generic_handle_irq+0x3c/0x4c
                       [<c000f83c>] handle_IRQ+0x50/0xa0
                       [<c0008674>] gic_handle_irq+0x3c/0x6c
                       [<c0707a04>] __irq_svc+0x44/0x8c
                       [<c000fc44>] arch_cpu_idle+0x40/0x4c
                       [<c009aadc>] cpu_startup_entry+0x270/0x2e0
                       [<c06fcbf8>] rest_init+0xd4/0xe4
                       [<c0a44bfc>] start_kernel+0x3d0/0x3dc
                       [<80008084>] 0x80008084
     INITIAL USE at:
                      [<c0090040>] lock_acquire+0x9c/0x158
                      [<c070674c>] _raw_spin_lock_irqsave+0x54/0x68
                      [<c009aff8>] __irq_get_desc_lock+0x64/0xa4
                      [<c009e38c>] irq_set_chip+0x30/0x78
                      [<c009ec30>] irq_set_chip_and_handler_name+0x24/0x3c
                      [<c036ca10>] gic_irq_domain_map+0x48/0xb4
                      [<c00a0a80>] irq_domain_associate+0x84/0x1d4
                      [<c00a1154>] irq_create_mapping+0x80/0x11c
                      [<c00a1270>] irq_create_of_mapping+0x80/0x120
                      [<c05cdaa8>] irq_of_parse_and_map+0x34/0x3c
                      [<c0a4ea24>] omap_dm_timer_init_one+0x90/0x30c
                      [<c0a4eef0>] omap5_realtime_timer_init+0x8c/0x48c
                      [<c0a486b0>] time_init+0x28/0x38
                      [<c0a44a6c>] start_kernel+0x240/0x3dc
                      [<80008084>] 0x80008084
   }
   ... key      at: [<c1049ce0>] irq_desc_lock_class+0x0/0x8
   ... acquired at:
   [<c07065c8>] _raw_spin_lock+0x48/0x58
   [<c0375a90>] pcs_irq_unmask+0x58/0xa0
   [<c009ea48>] irq_enable+0x38/0x48
   [<c009ead0>] irq_startup+0x78/0x7c
   [<c009d440>] __setup_irq+0x4a8/0x4f4
   [<c009d5dc>] request_threaded_irq+0xb8/0x138
   [<c0415a5c>] omap_8250_startup+0x4c/0x148
   [<c041276c>] serial8250_startup+0x24/0x30
   [<c040d0ec>] uart_startup.part.9+0x5c/0x1b4
   [<c040dbcc>] uart_open+0xf4/0x16c
   [<c03f0540>] tty_open+0x170/0x61c
   [<c0157028>] chrdev_open+0xbc/0x1b4
   [<c0150494>] do_dentry_open+0x1e8/0x2bc
   [<c0150a84>] finish_open+0x44/0x5c
   [<c0160d50>] do_last.isra.47+0x710/0xca0
   [<c01613a4>] path_openat+0xc4/0x640
   [<c0162904>] do_filp_open+0x3c/0x98
   [<c0151bdc>] do_sys_open+0x114/0x1d8
   [<c0151cc8>] SyS_open+0x28/0x2c
   [<c0a44d70>] kernel_init_freeable+0x168/0x1e4
   [<c06fcc24>] kernel_init+0x1c/0xf8
   [<c000eee8>] ret_from_fork+0x14/0x20

-> (&pcs->lock){+.....} ops: 65 {
   HARDIRQ-ON-W at:
                    [<c0090040>] lock_acquire+0x9c/0x158
                    [<c07065c8>] _raw_spin_lock+0x48/0x58
                    [<c0375b54>] pcs_irq_handle+0x48/0x9c
                    [<c0375c5c>] pcs_irq_handler+0x1c/0x28
                    [<c009c458>] irq_forced_thread_fn+0x30/0x74
                    [<c009c784>] irq_thread+0x158/0x1c4
                    [<c0063fc4>] kthread+0xd4/0xe8
                    [<c000eee8>] ret_from_fork+0x14/0x20
   INITIAL USE at:
                   [<c0090040>] lock_acquire+0x9c/0x158
                   [<c070674c>] _raw_spin_lock_irqsave+0x54/0x68
                   [<c0375344>] pcs_enable+0x7c/0xe8
                   [<c0372a44>] pinmux_enable_setting+0x178/0x220
                   [<c036fecc>] pinctrl_select_state+0x110/0x194
                   [<c04732dc>] pinctrl_bind_pins+0x7c/0x108
                   [<c045853c>] driver_probe_device+0x70/0x254
                   [<c0458810>] __driver_attach+0x9c/0xa0
                   [<c045674c>] bus_for_each_dev+0x78/0xac
                   [<c0458030>] driver_attach+0x2c/0x30
                   [<c0457c78>] bus_add_driver+0x15c/0x204
                   [<c0458ee0>] driver_register+0x88/0x108
                   [<c045a168>] __platform_driver_register+0x64/0x6c
                   [<c0a8170c>] omap_hsmmc_driver_init+0x1c/0x20
                   [<c0008a94>] do_one_initcall+0x110/0x170
                   [<c0a44d48>] kernel_init_freeable+0x140/0x1e4
                   [<c06fcc24>] kernel_init+0x1c/0xf8
                   [<c000eee8>] ret_from_fork+0x14/0x20
 }
 ... key      at: [<c1088a8c>] __key.18572+0x0/0x8
 ... acquired at:
   [<c008cdd4>] mark_lock+0x388/0x76c
   [<c008df40>] __lock_acquire+0x6d0/0x1f98
   [<c0090040>] lock_acquire+0x9c/0x158
   [<c07065c8>] _raw_spin_lock+0x48/0x58
   [<c0375b54>] pcs_irq_handle+0x48/0x9c
   [<c0375c5c>] pcs_irq_handler+0x1c/0x28
   [<c009c458>] irq_forced_thread_fn+0x30/0x74
   [<c009c784>] irq_thread+0x158/0x1c4
   [<c0063fc4>] kthread+0xd4/0xe8
   [<c000eee8>] ret_from_fork+0x14/0x20

stack backtrace:
CPU: 1 PID: 927 Comm: irq/369-pinctrl Not tainted 3.14.43-rt42-00360-g96ff499-dirty #24
[<c00177e0>] (unwind_backtrace) from [<c00130b0>] (show_stack+0x20/0x24)
[<c00130b0>] (show_stack) from [<c0702958>] (dump_stack+0x84/0xd0)
[<c0702958>] (dump_stack) from [<c008bcfc>] (print_irq_inversion_bug+0x1d0/0x21c)
[<c008bcfc>] (print_irq_inversion_bug) from [<c008bf18>] (check_usage_backwards+0xb4/0x11c)
[<c008bf18>] (check_usage_backwards) from [<c008cdd4>] (mark_lock+0x388/0x76c)
[<c008cdd4>] (mark_lock) from [<c008df40>] (__lock_acquire+0x6d0/0x1f98)
[<c008df40>] (__lock_acquire) from [<c0090040>] (lock_acquire+0x9c/0x158)
[<c0090040>] (lock_acquire) from [<c07065c8>] (_raw_spin_lock+0x48/0x58)
[<c07065c8>] (_raw_spin_lock) from [<c0375b54>] (pcs_irq_handle+0x48/0x9c)
[<c0375b54>] (pcs_irq_handle) from [<c0375c5c>] (pcs_irq_handler+0x1c/0x28)
[<c0375c5c>] (pcs_irq_handler) from [<c009c458>] (irq_forced_thread_fn+0x30/0x74)
[<c009c458>] (irq_forced_thread_fn) from [<c009c784>] (irq_thread+0x158/0x1c4)
[<c009c784>] (irq_thread) from [<c0063fc4>] (kthread+0xd4/0xe8)
[<c0063fc4>] (kthread) from [<c000eee8>] (ret_from_fork+0x14/0x20)

To fix it use IRQF_NO_THREAD to ensure that pcs irq will not be forced threaded.

Cc: Tony Lindgren <tony@atomide.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/pinctrl/pinctrl-single.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index b2de09d..0b8d480 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1760,7 +1760,8 @@  static int pcs_irq_init_chained_handler(struct pcs_device *pcs,
 		int res;
 
 		res = request_irq(pcs_soc->irq, pcs_irq_handler,
-				  IRQF_SHARED | IRQF_NO_SUSPEND,
+				  IRQF_SHARED | IRQF_NO_SUSPEND |
+				  IRQF_NO_THREAD,
 				  name, pcs_soc);
 		if (res) {
 			pcs_soc->irq = -1;