Re: [PATCH] dmaengine: plx_dma: Fix potential deadlock on &plxdev->ring_lock

From: Chengfeng Ye
Date: Thu Jul 27 2023 - 10:35:17 EST


Hi Eric,

> W/ special emphasis on commit edf10919e5fc ("dmaengine: altera: fix
> spinlock usage")
> spin_lock_bh was changed to spin_lock_irqsave w/ this patch.

Yeah, that patch is to fix the deadlock between irq and kernel thread
thus use spin_lock_irqsave(), exactly the kind of bug that I am reporting
about. But slight difference is the deadlock on that commit concerns
hardirq and thus fixed by spin_lock_irqsave(), but this one concerns softirq.

> For uniformity reason across drivers and also that not something else
> gets missed please compare your requirements and solution to the
> implementation of the "altera-msgdma" driver (altera-msgdma.c).

Maybe for our case spin_lock_bh() seems to be enough since we are handling
deadlock between tasklet and kernel thread. It's truth that uniformity
is an important concern, also as I glance at other DMA driver, in general
spin_lock_irqsave() is more frequently used than spin_lock_bh(). But this driver
already consistently uses spin_lock_bh() on &plxdev->ring_lock, only change to
spin_lock_irqsave() at this function could instead make the code seems
a bit weird.

Even though my original patch did use spin_lock_irqsave(), now I think at least
for this bug fix just revert back to spin_lock_bh() may be better and
will not complicate
things much.

Thanks,
Chengfeng