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