Hi, This patchset comes from my former RFC: [RFC][PATCH 0/3] ALSA: pcm/firewire: allow to queue period elapse event in process context * https://lore.kernel.org/alsa-devel/20210606091838.80812-1-o-takashi@xxxxxxxxxxxxx/ All of drivers in ALSA firewire stack have two chances to process isochronous packets of any isochronous context; in software IRQ context for 1394 OHCI, and in process context of ALSA PCM application. In the process context, callbacks of .pointer and .ack are utilized. The callbacks are done by ALSA PCM core under acquiring lock of PCM substream, In design of ALSA PCM core, call of snd_pcm_period_elapsed() is used for drivers to awaken user processes from waiting for available frames. The function voluntarily acquires lock of PCM substream, therefore it is not called in the process context since it causes dead lock. As a workaround to avoid the dead lock, all of drivers in ALSA firewire stack use workqueue to delegate the call. This patchset is my attempt for the issue. A variant of 'snd_pcm_period_elapsed()' without lock acquisition is going to be added, named 'snd_pcm_period_elapsed_under_stream_lock()'. The call is available in callbacks of .pointer and .ack of snd_pcm_ops structure. Changes from RFC: * dismiss inlining * rename function name * improve function comments I tested the patchset with ALSA OXFW driver and ftrace, covering axfer and jackd cases since pulseaudio and pipewire are programmed with timer-based scheduling model and ALSA runtime expects drivers not to call snd_pcm_period_elapsed(). The configuration of tracer and filters is: ``` $ sudo trace-cmd record -p function_graph \ -l :mod:snd_firewire_lib \ -l fw_iso_context_queue:mod:firewire_core \ -l snd_pcm_ioctl:mod:snd_pcm \ -l snd_pcm_period_elapsed*:mod:snd_pcm ``` The runtime of axfer without '-M' option can often call .pointer and .ack. Below is a sample about .pointer case via SNDRV_PCM_IOCT_HWSYNC. The new snd_pcm_period_elapsed_under_stream_lock() should be called. ``` 3929.769359: funcgraph_entry: | snd_pcm_ioctl() { 3929.769360: funcgraph_entry: | amdtp_domain_stream_pcm_pointer() { 3929.769361: funcgraph_entry: | irq_target_callback() { 3929.769361: funcgraph_entry: | process_rx_packets() { 3929.769362: funcgraph_entry: | process_ctx_payloads() { 3929.769363: funcgraph_entry: 0.391 us | process_it_ctx_payloads(); 3929.769363: funcgraph_entry: | snd_pcm_period_elapsed_under_stream_lock() { 3929.769364: funcgraph_entry: 0.411 us | amdtp_domain_stream_pcm_pointer(); 3929.769365: funcgraph_exit: 1.723 us | } 3929.769365: funcgraph_exit: 3.106 us | } 3929.769366: funcgraph_entry: | queue_packet() { 3929.769366: funcgraph_entry: 0.561 us | fw_iso_context_queue(); 3929.769367: funcgraph_exit: 1.122 us | } 3929.769367: funcgraph_exit: 5.731 us | } 3929.769367: funcgraph_entry: | process_ctxs_in_domain() { 3929.769368: funcgraph_entry: | process_tx_packets() { 3929.769369: funcgraph_entry: | process_ctx_payloads() { 3929.769369: funcgraph_entry: 0.321 us | process_ir_ctx_payloads(); 3929.769369: funcgraph_exit: 0.962 us | } 3929.769370: funcgraph_entry: 0.491 us | fw_iso_context_queue(); 3929.769371: funcgraph_exit: 2.364 us | } 3929.769371: funcgraph_exit: 3.427 us | } 3929.769371: funcgraph_exit: 10.038 us | } 3929.769372: funcgraph_exit: 11.271 us | } 3929.769372: funcgraph_exit: 13.606 us | } ``` The runtime of jackd heavily relies on period wakeup scheduled in invocation of interrupt handler. Below is a sample about software IRQ case of 1394 OHCI. The snd_pcm_period_elapsed() should be called. ``` 5318.980502: funcgraph_entry: | irq_target_callback() { 5318.980503: funcgraph_entry: | process_rx_packets() { 5318.980503: funcgraph_entry: | process_ctx_payloads() { 5318.980503: funcgraph_entry: 0.531 us | process_it_ctx_payloads(); 5318.980505: funcgraph_entry: | snd_pcm_period_elapsed() { 5318.980505: funcgraph_entry: | snd_pcm_period_elapsed_under_stream_lock() { 5318.980505: funcgraph_entry: 0.180 us | amdtp_domain_stream_pcm_pointer(); 5318.980508: funcgraph_exit: 3.106 us | } 5318.980508: funcgraph_exit: 3.497 us | } 5318.980508: funcgraph_exit: 4.990 us | } 5318.980508: funcgraph_entry: | queue_packet() { 5318.980509: funcgraph_entry: 0.350 us | fw_iso_context_queue(); 5318.980509: funcgraph_exit: 0.702 us | } 5318.980509: funcgraph_entry: | queue_packet() { 5318.980509: funcgraph_entry: 0.291 us | fw_iso_context_queue(); 5318.980510: funcgraph_exit: 0.651 us | } 5318.980510: funcgraph_entry: | queue_packet() { 5318.980510: funcgraph_entry: 0.280 us | fw_iso_context_queue(); 5318.980511: funcgraph_exit: 0.641 us | } 5318.980511: funcgraph_exit: 8.416 us | } 5318.980511: funcgraph_entry: | process_ctxs_in_domain() { 5318.980512: funcgraph_entry: | process_tx_packets() { 5318.980512: funcgraph_entry: | process_ctx_payloads() { 5318.980512: funcgraph_entry: 0.531 us | process_ir_ctx_payloads(); 5318.980513: funcgraph_entry: | snd_pcm_period_elapsed() { 5318.980513: funcgraph_entry: | snd_pcm_period_elapsed_under_stream_lock() { 5318.980513: funcgraph_entry: 0.170 us | amdtp_domain_stream_pcm_pointer(); 5318.980514: funcgraph_exit: 0.842 us | } 5318.980514: funcgraph_exit: 1.242 us | } 5318.980514: funcgraph_exit: 2.335 us | } 5318.980515: funcgraph_entry: 0.301 us | fw_iso_context_queue(); 5318.980515: funcgraph_entry: 0.291 us | fw_iso_context_queue(); 5318.980516: funcgraph_entry: 0.290 us | fw_iso_context_queue(); 5318.980516: funcgraph_exit: 4.198 us | } 5318.980516: funcgraph_exit: 5.119 us | } 5318.980516: funcgraph_exit: 14.077 us | } ``` ALSA OXFW driver works well to select appropriate kernel API by distinguishing running context. I also tested for error cases that the sequence of tx packets is invalid or packet queueing fails. In both cases, the error doesn't bring system corruption. Takashi Sakamoto (3): ALSA: pcm: add snd_pcm_period_elapsed() variant without acquiring lock of PCM substream ALSA: firewire-lib: operate for period elapse event in process context ALSA: firewire-lib: obsolete workqueue for period update include/sound/pcm.h | 1 + sound/core/pcm_lib.c | 68 +++++++++++++++++++++++++++-------- sound/firewire/amdtp-stream.c | 46 ++++++++---------------- sound/firewire/amdtp-stream.h | 1 - 4 files changed, 68 insertions(+), 48 deletions(-) -- 2.27.0
