mmc: mxs: DEADLOCK

PATCH Lauri Hintsala 5 participants 13 messages Jul 10, 2012 – Jul 17, 2012

Subsystems (2)

Arm i.MX and Layerscape MMC, SD and SDIO

lore

Share & tools

lore

Lauri Hintsala

author

2012-07-10 14:04 UTC #0 lore raw

Hi,

I was able to get deadlock with CONFIG_DEBUG_SPINLOCK enabled. I added also CONFIG_PROVE_LOCKING to get more verbose output. I got following error message after SDIO device has been powered.

I'm able to replicate issue with Linux next-20120710. Platform is imx28.

[   79.660000] =============================================
[   79.660000] [ INFO: possible recursive locking detected ]
[   79.660000] 3.4.0-00009-g3e96082-dirty #11 Not tainted
[   79.660000] ---------------------------------------------
[   79.660000] swapper/0 is trying to acquire lock:
[   79.660000]  (&(&host->lock)->rlock#2){-.....}, at: [<c026ea3c>] 
mxs_mmc_enable_sdio_irq+0x18/0xd4
[   79.660000]
[   79.660000] but task is already holding lock:
[   79.660000]  (&(&host->lock)->rlock#2){-.....}, at: [<c026f744>] 
mxs_mmc_irq_handler+0x1c/0xe8
[   79.660000]
[   79.660000] other info that might help us debug this:
[   79.660000]  Possible unsafe locking scenario:
[   79.660000]
[   79.660000]        CPU0
[   79.660000]        ----
[   79.660000]   lock(&(&host->lock)->rlock#2);
[   79.660000]   lock(&(&host->lock)->rlock#2);
[   79.660000]
[   79.660000]  *** DEADLOCK ***
[   79.660000]
[   79.660000]  May be due to missing lock nesting notation
[   79.660000]
[   79.660000] 1 lock held by swapper/0:
[   79.660000]  #0:  (&(&host->lock)->rlock#2){-.....}, at: [<c026f744>] 
mxs_mmc_irq_handler+0x1c/0xe8
[   79.660000]
[   79.660000] stack backtrace:
[   79.660000] [<c0014bd0>] (unwind_backtrace+0x0/0xf4) from 
[<c005f9c0>] (__lock_acquire+0x1948/0x1d48)
[   79.660000] [<c005f9c0>] (__lock_acquire+0x1948/0x1d48) from 
[<c005fea0>] (lock_acquire+0xe0/0xf8)
[   79.660000] [<c005fea0>] (lock_acquire+0xe0/0xf8) from [<c03a8460>] 
(_raw_spin_lock_irqsave+0x44/0x58)
[   79.660000] [<c03a8460>] (_raw_spin_lock_irqsave+0x44/0x58) from 
[<c026ea3c>] (mxs_mmc_enable_sdio_irq+0x18/0xd4)
[   79.660000] [<c026ea3c>] (mxs_mmc_enable_sdio_irq+0x18/0xd4) from 
[<c026f7fc>] (mxs_mmc_irq_handler+0xd4/0xe8)
[   79.660000] [<c026f7fc>] (mxs_mmc_irq_handler+0xd4/0xe8) from 
[<c006bdd8>] (handle_irq_event_percpu+0x70/0x254)
[   79.660000] [<c006bdd8>] (handle_irq_event_percpu+0x70/0x254) from 
[<c006bff8>] (handle_irq_event+0x3c/0x5c)
[   79.660000] [<c006bff8>] (handle_irq_event+0x3c/0x5c) from 
[<c006e6d0>] (handle_level_irq+0x90/0x110)
[   79.660000] [<c006e6d0>] (handle_level_irq+0x90/0x110) from 
[<c006b930>] (generic_handle_irq+0x38/0x50)
[   79.660000] [<c006b930>] (generic_handle_irq+0x38/0x50) from 
[<c00102fc>] (handle_IRQ+0x30/0x84)
[   79.660000] [<c00102fc>] (handle_IRQ+0x30/0x84) from [<c000f058>] 
(__irq_svc+0x38/0x60)
[   79.660000] [<c000f058>] (__irq_svc+0x38/0x60) from [<c0010520>] 
(default_idle+0x2c/0x40)
[   79.660000] [<c0010520>] (default_idle+0x2c/0x40) from [<c0010a90>] 
(cpu_idle+0x64/0xcc)
[   79.660000] [<c0010a90>] (cpu_idle+0x64/0xcc) from [<c04ff858>] 
(start_kernel+0x244/0x2c8)
[   79.660000] BUG: spinlock lockup on CPU#0, swapper/0
[   79.660000]  lock: c398cb2c, .magic: dead4ead, .owner: swapper/0, 
.owner_cpu: 0
[   79.660000] [<c0014bd0>] (unwind_backtrace+0x0/0xf4) from 
[<c01ddb1c>] (do_raw_spin_lock+0xf0/0x144)
[   79.660000] [<c01ddb1c>] (do_raw_spin_lock+0xf0/0x144) from 
[<c03a8468>] (_raw_spin_lock_irqsave+0x4c/0x58)
[   79.660000] [<c03a8468>] (_raw_spin_lock_irqsave+0x4c/0x58) from 
[<c026ea3c>] (mxs_mmc_enable_sdio_irq+0x18/0xd4)
[   79.660000] [<c026ea3c>] (mxs_mmc_enable_sdio_irq+0x18/0xd4) from 
[<c026f7fc>] (mxs_mmc_irq_handler+0xd4/0xe8)
[   79.660000] [<c026f7fc>] (mxs_mmc_irq_handler+0xd4/0xe8) from 
[<c006bdd8>] (handle_irq_event_percpu+0x70/0x254)
[   79.660000] [<c006bdd8>] (handle_irq_event_percpu+0x70/0x254) from 
[<c006bff8>] (handle_irq_event+0x3c/0x5c)
[   79.660000] [<c006bff8>] (handle_irq_event+0x3c/0x5c) from 
[<c006e6d0>] (handle_level_irq+0x90/0x110)
[   79.660000] [<c006e6d0>] (handle_level_irq+0x90/0x110) from 
[<c006b930>] (generic_handle_irq+0x38/0x50)
[   79.660000] [<c006b930>] (generic_handle_irq+0x38/0x50) from 
[<c00102fc>] (handle_IRQ+0x30/0x84)
[   79.660000] [<c00102fc>] (handle_IRQ+0x30/0x84) from [<c000f058>] 
(__irq_svc+0x38/0x60)
[   79.660000] [<c000f058>] (__irq_svc+0x38/0x60) from [<c0010520>] 
(default_idle+0x2c/0x40)
[   79.660000] [<c0010520>] (default_idle+0x2c/0x40) from [<c0010a90>] 
(cpu_idle+0x64/0xcc)
[   79.660000] [<c0010a90>] (cpu_idle+0x64/0xcc) from [<c04ff858>]

(start_kernel+0x244/0x2c8)

I found a way to fix this issue:

drivers/mmc/host/mxs-mmc.c +0 -0

@@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int irq,

void *dev_id)
writel(stat & MXS_MMC_IRQ_BITS,
host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);

+ spin_unlock(&host->lock);
+
if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
mmc_signal_sdio_irq(host->mmc);

- spin_unlock(&host->lock);
-
if (stat & BM_SSP_CTRL1_RESP_TIMEOUT_IRQ)
cmd->error = -ETIMEDOUT;
else if (stat & BM_SSP_CTRL1_RESP_ERR_IRQ)

Is there any reason to keep mmc_signal_sdio_irq inside the spinlock? mmc_signal_sdio_irq calls mxs_mmc_enable_sdio_irq and it tries to acquire lock while it is already acquired.

Best regards,
Lauri Hintsala

Marek Vasut

2012-07-10 15:02 UTC #1 lore raw

Dear Lauri Hintsala,

[...]

Quoted 4 lines

drivers/mmc/host/mxs-mmc.c +0 -0

@@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int irq,
 void *dev_id)

drivers/mmc/host/mxs-mmc.c static irqreturn_t mxs_mmc_irq_handler(int irq, · line 286

Quoted 10 lines

drivers/mmc/host/mxs-mmc.c +2 -2

  	writel(stat & MXS_MMC_IRQ_BITS,
  	       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
 
+	spin_unlock(&host->lock);
+
  	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
  		mmc_signal_sdio_irq(host->mmc);
 
-	spin_unlock(&host->lock);
-

Spinlock in irq handler is interesting too ;-)

Quoted 5 lines

drivers/mmc/host/mxs-mmc.c +0 -0
  	if (stat & BM_SSP_CTRL1_RESP_TIMEOUT_IRQ)
  		cmd->error = -ETIMEDOUT;
else if (stat & BM_SSP_CTRL1_RESP_ERR_IRQ)

Quoted 7 lines

Is there any reason to keep mmc_signal_sdio_irq inside the spinlock? mmc_signal_sdio_irq calls mxs_mmc_enable_sdio_irq and it tries to acquire lock while it is already acquired.
Best regards,
Lauri Hintsala

Best regards,
Marek Vasut

Shawn Guo

Acked-by

2012-07-11 06:06 UTC #3 lore raw

In reply to Lauri Hintsala

Quoted 8 lines · Lauri Hintsala

Hi,
I was able to get deadlock with CONFIG_DEBUG_SPINLOCK enabled. I added also CONFIG_PROVE_LOCKING to get more verbose output. I got following error message after SDIO device has been powered.
I'm able to replicate issue with Linux next-20120710. Platform is imx28.

The bug is there probably because the driver hasn't been widely tested on SDIO card.

Quoted 21 lines

I found a way to fix this issue:

drivers/mmc/host/mxs-mmc.c +2 -2

@@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int
 irq, void *dev_id)
 	writel(stat & MXS_MMC_IRQ_BITS,
 	       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
 
+	spin_unlock(&host->lock);
+
 	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
 		mmc_signal_sdio_irq(host->mmc);
 
-	spin_unlock(&host->lock);
-
 	if (stat & BM_SSP_CTRL1_RESP_TIMEOUT_IRQ)
 		cmd->error = -ETIMEDOUT;

else if (stat & BM_SSP_CTRL1_RESP_ERR_IRQ)

Quoted 4 lines

Is there any reason to keep mmc_signal_sdio_irq inside the spinlock? mmc_signal_sdio_irq calls mxs_mmc_enable_sdio_irq and it tries to acquire lock while it is already acquired.

The fix looks right to me. You can have my ack when you send a patch for it.

Acked-by Shawn Guo <[email protected]>

Signature · 2 lines

Regards,
Shawn

Lauri Hintsala

author

2012-07-11 06:08 UTC #4 lore raw

Quoted 19 lines · Shawn Guo

Quoted 19 lines

drivers/mmc/host/mxs-mmc.c +2 -2

@@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int
 irq, void *dev_id)
  	writel(stat & MXS_MMC_IRQ_BITS,
  	       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
 
+	spin_unlock(&host->lock);
+
  	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
  		mmc_signal_sdio_irq(host->mmc);
 
-	spin_unlock(&host->lock);
-
  	if (stat & BM_SSP_CTRL1_RESP_TIMEOUT_IRQ)
  		cmd->error = -ETIMEDOUT;

else if (stat & BM_SSP_CTRL1_RESP_ERR_IRQ)

Quoted 8 lines

Quoted 4 lines
Is there any reason to keep mmc_signal_sdio_irq inside the spinlock? mmc_signal_sdio_irq calls mxs_mmc_enable_sdio_irq and it tries to acquire lock while it is already acquired.
The fix looks right to me. You can have my ack when you send a patch for it.
Acked-by Shawn Guo <[email protected]>

OK, I'll send a patch. Thanks!

Lauri

Shawn Guo

2012-07-11 06:09 UTC #2 lore raw

In reply to Marek Vasut

Quoted 16 lines · Marek Vasut

Dear Lauri Hintsala,

[...]

Quoted 12 lines

drivers/mmc/host/mxs-mmc.c +2 -0

@@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int irq,
 void *dev_id)
  	writel(stat & MXS_MMC_IRQ_BITS,
  	       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
 
+	spin_unlock(&host->lock);
+
  	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
  		mmc_signal_sdio_irq(host->mmc);

Quoted 5 lines

drivers/mmc/host/mxs-mmc.c static irqreturn_t mxs_mmc_irq_handler(int irq, · line 286
Quoted 2 lines
drivers/mmc/host/mxs-mmc.c +0 -2
-	spin_unlock(&host->lock);
-
Spinlock in irq handler is interesting too ;-)

For you information, the following is what I learnt from Arnd when I was a beginner.

Regards,
Shawn

--- Quote Begins ---

A short form of the strict rules (there are better documentations out there) is:

* If all users are outside of interrupt or tasklet context, use a bare spin_lock().

* If one user is in a tasklet context, use spin_lock() inside the tasklet, but spin_lock_bh() outside, to prevent the tasklet from interrupting the critical section. Code that can be called in either tasklet or regular context needs to use spin_lock_bh() as well.

* If one user is in interrupt context, use spin_lock() inside of the interrupt handler, but spin_lock_irq() in tasklet and normal context (not spin_lock_irqsave()), to prevent the interrupt from happening during the critical section.

* Use spin_lock_irqsave() only for functions that can be called in either interrupt or non-interrupt context. Most drivers don't need this at all.

The simplified rule would be to always use spin_lock_irqsave(), because that does not require you to understand what you are doing.

My position is that it is better to use the stricter rules, because that documents that you actually do understand what you are doing ;-) It's also slightly more efficient, because it avoids having to save the interrupt status in a variable.

--- Quote Ends ---

Attila Kinali

2012-07-12 14:00 UTC #5 lore raw

In reply to Shawn Guo

Quoted 21 lines · On Wed, 11 Jul 2012 14:06:09 +0800 Shawn Guo <[email protected]> wrote:

Quoted 21 lines

I found a way to fix this issue:

drivers/mmc/host/mxs-mmc.c +2 -2

@@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int
 irq, void *dev_id)
 	writel(stat & MXS_MMC_IRQ_BITS,
 	       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
 
+	spin_unlock(&host->lock);
+
 	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
 		mmc_signal_sdio_irq(host->mmc);
 
-	spin_unlock(&host->lock);
-
 	if (stat & BM_SSP_CTRL1_RESP_TIMEOUT_IRQ)
 		cmd->error = -ETIMEDOUT;

else if (stat & BM_SSP_CTRL1_RESP_ERR_IRQ)

Quoted 8 lines

Quoted 4 lines
Is there any reason to keep mmc_signal_sdio_irq inside the spinlock? mmc_signal_sdio_irq calls mxs_mmc_enable_sdio_irq and it tries to acquire lock while it is already acquired.
The fix looks right to me. You can have my ack when you send a patch for it.
Acked-by Shawn Guo <[email protected]>

I ran into the same problem today, but the proposed fix doesn't seem to work for me:

---schnipp---

# modprobe libertas_sdio
[   59.200000] lib80211: common routines for IEEE802.11 drivers
[   59.240000] cfg80211: Calling CRDA to update world regulatory domain
[   59.320000] libertas_sdio: Libertas SDIO driver
[   59.330000] libertas_sdio: Copyright Pierre Ossman
# modprobe mxs-mmc
[   64.210000] mxs-mmc 80010000.ssp: initialized
[   64.260000] mxs-mmc 80034000.ssp: initialized
[   64.270000] mmc0: new SDIO card at address 0001
# [   65.440000] libertas_sdio mmc0:0001:1: (unregistered net_device): 00:13:04:80:00:3f, fw 9.70.3p24, cap 0x00000303
[   65.470000] 
[   65.470000] =============================================
[   65.470000] [ INFO: possible recursive locking detected ]
[   65.470000] 3.5.0-rc5 #2 Not tainted
[   65.470000] ---------------------------------------------
[   65.470000] ksdioirqd/mmc0/73 is trying to acquire lock:
[   65.470000]  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000] 
[   65.470000] but task is already holding lock:
[   65.470000]  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000] 
[   65.470000] other info that might help us debug this:
[   65.470000]  Possible unsafe locking scenario:
[   65.470000] 
[   65.470000]        CPU0
[   65.470000]        ----
[   65.470000]   lock(&(&host->lock)->rlock#2);
[   65.470000]   lock(&(&host->lock)->rlock#2);
[   65.470000] 
[   65.470000]  *** DEADLOCK ***
[   65.470000] 
[   65.470000]  May be due to missing lock nesting notation
[   65.470000] 
[   65.470000] 1 lock held by ksdioirqd/mmc0/73:
[   65.470000]  #0:  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000] 
[   65.470000] stack backtrace:
[   65.470000] [<c0014990>] (unwind_backtrace+0x0/0xf4) from [<c005ccb8>] (__lock_acquire+0x14f8/0x1b98)
[   65.470000] [<c005ccb8>] (__lock_acquire+0x14f8/0x1b98) from [<c005d3f8>] (lock_acquire+0xa0/0x108)
[   65.470000] [<c005d3f8>] (lock_acquire+0xa0/0x108) from [<c02f671c>] (_raw_spin_lock_irqsave+0x48/0x5c)
[   65.470000] [<c02f671c>] (_raw_spin_lock_irqsave+0x48/0x5c) from [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc])
[   65.470000] [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]) from [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc])
[   65.470000] [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc]) from [<c0219b38>] (sdio_irq_thread+0x1bc/0x274)
[   65.470000] [<c0219b38>] (sdio_irq_thread+0x1bc/0x274) from [<c003c324>] (kthread+0x8c/0x98)
[   65.470000] [<c003c324>] (kthread+0x8c/0x98) from [<c00101ac>] (kernel_thread_exit+0x0/0x8)
[   65.470000] BUG: spinlock lockup suspected on CPU#0, ksdioirqd/mmc0/73
[   65.470000]  lock: 0xc3358724, .magic: dead4ead, .owner: ksdioirqd/mmc0/73, .owner_cpu: 0
[   65.470000] [<c0014990>] (unwind_backtrace+0x0/0xf4) from [<c01b46b0>] (do_raw_spin_lock+0x100/0x144)
[   65.470000] [<c01b46b0>] (do_raw_spin_lock+0x100/0x144) from [<c02f6724>] (_raw_spin_lock_irqsave+0x50/0x5c)
[   65.470000] [<c02f6724>] (_raw_spin_lock_irqsave+0x50/0x5c) from [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc])
[   65.470000] [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]) from [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc])
[   65.470000] [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc]) from [<c0219b38>] (sdio_irq_thread+0x1bc/0x274)
[   65.470000] [<c0219b38>] (sdio_irq_thread+0x1bc/0x274) from [<c003c324>] (kthread+0x8c/0x98)
[   65.470000] [<c003c324>] (kthread+0x8c/0x98) from [<c00101ac>] (kernel_thread_exit+0x0/0x8)
---schnapp---

