On 04/03/2014 12:35 AM, Vinod Koul wrote:
On Mon, Mar 31, 2014 at 12:08:55PM +0800, Hongbo Zhang wrote:
On 03/29/2014 09:45 PM, Vinod Koul wrote:
On Fri, Mar 28, 2014 at 02:33:37PM +0800, Hongbo Zhang wrote:
On 03/26/2014 03:01 PM, Vinod Koul wrote:
On Thu, 2014-01-16 at 13:47 +0800, hongbo.zhang@xxxxxxxxxxxxx wrote:
From: Hongbo Zhang <hongbo.zhang@xxxxxxxxxxxxx>
The usage of spin_lock_irqsave() is a stronger locking mechanism than is
required throughout the driver. The minimum locking required should be used
instead. Interrupts will be turned off and context will be saved, it is
unnecessary to use irqsave.
This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). All
manipulation of protected fields is done using tasklet context or weaker, which
makes spin_lock_bh() the correct choice.
/**
@@ -1124,11 +1120,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
static void dma_do_tasklet(unsigned long data)
{
struct fsldma_chan *chan = (struct fsldma_chan *)data;
- unsigned long flags;
chan_dbg(chan, "tasklet entry\n");
- spin_lock_irqsave(&chan->desc_lock, flags);
+ spin_lock_bh(&chan->desc_lock);
okay here is the problem :(
You moved to _bh variant. So if you grab the lock in rest of the code
and irq gets triggered then here we will be spinning to grab the lock.
So effectively you made right locking solution into deadlock situation!
If the rest code grabs lock by spin_lock_bh(), and if irq raised,
the tasklet could not be executed because it has been disabled by
the _bh variant function.
yes if you are accessing resources only in tasklet and rest of the code, then
_bh variant works well. The problem here is usage in irq handler
The name dma_do_tasklet may mislead, it is tasklet handler, not irq
handler, not a trigger to load tasklet.
the irq handler is fsldma_chan_irq, and I don't use lock in it.
sorry my bad, i misread this as code in fsldma_chan_irq() insteadof
dma_do_tasklet(). In that case patch is doing the right thing.
OK, so I will send a v2 series with only updating 3/7 soon.
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
