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

[Ulf Hansson]

On 5 January 2015 at 10:50, Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> Fixes: 94144a465dd0 ("mmc: sdhci: add get_cd() implementation")
> Cc: <stable@xxxxxxxxxxxxxxx>

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/