From patchwork Mon Jun 20 16:27:50 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guennadi Liakhovetski X-Patchwork-Id: 897692 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p5KGS1hM017937 for ; Mon, 20 Jun 2011 16:28:01 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751439Ab1FTQ2A (ORCPT ); Mon, 20 Jun 2011 12:28:00 -0400 Received: from moutng.kundenserver.de ([212.227.17.8]:50590 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750844Ab1FTQ17 (ORCPT ); Mon, 20 Jun 2011 12:27:59 -0400 Received: from axis700.grange (dslb-088-076-027-104.pools.arcor-ip.net [88.76.27.104]) by mrelayeu.kundenserver.de (node=mrbap3) with ESMTP (Nemesis) id 0LtnUL-1PXkz804Fp-011AVV; Mon, 20 Jun 2011 18:27:55 +0200 Received: by axis700.grange (Postfix, from userid 1000) id 967DE189B83; Mon, 20 Jun 2011 18:27:50 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by axis700.grange (Postfix) with ESMTP id 8E695189B82; Mon, 20 Jun 2011 18:27:50 +0200 (CEST) Date: Mon, 20 Jun 2011 18:27:50 +0200 (CEST) From: Guennadi Liakhovetski X-X-Sender: lyakh@axis700.grange To: linux-mmc@vger.kernel.org cc: linux-sh@vger.kernel.org, Ian Molton , Kuninori Morimoto , Chris Ball , Magnus Damm Subject: [PATCH v2] mmc: tmio: fix recursive spinlock, don't schedule with interrupts disabled In-Reply-To: Message-ID: References: MIME-Version: 1.0 X-Provags-ID: V02:K0:FNVAwWCTMlpJUfeKg5HaqidMCJrzHrGdXZL74GVSIjh 3jCHRzkRL1MOmnpO+14Iy/9OUW+OFHAjONjHPa9Yxm0QqR/H6d zgVALRR1cLyPGqW0pm8E7SC2RSrE8boYf/6INnwf/WGHD02aJQ /ATRu3xjWcyM0NaZOPPwobtM7NLziEN++1D2eE4r0BmumUseKA mojsZ8GjHeyoV0nhBPKgEqPm7ygDvm85pe583azwgg= Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Mon, 20 Jun 2011 16:28:02 +0000 (UTC) Calling mmc_request_done() under a spinlock with interrupts disabled leads to a recursive spin-lock on request retry path and to scheduling in atomic context. This patch fixes both these problems by moving mmc_request_done() to the scheduler workqueue. Signed-off-by: Guennadi Liakhovetski Tested-by: Kuninori Morimoto --- This is a bug-fix: without it the system Oopses with LOCKDEP enabled, so, it should really go in 3.0. OTOH it is pretty intrusine and non-trivial, so, reviews and tests are highly appreciated! Also, unfortunately, I wasn't able to test it well enough with SDIO, because the driver for the only SDIO card, that I have, reproducibly crashes the kernel: http://www.spinics.net/lists/kernel/msg1203334.html So, mainly tested with SD-cards and only lightly with SDIO. v2: 1. added a mutex to properly complete each .set_ios() call instead of returning an error, when racing with another one. 2. oritect data inside tmio_mmc_finish_request() 3. don't reschedule card detection, if one is already pending drivers/mmc/host/tmio_mmc.h | 6 +++++- drivers/mmc/host/tmio_mmc_pio.c | 35 +++++++++++++++++++++++++++++------ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h index 8260bc2..5b83e46 100644 --- a/drivers/mmc/host/tmio_mmc.h +++ b/drivers/mmc/host/tmio_mmc.h @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -73,8 +74,11 @@ struct tmio_mmc_host { /* Track lost interrupts */ struct delayed_work delayed_reset_work; - spinlock_t lock; + struct work_struct done; + + spinlock_t lock; /* protect host private data */ unsigned long last_req_ts; + struct mutex ios_lock; /* protect set_ios() context */ }; int tmio_mmc_host_probe(struct tmio_mmc_host **host, diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c index 0b09e82..eabda39 100644 --- a/drivers/mmc/host/tmio_mmc_pio.c +++ b/drivers/mmc/host/tmio_mmc_pio.c @@ -284,10 +284,16 @@ static void tmio_mmc_reset_work(struct work_struct *work) /* called with host->lock held, interrupts disabled */ static void tmio_mmc_finish_request(struct tmio_mmc_host *host) { - struct mmc_request *mrq = host->mrq; + struct mmc_request *mrq; + unsigned long flags; - if (!mrq) + spin_lock_irqsave(&host->lock, flags); + + mrq = host->mrq; + if (IS_ERR_OR_NULL(mrq)) { + spin_unlock_irqrestore(&host->lock, flags); return; + } host->cmd = NULL; host->data = NULL; @@ -296,11 +302,18 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host *host) cancel_delayed_work(&host->delayed_reset_work); host->mrq = NULL; + spin_unlock_irqrestore(&host->lock, flags); - /* FIXME: mmc_request_done() can schedule! */ mmc_request_done(host->mmc, mrq); } +static void tmio_mmc_done_work(struct work_struct *work) +{ + struct tmio_mmc_host *host = container_of(work, struct tmio_mmc_host, + done); + tmio_mmc_finish_request(host); +} + /* These are the bitmasks the tmio chip requires to implement the MMC response * types. Note that R1 and R6 are the same in this scheme. */ #define APP_CMD 0x0040 @@ -467,7 +480,7 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host) BUG(); } - tmio_mmc_finish_request(host); + schedule_work(&host->done); } static void tmio_mmc_data_irq(struct tmio_mmc_host *host) @@ -557,7 +570,7 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, tasklet_schedule(&host->dma_issue); } } else { - tmio_mmc_finish_request(host); + schedule_work(&host->done); } out: @@ -618,7 +631,8 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid) if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) { tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE); - mmc_detect_change(host->mmc, msecs_to_jiffies(100)); + if (!work_pending(&host->mmc->detect.work)) + mmc_detect_change(host->mmc, msecs_to_jiffies(100)); } /* CRC and other errors */ @@ -749,6 +763,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) struct tmio_mmc_data *pdata = host->pdata; unsigned long flags; + mutex_lock(&host->ios_lock); + spin_lock_irqsave(&host->lock, flags); if (host->mrq) { if (IS_ERR(host->mrq)) { @@ -764,6 +780,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) host->mrq->cmd->opcode, host->last_req_ts, jiffies); } spin_unlock_irqrestore(&host->lock, flags); + + mutex_unlock(&host->ios_lock); return; } @@ -817,6 +835,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) current->comm, task_pid_nr(current), ios->clock, ios->power_mode); host->mrq = NULL; + + mutex_unlock(&host->ios_lock); } static int tmio_mmc_get_ro(struct mmc_host *mmc) @@ -913,9 +933,11 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host, tmio_mmc_enable_sdio_irq(mmc, 0); spin_lock_init(&_host->lock); + mutex_init(&_host->ios_lock); /* Init delayed work for request timeouts */ INIT_DELAYED_WORK(&_host->delayed_reset_work, tmio_mmc_reset_work); + INIT_WORK(&_host->done, tmio_mmc_done_work); /* See if we also get DMA */ tmio_mmc_request_dma(_host, pdata); @@ -963,6 +985,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host) pm_runtime_get_sync(&pdev->dev); mmc_remove_host(host->mmc); + cancel_work_sync(&host->done); cancel_delayed_work_sync(&host->delayed_reset_work); tmio_mmc_release_dma(host);