mbox series

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

Message ID cover.1645784757.git.quic_rbankapu@quicinc.com
Headers show
Series ALSA: pcm: fix blocking while loop in | expand

Message

Raghu Ballappa Bankapur Feb. 25, 2022, 10:39 a.m. UTC 
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(-)

Comments

Raghu Ballappa Bankapur Feb. 25, 2022, 12:26 p.m. UTC | #1 
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

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
Takashi Iwai Feb. 28, 2022, 3:58 p.m. UTC | #2 
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
> 
>