[RFC] ALSA: pcm: fix blocking while loop in snd_pcm_update_hw_ptr0()

Message ID 20191023120114.17445-1-srinivas.kandagatla@linaro.org
State New
Headers show
Series
  • [RFC] ALSA: pcm: fix blocking while loop in snd_pcm_update_hw_ptr0()
Related show

Commit Message

Srinivas Kandagatla Oct. 23, 2019, 12:01 p.m.
From: Preetam Singh Ranawat <apranawat@codeaurora.org>


When period interrupts are disabled, while loop in snd_pcm_update_hw_ptr0()
results in the machine locking up if runtime->hw_ptr_buffer_jiffies is 0.

Validate runtime->hw_ptr_buffer_jiffies value before while loop to avoid
delta check.

Signed-off-by: Preetam Singh Ranawat <apranawat@codeaurora.org>

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

---

This issue was noticed in Android setup from fastcapture path.

 sound/core/pcm_lib.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.21.0

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

Comments

Takashi Iwai Oct. 23, 2019, 1:29 p.m. | #1
On Wed, 23 Oct 2019 14:01:14 +0200,
Srinivas Kandagatla wrote:
> 

> From: Preetam Singh Ranawat <apranawat@codeaurora.org>

> 

> When period interrupts are disabled, while loop in snd_pcm_update_hw_ptr0()

> results in the machine locking up if runtime->hw_ptr_buffer_jiffies is 0.

> 

> Validate runtime->hw_ptr_buffer_jiffies value before while loop to avoid

> delta check.


Although the fix itself is fine, I still wonder how this is
triggered.  Could you check in what kind of PCM state is it?

Basically snd_pcm_update_hw_ptr*() is called only in the running PCM
state, and for that, we should have set hw_ptr_buffer_jiffies at
snd_pcm_post_start().  It's

	runtime->hw_ptr_buffer_jiffies = (runtime->buffer_size * HZ) / 
							    runtime->rate;

So maybe the given buffer size is so small and hw_ptr_buffer_jiffies
is really 0?


thanks,

Takashi

> 

> Signed-off-by: Preetam Singh Ranawat <apranawat@codeaurora.org>

> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

> ---

> 

> This issue was noticed in Android setup from fastcapture path.

> 

>  sound/core/pcm_lib.c | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

> 

> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c

> index d80041ea4e01..f407f6895f8a 100644

> --- a/sound/core/pcm_lib.c

> +++ b/sound/core/pcm_lib.c

> @@ -341,7 +341,8 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,

>  		 * the elapsed time to detect xruns.

>  		 */

>  		jdelta = curr_jiffies - runtime->hw_ptr_jiffies;

> -		if (jdelta < runtime->hw_ptr_buffer_jiffies / 2)

> +		if ((jdelta < runtime->hw_ptr_buffer_jiffies / 2) ||

> +		    !runtime->hw_ptr_buffer_jiffies)

>  			goto no_delta_check;

>  		hdelta = jdelta - delta * HZ / runtime->rate;

>  		xrun_threshold = runtime->hw_ptr_buffer_jiffies / 2 + 1;

> -- 

> 2.21.0

> 

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

Patch

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index d80041ea4e01..f407f6895f8a 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -341,7 +341,8 @@  static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 		 * the elapsed time to detect xruns.
 		 */
 		jdelta = curr_jiffies - runtime->hw_ptr_jiffies;
-		if (jdelta < runtime->hw_ptr_buffer_jiffies / 2)
+		if ((jdelta < runtime->hw_ptr_buffer_jiffies / 2) ||
+		    !runtime->hw_ptr_buffer_jiffies)
 			goto no_delta_check;
 		hdelta = jdelta - delta * HZ / runtime->rate;
 		xrun_threshold = runtime->hw_ptr_buffer_jiffies / 2 + 1;