Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 9 Dec 2022 11:16:40 +0000
From:   Charles Keepax <ckeepax@opensource.cirrus.com>
To:     Marek Szyprowski <m.szyprowski@samsung.com>
CC:     <patches@opensource.cirrus.com>, <alsa-devel@alsa-project.org>,
        <linux-kernel@vger.kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>
Subject: Re: [PATCH] ASoC: wm8994: Fix potential deadlock
Message-ID: <20221209111640.GK105268@ediswmail.ad.cirrus.com>
References: <CGME20221209091928eucas1p1cfc768d888a6e6c57fcaa0fe320cfced@eucas1p1.samsung.com>
 <20221209091657.1183-1-m.szyprowski@samsung.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221209091657.1183-1-m.szyprowski@samsung.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

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