Re: [PATCH 5/5] echoaudio: Address bugs in the interrupt handling

[Takashi Iwai <tiwai@suse.de>]

On Wed, 08 Jul 2020 12:18:48 +0200,
Mark Hills wrote:
> 
> Distorted audio appears occasionally, affecting either playback or
> capture and requiring the affected substream to be closed by all
> applications and re-opened.
> 
> The best way I have found to reproduce the bug is to use dmix in
> combination with Chromium, which opens the audio device multiple times
> in threads. Anecdotally, the problems appear to have increased with
> faster CPUs. I ruled out 32-bit counter wrapping; it often happens
> much earlier.
> 
> Since applying this patch I have not had problems, where previously
> they would occur several times a day.
> 
> The patch targets the following issues:
> 
> * Check for progress using the counter from the hardware, not after it
>   has been truncated to the buffer.
> 
>   This is a clean way to address a possible bug where if a whole
>   ringbuffer advances between interrupts, it goes unnoticed.
> 
> * Move last_period state from chip to pipe
> 
>   This more logically belongs as part of pipe, and code is reasier to
>   read if it is "counter position last time a period elapsed".
> 
>   Now the code has no references to period count. A period is just
>   when the regular counter crosses a threshold. This increases
>   readability and reduces scope for bugs.
> 
> * Treat period notification and buffer advance independently:
> 
>   This helps to clarify what is the responsibility of the interrupt
>   handler, and what is pcm_pointer().
> 
>   Removing shared state between these operations means race conditions
>   are fixed without introducing locks. Synchronisation is only around
>   the read of pipe->dma_counter. There may be cache line contention
>   around "struct audiopipe" but I did not have cause to profile this.
> 
> Pay attention to be robust where dma_counter wrapping is not a
> multiple of period_size or buffer_size.
> 
> This is a revised patch based on feedback from Takashi and Giuliano.
> 
> Signed-off-by: Mark Hills <mark@xwax.org>

Applied now.  Thanks.


Takashi