Re: ALSA: hda - Fix possible races in HDMI driver - lockup on shutdownwhen radeon.audio=1 after using audacity

From: Arthur Marsh
Date: Tue Jan 21 2014 - 16:58:09 EST




Takashi Iwai wrote, on 21/01/14 21:43:
At Tue, 21 Jan 2014 11:50:43 +1030,
Arthur Marsh wrote:



Takashi Iwai wrote, on 20/01/14 19:22:
At Sun, 19 Jan 2014 17:32:16 +1030,
Arthur Marsh wrote:

I have had reproducible lock-ups on shut-down (at the shutting down ALSA
stage) of my AMD64 machine (Asus M3A78Pro motherboard, BIOS 1701
01/27/2011, CPU AMD Athlon(tm) II X4 640 Processor) running the 64 bit
Linux kernel more recent than 3.12 when *both* radeon.audio=1 was set
and I had been running audacity 2.0.5. (iommu=noaperture is also set).

The problem was reproducible with the stock Debian kernel
linux-image-3.13-rc6-amd64 version 3.13~rc6-1~exp1.

The machine is using an ATI/AMD 3850HD video card with a DVI cable to a
DVI input on my monitor, and the default audio device is the
motherboard's on-board audio device:

00:14.2 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] SBx00
Azalia (Intel HDA)

01:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
[AMD/ATI] RV670 [Radeon HD 3690/3850]
01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] RV670/680
HDMI Audio [Radeon HD 3690/3800 Series]

$ git bisect bad
cbbaa603a03cc46681e24d6b2804b62fde95a2af is the first bad commit
commit cbbaa603a03cc46681e24d6b2804b62fde95a2af
Author: Takashi Iwai <tiwai@xxxxxxx>
Date: Thu Oct 17 18:03:24 2013 +0200

ALSA: hda - Fix possible races in HDMI driver

Some per_pin fields and ELD contents might be changed dynamically in
multiple ways where the concurrent accesses are still opened in the
current code. This patch fixes such possible races by using eld->lock
in appropriate places.

Reported-by: Anssi Hannula <anssi.hannula@xxxxxx>
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>

:040000 040000 0c29281f82a3ebd9a704d481114f9cfcefea07c8
d71fd101125cd29a628cb5e66c7ee4f56e28329b M sound

When running audacity from the command line there was the following output:

ALSA lib pcm_dsnoop.c:618:(snd_pcm_dsnoop_open) unable to open slave
ALSA lib pcm_dmix.c:1022:(snd_pcm_dmix_open) unable to open slave
ALSA lib pcm.c:2239:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.rear
ALSA lib pcm.c:2239:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.center_lfe
ALSA lib pcm.c:2239:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.side
ALSA lib pcm_dmix.c:1022:(snd_pcm_dmix_open) unable to open slave
Expression 'stream->playback.pcm' failed in
'src/hostapi/alsa/pa_linux_alsa.c', line: 4611
Expression 'stream->playback.pcm' failed in
'src/hostapi/alsa/pa_linux_alsa.c', line: 4611
Expression 'stream->playback.pcm' failed in
'src/hostapi/alsa/pa_linux_alsa.c', line: 4611

I am happy to supply further information or run further tests to help in
isolating the problem and verifying a solution.

Could you build the kernel with lockdep kconfig and see whether it
reports errors?

Reverting the commit doesn't work cleanly. Instead, you can try to
simply comment out all mutex_lock(&per_pin->lock) and
mutex_unlock(&per_pin->lock) calls in patch_hdmi.c to see whether it's
a mutex deadlock.


thanks,

Takashi


I rebuilt the kernel after commenting out all mutex_lock(&per_pin->lock)
and mutex_unlock(&per_pin->lock) calls in patch_hdmi.c, and the
resulting kernel shutdown without hanging.

Thanks. Could you try the patch below? It's already queued for
3.14. If this patch really fixes the issue, I'm going to send it to
stable 3.13.x.


Takashi

-- 8< --
From: David Henningsson <david.henningsson@xxxxxxxxxxxxx>
Subject: [PATCH] ALSA: hda - Explicitly keep codec powered up in
hdmi_present_sense

This should help us avoid the following mutex deadlock:

[] mutex_lock+0x2a/0x50
[] hdmi_present_sense+0x53/0x3a0 [snd_hda_codec_hdmi]
[] generic_hdmi_resume+0x5a/0x70 [snd_hda_codec_hdmi]
[] hda_call_codec_resume+0xec/0x1d0 [snd_hda_codec]
[] snd_hda_power_save+0x1e4/0x280 [snd_hda_codec]
[] codec_exec_verb+0x5f/0x290 [snd_hda_codec]
[] snd_hda_codec_read+0x5b/0x90 [snd_hda_codec]
[] snd_hdmi_get_eld_size+0x1e/0x20 [snd_hda_codec_hdmi]
[] snd_hdmi_get_eld+0x2c/0xd0 [snd_hda_codec_hdmi]
[] hdmi_present_sense+0x9a/0x3a0 [snd_hda_codec_hdmi]
[] hdmi_repoll_eld+0x34/0x50 [snd_hda_codec_hdmi]

Signed-off-by: David Henningsson <david.henningsson@xxxxxxxxxxxxx>
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
sound/pci/hda/patch_hdmi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index f5060fc7c303..977db17db26c 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1496,11 +1496,14 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
* specification worked this way. Hence, we just ignore the data in
* the unsolicited response to avoid custom WARs.
*/
- int present = snd_hda_pin_sense(codec, pin_nid);
+ int present;
bool update_eld = false;
bool eld_changed = false;
bool ret;

+ snd_hda_power_up(codec);
+ present = snd_hda_pin_sense(codec, pin_nid);
+
mutex_lock(&per_pin->lock);
pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
if (pin_eld->monitor_present)
@@ -1573,6 +1576,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
jack->block_report = !ret;

mutex_unlock(&per_pin->lock);
+ snd_hda_power_down(codec);
return ret;
}



The patch worked thanks.

I had to revert this patch to update the kernel to the current Linus' git head.

Regards,

Arthur.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/