[alsa-devel] New snd-hda warning spew

[Takashi Iwai]

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 at suse.de>
> > 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 at linux.intel.com>
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> 
> Yep, my ILK seems happy with this. No deadlocks, and HDMI audio works.
> 
> Tested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Great, now queued with your tested-by tag.  Thanks for quick testing!


Takashi