[alsa-devel] [PATCH v3] ALSA: compress: fix drain calls blocking other compress functions

[Takashi Iwai]

At Wed, 30 Oct 2013 20:48:40 +0530,
Vinod Koul wrote:
> 
> On Wed, Oct 30, 2013 at 05:01:38PM +0100, Takashi Iwai wrote:
>  
> > > > > +static int snd_compress_wait_for_drain(struct snd_compr_stream *stream)
> > > > > +{
> > > > > +	/*
> > > > > +	 * We are called with lock held. So drop the lock while we wait for
> > > > > +	 * drain complete notfication from the driver
> > > > > +	 *
> > > > > +	 * It is expected that driver will notify the drain completion and then
> > > > > +	 * stream will be moved to SETUP state, even if draining resulted in an
> > > > > +	 * error. We can trigger next track after this.
> > > > > +	 */
> > > > > +	stream->runtime->state = SNDRV_PCM_STATE_DRAINING;
> > > > > +	mutex_unlock(&stream->device->lock);
> > > > > +
> > > > > +	wait_event(stream->runtime->wait, stream->runtime->drain_wake);
> > > > 
> > > > Don't you really want the interruption?
> > > > Or use wait_event_interruptible() and return error appropriately.
> > > any interruption from usermode should trigger the compress_stop/compress_free
> > 
> > ... but how?
> > 
> > > which will unblock this. I dont see the need to have interruptible here
> > 
> > Suppose you're running a single thread program.  Then who triggers the
> > stop?
> Your signal catcher which should be registered for signals intrested and then
> calls stop or free. Assuming this should be considered single threaded. IIRC,
> this is how you do in aplay, right?

No, issuing an ioctl in a signal isn't guaranteed to work at all.
You shouldn't code the kernel driver relying on that userspace
behavior.


Takashi