Re: [PATCH 26/32] omap_hsmmc: prevent races with irq handler
Andrew Morton wrote:
On Fri, 10 Jul 2009 15:43:09 +0300
Adrian Hunter adrian.hun...@nokia.com wrote:
From 242fae6293adec671b14354f215217354f5076a0 Mon Sep 17 00:00:00 2001
From: Adrian Hunter adrian.hun...@nokia.com
Date: Sat, 16 May 2009 10:32:34 +0300
Subject: [PATCH] omap_hsmmc: prevent races with irq handler
If an unexpected interrupt occurs while preparing the
next request, an oops can occur.
For example, a new request is setting up DMA for data
transfer so host-data is not NULL. An unexpected
transfer complete (TC) interrupt comes along and
the interrupt handler sets host-data to NULL. Oops!
Prevent that by disabling interrupts while setting up
a new request.
Signed-off-by: Adrian Hunter adrian.hun...@nokia.com
---
drivers/mmc/host/omap_hsmmc.c | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 28563d6..38e1410 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -452,6 +452,13 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct
mmc_command *cmd,
if (host-use_dma)
cmdreg |= DMA_EN;
+ /*
+* In an interrupt context (i.e. STOP command), the interrupt is already
+* enabled, otherwise it is not (i.e. new request).
+*/
+ if (!in_interrupt())
+ enable_irq(host-irq);
+
OMAP_HSMMC_WRITE(host-base, ARG, cmd-arg);
OMAP_HSMMC_WRITE(host-base, CMD, cmdreg);
}
@@ -1011,6 +1018,13 @@ static void omap_mmc_request(struct mmc_host *mmc,
struct mmc_request *req)
struct mmc_omap_host *host = mmc_priv(mmc);
int err;
+ /*
+* Prevent races with the interrupt handler because of unexpected
+* interrupts, but not if we are already in interrupt context i.e.
+* retries.
+*/
+ if (!in_interrupt())
+ disable_irq(host-irq);
WARN_ON(host-mrq != NULL);
host-mrq = req;
err = mmc_omap_prepare_data(host, req);
@@ -1019,6 +1033,8 @@ static void omap_mmc_request(struct mmc_host *mmc, struct
mmc_request *req)
if (req-data)
req-data-error = err;
host-mrq = NULL;
+ if (!in_interrupt())
+ enable_irq(host-irq);
mmc_request_done(mmc, req);
return;
}
That seems pretty crude. Disabling an interrupt line can be expensive,
and will shut off any other innocent devices which share the line.
The usual and superior way of fixing races such as this is
spin_lock_irq[save]().
From f54be3678907b85a00d9f51e3ce4c828fc21ee95 Mon Sep 17 00:00:00 2001
From: Adrian Hunter adrian.hun...@nokia.com
Date: Mon, 27 Jul 2009 12:00:17 +0300
Subject: [PATCH] omap_hsmmc: use spin_lock not disable_irq
Disabling an interrupt line can be expensive. The usual and
superior way of fixing races such as this is spin_lock_irqsave.
Signed-off-by: Adrian Hunter adrian.hun...@nokia.com
---
drivers/mmc/host/omap_hsmmc.c | 16
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 8900bae..9850280 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -146,6 +146,8 @@ struct omap_hsmmc_host {
struct work_struct mmc_carddetect_work;
void__iomem *base;
resource_size_t mapbase;
+ spinlock_t irq_lock;
+ unsigned long flags;
unsigned intid;
unsigned intdma_len;
unsigned intdma_sg_idx;
@@ -462,7 +464,7 @@ omap_hsmmc_start_command(struct omap_hsmmc_host *host,
struct mmc_command *cmd,
* enabled, otherwise it is not (i.e. new request).
*/
if (!in_interrupt())
- enable_irq(host-irq);
+ spin_unlock_irqrestore(host-irq_lock, host-flags);
OMAP_HSMMC_WRITE(host-base, ARG, cmd-arg);
OMAP_HSMMC_WRITE(host-base, CMD, cmdreg);
@@ -626,11 +628,14 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
struct mmc_data *data;
int end_cmd = 0, end_trans = 0, status;
+ spin_lock(host-irq_lock);
+
if (host-mrq == NULL) {
OMAP_HSMMC_WRITE(host-base, STAT,
OMAP_HSMMC_READ(host-base, STAT));
/* Flush posted write */
OMAP_HSMMC_READ(host-base, STAT);
+ spin_unlock(host-irq_lock);
return IRQ_HANDLED;
}
@@ -696,6 +701,8 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
if ((end_trans || (status TC)) host-mrq)
omap_hsmmc_xfer_done(host, data);
+ spin_unlock(host-irq_lock);
+
return IRQ_HANDLED;
}
@@ -1066,7 +1073,7 @@ static void omap_hsmmc_request(struct mmc_host *mmc,
struct mmc_request *req)
*
Re: [PATCH 26/32] omap_hsmmc: prevent races with irq handler
On Fri, 10 Jul 2009 15:43:09 +0300 Adrian Hunter adrian.hun...@nokia.com wrote: From 242fae6293adec671b14354f215217354f5076a0 Mon Sep 17 00:00:00 2001 From: Adrian Hunter adrian.hun...@nokia.com Date: Sat, 16 May 2009 10:32:34 +0300 Subject: [PATCH] omap_hsmmc: prevent races with irq handler If an unexpected interrupt occurs while preparing the next request, an oops can occur. For example, a new request is setting up DMA for data transfer so host-data is not NULL. An unexpected transfer complete (TC) interrupt comes along and the interrupt handler sets host-data to NULL. Oops! Prevent that by disabling interrupts while setting up a new request. Signed-off-by: Adrian Hunter adrian.hun...@nokia.com --- drivers/mmc/host/omap_hsmmc.c | 16 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 28563d6..38e1410 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -452,6 +452,13 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd, if (host-use_dma) cmdreg |= DMA_EN; + /* + * In an interrupt context (i.e. STOP command), the interrupt is already + * enabled, otherwise it is not (i.e. new request). + */ + if (!in_interrupt()) + enable_irq(host-irq); + OMAP_HSMMC_WRITE(host-base, ARG, cmd-arg); OMAP_HSMMC_WRITE(host-base, CMD, cmdreg); } @@ -1011,6 +1018,13 @@ static void omap_mmc_request(struct mmc_host *mmc, struct mmc_request *req) struct mmc_omap_host *host = mmc_priv(mmc); int err; + /* + * Prevent races with the interrupt handler because of unexpected + * interrupts, but not if we are already in interrupt context i.e. + * retries. + */ + if (!in_interrupt()) + disable_irq(host-irq); WARN_ON(host-mrq != NULL); host-mrq = req; err = mmc_omap_prepare_data(host, req); @@ -1019,6 +1033,8 @@ static void omap_mmc_request(struct mmc_host *mmc, struct mmc_request *req) if (req-data) req-data-error = err; host-mrq = NULL; + if (!in_interrupt()) + enable_irq(host-irq); mmc_request_done(mmc, req); return; } That seems pretty crude. Disabling an interrupt line can be expensive, and will shut off any other innocent devices which share the line. The usual and superior way of fixing races such as this is spin_lock_irq[save](). -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 26/32] omap_hsmmc: prevent races with irq handler
From 242fae6293adec671b14354f215217354f5076a0 Mon Sep 17 00:00:00 2001 From: Adrian Hunter adrian.hun...@nokia.com Date: Sat, 16 May 2009 10:32:34 +0300 Subject: [PATCH] omap_hsmmc: prevent races with irq handler If an unexpected interrupt occurs while preparing the next request, an oops can occur. For example, a new request is setting up DMA for data transfer so host-data is not NULL. An unexpected transfer complete (TC) interrupt comes along and the interrupt handler sets host-data to NULL. Oops! Prevent that by disabling interrupts while setting up a new request. Signed-off-by: Adrian Hunter adrian.hun...@nokia.com --- drivers/mmc/host/omap_hsmmc.c | 16 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 28563d6..38e1410 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -452,6 +452,13 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd, if (host-use_dma) cmdreg |= DMA_EN; + /* +* In an interrupt context (i.e. STOP command), the interrupt is already +* enabled, otherwise it is not (i.e. new request). +*/ + if (!in_interrupt()) + enable_irq(host-irq); + OMAP_HSMMC_WRITE(host-base, ARG, cmd-arg); OMAP_HSMMC_WRITE(host-base, CMD, cmdreg); } @@ -1011,6 +1018,13 @@ static void omap_mmc_request(struct mmc_host *mmc, struct mmc_request *req) struct mmc_omap_host *host = mmc_priv(mmc); int err; + /* +* Prevent races with the interrupt handler because of unexpected +* interrupts, but not if we are already in interrupt context i.e. +* retries. +*/ + if (!in_interrupt()) + disable_irq(host-irq); WARN_ON(host-mrq != NULL); host-mrq = req; err = mmc_omap_prepare_data(host, req); @@ -1019,6 +1033,8 @@ static void omap_mmc_request(struct mmc_host *mmc, struct mmc_request *req) if (req-data) req-data-error = err; host-mrq = NULL; + if (!in_interrupt()) + enable_irq(host-irq); mmc_request_done(mmc, req); return; } -- 1.5.6.3 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
