Skip to content

Commit

Permalink
dmaengine: shdma: fix locking
Browse files Browse the repository at this point in the history 
Close multiple theoretical races, especially the one in
.device_free_chan_resources().

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Paul Mundt <lethal@linux-sh.org>
  • Loading branch information
Guennadi Liakhovetski authored and Paul Mundt committed May 23, 2011
1 parent 5b02c51 commit 2dc6666
Showing 1 changed file with 68 additions and 36 deletions.
104 changes: 68 additions & 36 deletions drivers/dma/shdma.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ enum sh_dmae_desc_status {

/*
* Used for write-side mutual exclusion for the global device list,
* read-side synchronization by way of RCU.
* read-side synchronization by way of RCU, and per-controller data.
*/
static DEFINE_SPINLOCK(sh_dmae_lock);
static LIST_HEAD(sh_dmae_devices);
Expand Down Expand Up @@ -85,22 +85,35 @@ static void dmaor_write(struct sh_dmae_device *shdev, u16 data)
*/
static void sh_dmae_ctl_stop(struct sh_dmae_device *shdev)
{
unsigned short dmaor = dmaor_read(shdev);
unsigned short dmaor;
unsigned long flags;

spin_lock_irqsave(&sh_dmae_lock, flags);

dmaor = dmaor_read(shdev);
dmaor_write(shdev, dmaor & ~(DMAOR_NMIF | DMAOR_AE | DMAOR_DME));

spin_unlock_irqrestore(&sh_dmae_lock, flags);
}

static int sh_dmae_rst(struct sh_dmae_device *shdev)
{
unsigned short dmaor;
unsigned long flags;

sh_dmae_ctl_stop(shdev);
dmaor = dmaor_read(shdev) | shdev->pdata->dmaor_init;
spin_lock_irqsave(&sh_dmae_lock, flags);

dmaor_write(shdev, dmaor);
if (dmaor_read(shdev) & (DMAOR_AE | DMAOR_NMIF)) {
pr_warning("dma-sh: Can't initialize DMAOR.\n");
return -EINVAL;
dmaor = dmaor_read(shdev) & ~(DMAOR_NMIF | DMAOR_AE | DMAOR_DME);

dmaor_write(shdev, dmaor | shdev->pdata->dmaor_init);

dmaor = dmaor_read(shdev);

spin_unlock_irqrestore(&sh_dmae_lock, flags);

if (dmaor & (DMAOR_AE | DMAOR_NMIF)) {
dev_warn(shdev->common.dev, "Can't initialize DMAOR.\n");
return -EIO;
}
return 0;
}
Expand Down Expand Up @@ -184,7 +197,7 @@ static void dmae_init(struct sh_dmae_chan *sh_chan)

static int dmae_set_chcr(struct sh_dmae_chan *sh_chan, u32 val)
{
/* When DMA was working, can not set data to CHCR */
/* If DMA is active, cannot set CHCR. TODO: remove this superfluous check */
if (dmae_is_busy(sh_chan))
return -EBUSY;

Expand Down Expand Up @@ -374,7 +387,12 @@ static void sh_dmae_free_chan_resources(struct dma_chan *chan)
LIST_HEAD(list);
int descs = sh_chan->descs_allocated;

/* Protect against ISR */
spin_lock_irq(&sh_chan->desc_lock);
dmae_halt(sh_chan);
spin_unlock_irq(&sh_chan->desc_lock);

/* Now no new interrupts will occur */

/* Prepared and not submitted descriptors can still be on the queue */
if (!list_empty(&sh_chan->ld_queue))
Expand All @@ -384,6 +402,7 @@ static void sh_dmae_free_chan_resources(struct dma_chan *chan)
/* The caller is holding dma_list_mutex */
struct sh_dmae_slave *param = chan->private;
clear_bit(param->slave_id, sh_dmae_slave_used);
chan->private = NULL;
}

spin_lock_bh(&sh_chan->desc_lock);
Expand Down Expand Up @@ -563,8 +582,6 @@ static struct dma_async_tx_descriptor *sh_dmae_prep_memcpy(
if (!chan || !len)
return NULL;

chan->private = NULL;

sh_chan = to_sh_chan(chan);

sg_init_table(&sg, 1);
Expand Down Expand Up @@ -620,9 +637,9 @@ static int sh_dmae_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
if (!chan)
return -EINVAL;

spin_lock_bh(&sh_chan->desc_lock);
dmae_halt(sh_chan);

spin_lock_bh(&sh_chan->desc_lock);
if (!list_empty(&sh_chan->ld_queue)) {
/* Record partial transfer */
struct sh_desc *desc = list_entry(sh_chan->ld_queue.next,
Expand Down Expand Up @@ -716,6 +733,14 @@ static dma_async_tx_callback __ld_cleanup(struct sh_dmae_chan *sh_chan, bool all
list_move(&desc->node, &sh_chan->ld_free);
}
}

if (all && !callback)
/*
* Terminating and the loop completed normally: forgive
* uncompleted cookies
*/
sh_chan->completed_cookie = sh_chan->common.cookie;

spin_unlock_bh(&sh_chan->desc_lock);

if (callback)
Expand All @@ -733,10 +758,6 @@ static void sh_dmae_chan_ld_cleanup(struct sh_dmae_chan *sh_chan, bool all)
{
while (__ld_cleanup(sh_chan, all))
;

if (all)
/* Terminating - forgive uncompleted cookies */
sh_chan->completed_cookie = sh_chan->common.cookie;
}

static void sh_chan_xfer_ld_queue(struct sh_dmae_chan *sh_chan)
Expand Down Expand Up @@ -782,8 +803,10 @@ static enum dma_status sh_dmae_tx_status(struct dma_chan *chan,

sh_dmae_chan_ld_cleanup(sh_chan, false);

last_used = chan->cookie;
/* First read completed cookie to avoid a skew */
last_complete = sh_chan->completed_cookie;
rmb();
last_used = chan->cookie;
BUG_ON(last_complete < 0);
dma_set_tx_state(txstate, last_complete, last_used, 0);

Expand Down Expand Up @@ -813,8 +836,12 @@ static enum dma_status sh_dmae_tx_status(struct dma_chan *chan,
static irqreturn_t sh_dmae_interrupt(int irq, void *data)
{
irqreturn_t ret = IRQ_NONE;
struct sh_dmae_chan *sh_chan = (struct sh_dmae_chan *)data;
u32 chcr = sh_dmae_readl(sh_chan, CHCR);
struct sh_dmae_chan *sh_chan = data;
u32 chcr;

spin_lock(&sh_chan->desc_lock);

chcr = sh_dmae_readl(sh_chan, CHCR);

if (chcr & CHCR_TE) {
/* DMA stop */
Expand All @@ -824,10 +851,13 @@ static irqreturn_t sh_dmae_interrupt(int irq, void *data)
tasklet_schedule(&sh_chan->tasklet);
}

spin_unlock(&sh_chan->desc_lock);

return ret;
}

static unsigned int sh_dmae_reset(struct sh_dmae_device *shdev)
/* Called from error IRQ or NMI */
static bool sh_dmae_reset(struct sh_dmae_device *shdev)
{
unsigned int handled = 0;
int i;
Expand All @@ -839,22 +869,32 @@ static unsigned int sh_dmae_reset(struct sh_dmae_device *shdev)
for (i = 0; i < SH_DMAC_MAX_CHANNELS; i++) {
struct sh_dmae_chan *sh_chan = shdev->chan[i];
struct sh_desc *desc;
LIST_HEAD(dl);

if (!sh_chan)
continue;

spin_lock(&sh_chan->desc_lock);

/* Stop the channel */
dmae_halt(sh_chan);

list_splice_init(&sh_chan->ld_queue, &dl);

spin_unlock(&sh_chan->desc_lock);

/* Complete all */
list_for_each_entry(desc, &sh_chan->ld_queue, node) {
list_for_each_entry(desc, &dl, node) {
struct dma_async_tx_descriptor *tx = &desc->async_tx;
desc->mark = DESC_IDLE;
if (tx->callback)
tx->callback(tx->callback_param);
}

list_splice_init(&sh_chan->ld_queue, &sh_chan->ld_free);
spin_lock(&sh_chan->desc_lock);
list_splice(&dl, &sh_chan->ld_free);
spin_unlock(&sh_chan->desc_lock);

handled++;
}

Expand All @@ -867,10 +907,11 @@ static irqreturn_t sh_dmae_err(int irq, void *data)
{
struct sh_dmae_device *shdev = data;

if (dmaor_read(shdev) & DMAOR_AE)
return IRQ_RETVAL(sh_dmae_reset(data));
else
if (!(dmaor_read(shdev) & DMAOR_AE))
return IRQ_NONE;

sh_dmae_reset(data);
return IRQ_HANDLED;
}

static void dmae_do_tasklet(unsigned long data)
Expand Down Expand Up @@ -902,17 +943,11 @@ static void dmae_do_tasklet(unsigned long data)

static bool sh_dmae_nmi_notify(struct sh_dmae_device *shdev)
{
unsigned int handled;

/* Fast path out if NMIF is not asserted for this controller */
if ((dmaor_read(shdev) & DMAOR_NMIF) == 0)
return false;

handled = sh_dmae_reset(shdev);
if (handled)
return true;

return false;
return sh_dmae_reset(shdev);
}

static int sh_dmae_nmi_handler(struct notifier_block *self,
Expand Down Expand Up @@ -982,9 +1017,6 @@ static int __devinit sh_dmae_chan_probe(struct sh_dmae_device *shdev, int id,
tasklet_init(&new_sh_chan->tasklet, dmae_do_tasklet,
(unsigned long)new_sh_chan);

/* Init the channel */
dmae_init(new_sh_chan);

spin_lock_init(&new_sh_chan->desc_lock);

/* Init descripter manage list */
Expand Down Expand Up @@ -1115,7 +1147,7 @@ static int __init sh_dmae_probe(struct platform_device *pdev)
list_add_tail_rcu(&shdev->node, &sh_dmae_devices);
spin_unlock_irqrestore(&sh_dmae_lock, flags);

/* reset dma controller */
/* reset dma controller - only needed as a test */
err = sh_dmae_rst(shdev);
if (err)
goto rst_err;
Expand Down

0 comments on commit 2dc6666

Please sign in to comment.