diff mbox

[RESEND] mmc: sdhci: Fix sleep in atomic after inserting SD card

Message ID 1420451415-11828-1-git-send-email-k.kozlowski@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Krzysztof Kozlowski Jan. 5, 2015, 9:50 a.m. UTC
Sleep in atomic context happened on Trats2 board after inserting or
removing SD card because mmc_gpio_get_cd() was called under spin lock.

Fix this by moving card detection earlier, before acquiring spin lock.
The mmc_gpio_get_cd() call does not have to be protected by spin lock
because it does not access any sdhci internal data.
The sdhci_do_get_cd() call access host flags (SDHCI_DEVICE_DEAD). After
moving it out side of spin lock it could theoretically race with driver
removal but still there is no actual protection against manual card
eject.

Dmesg after inserting SD card:
[   41.663414] BUG: sleeping function called from invalid context at drivers/gpio/gpiolib.c:1511
[   41.670469] in_atomic(): 1, irqs_disabled(): 128, pid: 30, name: kworker/u8:1
[   41.677580] INFO: lockdep is turned off.
[   41.681486] irq event stamp: 61972
[   41.684872] hardirqs last  enabled at (61971): [<c0490ee0>] _raw_spin_unlock_irq+0x24/0x5c
[   41.693118] hardirqs last disabled at (61972): [<c04907ac>] _raw_spin_lock_irq+0x18/0x54
[   41.701190] softirqs last  enabled at (61648): [<c0026fd4>] __do_softirq+0x234/0x2c8
[   41.708914] softirqs last disabled at (61631): [<c00273a0>] irq_exit+0xd0/0x114
[   41.716206] Preemption disabled at:[<  (null)>]   (null)
[   41.721500]
[   41.722985] CPU: 3 PID: 30 Comm: kworker/u8:1 Tainted: G        W      3.18.0-rc5-next-20141121 #883
[   41.732111] Workqueue: kmmcd mmc_rescan
[   41.735945] [<c0014d2c>] (unwind_backtrace) from [<c0011c80>] (show_stack+0x10/0x14)
[   41.743661] [<c0011c80>] (show_stack) from [<c0489d14>] (dump_stack+0x70/0xbc)
[   41.750867] [<c0489d14>] (dump_stack) from [<c0228b74>] (gpiod_get_raw_value_cansleep+0x18/0x30)
[   41.759628] [<c0228b74>] (gpiod_get_raw_value_cansleep) from [<c03646e8>] (mmc_gpio_get_cd+0x38/0x58)
[   41.768821] [<c03646e8>] (mmc_gpio_get_cd) from [<c036d378>] (sdhci_request+0x50/0x1a4)
[   41.776808] [<c036d378>] (sdhci_request) from [<c0357934>] (mmc_start_request+0x138/0x268)
[   41.785051] [<c0357934>] (mmc_start_request) from [<c0357cc8>] (mmc_wait_for_req+0x58/0x1a0)
[   41.793469] [<c0357cc8>] (mmc_wait_for_req) from [<c0357e68>] (mmc_wait_for_cmd+0x58/0x78)
[   41.801714] [<c0357e68>] (mmc_wait_for_cmd) from [<c0361c00>] (mmc_io_rw_direct_host+0x98/0x124)
[   41.810480] [<c0361c00>] (mmc_io_rw_direct_host) from [<c03620f8>] (sdio_reset+0x2c/0x64)
[   41.818641] [<c03620f8>] (sdio_reset) from [<c035a3d8>] (mmc_rescan+0x254/0x2e4)
[   41.826028] [<c035a3d8>] (mmc_rescan) from [<c003a0e0>] (process_one_work+0x180/0x3f4)
[   41.833920] [<c003a0e0>] (process_one_work) from [<c003a3bc>] (worker_thread+0x34/0x4b0)
[   41.841991] [<c003a3bc>] (worker_thread) from [<c003fed8>] (kthread+0xe4/0x104)
[   41.849285] [<c003fed8>] (kthread) from [<c000f268>] (ret_from_fork+0x14/0x2c)
[   42.038276] mmc0: new high speed SDHC card at address 1234

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Fixes: 94144a465dd0 ("mmc: sdhci: add get_cd() implementation")
Cc: <stable@vger.kernel.org>
---
 drivers/mmc/host/sdhci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Ulf Hansson Jan. 5, 2015, 3:22 p.m. UTC | #1
