[Intel-gfx] pcm_lock deadlock

Takashi Iwai tiwai at suse.de
Tue Oct 29 20:52:57 UTC 2019


On Tue, 29 Oct 2019 20:10:50 +0100,
Ville Syrjälä wrote:
> 
> Hi Takashi,
> 
> I just got this deadlock when I tried to modprobe i915 on an ELK:
> 
> [  203.716416] ============================================
> [  203.716417] WARNING: possible recursive locking detected
> [  203.716418] 5.4.0-rc5-elk+ #206 Not tainted
> [  203.716419] --------------------------------------------
> [  203.716420] kworker/0:1/12 is trying to acquire lock:
> [  203.716421] efb1c138 (&spec->pcm_lock){+.+.}, at: generic_hdmi_init+0x21/0x140 [snd_hda_codec_hdmi]
> [  203.716426] 
>                but task is already holding lock:
> [  203.716427] efb1c138 (&spec->pcm_lock){+.+.}, at: check_presence_and_report+0x67/0xb0 [snd_hda_codec_hdmi]
> [  203.716430] 
>                other info that might help us debug this:
> [  203.716431]  Possible unsafe locking scenario:
> 
> [  203.716431]        CPU0
> [  203.716432]        ----
> [  203.716432]   lock(&spec->pcm_lock);
> [  203.716433]   lock(&spec->pcm_lock);
> [  203.716434] 
>                 *** DEADLOCK ***
> 
> [  203.716435]  May be due to missing lock nesting notation
> 
> [  203.716436] 3 locks held by kworker/0:1/12:
> [  203.716436]  #0: f14096a0 ((wq_completion)events){+.+.}, at: process_one_work+0x1b8/0x530
> [  203.716442]  #1: f14dbf4c ((work_completion)(&bus->unsol_work)){+.+.}, at: process_one_work+0x1b8/0x530
> [  203.716444]  #2: efb1c138 (&spec->pcm_lock){+.+.}, at: check_presence_and_report+0x67/0xb0 [snd_hda_codec_hdmi]
> [  203.716448] 
>                stack backtrace:
> [  203.716449] CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.4.0-rc5-elk+ #206
> [  203.716450] Hardware name: System manufacturer P5Q-EM/P5Q-EM, BIOS 2203    07/08/2009
> [  203.716457] Workqueue: events snd_hdac_bus_process_unsol_events [snd_hda_core]
> [  203.716459] Call Trace:
> [  203.716463]  dump_stack+0x66/0x8e
> [  203.716466]  __lock_acquire.cold.62+0x3bf/0x3c7
> [  203.716468]  ? mark_held_locks+0x3f/0x60
> [  203.716470]  ? _raw_spin_unlock_irq+0x22/0x30
> [  203.716478]  ? azx_rirb_get_response+0xd7/0x220 [snd_hda_codec]
> [  203.716479]  ? lockdep_hardirqs_on+0xec/0x1a0
> [  203.716480]  ? _raw_spin_unlock_irq+0x22/0x30
> [  203.716483]  ? trace_hardirqs_on+0x4a/0xf0
> [  203.716484]  ? find_held_lock+0x26/0xb0
> [  203.716486]  lock_acquire+0x74/0x150
> [  203.716488]  ? generic_hdmi_init+0x21/0x140 [snd_hda_codec_hdmi]
> [  203.716490]  __mutex_lock+0x60/0x810
> [  203.716492]  ? generic_hdmi_init+0x21/0x140 [snd_hda_codec_hdmi]
> [  203.716496]  ? snd_hdac_exec_verb+0x16/0x40 [snd_hda_core]
> [  203.716499]  ? codec_read+0x29/0x40 [snd_hda_core]
> [  203.716501]  mutex_lock_nested+0x14/0x20
> [  203.716503]  ? generic_hdmi_init+0x21/0x140 [snd_hda_codec_hdmi]
> [  203.716505]  generic_hdmi_init+0x21/0x140 [snd_hda_codec_hdmi]
> [  203.716507]  generic_hdmi_resume+0x18/0x60 [snd_hda_codec_hdmi]
> [  203.716512]  hda_call_codec_resume+0xc2/0x130 [snd_hda_codec]
> [  203.716517]  hda_codec_runtime_resume+0x2a/0x60 [snd_hda_codec]
> [  203.716520]  __rpm_callback+0x7a/0x140
> [  203.716524]  ? snd_hda_codec_device_new+0x2a0/0x2a0 [snd_hda_codec]
> [  203.716529]  ? snd_hda_codec_device_new+0x2a0/0x2a0 [snd_hda_codec]
> [  203.716531]  rpm_callback+0x1a/0x70
> [  203.716535]  ? snd_hda_codec_device_new+0x2a0/0x2a0 [snd_hda_codec]
> [  203.716537]  rpm_resume+0x52c/0x700
> [  203.716538]  ? _raw_spin_lock_irqsave+0x32/0x40
> [  203.716540]  __pm_runtime_resume+0x43/0x90
> [  203.716543]  snd_hdac_power_up_pm+0x4d/0x50 [snd_hda_core]
> [  203.716546]  hdmi_present_sense+0x34/0x340 [snd_hda_codec_hdmi]
> [  203.716548]  ? finish_task_switch+0x89/0x210
> [  203.716550]  check_presence_and_report+0x7a/0xb0 [snd_hda_codec_hdmi]
> [  203.716553]  hdmi_unsol_event+0x57/0x60 [snd_hda_codec_hdmi]
> [  203.716557]  ? hda_codec_match+0x70/0x70 [snd_hda_codec]
> [  203.716561]  hda_codec_unsol_event+0x12/0x20 [snd_hda_codec]
> [  203.716564]  snd_hdac_bus_process_unsol_events+0x51/0x60 [snd_hda_core]
> [  203.716566]  process_one_work+0x230/0x530
> [  203.716567]  worker_thread+0x37/0x410
> [  203.716569]  kthread+0xf5/0x110
> [  203.716570]  ? process_one_work+0x530/0x530
> [  203.716572]  ? kthread_create_worker_on_cpu+0x20/0x20
> [  203.716574]  ret_from_fork+0x2e/0x38
> 
> Looks like commit ade49db337a9 ("ALSA: hda/hdmi - Allow audio
> component for AMD/ATI and Nvidia HDMI") introduced pcm_lock
> to generic_hdmi_init().

Indeed, that can lead to a deadlock.
The patch below should address the issue.  I'm going to queue it
later.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] ALSA: hda - Fix mutex deadlock in HDMI codec driver

