[PATCH] ALSA: aloop: Fix spinlock deadlock in loopback_hrtimer_stop()

[PATCH] ALSA: aloop: Fix spinlock deadlock in loopback_hrtimer_stop()

[Yu-Hsuan Hsu]

In loopback_hrtimer_stop(), calling hrtimer_cancel() while holding
cable->lock triggers an AB-BA spinlock deadlock if the hrtimer softirq
is executing concurrently on another CPU:

1) CPU A runs loopback_trigger(STOP), acquires spin_lock(&cable->lock),
   and calls hrtimer_cancel(). Since hrtimer_cancel() is synchronous,
   it spins waiting for the executing callback to complete before
   returning.
2) CPU B executes loopback_hrtimer_function(), which immediately tries
   to acquire spin_lock(&cable->lock).

This mutual dependency leads to a CPU hard lockup and NMI watchdog
panic when multiple streams start and stop concurrently with small
period sizes.

Replace hrtimer_cancel() in loopback_hrtimer_stop() with the non-blocking
hrtimer_try_to_cancel(), matching the behavior of jiffies timers
(timer_delete vs timer_delete_sync). If try_to_cancel returns -1
because the handler is running, CPU A releases cable->lock cleanly.
When the running handler subsequently acquires cable->lock, it observes
that the stream is no longer in running state (cleared by trigger STOP)
and terminates without re-arming the timer. Synchronous hrtimer_cancel()
remains preserved in loopback_hrtimer_stop_sync() where cable->lock is
not held.

Fixes: bf08a5f698dc ("ALSA: aloop: Add 'hrtimer' option to timer_source")
Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
---
 sound/drivers/aloop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index 236f49a7fb8b..60f5bf1e48bb 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -303,7 +303,7 @@ static inline int loopback_jiffies_timer_stop(struct loopback_pcm *dpcm)
 /* call in cable->lock */
 static inline int loopback_hrtimer_stop(struct loopback_pcm *dpcm)
 {
-	hrtimer_cancel(&dpcm->hrtimer);
+	hrtimer_try_to_cancel(&dpcm->hrtimer);
 
 	return 0;
 }
-- 
2.55.0.508.g3f0d502094-goog

Re: [PATCH] ALSA: aloop: Fix spinlock deadlock in loopback_hrtimer_stop()

[Takashi Iwai]

On Fri, 31 Jul 2026 09:39:35 +0200,
Yu-Hsuan Hsu wrote:
> 
> In loopback_hrtimer_stop(), calling hrtimer_cancel() while holding
> cable->lock triggers an AB-BA spinlock deadlock if the hrtimer softirq
> is executing concurrently on another CPU:
> 
> 1) CPU A runs loopback_trigger(STOP), acquires spin_lock(&cable->lock),
>    and calls hrtimer_cancel(). Since hrtimer_cancel() is synchronous,
>    it spins waiting for the executing callback to complete before
>    returning.
> 2) CPU B executes loopback_hrtimer_function(), which immediately tries
>    to acquire spin_lock(&cable->lock).
> 
> This mutual dependency leads to a CPU hard lockup and NMI watchdog
> panic when multiple streams start and stop concurrently with small
> period sizes.
> 
> Replace hrtimer_cancel() in loopback_hrtimer_stop() with the non-blocking
> hrtimer_try_to_cancel(), matching the behavior of jiffies timers
> (timer_delete vs timer_delete_sync). If try_to_cancel returns -1
> because the handler is running, CPU A releases cable->lock cleanly.
> When the running handler subsequently acquires cable->lock, it observes
> that the stream is no longer in running state (cleared by trigger STOP)
> and terminates without re-arming the timer. Synchronous hrtimer_cancel()
> remains preserved in loopback_hrtimer_stop_sync() where cable->lock is
> not held.
> 
> Fixes: bf08a5f698dc ("ALSA: aloop: Add 'hrtimer' option to timer_source")
> Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>

While I find it's fine to change like this, I wonder whether you
really hit a CPU deadlock.  Or it's just hypothetical?


thanks,

Takashi

Re: [PATCH] ALSA: aloop: Fix spinlock deadlock in loopback_hrtimer_stop()

[Yu-Hsuan Hsu]

Hi Takashi,

Thanks. I actually encountered a CPU deadlock during the aloop stress
test. (Resent as plain text.)

Best,
Yu-Hsuan