Any hints how to work around or fix this, would be appreciated

Attila Kinali

Signature · 4 lines

It is upon moral qualities that a society is ultimately founded. All 
the prosperity and technological sophistication in the world is of no 
use without that foundation.
                 -- Miss Matheson, The Diamond Age, Neil Stephenson

Shawn Guo

2012-07-12 14:39 UTC #6 lore raw

Quoted 32 lines · Attila Kinali

Quoted 28 lines · On Wed, 11 Jul 2012 14:06:09 +0800 Shawn Guo <[email protected]> wrote:
Quoted 21 lines
I found a way to fix this issue:
drivers/mmc/host/mxs-mmc.c +2 -2
@@ -278,11 +278,11 @@ static irqreturn_t mxs_mmc_irq_handler(int
 irq, void *dev_id)
 	writel(stat & MXS_MMC_IRQ_BITS,
 	       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
 
+	spin_unlock(&host->lock);
+
 	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
 		mmc_signal_sdio_irq(host->mmc);
 
-	spin_unlock(&host->lock);
-
 	if (stat & BM_SSP_CTRL1_RESP_TIMEOUT_IRQ)
 		cmd->error = -ETIMEDOUT;
else if (stat & BM_SSP_CTRL1_RESP_ERR_IRQ)
Quoted 4 lines
Is there any reason to keep mmc_signal_sdio_irq inside the spinlock? mmc_signal_sdio_irq calls mxs_mmc_enable_sdio_irq and it tries to acquire lock while it is already acquired.
The fix looks right to me. You can have my ack when you send a patch for it.

Quoted 5 lines

Quoted 1 line
Acked-by Shawn Guo <[email protected]>
I ran into the same problem today, but the proposed fix doesn't seem to work for me:

It's a different problem from what Lauri reported and fixed. I haven't played SDIO card that much, so I'm not completely clear about the SDIO calling sequence, but is it reasonable that mxs_mmc_enable_sdio_irq is being called recursively?

Regards,
Shawn

Quoted 66 lines

---schnipp---

# modprobe libertas_sdio
[   59.200000] lib80211: common routines for IEEE802.11 drivers
[   59.240000] cfg80211: Calling CRDA to update world regulatory domain
[   59.320000] libertas_sdio: Libertas SDIO driver
[   59.330000] libertas_sdio: Copyright Pierre Ossman
# modprobe mxs-mmc
[   64.210000] mxs-mmc 80010000.ssp: initialized
[   64.260000] mxs-mmc 80034000.ssp: initialized
[   64.270000] mmc0: new SDIO card at address 0001
# [   65.440000] libertas_sdio mmc0:0001:1: (unregistered net_device): 00:13:04:80:00:3f, fw 9.70.3p24, cap 0x00000303
[   65.470000] 
[   65.470000] =============================================
[   65.470000] [ INFO: possible recursive locking detected ]
[   65.470000] 3.5.0-rc5 #2 Not tainted
[   65.470000] ---------------------------------------------
[   65.470000] ksdioirqd/mmc0/73 is trying to acquire lock:
[   65.470000]  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000] 
[   65.470000] but task is already holding lock:
[   65.470000]  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000] 
[   65.470000] other info that might help us debug this:
[   65.470000]  Possible unsafe locking scenario:
[   65.470000] 
[   65.470000]        CPU0
[   65.470000]        ----
[   65.470000]   lock(&(&host->lock)->rlock#2);
[   65.470000]   lock(&(&host->lock)->rlock#2);
[   65.470000] 
[   65.470000]  *** DEADLOCK ***
[   65.470000] 
[   65.470000]  May be due to missing lock nesting notation
[   65.470000] 
[   65.470000] 1 lock held by ksdioirqd/mmc0/73:
[   65.470000]  #0:  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000] 
[   65.470000] stack backtrace:
[   65.470000] [<c0014990>] (unwind_backtrace+0x0/0xf4) from [<c005ccb8>] (__lock_acquire+0x14f8/0x1b98)
[   65.470000] [<c005ccb8>] (__lock_acquire+0x14f8/0x1b98) from [<c005d3f8>] (lock_acquire+0xa0/0x108)
[   65.470000] [<c005d3f8>] (lock_acquire+0xa0/0x108) from [<c02f671c>] (_raw_spin_lock_irqsave+0x48/0x5c)
[   65.470000] [<c02f671c>] (_raw_spin_lock_irqsave+0x48/0x5c) from [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc])
[   65.470000] [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]) from [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc])
[   65.470000] [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc]) from [<c0219b38>] (sdio_irq_thread+0x1bc/0x274)
[   65.470000] [<c0219b38>] (sdio_irq_thread+0x1bc/0x274) from [<c003c324>] (kthread+0x8c/0x98)
[   65.470000] [<c003c324>] (kthread+0x8c/0x98) from [<c00101ac>] (kernel_thread_exit+0x0/0x8)
[   65.470000] BUG: spinlock lockup suspected on CPU#0, ksdioirqd/mmc0/73
[   65.470000]  lock: 0xc3358724, .magic: dead4ead, .owner: ksdioirqd/mmc0/73, .owner_cpu: 0
[   65.470000] [<c0014990>] (unwind_backtrace+0x0/0xf4) from [<c01b46b0>] (do_raw_spin_lock+0x100/0x144)
[   65.470000] [<c01b46b0>] (do_raw_spin_lock+0x100/0x144) from [<c02f6724>] (_raw_spin_lock_irqsave+0x50/0x5c)
[   65.470000] [<c02f6724>] (_raw_spin_lock_irqsave+0x50/0x5c) from [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc])
[   65.470000] [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]) from [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc])
[   65.470000] [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc]) from [<c0219b38>] (sdio_irq_thread+0x1bc/0x274)
[   65.470000] [<c0219b38>] (sdio_irq_thread+0x1bc/0x274) from [<c003c324>] (kthread+0x8c/0x98)
[   65.470000] [<c003c324>] (kthread+0x8c/0x98) from [<c00101ac>] (kernel_thread_exit+0x0/0x8)
---schnapp---