On 5 January 2015 at 10:50, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
> Sleep in atomic context happened on Trats2 board after inserting or
> removing SD card because mmc_gpio_get_cd() was called under spin lock.
>
> Fix this by moving card detection earlier, before acquiring spin lock.
> The mmc_gpio_get_cd() call does not have to be protected by spin lock
> because it does not access any sdhci internal data.
> The sdhci_do_get_cd() call access host flags (SDHCI_DEVICE_DEAD). After
> moving it out side of spin lock it could theoretically race with driver
> removal but still there is no actual protection against manual card
> eject.
>
> Dmesg after inserting SD card:
> [   41.663414] BUG: sleeping function called from invalid context at drivers/gpio/gpiolib.c:1511
> [   41.670469] in_atomic(): 1, irqs_disabled(): 128, pid: 30, name: kworker/u8:1
> [   41.677580] INFO: lockdep is turned off.
> [   41.681486] irq event stamp: 61972
> [   41.684872] hardirqs last  enabled at (61971): [<c0490ee0>] _raw_spin_unlock_irq+0x24/0x5c
> [   41.693118] hardirqs last disabled at (61972): [<c04907ac>] _raw_spin_lock_irq+0x18/0x54
> [   41.701190] softirqs last  enabled at (61648): [<c0026fd4>] __do_softirq+0x234/0x2c8
> [   41.708914] softirqs last disabled at (61631): [<c00273a0>] irq_exit+0xd0/0x114
> [   41.716206] Preemption disabled at:[<  (null)>]   (null)
> [   41.721500]
> [   41.722985] CPU: 3 PID: 30 Comm: kworker/u8:1 Tainted: G        W      3.18.0-rc5-next-20141121 #883
> [   41.732111] Workqueue: kmmcd mmc_rescan
> [   41.735945] [<c0014d2c>] (unwind_backtrace) from [<c0011c80>] (show_stack+0x10/0x14)
> [   41.743661] [<c0011c80>] (show_stack) from [<c0489d14>] (dump_stack+0x70/0xbc)
> [   41.750867] [<c0489d14>] (dump_stack) from [<c0228b74>] (gpiod_get_raw_value_cansleep+0x18/0x30)
> [   41.759628] [<c0228b74>] (gpiod_get_raw_value_cansleep) from [<c03646e8>] (mmc_gpio_get_cd+0x38/0x58)
> [   41.768821] [<c03646e8>] (mmc_gpio_get_cd) from [<c036d378>] (sdhci_request+0x50/0x1a4)
> [   41.776808] [<c036d378>] (sdhci_request) from [<c0357934>] (mmc_start_request+0x138/0x268)
> [   41.785051] [<c0357934>] (mmc_start_request) from [<c0357cc8>] (mmc_wait_for_req+0x58/0x1a0)
> [   41.793469] [<c0357cc8>] (mmc_wait_for_req) from [<c0357e68>] (mmc_wait_for_cmd+0x58/0x78)
> [   41.801714] [<c0357e68>] (mmc_wait_for_cmd) from [<c0361c00>] (mmc_io_rw_direct_host+0x98/0x124)
> [   41.810480] [<c0361c00>] (mmc_io_rw_direct_host) from [<c03620f8>] (sdio_reset+0x2c/0x64)
> [   41.818641] [<c03620f8>] (sdio_reset) from [<c035a3d8>] (mmc_rescan+0x254/0x2e4)
> [   41.826028] [<c035a3d8>] (mmc_rescan) from [<c003a0e0>] (process_one_work+0x180/0x3f4)
> [   41.833920] [<c003a0e0>] (process_one_work) from [<c003a3bc>] (worker_thread+0x34/0x4b0)
> [   41.841991] [<c003a3bc>] (worker_thread) from [<c003fed8>] (kthread+0xe4/0x104)
> [   41.849285] [<c003fed8>] (kthread) from [<c000f268>] (ret_from_fork+0x14/0x2c)
> [   42.038276] mmc0: new high speed SDHC card at address 1234
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Fixes: 94144a465dd0 ("mmc: sdhci: add get_cd() implementation")
> Cc: <stable@vger.kernel.org>

Thanks! Applied for fixes.

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index cbb245b58538..8a94c126c576 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1353,6 +1353,8 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>
>         sdhci_runtime_pm_get(host);
>
> +       present = mmc_gpio_get_cd(host->mmc);
> +
>         spin_lock_irqsave(&host->lock, flags);
>
>         WARN_ON(host->mrq != NULL);
> @@ -1381,7 +1383,6 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>          *     zero: cd-gpio is used, and card is removed
>          *     one: cd-gpio is used, and card is present
>          */
> -       present = mmc_gpio_get_cd(host->mmc);
>         if (present < 0) {
>                 /* If polling, assume that the card is always present. */
>                 if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> @@ -2110,15 +2111,18 @@ static void sdhci_card_event(struct mmc_host *mmc)
>  {
>         struct sdhci_host *host = mmc_priv(mmc);
>         unsigned long flags;
> +       int present;
>
>         /* First check if client has provided their own card event */
>         if (host->ops->card_event)
>                 host->ops->card_event(host);
>
> +       present = sdhci_do_get_cd(host);
> +
>         spin_lock_irqsave(&host->lock, flags);
>
>         /* Check host->mrq first in case we are runtime suspended */
> -       if (host->mrq && !sdhci_do_get_cd(host)) {
> +       if (host->mrq && !present) {
>                 pr_err("%s: Card removed during transfer!\n",
>                         mmc_hostname(host->mmc));
>                 pr_err("%s: Resetting controller.\n",
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cbb245b58538..8a94c126c576 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1353,6 +1353,8 @@  static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	sdhci_runtime_pm_get(host);
 
+	present = mmc_gpio_get_cd(host->mmc);
+
 	spin_lock_irqsave(&host->lock, flags);
 
 	WARN_ON(host->mrq != NULL);
@@ -1381,7 +1383,6 @@  static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	 *     zero: cd-gpio is used, and card is removed
 	 *     one: cd-gpio is used, and card is present
 	 */
-	present = mmc_gpio_get_cd(host->mmc);
 	if (present < 0) {
 		/* If polling, assume that the card is always present. */
 		if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
@@ -2110,15 +2111,18 @@  static void sdhci_card_event(struct mmc_host *mmc)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
 	unsigned long flags;
+	int present;
 
 	/* First check if client has provided their own card event */
 	if (host->ops->card_event)
 		host->ops->card_event(host);
 
+	present = sdhci_do_get_cd(host);
+
 	spin_lock_irqsave(&host->lock, flags);
 
 	/* Check host->mrq first in case we are runtime suspended */
-	if (host->mrq && !sdhci_do_get_cd(host)) {
+	if (host->mrq && !present) {
 		pr_err("%s: Card removed during transfer!\n",
 			mmc_hostname(host->mmc));
 		pr_err("%s: Resetting controller.\n",