* [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock
@ 2013-05-23 1:51 Wang Xingchao
2013-05-23 6:49 ` Takashi Iwai
0 siblings, 1 reply; 7+ messages in thread
From: Wang Xingchao @ 2013-05-23 1:51 UTC (permalink / raw)
To: tiwai
Cc: alsa-devel, jocelyn.li, mengdong.lin, intel-gfx, Wang Xingchao,
xingchao.wang, liam.r.girdwood, david.henningsson
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@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");
}
- put_power = symbol_request(i915_release_power_well);
+ put_power = symbol_get(i915_release_power_well);
if (!put_power) {
symbol_put(i915_request_power_well);
get_power = NULL;
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index f20a88c..1bc7c3b 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -76,6 +76,7 @@ static int probe_only[SNDRV_CARDS];
static int jackpoll_ms[SNDRV_CARDS];
static bool single_cmd;
static int enable_msi = -1;
+static int dev;
#ifdef CONFIG_SND_HDA_PATCH_LOADER
static char *patch[SNDRV_CARDS];
#endif
@@ -542,6 +543,8 @@ struct azx {
/* for pending irqs */
struct work_struct irq_pending_work;
+ struct delayed_work probe_work;
+
/* reboot notifier (for mysterious hangup problem at power-down) */
struct notifier_block reboot_notifier;
@@ -3670,58 +3673,22 @@ static void azx_firmware_cb(const struct firmware *fw, void *context)
}
#endif
-static int azx_probe(struct pci_dev *pci,
- const struct pci_device_id *pci_id)
+static void azx_probe_work(struct work_struct *work)
{
- static int dev;
- struct snd_card *card;
- struct azx *chip;
+ struct azx *chip =
+ container_of(work, struct azx, probe_work.work);
+ struct snd_card *card = chip->card;
+ struct pci_dev *pci = chip->pci;
bool probe_now;
int err;
- if (dev >= SNDRV_CARDS)
- return -ENODEV;
- if (!enable[dev]) {
- dev++;
- return -ENOENT;
- }
-
- err = snd_card_create(index[dev], id[dev], THIS_MODULE, 0, &card);
- if (err < 0) {
- snd_printk(KERN_ERR "hda-intel: Error creating card!\n");
- return err;
- }
-
- snd_card_set_dev(card, &pci->dev);
-
- err = azx_create(card, pci, dev, pci_id->driver_data, &chip);
- if (err < 0)
- goto out_free;
- card->private_data = chip;
-
- pci_set_drvdata(pci, card);
-
- err = register_vga_switcheroo(chip);
- if (err < 0) {
- snd_printk(KERN_ERR SFX
- "%s: Error registering VGA-switcheroo client\n", pci_name(pci));
- goto out_free;
- }
-
- if (check_hdmi_disabled(pci)) {
- snd_printk(KERN_INFO SFX "%s: VGA controller is disabled\n",
- pci_name(pci));
- snd_printk(KERN_INFO SFX "%s: Delaying initialization\n", pci_name(pci));
- chip->disabled = true;
- }
-
/* Request power well for Haswell HDA controller and codec */
if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
#ifdef CONFIG_SND_HDA_I915
err = hda_i915_init();
if (err < 0) {
snd_printk(KERN_ERR SFX "Error request power-well from i915\n");
- return err;
+ goto out_free;
}
hda_display_power(true);
#else
@@ -3760,7 +3727,7 @@ static int azx_probe(struct pci_dev *pci,
dev++;
complete_all(&chip->probe_wait);
- return 0;
+ return;
out_free_power:
if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
hda_display_power(false);
@@ -3769,6 +3736,58 @@ out_free_power:
out_free:
snd_card_free(card);
pci_set_drvdata(pci, NULL);
+}
+
+static int azx_probe(struct pci_dev *pci,
+ const struct pci_device_id *pci_id)
+{
+ struct snd_card *card;
+ struct azx *chip;
+ int err;
+
+ if (dev >= SNDRV_CARDS)
+ return -ENODEV;
+ if (!enable[dev]) {
+ dev++;
+ return -ENOENT;
+ }
+
+ err = snd_card_create(index[dev], id[dev], THIS_MODULE, 0, &card);
+ if (err < 0) {
+ snd_printk(KERN_ERR "hda-intel: Error creating card!\n");
+ return err;
+ }
+
+ snd_card_set_dev(card, &pci->dev);
+
+ err = azx_create(card, pci, dev, pci_id->driver_data, &chip);
+ if (err < 0)
+ goto out_free;
+ card->private_data = chip;
+
+ pci_set_drvdata(pci, card);
+
+ err = register_vga_switcheroo(chip);
+ if (err < 0) {
+ snd_printk(KERN_ERR SFX
+ "%s: Error registering VGA-switcheroo client\n", pci_name(pci));
+ goto out_free;
+ }
+
+ if (check_hdmi_disabled(pci)) {
+ snd_printk(KERN_INFO SFX "%s: VGA controller is disabled\n",
+ pci_name(pci));
+ snd_printk(KERN_INFO SFX "%s: Delaying initialization\n", pci_name(pci));
+ chip->disabled = true;
+ }
+
+ /* continue probing in work context as may trigger request module */
+ INIT_DELAYED_WORK(&chip->probe_work, azx_probe_work);
+ schedule_delayed_work(&chip->probe_work, 0);
+ return 0;
+out_free:
+ snd_card_free(card);
+ pci_set_drvdata(pci, NULL);
return err;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock
2013-05-23 1:51 [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock Wang Xingchao
@ 2013-05-23 6:49 ` Takashi Iwai
2013-05-23 10:19 ` Wang, Xingchao
0 siblings, 1 reply; 7+ messages in thread
From: Takashi Iwai @ 2013-05-23 6:49 UTC (permalink / raw)
To: Wang Xingchao
Cc: alsa-devel, jocelyn.li, mengdong.lin, intel-gfx, liam.r.girdwood,
david.henningsson
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@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?
Takashi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock
2013-05-23 6:49 ` Takashi Iwai
@ 2013-05-23 10:19 ` Wang, Xingchao
2013-05-23 10:26 ` Takashi Iwai
0 siblings, 1 reply; 7+ messages in thread
From: Wang, Xingchao @ 2013-05-23 10:19 UTC (permalink / raw)
To: Takashi Iwai, Wang Xingchao
Cc: alsa-devel@alsa-project.org, Li, Jocelyn, Lin, Mengdong,
intel-gfx@lists.freedesktop.org, Girdwood, Liam R,
david.henningsson@canonical.com
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, May 23, 2013 2:49 PM
> To: Wang Xingchao
> Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org;
> david.henningsson@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@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.
Thanks
--xingchao
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock
2013-05-23 10:19 ` Wang, Xingchao
@ 2013-05-23 10:26 ` Takashi Iwai
2013-05-23 10:29 ` Wang, Xingchao
0 siblings, 1 reply; 7+ messages in thread
From: Takashi Iwai @ 2013-05-23 10:26 UTC (permalink / raw)
To: Wang, Xingchao
Cc: alsa-devel@alsa-project.org, Li, Jocelyn, Lin, Mengdong,
intel-gfx@lists.freedesktop.org, Wang Xingchao, Girdwood, Liam R,
david.henningsson@canonical.com
At Thu, 23 May 2013 10:19:27 +0000,
Wang, Xingchao wrote:
>
> Hi Takashi,
>
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Thursday, May 23, 2013 2:49 PM
> > To: Wang Xingchao
> > Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org;
> > david.henningsson@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@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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock
2013-05-23 10:26 ` Takashi Iwai
@ 2013-05-23 10:29 ` Wang, Xingchao
2013-05-23 17:41 ` David Henningsson
0 siblings, 1 reply; 7+ messages in thread
From: Wang, Xingchao @ 2013-05-23 10:29 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel@alsa-project.org, Li, Jocelyn, Lin, Mengdong,
intel-gfx@lists.freedesktop.org, Wang Xingchao, Girdwood, Liam R,
david.henningsson@canonical.com
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, May 23, 2013 6:27 PM
> To: Wang, Xingchao
> Cc: Wang Xingchao; alsa-devel@alsa-project.org;
> intel-gfx@lists.freedesktop.org; david.henningsson@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@suse.de]
> > > Sent: Thursday, May 23, 2013 2:49 PM
> > > To: Wang Xingchao
> > > Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org;
> > > david.henningsson@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@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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock
2013-05-23 10:29 ` Wang, Xingchao
@ 2013-05-23 17:41 ` David Henningsson
2013-05-24 8:48 ` Takashi Iwai
0 siblings, 1 reply; 7+ messages in thread
From: David Henningsson @ 2013-05-23 17:41 UTC (permalink / raw)
To: Wang, Xingchao
Cc: alsa-devel@alsa-project.org, Takashi Iwai, Lin, Mengdong,
intel-gfx@lists.freedesktop.org, Wang Xingchao, Li, Jocelyn,
Girdwood, Liam R
On 05/23/2013 12:29 PM, Wang, Xingchao wrote:
>
>
>> -----Original Message-----
>> From: Takashi Iwai [mailto:tiwai@suse.de]
>> Sent: Thursday, May 23, 2013 6:27 PM
>> To: Wang, Xingchao
>> Cc: Wang Xingchao; alsa-devel@alsa-project.org;
>> intel-gfx@lists.freedesktop.org; david.henningsson@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@suse.de]
>>>> Sent: Thursday, May 23, 2013 2:49 PM
>>>> To: Wang Xingchao
>>>> Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org;
>>>> david.henningsson@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@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.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock
2013-05-23 17:41 ` David Henningsson
@ 2013-05-24 8:48 ` Takashi Iwai
0 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2013-05-24 8:48 UTC (permalink / raw)
To: David Henningsson
Cc: alsa-devel@alsa-project.org, Li, Jocelyn, Lin, Mengdong,
intel-gfx@lists.freedesktop.org, Wang Xingchao, Girdwood, Liam R
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@suse.de]
> >> Sent: Thursday, May 23, 2013 6:27 PM
> >> To: Wang, Xingchao
> >> Cc: Wang Xingchao; alsa-devel@alsa-project.org;
> >> intel-gfx@lists.freedesktop.org; david.henningsson@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@suse.de]
> >>>> Sent: Thursday, May 23, 2013 2:49 PM
> >>>> To: Wang Xingchao
> >>>> Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org;
> >>>> david.henningsson@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@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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-05-24 8:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-23 1:51 [PATCH 4/4 V2] ALSA: hda - Continue probe in work context to avoid request_module deadlock Wang Xingchao
2013-05-23 6:49 ` Takashi Iwai
2013-05-23 10:19 ` Wang, Xingchao
2013-05-23 10:26 ` Takashi Iwai
2013-05-23 10:29 ` Wang, Xingchao
2013-05-23 17:41 ` David Henningsson
2013-05-24 8:48 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.