[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAo+4rXJkWsTtsjDCY-b7bfqhMms1HSRQiXVHL2WfXNMLD8uSg@mail.gmail.com>
Date: Thu, 27 Jul 2023 22:34:55 +0800
From: Chengfeng Ye <dg573847474@...il.com>
To: Eric Schwarz <eas@...optimization.com>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
vkoul@...nel.org, Yunbo Yu <yuyunbo519@...il.com>,
Logan Gunthorpe <logang@...tatee.com>
Cc: dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dmaengine: plx_dma: Fix potential deadlock on &plxdev->ring_lock
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
Powered by blists - more mailing lists