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

[Takashi Iwai]

At Thu, 23 May 2013 19:41:58 +0200,
David Henningsson wrote:
> 
> On 05/23/2013 12:29 PM, Wang, Xingchao wrote:
> >
> >
> >> -----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. :(
> 
> To the rescue!
> 
> I've tried to track this down, and I think this is a problem in modprobe 
> with blacklisted modules. I've just emailed the kmod maintainer (with 
> Wang Xingchao in cc) and asked for clarification.

Thanks for figuring out!


Takashi