]> www.infradead.org Git - nvme.git/commitdiff
Revert "ALSA: firewire-lib: operate for period elapse event in process context"
authorEdmund Raile <edmund.raile@protonmail.com>
Tue, 30 Jul 2024 19:53:29 +0000 (19:53 +0000)
committerTakashi Iwai <tiwai@suse.de>
Wed, 31 Jul 2024 09:28:33 +0000 (11:28 +0200)
Commit 7ba5ca32fe6e ("ALSA: firewire-lib: operate for period elapse event
in process context") removed the process context workqueue from
amdtp_domain_stream_pcm_pointer() and update_pcm_pointers() to remove
its overhead.

With RME Fireface 800, this lead to a regression since
Kernels 5.14.0, causing an AB/BA deadlock competition for the
substream lock with eventual system freeze under ALSA operation:

thread 0:
    * (lock A) acquire substream lock by
snd_pcm_stream_lock_irq() in
snd_pcm_status64()
    * (lock B) wait for tasklet to finish by calling
     tasklet_unlock_spin_wait() in
tasklet_disable_in_atomic() in
ohci_flush_iso_completions() of ohci.c

thread 1:
    * (lock B) enter tasklet
    * (lock A) attempt to acquire substream lock,
     waiting for it to be released:
snd_pcm_stream_lock_irqsave() in
     snd_pcm_period_elapsed() in
update_pcm_pointers() in
process_ctx_payloads() in
process_rx_packets() of amdtp-stream.c

? tasklet_unlock_spin_wait
 </NMI>
 <TASK>
ohci_flush_iso_completions firewire_ohci
amdtp_domain_stream_pcm_pointer snd_firewire_lib
snd_pcm_update_hw_ptr0 snd_pcm
snd_pcm_status64 snd_pcm

? native_queued_spin_lock_slowpath
 </NMI>
 <IRQ>
_raw_spin_lock_irqsave
snd_pcm_period_elapsed snd_pcm
process_rx_packets snd_firewire_lib
irq_target_callback snd_firewire_lib
handle_it_packet firewire_ohci
context_tasklet firewire_ohci

Restore the process context work queue to prevent deadlock
AB/BA deadlock competition for ALSA substream lock of
snd_pcm_stream_lock_irq() in snd_pcm_status64()
and snd_pcm_stream_lock_irqsave() in snd_pcm_period_elapsed().

revert commit 7ba5ca32fe6e ("ALSA: firewire-lib: operate for period
elapse event in process context")

Replace inline description to prevent future deadlock.

Cc: stable@vger.kernel.org
Fixes: 7ba5ca32fe6e ("ALSA: firewire-lib: operate for period elapse event in process context")
Reported-by: edmund.raile <edmund.raile@proton.me>
Closes: https://lore.kernel.org/r/kwryofzdmjvzkuw6j3clftsxmoolynljztxqwg76hzeo4simnl@jn3eo7pe642q/
Signed-off-by: Edmund Raile <edmund.raile@protonmail.com>
Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20240730195318.869840-3-edmund.raile@protonmail.com
sound/firewire/amdtp-stream.c

index 31201d506a218f6e177a32da33754af32e1d5a2b..7438999e0510d040f2f511f40a014819dc9ef5a1 100644 (file)
@@ -615,16 +615,8 @@ static void update_pcm_pointers(struct amdtp_stream *s,
                // The program in user process should periodically check the status of intermediate
                // buffer associated to PCM substream to process PCM frames in the buffer, instead
                // of receiving notification of period elapsed by poll wait.
-               if (!pcm->runtime->no_period_wakeup) {
-                       if (in_softirq()) {
-                               // In software IRQ context for 1394 OHCI.
-                               snd_pcm_period_elapsed(pcm);
-                       } else {
-                               // In process context of ALSA PCM application under acquired lock of
-                               // PCM substream.
-                               snd_pcm_period_elapsed_under_stream_lock(pcm);
-                       }
-               }
+               if (!pcm->runtime->no_period_wakeup)
+                       queue_work(system_highpri_wq, &s->period_work);
        }
 }
 
@@ -1864,11 +1856,14 @@ unsigned long amdtp_domain_stream_pcm_pointer(struct amdtp_domain *d,
 {
        struct amdtp_stream *irq_target = d->irq_target;
 
-       // Process isochronous packets queued till recent isochronous cycle to handle PCM frames.
        if (irq_target && amdtp_stream_running(irq_target)) {
-               // In software IRQ context, the call causes dead-lock to disable the tasklet
-               // synchronously.
-               if (!in_softirq())
+               // use wq to prevent AB/BA deadlock competition for
+               // substream lock:
+               // fw_iso_context_flush_completions() acquires
+               // lock by ohci_flush_iso_completions(),
+               // amdtp-stream process_rx_packets() attempts to
+               // acquire same lock by snd_pcm_elapsed()
+               if (current_work() != &s->period_work)
                        fw_iso_context_flush_completions(irq_target->context);
        }