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

commit 58148c0 ("CHROMIUM: dmaengine: tegra-adma: Fix
another race between tasklet/terminate_all") added
tasklet_kill which is called from atomic context, causing
scheduling while atomic bug. This patch moves the the sync
point to device_free_chan_resources which is not atomic.

BUG=chrome-os-partner:46247, b:26806769
TEST=Add udelay(10000) in the tasklet before calling the
callback to simulate heavy system load. Observe the
scheduling while atomic bug. After applying this patch,
observe the bug is gone and audio playback works on Ryu.

Change-Id: I8d4408364cea492f094e15306c09d153315c9782
Signed-off-by: Ben Zhang <benzh@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/342355
Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
(cherry picked from commit 20115e8a450a03cea5ff7b2e5db5fc35081b4637)
Reviewed-on: https://chromium-review.googlesource.com/342835
Commit-Queue: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
  • Loading branch information
Ben Zhang authored and Andrew Bresticker committed May 6, 2016
1 parent 933cb6e commit 161314b
Showing 1 changed file with 21 additions and 9 deletions.
30 changes: 21 additions & 9 deletions drivers/dma/tegra210-adma.c
Original file line number Diff line number Diff line change
Expand Up @@ -734,18 +734,18 @@ static void tegra_adma_terminate_all(struct dma_chan *dc)
/*
* The tasklet may be active and so set the current callback
* count to zero to terminate the tasklet.
* 1. If tegra_adma_terminate_all is called by the tasklet callback
* itself due to xrun, callback_count=0 ensures the callback is not
* called again after the current callback returns.
* 2. If tegra_adma_terminate_all is called by the client thread that
* wants to stop audio, no new callbacks are going to be scheduled
* after this point. However, the callback can still be running on
* another CPU at this point if it was started before tdc->lock is
* grabbed. We sync with the callback in device_free_chan_resources
* since the current context is atomic.
*/
tdc->callback_count = 0;

/* Make sure the tasklet has stopped running before we return. */
if (!in_interrupt()) {
tdc->busy = true;
spin_unlock_irqrestore(&tdc->lock, flags);
tasklet_kill(&tdc->tasklet);
spin_lock_irqsave(&tdc->lock, flags);
tdc->busy = false;
}

spin_unlock_irqrestore(&tdc->lock, flags);
}

Expand Down Expand Up @@ -1141,6 +1141,18 @@ static void tegra_adma_free_chan_resources(struct dma_chan *dc)

if (tdc->busy)
tegra_adma_terminate_all(dc);

/* Ensure the callback in the tasklet finishes before freeing resources.
* We cannot sync in tegra_adma_terminate_all, because it is called
* from atomic context SNDRV_PCM_TRIGGER_STOP->DMA_TERMINATE_ALL.
* The current context is not atomic (from pcm_close op), so it's OK
* to sleep and sync here.
* Without this sync, the callback (e.g. dmaengine_pcm_dma_complete)
* may still be running after snd_pcm_release which frees
* snd_pcm_substream, causing use-after-free crash.
*/
tasklet_kill(&tdc->tasklet);

pm_runtime_put(tdc->tdma->dev);
spin_lock_irqsave(&tdc->lock, flags);
list_splice_init(&tdc->pending_sg_req, &sg_req_list);
Expand Down

0 comments on commit 161314b

Please sign in to comment.