[alsa-devel] [PATCH] ad1848 ac loop

[Krzysztof Helt]

On Wed, 19 Sep 2007 12:50:54 -0700 (PDT)
Trent Piepho <xyzzy at speakeasy.org> wrote:

> On Wed, 19 Sep 2007, Krzysztof Helt wrote:
> >  However, quick test showed that playback is stopped
> >  from the irq handler if there is no more data (indirect register 9 write).
> 
> Hmm, I don't see where that happens.  Unless snd_pcm_period_elapsed() is
> allowed to call the trigger callback?  I know it will call the pointer
> callback, which doesn't use any indirect registers.
> 
> I know that trigger, ack, and pointer are called with interrupts disabled and
> a spinlock held.  But that is not the same as being called from interrupt
> context.  

I do not understand these two sentences so probably you are right.
I can provide you backtrace how the snd_ad1848_trigger() is called:

...
snd_ad1848_trigger
snd_ad1848_playback_trigger
snd_pcm_do_stop
snd_pcm_action_single
snd_pcm_period_elapsed
scheduler_tick
run_timer_softirq
snd_ad1848_interrupt
...
do_IRQ
...

> > > Another CPU could close the substream between the check and the call to
> > > snd_pcm_period_elapsed().  snd_pcm_period_elapsed() could be called with
> > > either NULL or a stale substream pointer depending on how the code compiled.
> > > I'd think creating an irq spinlock would be an easy way to fix this.  The irq
> > > handler would grab it, and the same with the open() and close() functions
> > > around the code that sets the substream pointers.  Alternatively, one could
> > > disabled the irq handler during open and close.
> >
> > I would not do this in the driver code. This should be handled in the alsa pcm layer
> > and probably it is. At least the snd_pcm_period_elapsed() does locking and irq
> > disabling. Could any alsa guru enlighten us on pcm open/close locking
> > and interrupt disabling?
> 
> I don't see how the alsa pcm layer could lock around the interrupt handler.
> The handler is registered directly to the kernel, not via alsa, so there's no
> way the alsa pcm layer could know if the irq handler is running, or if one
> even exists.

The pcm layer may disable interrupts. IMO, this should be enough to stop entering 
any interrupt handler.
I am not expert on this but following your reasoning any core kernel code 
(vm, scheduler) could not be interrupt-safe because most of interrupt handlers
are in drivers and the core has no idea that drivers exist.

Regards,
Krzysztof