Commit c0feea594e05 ("workqueue: don't skip lockdep work dependency in
cancel_work_sync()") revealed the following locking issue in the wm8994
codec:
======================================================
WARNING: possible circular locking dependency detected
6.0.0-rc1-00001-gc0feea594e05-dirty #13097 Not tainted
------------------------------------------------------
kworker/1:1/32 is trying to acquire lock:
c2bd4300 (&wm8994->accdet_lock){+.+.}-{3:3}, at: wm1811_mic_work+0x38/0xdc
but task is already holding lock:
f08f5f28 ((work_completion)(&(&wm8994->mic_work)->work)){+.+.}-{0:0}, at: process_one_work+0x1e4/0x778
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 ((work_completion)(&(&wm8994->mic_work)->work)){+.+.}-{0:0}:
__cancel_work_timer+0x198/0x22c
wm1811_jackdet_irq+0x124/0x238
process_one_work+0x288/0x778
worker_thread+0x44/0x504
kthread+0xf0/0x124
ret_from_fork+0x14/0x2c
0x0
-> #0 (&wm8994->accdet_lock){+.+.}-{3:3}:
lock_acquire+0x124/0x3e4
__mutex_lock+0x90/0x948
mutex_lock_nested+0x1c/0x24
wm1811_mic_work+0x38/0xdc
process_one_work+0x288/0x778
worker_thread+0x44/0x504
kthread+0xf0/0x124
ret_from_fork+0x14/0x2c
0x0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock((work_completion)(&(&wm8994->mic_work)->work));
lock(&wm8994->accdet_lock);
lock((work_completion)(&(&wm8994->mic_work)->work));
lock(&wm8994->accdet_lock);
*** DEADLOCK ***
2 locks held by kworker/1:1/32:
#0: c1c072a8 ((wq_completion)events_power_efficient){+.+.}-{0:0}, at: process_one_work+0x1e4/0x778
#1: f08f5f28 ((work_completion)(&(&wm8994->mic_work)->work)){+.+.}-{0:0}, at: process_one_work+0x1e4/0x778
stack backtrace:
CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 6.0.0-rc1-00001-gc0feea594e05-dirty #13097
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: events_power_efficient wm1811_mic_work
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x58/0x70
dump_stack_lvl from check_noncircular+0xf0/0x158
check_noncircular from __lock_acquire+0x15e8/0x2a7c
__lock_acquire from lock_acquire+0x124/0x3e4
lock_acquire from __mutex_lock+0x90/0x948
__mutex_lock from mutex_lock_nested+0x1c/0x24
mutex_lock_nested from wm1811_mic_work+0x38/0xdc
wm1811_mic_work from process_one_work+0x288/0x778
process_one_work from worker_thread+0x44/0x504
worker_thread from kthread+0xf0/0x124
kthread from ret_from_fork+0x14/0x2c
Exception stack(0xf08f5fb0 to 0xf08f5ff8)
...
--->8---
Fix this by dropping wm8994->accdet_lock while calling
cancel_delayed_work_sync(&wm8994->mic_work) in wm1811_jackdet_irq().
Fixes: c0cc3f166525 ("ASoC: wm8994: Allow a delay between jack insertion and microphone detect")
Signed-off-by: Marek Szyprowski <[email protected]>
---
sound/soc/codecs/wm8994.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index d3cfd3788f2a..f810135e28d0 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -3853,7 +3853,12 @@ static irqreturn_t wm1811_jackdet_irq(int irq, void *data)
} else {
dev_dbg(component->dev, "Jack not detected\n");
+ /* Release wm8994->accdet_lock to avoid deadlock:
+ * cancel_delayed_work_sync() takes wm8994->mic_work internal
+ * lock and wm1811_mic_work takes wm8994->accdet_lock */
+ mutex_unlock(&wm8994->accdet_lock);
cancel_delayed_work_sync(&wm8994->mic_work);
+ mutex_lock(&wm8994->accdet_lock);
snd_soc_component_update_bits(component, WM8958_MICBIAS2,
WM8958_MICB2_DISCH, WM8958_MICB2_DISCH);
--
2.38.1
On Fri, Dec 09, 2022 at 10:16:57AM +0100, Marek Szyprowski wrote:
> Commit c0feea594e05 ("workqueue: don't skip lockdep work dependency in
> cancel_work_sync()") revealed the following locking issue in the wm8994
> codec:
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 6.0.0-rc1-00001-gc0feea594e05-dirty #13097 Not tainted
> ------------------------------------------------------
> kworker/1:1/32 is trying to acquire lock:
> c2bd4300 (&wm8994->accdet_lock){+.+.}-{3:3}, at: wm1811_mic_work+0x38/0xdc
>
> but task is already holding lock:
> f08f5f28 ((work_completion)(&(&wm8994->mic_work)->work)){+.+.}-{0:0}, at: process_one_work+0x1e4/0x778
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 ((work_completion)(&(&wm8994->mic_work)->work)){+.+.}-{0:0}:
> __cancel_work_timer+0x198/0x22c
> wm1811_jackdet_irq+0x124/0x238
> process_one_work+0x288/0x778
> worker_thread+0x44/0x504
> kthread+0xf0/0x124
> ret_from_fork+0x14/0x2c
> 0x0
>
> -> #0 (&wm8994->accdet_lock){+.+.}-{3:3}:
> lock_acquire+0x124/0x3e4
> __mutex_lock+0x90/0x948
> mutex_lock_nested+0x1c/0x24
> wm1811_mic_work+0x38/0xdc
> process_one_work+0x288/0x778
> worker_thread+0x44/0x504
> kthread+0xf0/0x124
> ret_from_fork+0x14/0x2c
> 0x0
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock((work_completion)(&(&wm8994->mic_work)->work));
> lock(&wm8994->accdet_lock);
> lock((work_completion)(&(&wm8994->mic_work)->work));
> lock(&wm8994->accdet_lock);
>
> *** DEADLOCK ***
>
> 2 locks held by kworker/1:1/32:
> #0: c1c072a8 ((wq_completion)events_power_efficient){+.+.}-{0:0}, at: process_one_work+0x1e4/0x778
> #1: f08f5f28 ((work_completion)(&(&wm8994->mic_work)->work)){+.+.}-{0:0}, at: process_one_work+0x1e4/0x778
>
> stack backtrace:
> CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 6.0.0-rc1-00001-gc0feea594e05-dirty #13097
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: events_power_efficient wm1811_mic_work
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x58/0x70
> dump_stack_lvl from check_noncircular+0xf0/0x158
> check_noncircular from __lock_acquire+0x15e8/0x2a7c
> __lock_acquire from lock_acquire+0x124/0x3e4
> lock_acquire from __mutex_lock+0x90/0x948
> __mutex_lock from mutex_lock_nested+0x1c/0x24
> mutex_lock_nested from wm1811_mic_work+0x38/0xdc
> wm1811_mic_work from process_one_work+0x288/0x778
> process_one_work from worker_thread+0x44/0x504
> worker_thread from kthread+0xf0/0x124
> kthread from ret_from_fork+0x14/0x2c
> Exception stack(0xf08f5fb0 to 0xf08f5ff8)
> ...
> --->8---
>
> Fix this by dropping wm8994->accdet_lock while calling
> cancel_delayed_work_sync(&wm8994->mic_work) in wm1811_jackdet_irq().
>
> Fixes: c0cc3f166525 ("ASoC: wm8994: Allow a delay between jack insertion and microphone detect")
> Signed-off-by: Marek Szyprowski <[email protected]>
> ---
Acked-by: Charles Keepax <[email protected]>
Thanks,
Charles
On Fri, 09 Dec 2022 10:16:57 +0100, Marek Szyprowski wrote:
> Fix this by dropping wm8994->accdet_lock while calling
> cancel_delayed_work_sync(&wm8994->mic_work) in wm1811_jackdet_irq().
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: wm8994: Fix potential deadlock
commit: 9529dc167ffcdfd201b9f0eda71015f174095f7e
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark