On Thu, 17 Mar 2016 15:25:10 +0100,
Ville Syrjälä wrote:
>
> On Thu, Mar 17, 2016 at 03:12:22PM +0100, Takashi Iwai wrote:
> > On Thu, 17 Mar 2016 14:38:42 +0100,
> > Takashi Iwai wrote:
> > >
> > > On Wed, 16 Mar 2016 15:04:20 +0100,
> > > Ville Syrjälä wrote:
> > > >
> > > > But now I got a lockdep spew when I enabled the HDMI video output [1]
> > > >
> > > > And sure enough mplayer got stuck in the kernel when I tried to use
> > > > the HDMI audio device [2]
> > > >
> > > > [1]
> > > > [ 1939.476458] =============================================
> > > > [ 1939.476460] [ INFO: possible recursive locking detected ]
> > > > [ 1939.476463] 4.5.0-vga+ #13 Not tainted
> > > > [ 1939.476464] ---------------------------------------------
> > > > [ 1939.476466] kworker/2:2/1016 is trying to acquire lock:
> > > > [ 1939.476469] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi]
> > > > [ 1939.476480]
> > > > but task is already holding lock:
> > > > [ 1939.476482] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi]
> > > > [ 1939.476489]
> > > > other info that might help us debug this:
> > > > [ 1939.476491] Possible unsafe locking scenario:
> > > >
> > > > [ 1939.476493] CPU0
> > > > [ 1939.476495] ----
> > > > [ 1939.476496] lock(&spec->pcm_lock);
> > > > [ 1939.476499] lock(&spec->pcm_lock);
> > > > [ 1939.476502]
> > > > *** DEADLOCK ***
> > > >
> > > > [ 1939.476504] May be due to missing lock nesting notation
> > >
> > > Unfortunately, no this is a real deadlock.
> > > Let's see below: hdmi_present_sense() gets called twice because the
> > > function issues a verb that does self-resume and it invokes
> > > hdmi_present_sense() again in runtime resume.
> > >
> > > > [ 1939.476622] [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi]
> > > ....
> > > > [ 1939.476642] [<ffffffffa020bd6d>] generic_hdmi_resume+0x4d/0x60 [snd_hda_codec_hdmi]
> > > ....
> > > > [ 1939.476690] [<ffffffffa017de62>] snd_hdac_power_up_pm+0x52/0x60 [snd_hda_core]
> > > > [ 1939.476694] [<ffffffffa020b9c3>] hdmi_present_sense+0x193/0x300 [snd_hda_codec_hdmi]
> > > > [ 1939.476699] [<ffffffffa020bba0>] check_presence_and_report+0x70/0x90 [snd_hda_codec_hdmi]
> > > > [ 1939.476703] [<ffffffffa020bcba>] hdmi_unsol_event+0x9a/0xb0 [snd_hda_codec_hdmi]
> > >
> > > This wasn't seen until now because the code path using i915 audio
> > > notifier doesn't need to power up the codec. Now we switched to the
> > > old method for old chips, and the bug is revealed. It's good to have
> > > caught it now, because basically this hits all non-Intel chips.
> >
> > ... and the fix patch is attached below.
> >
> >
> > Takashi
> >
> > -- 8< --
> > From: Takashi Iwai <tiwai@xxxxxxx>
> > Subject: [PATCH] ALSA: hda - Fix mutex deadlock at HDMI/DP hotplug
> >
> > The recent change in HD-audio HDMI/DP codec driver for allowing the
> > dynamic PCM binding introduced a new spec->pcm_mutex. One of the
> > protected area by this mutex is hdmi_present_sense(). As reported by
> > Intel CI tests, unfortunately, the new mutex causes a deadlock when
> > the hotplug/unplug is triggered during the codec is in runtime
> > suspend. The buggy code path is like the following:
> >
> > hdmi_unsol_event() -> ...
> > -> hdmi_present_sense()
> > ==> ** here taking pcm_mutex
> > -> hdmi_present_sense_via_verbs()
> > -> snd_hda_power_up_pm() -> ... (runtime resume calls)
> > -> generic_hdmi_resume()
> > -> hdmi_present_sense()
> > ==> ** here taking pcm_mutex again!
> >
> > As we can see here, the problem is that the mutex is taken before
> > snd_hda_power_up_pm() call that triggers the runtime resume. That is,
> > the obvious solution is to move the power up/down call outside the
> > mutex; it is exactly what this patch provides.
> >
> > The patch also clarifies why this bug wasn't caught beforehand. We
> > used to have the i915 audio component for hotplug for all Intel chips,
> > and in that code path, there is no power up required but the
> > information is taken directly from the graphics side. However, we
> > recently switched back to the old method for some old Intel chips due
> > to regressions, and now the deadlock issue is surfaced.
> >
> > Fixes: a76056f2e57e ('ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug')
> > Reported-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
>
> Yep, my ILK seems happy with this. No deadlocks, and HDMI audio works.
>
> Tested-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Great, now queued with your tested-by tag. Thanks for quick testing!
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
