[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACRpkdZ1nFAmzRvsvKvZ08fsP_MgsnsiNpD7LdRRXUDWtO_w=Q@mail.gmail.com>
Date: Sat, 19 Feb 2022 02:13:12 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: Samuel Holland <samuel@...lland.org>
Cc: Maxime Ripard <mripard@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
linux-sunxi@...ts.linux.dev, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org, Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH] pinctrl: sunxi: Use unique lockdep classes for IRQs
On Wed, Feb 16, 2022 at 5:00 AM Samuel Holland <samuel@...lland.org> wrote:
> This driver, like several others, uses a chained IRQ for each GPIO bank,
> and forwards .irq_set_wake to the GPIO bank's upstream IRQ. As a result,
> a call to irq_set_irq_wake() needs to lock both the upstream and
> downstream irq_desc's. Lockdep considers this to be a possible deadlock
> when the irq_desc's share lockdep classes, which they do by default:
>
> ============================================
> WARNING: possible recursive locking detected
> 5.17.0-rc3-00394-gc849047c2473 #1 Not tainted
> --------------------------------------------
> init/307 is trying to acquire lock:
> c2dfe27c (&irq_desc_lock_class){-.-.}-{2:2}, at: __irq_get_desc_lock+0x58/0xa0
>
> but task is already holding lock:
> c3c0ac7c (&irq_desc_lock_class){-.-.}-{2:2}, at: __irq_get_desc_lock+0x58/0xa0
>
> 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
>
> 4 locks held by init/307:
> #0: c1f29f18 (system_transition_mutex){+.+.}-{3:3}, at: __do_sys_reboot+0x90/0x23c
> #1: c20f7760 (&dev->mutex){....}-{3:3}, at: device_shutdown+0xf4/0x224
> #2: c2e804d8 (&dev->mutex){....}-{3:3}, at: device_shutdown+0x104/0x224
> #3: c3c0ac7c (&irq_desc_lock_class){-.-.}-{2:2}, at: __irq_get_desc_lock+0x58/0xa0
>
> stack backtrace:
> CPU: 0 PID: 307 Comm: init Not tainted 5.17.0-rc3-00394-gc849047c2473 #1
> Hardware name: Allwinner sun8i Family
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x68/0x90
> dump_stack_lvl from __lock_acquire+0x1680/0x31a0
> __lock_acquire from lock_acquire+0x148/0x3dc
> lock_acquire from _raw_spin_lock_irqsave+0x50/0x6c
> _raw_spin_lock_irqsave from __irq_get_desc_lock+0x58/0xa0
> __irq_get_desc_lock from irq_set_irq_wake+0x2c/0x19c
> irq_set_irq_wake from irq_set_irq_wake+0x13c/0x19c
> [tail call from sunxi_pinctrl_irq_set_wake]
> irq_set_irq_wake from gpio_keys_suspend+0x80/0x1a4
> gpio_keys_suspend from gpio_keys_shutdown+0x10/0x2c
> gpio_keys_shutdown from device_shutdown+0x180/0x224
> device_shutdown from __do_sys_reboot+0x134/0x23c
> __do_sys_reboot from ret_fast_syscall+0x0/0x1c
>
> However, this can never deadlock because the upstream and downstream
> IRQs are never the same (nor do they even involve the same irqchip).
>
> Silence this erroneous lockdep splat by applying what appears to be the
> usual fix of moving the GPIO IRQs to separate lockdep classes.
>
> Fixes: a59c99d9eaf9 ("pinctrl: sunxi: Forward calls to irq_set_irq_wake")
> Reported-by: Guenter Roeck <linux@...ck-us.net>
> Signed-off-by: Samuel Holland <samuel@...lland.org>
Patch applied.
Yours,
Linus Walleij
Powered by blists - more mailing lists