Any hints how to work around or fix this, would be appreciated

Attila Kinali

Signature · 4 lines

It is upon moral qualities that a society is ultimately founded. All 
the prosperity and technological sophistication in the world is of no 
use without that foundation.
                 -- Miss Matheson, The Diamond Age, Neil Stephenson

Attila Kinali

2012-07-12 15:13 UTC #7 lore raw

Quoted 5 lines · On Thu, 12 Jul 2012 22:39:53 +0800 Shawn Guo <[email protected]> wrote:

Quoted 4 lines
I ran into the same problem today, but the proposed fix doesn't seem to work for me:
It's a different problem from what Lauri reported and fixed.

Ok...

Quoted 4 lines

I haven't
played SDIO card that much, so I'm not completely clear about the SDIO calling sequence, but is it reasonable that mxs_mmc_enable_sdio_irq is being called recursively?

I don't know. I dont know the code at all and not how the sdio system works. But a quick check shows, that mxs_mmc_enable_sdio_irq does not call any other function (besides readel, writel) and hence cannot call itself.

For me it rather looks like that there seem to be two consequtive irqs that get passed to sdio_irq_thread which then calls mxs_mmc_enable_sdio_irq.

But with my limited knowledge i cannot check this theory. Can anyone give me some hints how i could verify this?

