diff mbox

[v2] mmc: mxcmmc: fix bug that may block a data transfer forever.

Message ID 5126421D.7010204@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fabio Estevam Feb. 21, 2013, 3:49 p.m. UTC
Hi Anatolij,

Anatolij Gustschin wrote:

> this change introduces a race condition for host->req (and maybe
> for host->data) accesses. The callback is running in soft-irq context and can
> be interrupted by the mxcmci_irq() interrupt which can finish the request and
> set host->req to NULL. Then mxcmci_data_done() crashes with a null pointer
> dereference. How extensively was it tested?

Does the patch below help?


--
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

Comments

Anatolij Gustschin March 14, 2013, 7:58 p.m. UTC | #1
Hi Fabio,

On Thu, 21 Feb 2013 12:49:49 -0300
Fabio Estevam <fabio.estevam@freescale.com> wrote:

> Hi Anatolij,
> 
> Anatolij Gustschin wrote:
> 
> > this change introduces a race condition for host->req (and maybe
> > for host->data) accesses. The callback is running in soft-irq context and can
> > be interrupted by the mxcmci_irq() interrupt which can finish the request and
> > set host->req to NULL. Then mxcmci_data_done() crashes with a null pointer
> > dereference. How extensively was it tested?
> 
> Does the patch below help?

Sorry for long delay. No, it doesn't help. With this patch applied
the task copying data to the sd-card hangs forever. I used following
patch to fix observed crashes:

 http://thread.gmane.org/gmane.linux.kernel.mmc/19658/focus=19662

Thanks anyway!

Anatolij
--
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

--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -309,9 +309,11 @@  static void mxcmci_dma_callback(void *data)
 {
        struct mxcmci_host *host = data;
        u32 stat;
+       unsigned int long flags;

        del_timer(&host->watchdog);

+       spin_lock_irqsave(&host->lock, flags);
        stat = readl(host->base + MMC_REG_STATUS);
        writel(stat & ~STATUS_DATA_TRANS_DONE, host->base + MMC_REG_STATUS);

@@ -320,6 +322,7 @@  static void mxcmci_dma_callback(void *data)
        if (stat & STATUS_READ_OP_DONE)
                writel(STATUS_READ_OP_DONE, host->base + MMC_REG_STATUS);

+       spin_unlock_irqrestore(&host->lock, flags);
        mxcmci_data_done(host, stat);
 }