Skip to contentSkip to conversation

omap_hsmmc: race between omap_hsmmc_start_command() and DMA callback

DISCUSSION Daniel Mack 2 participants 2 messages Jul 18, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hi,

I'm facing a NULL pointer dereference in omap_hsmmc_start_command() on an AM33xx board running 3.11-rc1 (DMA enabled).

A quick debug session showed that DMA engine timing leads to a very reproducable race condition. In omap_hsmmc_request(), we have:

        host->mrq = req;
        omap_hsmmc_prepare_data()
		omap_hsmmc_start_dma_transfer()
			tx->callback = omap_hsmmc_dma_callback;

	[*]

	omap_hsmmc_start_command()
		if (cmd == host->mrq->stop) [<-- oops]
			...

It turns out that omap_hsmmc_dma_callback() (which sets host->mrq = NULL) is entered just after the DMA submission, and *before* omap_hsmmc_start_command() is called, consequently leading to an Oops.

I can debug this in more depth, but maybe someone has an idea already?

Thanks,
Daniel

#1 lore raw
Quoted 19 lines · Daniel Mack

Hi,

I'm facing a NULL pointer dereference in omap_hsmmc_start_command() on an AM33xx board running 3.11-rc1 (DMA enabled).

A quick debug session showed that DMA engine timing leads to a very reproducable race condition. In omap_hsmmc_request(), we have:

         host->mrq = req;
         omap_hsmmc_prepare_data()
		omap_hsmmc_start_dma_transfer()
			tx->callback = omap_hsmmc_dma_callback;

	[*]

	omap_hsmmc_start_command()
		if (cmd == host->mrq->stop) [<-- oops]
			...
Quoted 6 lines

It turns out that omap_hsmmc_dma_callback() (which sets host->mrq = NULL) is entered just after the DMA submission, and *before* omap_hsmmc_start_command() is called, consequently leading to an Oops.

I can debug this in more depth, but maybe someone has an idea already?

Can you check with this hack patch in addition to other dependent patch for adding edma nodes to dt[1] and slave sg limit [2]

arch/arm/common/edma.c +2 -2
index a432e6c..5a19164 100644
@@ -1262,8 +1262,8 @@ int edma_start(unsigned channel)
 		if (test_bit(channel, edma_cc[ctlr]->edma_unused)) {
 			pr_debug("EDMA: ESR%d %08x\n", j,
 				edma_shadow0_read_array(ctlr, SH_ESR, j));
-			edma_shadow0_write_array(ctlr, SH_ESR, j, mask);
-			return 0;
+//			edma_shadow0_write_array(ctlr, SH_ESR, j, mask);
+//			return 0;
 		}
 
 		/* EDMA channel with event association */
Signature · 2 lines
[1] https://lkml.org/lkml/2013/6/18/49
[2] https://patchwork.kernel.org/patch/2228041/
Quoted 4 lines

Thanks,
Daniel