Attila Kinali

Signature · 4 lines

The trouble with you, Shev, is you don't say anything until you've saved
up a whole truckload of damned heavy brick arguments and then you dump
them all out and never look at the bleeding body mangled beneath the heap
		-- Tirin, The Dispossessed, U. Le Guin

Lauri Hintsala

author

2012-07-16 05:57 UTC #8 lore raw

In reply to Attila Kinali

Hi Attila,

Quoted 60 lines · Attila Kinali

I ran into the same problem today, but the proposed fix doesn't seem to work for me:

---schnipp---

# modprobe libertas_sdio
[   59.200000] lib80211: common routines for IEEE802.11 drivers
[   59.240000] cfg80211: Calling CRDA to update world regulatory domain
[   59.320000] libertas_sdio: Libertas SDIO driver
[   59.330000] libertas_sdio: Copyright Pierre Ossman
# modprobe mxs-mmc
[   64.210000] mxs-mmc 80010000.ssp: initialized
[   64.260000] mxs-mmc 80034000.ssp: initialized
[   64.270000] mmc0: new SDIO card at address 0001
# [   65.440000] libertas_sdio mmc0:0001:1: (unregistered net_device): 00:13:04:80:00:3f, fw 9.70.3p24, cap 0x00000303
[   65.470000]
[   65.470000] =============================================
[   65.470000] [ INFO: possible recursive locking detected ]
[   65.470000] 3.5.0-rc5 #2 Not tainted
[   65.470000] ---------------------------------------------
[   65.470000] ksdioirqd/mmc0/73 is trying to acquire lock:
[   65.470000]  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000]
[   65.470000] but task is already holding lock:
[   65.470000]  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000]
[   65.470000] other info that might help us debug this:
[   65.470000]  Possible unsafe locking scenario:
[   65.470000]
[   65.470000]        CPU0
[   65.470000]        ----
[   65.470000]   lock(&(&host->lock)->rlock#2);
[   65.470000]   lock(&(&host->lock)->rlock#2);
[   65.470000]
[   65.470000]  *** DEADLOCK ***
[   65.470000]
[   65.470000]  May be due to missing lock nesting notation
[   65.470000]
[   65.470000] 1 lock held by ksdioirqd/mmc0/73:
[   65.470000]  #0:  (&(&host->lock)->rlock#2){-.-...}, at: [<bf054120>] mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]
[   65.470000]
[   65.470000] stack backtrace:
[   65.470000] [<c0014990>] (unwind_backtrace+0x0/0xf4) from [<c005ccb8>] (__lock_acquire+0x14f8/0x1b98)
[   65.470000] [<c005ccb8>] (__lock_acquire+0x14f8/0x1b98) from [<c005d3f8>] (lock_acquire+0xa0/0x108)
[   65.470000] [<c005d3f8>] (lock_acquire+0xa0/0x108) from [<c02f671c>] (_raw_spin_lock_irqsave+0x48/0x5c)
[   65.470000] [<c02f671c>] (_raw_spin_lock_irqsave+0x48/0x5c) from [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc])
[   65.470000] [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]) from [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc])
[   65.470000] [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc]) from [<c0219b38>] (sdio_irq_thread+0x1bc/0x274)
[   65.470000] [<c0219b38>] (sdio_irq_thread+0x1bc/0x274) from [<c003c324>] (kthread+0x8c/0x98)
[   65.470000] [<c003c324>] (kthread+0x8c/0x98) from [<c00101ac>] (kernel_thread_exit+0x0/0x8)
[   65.470000] BUG: spinlock lockup suspected on CPU#0, ksdioirqd/mmc0/73
[   65.470000]  lock: 0xc3358724, .magic: dead4ead, .owner: ksdioirqd/mmc0/73, .owner_cpu: 0
[   65.470000] [<c0014990>] (unwind_backtrace+0x0/0xf4) from [<c01b46b0>] (do_raw_spin_lock+0x100/0x144)
[   65.470000] [<c01b46b0>] (do_raw_spin_lock+0x100/0x144) from [<c02f6724>] (_raw_spin_lock_irqsave+0x50/0x5c)
[   65.470000] [<c02f6724>] (_raw_spin_lock_irqsave+0x50/0x5c) from [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc])
[   65.470000] [<bf054120>] (mxs_mmc_enable_sdio_irq+0x18/0xdc [mxs_mmc]) from [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc])
[   65.470000] [<bf0541d0>] (mxs_mmc_enable_sdio_irq+0xc8/0xdc [mxs_mmc]) from [<c0219b38>] (sdio_irq_thread+0x1bc/0x274)
[   65.470000] [<c0219b38>] (sdio_irq_thread+0x1bc/0x274) from [<c003c324>] (kthread+0x8c/0x98)
[   65.470000] [<c003c324>] (kthread+0x8c/0x98) from [<c00101ac>] (kernel_thread_exit+0x0/0x8)
---schnapp---