The commit ade49db337a9 ("ALSA: hda/hdmi - Allow audio component for
AMD/ATI and Nvidia HDMI") introduced the spec->pcm_lock mutex lock to
the whole generic_hdmi_init() function for avoiding the race with the
audio component registration.  However, this caused a dead lock when
the unsolicited event is handled without the audio component, as the
codec gets runtime-resumed in hdmi_present_sense() which is already
inside the spec->pcm_lock in its caller.

For avoiding this deadlock, add a new mutex only for the audio
component binding that is used in both generic_hdmi_init() and the
audio notifier registration where the jack callbacks are handled /
re-registered.

Fixes: ade49db337a9 ("ALSA: hda/hdmi - Allow audio component for AMD/ATI and Nvidia HDMI")
Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/pci/hda/patch_hdmi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 795cbda32cbb..d9b5ba361409 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -145,6 +145,7 @@ struct hdmi_spec {
 	struct snd_array pins; /* struct hdmi_spec_per_pin */
 	struct hdmi_pcm pcm_rec[16];
 	struct mutex pcm_lock;
+	struct mutex bind_lock; /* for audio component binding */
 	/* pcm_bitmap means which pcms have been assigned to pins*/
 	unsigned long pcm_bitmap;
 	int pcm_used;	/* counter of pcm_rec[] */
@@ -2258,7 +2259,7 @@ static int generic_hdmi_init(struct hda_codec *codec)
 	struct hdmi_spec *spec = codec->spec;
 	int pin_idx;
 
-	mutex_lock(&spec->pcm_lock);
+	mutex_lock(&spec->bind_lock);
 	spec->use_jack_detect = !codec->jackpoll_interval;
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
@@ -2275,7 +2276,7 @@ static int generic_hdmi_init(struct hda_codec *codec)
 			snd_hda_jack_detect_enable_callback(codec, pin_nid,
 							    jack_callback);
 	}
-	mutex_unlock(&spec->pcm_lock);
+	mutex_unlock(&spec->bind_lock);
 	return 0;
 }
 
@@ -2451,7 +2452,7 @@ static void generic_acomp_notifier_set(struct drm_audio_component *acomp,
 	int i;
 
 	spec = container_of(acomp->audio_ops, struct hdmi_spec, drm_audio_ops);
-	mutex_lock(&spec->pcm_lock);
+	mutex_lock(&spec->bind_lock);
 	spec->use_acomp_notifier = use_acomp;
 	spec->codec->relaxed_resume = use_acomp;
 	/* reprogram each jack detection logic depending on the notifier */
@@ -2461,7 +2462,7 @@ static void generic_acomp_notifier_set(struct drm_audio_component *acomp,
 					      get_pin(spec, i)->pin_nid,
 					      use_acomp);
 	}
-	mutex_unlock(&spec->pcm_lock);
+	mutex_unlock(&spec->bind_lock);
 }
 
 /* enable / disable the notifier via master bind / unbind */
-- 
2.16.4



More information about the Intel-gfx mailing list