diff mbox series

ASoC: wm8994: Fix potential deadlock

Message ID 20221209091657.1183-1-m.szyprowski@samsung.com
State New
Headers
Series ASoC: wm8994: Fix potential deadlock |

Commit Message

Marek Szyprowski Dec. 9, 2022, 9:16 a.m. UTC 
  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 <m.szyprowski@samsung.com>
---
 sound/soc/codecs/wm8994.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Charles Keepax Dec. 9, 2022, 11:16 a.m. UTC | #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 <m.szyprowski@samsung.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles
Mark Brown Dec. 14, 2022, 1:31 p.m. UTC | #2 
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
diff mbox series

Patch

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);