Quoted 1 line

Any hints how to work around or fix this, would be appreciated

Does this patch fix your issue?

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/mmc/host/mxs-mmc.c +0 -0

@@ -637,11 +637,6 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host

*mmc, int enable)
host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_SET);
-
- if (readl(host->base + HW_SSP_STATUS(host)) &
- BM_SSP_STATUS_SDIO_IRQ)
- mmc_signal_sdio_irq(host->mmc);
-
} else {
writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
@@ -650,6 +645,11 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
}

spin_unlock_irqrestore(&host->lock, flags);
+
+ if (enable && readl(host->base + HW_SSP_STATUS(host)) &
+ BM_SSP_STATUS_SDIO_IRQ)
+ mmc_signal_sdio_irq(host->mmc);
+
}

static const struct mmc_host_ops mxs_mmc_ops = {
<<<<<<<

mxs_mmc_enable_sdio_irq was called by mmc_signal_sdio_irq. mmc_signal_sdio_irq was called inside spin lock. So the lock was tried to acquire before it was released.

Best regards,
Lauri Hintsala

Attila Kinali

2012-07-16 12:07 UTC #9 lore raw

Moin, Lauri,

Quoted 3 lines · On Mon, 16 Jul 2012 08:57:38 +0300 Lauri Hintsala…

Does this patch fix your issue?

A preliminary test shows that it at least fixes the oops at module loading. I haven't had the chance yet to give it a full test, but i would say it fixes it enough to be workable.

Thanks a lot!

Attila Kinali

Signature · 4 lines

The trouble with you, Shev, is you don't say anything until you've saved
up a whole truckload of damned heavy brick arguments and then you dump
them all out and never look at the bleeding body mangled beneath the heap
		-- Tirin, The Dispossessed, U. Le Guin

Lauri Hintsala

author

2012-07-17 04:54 UTC #10 lore raw

In reply to Lauri Hintsala

Shawn,

Could you review this patch? Attila reported it fixes his SDIO initialization issue.

Lauri

Quoted 47 lines · Lauri Hintsala

Quoted 1 line
Any hints how to work around or fix this, would be appreciated
Does this patch fix your issue?
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
drivers/mmc/host/mxs-mmc.c +0 -5
@@ -637,11 +637,6 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host
 *mmc, int enable)
                 host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
          writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
                 host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_SET);
