| Message ID | 20210606091838.80812-1-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| Headers | show |
| Series | ALSA: pcm/firewire: allow to queue period elapse event in process context | expand |
On Sun, 06 Jun 2021 11:18:35 +0200, Takashi Sakamoto wrote: > > Hi, > > All of drivers in ALSA firewire stack processes two chances to process > isochronous packets in 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 uses > 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_without_lock()'. This is used in callbacks > of .pointer and .ack of snd_pcm_ops structure. > > The patchset is still under my test, but it looks to work well in my > easy and rough test. Before posting for merge, I'd like to get your > comment to the idea. When evaluating, please merge below two histories: > * 64584f329352 (for-next) > * 9981b20a5e36 (for-linus) > > Takashi Sakamoto (3): > ALSA: pcm: add snd_pcm_period_elapsed() variant without acquiring lock > of PCM substream > ALSA: firewire-lib: queue event of period elapse in process context > ALSA: firewire-lib: obsolete workqueue for period update The idea is fine, but moving snd_pcm_period_elapsed() as inline static doesn't give much benefit, IMO. Although it can avoid an exported symbol, its cost is much higher, since it'd expand the code at each place of snd_pcm_period_elapsed(), i.e. almost in all driver code. Just provide two exported functions instead in a more straightforward way. thanks, Takashi
Hi, On Sun, Jun 06, 2021 at 12:20:57PM +0200, Takashi Iwai wrote: > On Sun, 06 Jun 2021 11:18:35 +0200, > Takashi Sakamoto wrote: > > > > Hi, > > > > All of drivers in ALSA firewire stack processes two chances to process > > isochronous packets in 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 uses > > 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_without_lock()'. This is used in callbacks > > of .pointer and .ack of snd_pcm_ops structure. > > > > The patchset is still under my test, but it looks to work well in my > > easy and rough test. Before posting for merge, I'd like to get your > > comment to the idea. When evaluating, please merge below two histories: > > * 64584f329352 (for-next) > > * 9981b20a5e36 (for-linus) > > > > Takashi Sakamoto (3): > > ALSA: pcm: add snd_pcm_period_elapsed() variant without acquiring lock > > of PCM substream > > ALSA: firewire-lib: queue event of period elapse in process context > > ALSA: firewire-lib: obsolete workqueue for period update > > The idea is fine, but moving snd_pcm_period_elapsed() as inline static > doesn't give much benefit, IMO. Although it can avoid an exported > symbol, its cost is much higher, since it'd expand the code at each > place of snd_pcm_period_elapsed(), i.e. almost in all driver code. > Just provide two exported functions instead in a more straightforward > way. Thanks for your positive comment. I agree with it. When adding parameters for function internal, we will discuss about the inlining for variations of kernel API again, I guess. After merging for-linus branch into for-next branch, I'm going to post it again. At the time, I may finish enough test. Thanks Takashi Sakamoto
Hi, All of drivers in ALSA firewire stack processes two chances to process isochronous packets in 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 uses 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_without_lock()'. This is used in callbacks of .pointer and .ack of snd_pcm_ops structure. The patchset is still under my test, but it looks to work well in my easy and rough test. Before posting for merge, I'd like to get your comment to the idea. When evaluating, please merge below two histories: * 64584f329352 (for-next) * 9981b20a5e36 (for-linus) Takashi Sakamoto (3): ALSA: pcm: add snd_pcm_period_elapsed() variant without acquiring lock of PCM substream ALSA: firewire-lib: queue event of period elapse in process context ALSA: firewire-lib: obsolete workqueue for period update include/sound/pcm.h | 53 ++++++++++++++++++++++++++++++++++- sound/core/pcm_lib.c | 25 +++-------------- sound/firewire/amdtp-stream.c | 46 +++++++++--------------------- sound/firewire/amdtp-stream.h | 1 - 4 files changed, 70 insertions(+), 55 deletions(-)