On Thu, Apr 07, 2016 at 12:40:03AM +0000, Koul, Vinod wrote:
On Wed, 2016-04-06 at 11:21 +0100, Charles Keepax wrote:
+int snd_compr_stop_xrun(struct snd_compr_stream *stream) +{
- if (stream->runtime->state == SNDRV_PCM_STATE_XRUN)
return 0;- stream->runtime->state = SNDRV_PCM_STATE_XRUN;
- queue_delayed_work(system_power_efficient_wq, &stream
->xrun_work, 0);
why do we want to do this in workqueue and not stop the compress stream immediately.
We need to defer this to a work queue because it is very likely we will detect the errors whilst already in a callback in the driver. For example we notice the stream is bad whilst processing a read or a pointer callback in the driver. Because this call by definition goes right back to the top of the stack rather than unwinding the stack nicely as returning an error would do, we need to be careful of all the locks that are likely to be held in between.
snd_compr_ioctl - takes stream->device->lock snd_compr_tstamp soc_compr_pointer - takes rtd->pcm_mutex wm_adsp_compr_pointer - takes dsp->pwr_lock snd_compr_stop_xrun snd_compr_stop soc_compr_trigger - Deadlock as we take rtd->pcm_mutex again
Also if we do this, then why should pointer return error?
The first patch in the chain could indeed be changed to have pointer calls not return an error status. But I feel that would be making the code worse. Ok the situation I am most interested here indicates a failure of the stream, but its a very small leap to imagine situations where pointer fails temporarily and the stream is still good.
Thanks, Charles