Re: [PATCH] pinctrl: at91-pio4: use dedicated lock class for IRQ

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Fri, Dec 15, 2023 at 10:35 PM Alexis Lothoré
<alexis.lothore@xxxxxxxxxxx> wrote:

> Trying to suspend to RAM on SAMA5D27 EVK leads to the following lockdep
> warning:
>
>  ============================================
>  WARNING: possible recursive locking detected
>  6.7.0-rc5-wt+ #532 Not tainted
>  --------------------------------------------
>  sh/92 is trying to acquire lock:
>  c3cf306c (&irq_desc_lock_class){-.-.}-{2:2}, at: __irq_get_desc_lock+0xe8/0x100
>
>  but task is already holding lock:
>  c3d7c46c (&irq_desc_lock_class){-.-.}-{2:2}, at: __irq_get_desc_lock+0xe8/0x100
>
>  other info that might help us debug this:
>   Possible unsafe locking scenario:
>
>         CPU0
>         ----
>    lock(&irq_desc_lock_class);
>    lock(&irq_desc_lock_class);
>
>   *** DEADLOCK ***
>
>   May be due to missing lock nesting notation
>
>  6 locks held by sh/92:
>   #0: c3aa0258 (sb_writers#6){.+.+}-{0:0}, at: ksys_write+0xd8/0x178
>   #1: c4c2df44 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter+0x138/0x284
>   #2: c32684a0 (kn->active){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x148/0x284
>   #3: c232b6d4 (system_transition_mutex){+.+.}-{3:3}, at: pm_suspend+0x13c/0x4e8
>   #4: c387b088 (&dev->mutex){....}-{3:3}, at: __device_suspend+0x1e8/0x91c
>   #5: c3d7c46c (&irq_desc_lock_class){-.-.}-{2:2}, at: __irq_get_desc_lock+0xe8/0x100
>
>  stack backtrace:
>  CPU: 0 PID: 92 Comm: sh Not tainted 6.7.0-rc5-wt+ #532
>  Hardware name: Atmel SAMA5
>   unwind_backtrace from show_stack+0x18/0x1c
>   show_stack from dump_stack_lvl+0x34/0x48
>   dump_stack_lvl from __lock_acquire+0x19ec/0x3a0c
>   __lock_acquire from lock_acquire.part.0+0x124/0x2d0
>   lock_acquire.part.0 from _raw_spin_lock_irqsave+0x5c/0x78
>   _raw_spin_lock_irqsave from __irq_get_desc_lock+0xe8/0x100
>   __irq_get_desc_lock from irq_set_irq_wake+0xa8/0x204
>   irq_set_irq_wake from atmel_gpio_irq_set_wake+0x58/0xb4
>   atmel_gpio_irq_set_wake from irq_set_irq_wake+0x100/0x204
>   irq_set_irq_wake from gpio_keys_suspend+0xec/0x2b8
>   gpio_keys_suspend from dpm_run_callback+0xe4/0x248
>   dpm_run_callback from __device_suspend+0x234/0x91c
>   __device_suspend from dpm_suspend+0x224/0x43c
>   dpm_suspend from dpm_suspend_start+0x9c/0xa8
>   dpm_suspend_start from suspend_devices_and_enter+0x1e0/0xa84
>   suspend_devices_and_enter from pm_suspend+0x460/0x4e8
>   pm_suspend from state_store+0x78/0xe4
>   state_store from kernfs_fop_write_iter+0x1a0/0x284
>   kernfs_fop_write_iter from vfs_write+0x38c/0x6f4
>   vfs_write from ksys_write+0xd8/0x178
>   ksys_write from ret_fast_syscall+0x0/0x1c
>  Exception stack(0xc52b3fa8 to 0xc52b3ff0)
>  3fa0:                   00000004 005a0ae8 00000001 005a0ae8 00000004 00000001
>  3fc0: 00000004 005a0ae8 00000001 00000004 00000004 b6c616c0 00000020 0059d190
>  3fe0: 00000004 b6c61678 aec5a041 aebf1a26
>
> This warning is raised because pinctrl-at91-pio4 uses chained IRQ. Whenever
> a wake up source configures an IRQ through irq_set_irq_wake, it will
> lock the corresponding IRQ desc, and then call irq_set_irq_wake on "parent"
> IRQ which will do the same on its own IRQ desc, but since those two locks
> share the same class, lockdep reports this as an issue.
>
> Fix lockdep false positive by setting a different class for parent and
> children IRQ
>
> Fixes: 776180848b57 ("pinctrl: introduce driver for Atmel PIO4 controller")
> Signed-off-by: Alexis Lothoré <alexis.lothore@xxxxxxxxxxx>

This is a serious bug, what do the PIO4 maintainers say?

Yours,
Linus Walleij