[PATCH V0 0/1] ALSA: pcm: fix blocking while loop in

[PATCH V0 0/1] ALSA: pcm: fix blocking while loop in

[Raghu Bankapur]

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.


when runtime->hw_ptr_buffer_jiffies is 0 then update of while condition 
hdelta > xrun_threshold will be always true because hdelta will not
change. This will lead to infinite loop causing lock-up

This issue found during stability test, below log explains CPU lock-up on CPU1

[ 922.249028] sched: RT throttling activated for rt_rq ffffffc0f9970778 (cpu 1)
[ 922.249028] potential CPU hogs:
[ 922.249028] **FastCapture (11426)
[ 922.249105] ------------[ cut here ]---------

To fix this lock-up issue, added new condition to check if runtime->
hw_ptr_buffer_jiffies is ZERO or negative.

Raghu Bankapur (1):
  ALSA: pcm: fix blocking while loop in snd_pcm_update_hw_ptr0()

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

-- 
2.17.1

[PATCH V0 1/1] ALSA: pcm: fix blocking while loop in snd_pcm_update_hw_ptr0()

[Raghu Bankapur]

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: Raghu Bankapur <quic_rbankapu@quicinc.com>
---
 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 f2090025236b..bb9097f658be 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -352,7 +352,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 <= 0))
 			goto no_delta_check;
 		hdelta = jdelta - delta * HZ / runtime->rate;
 		xrun_threshold = runtime->hw_ptr_buffer_jiffies / 2 + 1;
-- 
2.17.1

Re: [PATCH V0 1/1] ALSA: pcm: fix blocking while loop in snd_pcm_update_hw_ptr0()

[Jaroslav Kysela]

On 25. 02. 22 11:39, Raghu Bankapur wrote:
> 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.

I would set hw_ptr_buffer_jiffies to 1 in this case in snd_pcm_post_start().

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH V0 1/1] ALSA: pcm: fix blocking while loop in snd_pcm_update_hw_ptr0()

[Takashi Iwai]

On Fri, 25 Feb 2022 11:52:05 +0100,
Jaroslav Kysela wrote:
> 
> On 25. 02. 22 11:39, Raghu Bankapur wrote:
> > 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.
> 
> I would set hw_ptr_buffer_jiffies to 1 in this case in snd_pcm_post_start().

I thought of it at the first glance, but after reading the code again,
I doubt whether it makes sense at all to allow this condition.
Since the buffer size is too small and the rate is too high, we can't
calculate the buffer crossing condition accurately under such
condition.

But, having either this zero check or minimal hw_ptr_buffer_jiffies=1
would be good in anyway, even if we add more check for the hw_params
for no-period-wakeup case.


thanks,

Takashi

Re: [PATCH V0 1/1] ALSA: pcm: fix blocking while loop in snd_pcm_update_hw_ptr0()

[Takashi Iwai]

On Fri, 25 Feb 2022 13:26:36 +0100,
Raghu Ballappa Bankapur wrote:
> 
> Hi Takashi,
> 
> Thanks for your feedback.
> 
> I see your below statement
> 
> But, having either this zero check or minimal hw_ptr_buffer_jiffies=1
> would be good in anyway, even if we add more check for the hw_params
> for no-period-wakeup case.
> 
> Please review if those changes are Ok

If you mean about your posted patch for "those changes", as Jaroslav
suggested in the thread, we may take a different approach: just set
the minimal hw_ptr_buffer_jiffies to 1.

Could you try this and submit the fix if that works for you?


thanks,

Takashi

> 
> Regards
> Raghu
> 
> On 2/25/2022 4:53 PM, Takashi Iwai wrote:
> 
>     On Fri, 25 Feb 2022 11:52:05 +0100,
>     Jaroslav Kysela wrote:
>     
>         On 25. 02. 22 11:39, Raghu Bankapur wrote:
>         
>             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.
>             
>         I would set hw_ptr_buffer_jiffies to 1 in this case in snd_pcm_post_start().
>         
>     I thought of it at the first glance, but after reading the code again,
>     I doubt whether it makes sense at all to allow this condition.
>     Since the buffer size is too small and the rate is too high, we can't
>     calculate the buffer crossing condition accurately under such
>     condition.
>     
>     But, having either this zero check or minimal hw_ptr_buffer_jiffies=1
>     would be good in anyway, even if we add more check for the hw_params
>     for no-period-wakeup case.
> 
>     thanks,
>     
>     Takashi
> 
>