[PATCH 06/13] DMAENGINE: driver for the ARM PL080/PL081 PrimeCells

[Linus Walleij]

2010/12/23 Dan Williams <dan.j.williams at intel.com>:
> On Wed, Dec 22, 2010 at 4:29 AM, Russell King - ARM Linux
> <linux at arm.linux.org.uk> wrote:
>> As described above in my code analysis, pl08x_tasklet takes the spinlock,
>> calls pl011_dma_tx_callback and eventually back to pl08x_prep_slave_sg
>> and pl08x_prep_channel_resources which then try to take the spinlock
>> again, leading to deadlock.
>
> This is listed in the dmaengine documentation [1], but I obviously
> missed this before merging.  This also would have been caught by
> lockdep as required by SubmitChecklist.

Yeah, my bad. I'll get better at this... :-(
(I blame it partially on inaccessible hardware, sob sob. I do like to
run lockdep.)

> It looks like this driver needs a full scrub
> which seems unreasonable to complete and test over the holidays before
> .37 lands.  Linus we either need to mark this "depends on BROKEN" or
> revert it.

Isn't it really as simple as to release the spinlock during callbacks?
That lock is only intended to protect the plchan variables, not to block
anyone from queueing new stuff during the callback (as happens now).

It can release that lock, make a callback where a new descriptor
gets queued, and then take it again and start looking at the queue,
at which point it discovers the new desc and process it.

So something like this:


From: Linus Walleij <linus.walleij at stericsson.com>
Date: Thu, 23 Dec 2010 09:06:14 +0100
Subject: [PATCH] dma: release pl08x channel lock during callback

The spinlock is not really safeguarding any resources during the
callback, so let's release it before that and take it back
afterwards so as to avoid deadlocks.

Signed-off-by: Linus Walleij <linus.walleij at stericsson.com>
---
 drivers/dma/amba-pl08x.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index b605cc9..7879a22 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1651,8 +1651,11 @@ static void pl08x_tasklet(unsigned long data)
 		/*
 		 * Callback to signal completion
 		 */
-		if (callback)
-			callback(callback_param);
+		if (callback) {
+                        spin_unlock(&plchan->lock);
+                        callback(callback_param);
+                        spin_lock(&plchan->lock);
+                }

 		/*
 		 * Device callbacks should NOT clear
-- 
1.7.2.3

Yours,
Linus Walleij