-
-        if (readl(host->base + HW_SSP_STATUS(host)) &
-                BM_SSP_STATUS_SDIO_IRQ)
-            mmc_signal_sdio_irq(host->mmc);
-
      } else {
          writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
                 host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
drivers/mmc/host/mxs-mmc.c +5 -0
@@ -650,6 +645,11 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host
 *mmc, int enable)
      }
 
      spin_unlock_irqrestore(&host->lock, flags);
+
+    if (enable && readl(host->base + HW_SSP_STATUS(host)) &
+            BM_SSP_STATUS_SDIO_IRQ)
+        mmc_signal_sdio_irq(host->mmc);
+
  }
 
static const struct mmc_host_ops mxs_mmc_ops = {
<<<<<<<
mxs_mmc_enable_sdio_irq was called by mmc_signal_sdio_irq. mmc_signal_sdio_irq was called inside spin lock. So the lock was tried to acquire before it was released.
Best regards,
Lauri Hintsala
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Shawn Guo

Acked-by

2012-07-17 12:19 UTC #11 lore raw

Quoted 5 lines · Lauri Hintsala

Shawn,
Could you review this patch? Attila reported it fixes his SDIO initialization issue.

Thanks for fixing it.

Acked-by Shawn Guo <[email protected]>

Quoted 57 lines

Lauri
Quoted 41 lines · Lauri Hintsala
Quoted 1 line
Any hints how to work around or fix this, would be appreciated
Does this patch fix your issue?
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
drivers/mmc/host/mxs-mmc.c +0 -5
@@ -637,11 +637,6 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host
 *mmc, int enable)
                host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
         writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
                host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_SET);
