Skip to content

Commit 5596875

Browse files
Huang Shijiegregkh
Huang Shijie
authored andcommitted
mtd: gpmi: fix kernel BUG due to racing DMA operations
commit 7b3d2fb upstream. [1] The gpmi uses the nand_command_lp to issue the commands to NAND chips. The gpmi issues a DMA operation with gpmi_cmd_ctrl when it handles a NAND_CMD_NONE control command. So when we read a page(NAND_CMD_READ0) from the NAND, we may send two DMA operations back-to-back. If we do not serialize the two DMA operations, we will meet a bug when 1.1) we enable CONFIG_DMA_API_DEBUG, CONFIG_DMADEVICES_DEBUG, and CONFIG_DEBUG_SG. 1.2) Use the following commands in an UART console and a SSH console: cmd 1: while true;do dd if=/dev/mtd0 of=/dev/null;done cmd 1: while true;do dd if=/dev/mmcblk0 of=/dev/null;done The kernel log shows below: ----------------------------------------------------------------- kernel BUG at lib/scatterlist.c:28! Unable to handle kernel NULL pointer dereference at virtual address 00000000 ......................... [<80044a0c>] (__bug+0x18/0x24) from [<80249b74>] (sg_next+0x48/0x4c) [<80249b74>] (sg_next+0x48/0x4c) from [<80255398>] (debug_dma_unmap_sg+0x170/0x1a4) [<80255398>] (debug_dma_unmap_sg+0x170/0x1a4) from [<8004af58>] (dma_unmap_sg+0x14/0x6c) [<8004af58>] (dma_unmap_sg+0x14/0x6c) from [<8027e594>] (mxs_dma_tasklet+0x18/0x1c) [<8027e594>] (mxs_dma_tasklet+0x18/0x1c) from [<8007d444>] (tasklet_action+0x114/0x164) ----------------------------------------------------------------- 1.3) Assume the two DMA operations is X (first) and Y (second). The root cause of the bug: Assume process P issues DMA X, and sleep on the completion @this->dma_done. X's tasklet callback is dma_irq_callback. It firstly wake up the process sleeping on the completion @this->dma_done, and then trid to unmap the scatterlist S. The waked process P will issue Y in another ARM core. Y initializes S->sg_magic to zero with sg_init_one(), while dma_irq_callback is unmapping S at the same time. See the diagram: ARM core 0 | ARM core 1 ------------------------------------------------------------- (P issues DMA X, then sleep) --> | | (X's tasklet wakes P) --> | | | <-- (P begin to issue DMA Y) | (X's tasklet unmap the | scatterlist S with dma_unmap_sg) --> | <-- (Y calls sg_init_one() to init | scatterlist S) | [2] This patch serialize both the X and Y in the following way: Unmap the DMA scatterlist S firstly, and wake up the process at the end of the DMA callback, in such a way, Y will be executed after X. After this patch: ARM core 0 | ARM core 1 ------------------------------------------------------------- (P issues DMA X, then sleep) --> | | (X's tasklet unmap the | scatterlist S with dma_unmap_sg) --> | | (X's tasklet wakes P) --> | | | <-- (P begin to issue DMA Y) | | <-- (Y calls sg_init_one() to init | scatterlist S) | Signed-off-by: Huang Shijie <[email protected]> Signed-off-by: Brian Norris <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 8a631f3 commit 5596875

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

drivers/mtd/nand/gpmi-nand/gpmi-nand.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,6 @@ static void dma_irq_callback(void *param)
392392
struct gpmi_nand_data *this = param;
393393
struct completion *dma_c = &this->dma_done;
394394

395-
complete(dma_c);
396-
397395
switch (this->dma_type) {
398396
case DMA_FOR_COMMAND:
399397
dma_unmap_sg(this->dev, &this->cmd_sgl, 1, DMA_TO_DEVICE);
@@ -418,6 +416,8 @@ static void dma_irq_callback(void *param)
418416
default:
419417
pr_err("in wrong DMA operation.\n");
420418
}
419+
420+
complete(dma_c);
421421
}
422422

423423
int start_dma_without_bch_irq(struct gpmi_nand_data *this,

0 commit comments

Comments
 (0)