dmaengine: zynqmp_dma: lockdep warning

[Michael Tretter]

On Tue, 01 Jun 2021 15:01:08 +0200, Michael Tretter wrote:
> I get a lockdep warning in the zynqmp dma driver and I am not entirely sure
> how to fix it.
> 
> The code in drivers/dma/xilinx/zynqmp_dma.c looks as follows:
> 
> 604 static void zynqmp_dma_chan_desc_cleanup(struct zynqmp_dma_chan *chan)
> 605 {
> [...]
> 612	callback = desc->async_tx.callback;
> 613	callback_param = desc->async_tx.callback_param;
> 614	if (callback) {
> 615		spin_unlock(&chan->lock);
> 616		callback(callback_param);
> 617		spin_lock(&chan->lock);
> 618	}
> [...]
> 626 }
> [...]
> 747 static void zynqmp_dma_do_tasklet(struct tasklet_struct *t)
> 748 {
> [...]
> 753	spin_lock_irqsave(&chan->lock, irqflags);
> [...]
> 763	while (count) {
> 764		zynqmp_dma_complete_descriptor(chan);
> 765		zynqmp_dma_chan_desc_cleanup(chan);
> 766		count--;
> 767	}
> [...]
> 773	spin_unlock_irqrestore(&chan->lock, irqflags);
> 774 }
> 
> Lockdep reports that in line 617 spin_lock() is called from a non-hardirq
> context, while the same lock is used from a hardirq context. During runtime,
> the sequence is as follows:
> 
> line 753: acquire lock and disable interrupts
> line 615: release lock without enabling interrupts
> line 617: re-acquire lock with still disabled interrupts
> line 773: released lock and re-enable interrupts
> 
> Is this a false positive of lockdep, because it does not know that the irqs
> are still disabled in line 617? Is it actually OK to leave interrupts disabled
> over a spin_unlock() -> spin_lock() sequence or is this a problem?
> 
> Additionally, the lock is held for the entire tasklet that handles the
> finished dma transfer. This is conflict to the rule that spin locks should be
> held only for a short time. Is it necessary to hold the lock that long? I
> understand that the lock is only used to protect the descriptor lists and it
> would be better to only get the lock when descriptors are moved between lists.
> 
> Any guidance would be helpful.

Gentle ping.

Michael