Skip to content

Commit

Permalink
CHROMIUM: dmaengine: tegra-adma: fix race between tasklet/terminate_a…
Browse files Browse the repository at this point in the history
…ll (take 2)

Commit 92cd07d ("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 19218f4 ("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 <jonathanh@nvidia.com>
Reviewed-on: https://chromium-review.googlesource.com/305641
Commit-Ready: Anatol Pomazau <anatol@google.com>
Tested-by: Anatol Pomazau <anatol@google.com>
Reviewed-by: Anatol Pomazau <anatol@google.com>
Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
  • Loading branch information
Jon Hunter authored and chrome-bot committed Oct 15, 2015
1 parent 85a967b commit 3706266
Showing 1 changed file with 13 additions and 25 deletions.
38 changes: 13 additions & 25 deletions drivers/dma/tegra210-adma.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand Down

0 comments on commit 3706266

Please sign in to comment.