| 5a25de6d | 13-Aug-2020 |
Dinghao Liu <dinghao.liu@zju.edu.cn> |
ALSA: echoaudio: Fix potential Oops in snd_echo_resume()
Freeing chip on error may lead to an Oops at the next time
the system goes to resume. Fix this by removing all
snd_echo_free() calls on error
ALSA: echoaudio: Fix potential Oops in snd_echo_resume()
Freeing chip on error may lead to an Oops at the next time
the system goes to resume. Fix this by removing all
snd_echo_free() calls on error.
Fixes: 47b5d028fdce8 ("ALSA: Echoaudio - Add suspend support #2")
Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
Link: https://lore.kernel.org/r/20200813074632.17022-1-dinghao.liu@zju.edu.cn
Signed-off-by: Takashi Iwai <tiwai@suse.de>
show more ...
|
| a0b224b9 | 08-Jul-2020 |
Mark Hills <mark@xwax.org> |
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
applicatio
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>
show more ...
|
| f688a0df | 08-Jul-2020 |
Mark Hills <mark@xwax.org> |
ALSA: echoaudio: Prevent some noise on unloading the module
These are valid conditions in normal circumstances, so do not "warn" but
make them for debugging.
Signed-off-by: Mark Hills <mark@xwax.or
ALSA: echoaudio: Prevent some noise on unloading the module
These are valid conditions in normal circumstances, so do not "warn" but
make them for debugging.
Signed-off-by: Mark Hills <mark@xwax.org>
Link: https://lore.kernel.org/r/20200708101848.3457-4-mark@xwax.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
show more ...
|
| 6c331254 | 08-Jul-2020 |
Mark Hills <mark@xwax.org> |
ALSA: echoaudio: Prevent races in calls to set_audio_format()
The function uses chip->comm_page which needs locking against
other use at the same time.
Signed-off-by: Mark Hills <mark@xwax.org>
Lin
ALSA: echoaudio: Prevent races in calls to set_audio_format()
The function uses chip->comm_page which needs locking against
other use at the same time.
Signed-off-by: Mark Hills <mark@xwax.org>
Link: https://lore.kernel.org/r/20200708101848.3457-3-mark@xwax.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
show more ...
|
| 027c7002 | 08-Jul-2020 |
Mark Hills <mark@xwax.org> |
ALSA: echoaudio: Race conditions around "opencount"
Use of atomics does not make these statements robust:
atomic_inc(&chip->opencount);
if (atomic_read(&chip->opencount) > 1 && chip->
ALSA: echoaudio: Race conditions around "opencount"
Use of atomics does not make these statements robust:
atomic_inc(&chip->opencount);
if (atomic_read(&chip->opencount) > 1 && chip->rate_set)
chip->can_set_rate=0;
and
if (atomic_read(&chip->opencount)) {
if (chip->opencount) {
changed = -EAGAIN;
} else {
changed = set_digital_mode(chip, dmode);
It would be necessary to atomically increment or decrement the value
and use the returned result. And yet we still need to prevent other
threads making use of "can_set_rate" while we set it.
However in all but one case the atomic is misleading as they are already
running with "mode_mutex" held.
Decisions are made on mode setting are often intrinsically connected
to "opencount" because some operations are not permitted unless
there is sole ownership.
So instead simplify this, and use "mode_mutex" as a lock for all reference
counting and mode setting.
Signed-off-by: Mark Hills <mark@xwax.org>
Link: https://lore.kernel.org/r/20200708101848.3457-2-mark@xwax.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
show more ...
|