Re: [alsa-devel] [PATCH 6/8] ALSA: pcm: Add the support for sync-stop operation

[Takashi Iwai <tiwai@suse.de>]

On Mon, 18 Nov 2019 17:33:51 +0100,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 11/17/19 2:53 AM, Takashi Iwai wrote:
> > The standard programming model of a PCM sound driver is to process
> > snd_pcm_period_elapsed() from an interrupt handler.  When a running
> > stream is stopped, PCM core calls the trigger-STOP PCM ops, sets the
> > stream state to SETUP, and moves on to the next step.  This is
> > performed in an atomic manner -- this could be called from the interrupt
> > context, after all.
> >
> > The problem is that, if the stream goes further and reaches to the
> > CLOSE state immediately, the stream might be still being processed in
> > snd_pcm_period_elapsed() in the interrupt context, and hits a NULL
> > dereference.  Such a crash happens because of the atomic operation,
> > and we can't wait until the stream-stop finishes.
> >
> > For addressing such a problem, this commit adds a new PCM ops,
> > sync_stop.  This gets called at the appropriate places that need a
> > sync with the stream-stop, i.e. at hw_params, prepare and hw_free.
> >
> > Some drivers already have a similar mechanism implemented locally, and
> > we'll refactor the code later.
> 
> This rings a bell, for the SOF driver Keyon added a workqueue to avoid
> doing the call to snd_pcm_period_elapsed() while handling IPC
> interrupts. I believe the reason what that the IPC needs to be
> serialized, and the call to snd_pcm_period_elapsed could initiate a
> trigger stop IPC while we were dealing with an IPC, which broke the
> notion of serialization.
> 
> See "ASoC: SOF: PCM: add period_elapsed work to fix race condition in
> interrupt context"
> and "ASoC: SOF: ipc: use snd_sof_pcm_period_elapsed"

The new PCM ops is exactly for resolving such a thing.  Basically
snd_pcm_period_elapsed() is handled asynchronously, so we need the
synchronization.  For a simpler driver, it's merely synchronize_irq()
call (no matter whether it's a hard-irq or a threaded-irq), but for
drivers that deal with another async mechanism (e.g. USB-audio),
another sync procedure is required.

It's possible that the workaround you've done in SOF can be simplified
by this mechanism, but I didn't take a deeper look at it yet.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel