[alsa-devel] [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock

[Takashi Iwai]

At Thu, 23 May 2013 10:19:27 +0000,
Wang, Xingchao wrote:
> 
> Hi Takashi,
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai at suse.de]
> > Sent: Thursday, May 23, 2013 2:49 PM
> > To: Wang Xingchao
> > Cc: alsa-devel at alsa-project.org; intel-gfx at lists.freedesktop.org;
> > david.henningsson at canonical.com; Girdwood, Liam R; Li, Jocelyn; Wang,
> > Xingchao; Lin, Mengdong
> > Subject: Re: [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid
> > request_module deadlock
> > 
> > At Thu, 23 May 2013 09:51:07 +0800,
> > Wang Xingchao wrote:
> > >
> > > There's deadlock when request_module(i915) in azx_probe.
> > > It looks like:
> > > device_lock(audio pci device) -> azx_probe -> module_request (or
> > > symbol_request) -> modprobe (userspace) -> i915 init -> drm_pci_init
> > > -> pci_register_driver -> bus_add_driver -> driver_attach -> which in
> > > turn tries all locks on pci bus, and when it tries the one on the
> > > audio device, it will deadlock.
> > >
> > > This patch introduce a work to store remaining probe stuff, and let
> > > request_module run in safe work context.
> > >
> > > Signed-off-by: Wang Xingchao <xingchao.wang at linux.intel.com>
> > > ---
> > >  sound/pci/hda/hda_i915.c  |  13 ++++--  sound/pci/hda/hda_intel.c |
> > > 105 +++++++++++++++++++++++++++-------------------
> > >  2 files changed, 71 insertions(+), 47 deletions(-)
> > >
> > > diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c index
> > > 76c13d5..7547b20 100644
> > > --- a/sound/pci/hda/hda_i915.c
> > > +++ b/sound/pci/hda/hda_i915.c
> > > @@ -42,13 +42,18 @@ int hda_i915_init(void)  {
> > >  	int err = 0;
> > >
> > > -	get_power = symbol_request(i915_request_power_well);
> > > +	get_power = symbol_get(i915_request_power_well);
> > >  	if (!get_power) {
> > > -		snd_printk(KERN_WARNING "hda-i915: get_power symbol get
> > fail\n");
> > > -		return -ENODEV;
> > > +		request_module("i915");
> > > +		get_power = symbol_get(i915_request_power_well);
> > > +		if (!get_power) {
> > > +			snd_printk(KERN_WARNING "hda-i915: get_power symbol get
> > fail\n");
> > > +			return -ENODEV;
> > > +		}
> > > +		snd_printdd("hda-i915: get_power symbol get successful\n");
> > 
> > Why do you need this change?
> > 
> 
> symbol_request() should be the better API in such case but in my test it doesnot really load i915 module, that's why
> I call request_module(i915) directly here.
> 
> Please note there's parameter difference:
> request_module("i915")
> symbol_request(i915_reauest_power_well)-->request_module("symbol:i915_request_power_well")
> 
> I donot know why the second one did not really load the module.

Well, something is really fishy.  The patch can't be accepted only
because it just works by moon phase...


Takashi