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

[Wang, Xingchao]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Thursday, May 23, 2013 6:27 PM
> To: Wang, Xingchao
> Cc: Wang Xingchao; alsa-devel at alsa-project.org;
> intel-gfx at lists.freedesktop.org; david.henningsson at canonical.com; Girdwood,
> Liam R; Li, Jocelyn; 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 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...
> 
I will continue to figure out the reason it doesnot work. :(

--xingchao