-
-        if (readl(host->base + HW_SSP_STATUS(host)) &
-                BM_SSP_STATUS_SDIO_IRQ)
-            mmc_signal_sdio_irq(host->mmc);
-
     } else {
         writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
                host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
drivers/mmc/host/mxs-mmc.c +5 -0
@@ -650,6 +645,11 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host
 *mmc, int enable)
     }
 
     spin_unlock_irqrestore(&host->lock, flags);
+
+    if (enable && readl(host->base + HW_SSP_STATUS(host)) &
+            BM_SSP_STATUS_SDIO_IRQ)
+        mmc_signal_sdio_irq(host->mmc);
+
 }
 
static const struct mmc_host_ops mxs_mmc_ops = {
<<<<<<<
mxs_mmc_enable_sdio_irq was called by mmc_signal_sdio_irq. mmc_signal_sdio_irq was called inside spin lock. So the lock was tried to acquire before it was released.
Quoted 6 lines
Best regards,
Lauri Hintsala
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Lauri Hintsala

author

2012-07-17 13:03 UTC #12 lore raw

Quoted 9 lines · Shawn Guo

Quoted 5 lines · Lauri Hintsala
Shawn,
Could you review this patch? Attila reported it fixes his SDIO initialization issue.
Thanks for fixing it.
Acked-by Shawn Guo <[email protected]>

Thanks. I'll send both this and previous patches to mailing lists.

Lauri