Takashi Iwai <tiwai@suse.de> 於 2026年8月1日週六 下午3:33寫道：
>
> On Fri, 31 Jul 2026 09:39:35 +0200,
> Yu-Hsuan Hsu wrote:
> >
> > In loopback_hrtimer_stop(), calling hrtimer_cancel() while holding
> > cable->lock triggers an AB-BA spinlock deadlock if the hrtimer softirq
> > is executing concurrently on another CPU:
> >
> > 1) CPU A runs loopback_trigger(STOP), acquires spin_lock(&cable->lock),
> >    and calls hrtimer_cancel(). Since hrtimer_cancel() is synchronous,
> >    it spins waiting for the executing callback to complete before
> >    returning.
> > 2) CPU B executes loopback_hrtimer_function(), which immediately tries
> >    to acquire spin_lock(&cable->lock).
> >
> > This mutual dependency leads to a CPU hard lockup and NMI watchdog
> > panic when multiple streams start and stop concurrently with small
> > period sizes.
> >
> > Replace hrtimer_cancel() in loopback_hrtimer_stop() with the non-blocking
> > hrtimer_try_to_cancel(), matching the behavior of jiffies timers
> > (timer_delete vs timer_delete_sync). If try_to_cancel returns -1
> > because the handler is running, CPU A releases cable->lock cleanly.
> > When the running handler subsequently acquires cable->lock, it observes
> > that the stream is no longer in running state (cleared by trigger STOP)
> > and terminates without re-arming the timer. Synchronous hrtimer_cancel()
> > remains preserved in loopback_hrtimer_stop_sync() where cable->lock is
> > not held.
> >
> > Fixes: bf08a5f698dc ("ALSA: aloop: Add 'hrtimer' option to timer_source")
> > Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
>
> While I find it's fine to change like this, I wonder whether you
> really hit a CPU deadlock.  Or it's just hypothetical?
>
>
> thanks,
>
> Takashi

Re: [PATCH] ALSA: aloop: Fix spinlock deadlock in loopback_hrtimer_stop()

[Takashi Iwai]

On Sun, 02 Aug 2026 15:58:26 +0200,
Yu-Hsuan Hsu wrote:
> 
> Hi Takashi,
> 
> Thanks. I actually encountered a CPU deadlock during the aloop stress
> test. (Resent as plain text.)

Thanks for confirmation.  I asked it to judge for 7.2-rc7 fix, but I
missed the point that it's the fix for the new change for 7.3 :)
Now applied to for-next branch.


thanks,

Takashi


> 
> Best,
> Yu-Hsuan
> 
> Takashi Iwai <tiwai@suse.de> 於 2026年8月1日週六 下午3:33寫道：
> >
> > On Fri, 31 Jul 2026 09:39:35 +0200,
> > Yu-Hsuan Hsu wrote:
> > >
> > > In loopback_hrtimer_stop(), calling hrtimer_cancel() while holding
> > > cable->lock triggers an AB-BA spinlock deadlock if the hrtimer softirq
> > > is executing concurrently on another CPU:
> > >
> > > 1) CPU A runs loopback_trigger(STOP), acquires spin_lock(&cable->lock),
> > >    and calls hrtimer_cancel(). Since hrtimer_cancel() is synchronous,
> > >    it spins waiting for the executing callback to complete before
> > >    returning.
> > > 2) CPU B executes loopback_hrtimer_function(), which immediately tries
> > >    to acquire spin_lock(&cable->lock).
> > >
> > > This mutual dependency leads to a CPU hard lockup and NMI watchdog
> > > panic when multiple streams start and stop concurrently with small
> > > period sizes.
> > >
> > > Replace hrtimer_cancel() in loopback_hrtimer_stop() with the non-blocking
> > > hrtimer_try_to_cancel(), matching the behavior of jiffies timers
> > > (timer_delete vs timer_delete_sync). If try_to_cancel returns -1
> > > because the handler is running, CPU A releases cable->lock cleanly.
> > > When the running handler subsequently acquires cable->lock, it observes
> > > that the stream is no longer in running state (cleared by trigger STOP)
> > > and terminates without re-arming the timer. Synchronous hrtimer_cancel()
> > > remains preserved in loopback_hrtimer_stop_sync() where cable->lock is
> > > not held.
> > >
> > > Fixes: bf08a5f698dc ("ALSA: aloop: Add 'hrtimer' option to timer_source")
> > > Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
> >
> > While I find it's fine to change like this, I wonder whether you
> > really hit a CPU deadlock.  Or it's just hypothetical?
> >
> >
> > thanks,
> >
> > Takashi