ALSA: echoaudio: Address bugs in the interrupt handling
authorMark Hills <mark@xwax.org>
Wed, 8 Jul 2020 10:18:48 +0000 (11:18 +0100)
committerTakashi Iwai <tiwai@suse.de>
Thu, 9 Jul 2020 10:59:24 +0000 (12:59 +0200)
commita0b224b90bb60b3fbd2cae750227c995c9e61055
tree32f10ec19eaf9b15dbad84c3237f86f19b98804d
parentf688a0dfe247d71d94352a3ea69b5bb535117580
ALSA: echoaudio: Address bugs in the interrupt handling

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>
Link: https://lore.kernel.org/r/20200708101848.3457-5-mark@xwax.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
sound/pci/echoaudio/echoaudio.c
sound/pci/echoaudio/echoaudio.h