From 370626692e0cd7d3a9318a85948551974db03477 Mon Sep 17 00:00:00 2001 From: Jon Hunter Date: Fri, 9 Oct 2015 16:02:24 +0100 Subject: [PATCH] CHROMIUM: dmaengine: tegra-adma: fix race between tasklet/terminate_all (take 2) Commit 92cd07d23e63 ("CHROMIUM: dmaengine: tegra-adma: fix race between tasklet/terminate_all") attempted to fix a race condition between the tegra_adma_tasklet() and a call to tegra_adma_terminate_all() that would result in a kernel crash. This change added a call to tasklet_kill() to tegra_adma_terminate_all() to stop the tasklet if it was running in order to the crash. However, this triggered another kernel panic, when tegra_adma_terminate_all() was called from an interrupt context and tasklet_kill() then attempted to sleep. Commit 19218f465ccb ("CHROMIUM: dmaengine: tegra-adma: Avoid tasklet from self-destructing") workaround this new crash, but this still means that the crash from the original problem may still occur. On inspecting the most recent crash logs, it appears that the original crash is caused when the tasklet calling the DMA callback which then calls tegra_adma_terminate_all(). If the cb_count in the tegra_adma_tasklet() is greater than 1, when tegra_adma_terminate_all() is called, then it is possible for the DMA callback to be called again after the call to terminate the DMA transfers has completed. To prevent this store the current callback count in the ADMA channel data structure and force the count to zero if we are terminating the DMA transfers. BUG=chrome-os-partner:46247 TEST=Audio playback Change-Id: I0585f13e3386c07dd9fc420da5457300be71cfdc Signed-off-by: Jon Hunter Reviewed-on: https://chromium-review.googlesource.com/305641 Commit-Ready: Anatol Pomazau Tested-by: Anatol Pomazau Reviewed-by: Anatol Pomazau Reviewed-by: Andrew Bresticker --- drivers/dma/tegra210-adma.c | 38 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c index 7eaf8a92ec817..43431075c6442 100644 --- a/drivers/dma/tegra210-adma.c +++ b/drivers/dma/tegra210-adma.c @@ -125,9 +125,7 @@ struct tegra_adma_chan { /* ISR handler and tasklet for bottom half of isr handling */ dma_isr_handler isr_handler; struct tasklet_struct tasklet; - bool tasklet_active; - dma_async_tx_callback callback; - void *callback_param; + int callback_count; /* Channel-slave specific configuration */ struct dma_slave_config dma_sconfig; @@ -592,24 +590,22 @@ static void tegra_adma_tasklet(unsigned long data) void *callback_param = NULL; struct tegra_adma_desc *dma_desc; unsigned long flags; - int cb_count; spin_lock_irqsave(&tdc->lock, flags); - tdc->tasklet_active = true; while (!list_empty(&tdc->cb_desc)) { dma_desc = list_first_entry(&tdc->cb_desc, typeof(*dma_desc), cb_node); list_del(&dma_desc->cb_node); callback = dma_desc->txd.callback; callback_param = dma_desc->txd.callback_param; - cb_count = dma_desc->cb_count; + tdc->callback_count = dma_desc->cb_count; dma_desc->cb_count = 0; - spin_unlock_irqrestore(&tdc->lock, flags); - while (cb_count-- && callback) + while (tdc->callback_count-- && callback) { + spin_unlock_irqrestore(&tdc->lock, flags); callback(callback_param); - spin_lock_irqsave(&tdc->lock, flags); + spin_lock_irqsave(&tdc->lock, flags); + } } - tdc->tasklet_active = false; spin_unlock_irqrestore(&tdc->lock, flags); } @@ -724,21 +720,6 @@ static void tegra_adma_terminate_all(struct dma_chan *dc) } tegra_adma_resume(tdc); - /* - * Check for any running tasklets and kill them. - * But since this function can be invoked from tasklet itself, - * avoid self-destruct which would cause forever loop. - */ - if (tdc->tasklet_active) { - tdc->busy = true; - spin_unlock_irqrestore(&tdc->lock, flags); - if (!in_interrupt()) { - dev_warn(tdc2dev(tdc), "Killing tasklet\n"); - tasklet_kill(&tdc->tasklet); - } - spin_lock_irqsave(&tdc->lock, flags); - tdc->busy = false; - } skip_dma_stop: tegra_adma_abort_all(tdc); @@ -749,6 +730,13 @@ static void tegra_adma_terminate_all(struct dma_chan *dc) list_del(&dma_desc->cb_node); dma_desc->cb_count = 0; } + + /* + * The tasklet may be active and so set the current callback + * count to zero to terminate the tasklet. + */ + tdc->callback_count = 0; + spin_unlock_irqrestore(&tdc->lock, flags); }