All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Charles Keepax <ckeepax@opensource.cirrus.com>
Subject: [PATCH] ASoC: wm8994: Fix potential deadlock
Date: Fri,  9 Dec 2022 10:16:57 +0100	[thread overview]
Message-ID: <20221209091657.1183-1-m.szyprowski@samsung.com> (raw)
In-Reply-To: CGME20221209091928eucas1p1cfc768d888a6e6c57fcaa0fe320cfced@eucas1p1.samsung.com

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(+)

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


WARNING: multiple messages have this Message-ID (diff)
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Takashi Iwai <tiwai@suse.com>, Mark Brown <broonie@kernel.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: [PATCH] ASoC: wm8994: Fix potential deadlock
Date: Fri,  9 Dec 2022 10:16:57 +0100	[thread overview]
Message-ID: <20221209091657.1183-1-m.szyprowski@samsung.com> (raw)
In-Reply-To: CGME20221209091928eucas1p1cfc768d888a6e6c57fcaa0fe320cfced@eucas1p1.samsung.com

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(+)

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


       reply	other threads:[~2022-12-09  9:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20221209091928eucas1p1cfc768d888a6e6c57fcaa0fe320cfced@eucas1p1.samsung.com>
2022-12-09  9:16 ` Marek Szyprowski [this message]
2022-12-09  9:16   ` [PATCH] ASoC: wm8994: Fix potential deadlock Marek Szyprowski
2022-12-09 11:16   ` Charles Keepax
2022-12-09 11:16     ` Charles Keepax
2022-12-14 13:31   ` Mark Brown
2022-12-14 13:31     ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221209091657.1183-1-m.szyprowski@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.