* [PATCH] dmaengine: pl330: Fix lock-order inversion in the channel release path
@ 2026-10-07 15:01 Ginger Li
2026-10-08 21:23 ` Frank Li
0 siblings, 1 reply; 4+ messages in thread
From: Ginger Li @ 2026-10-07 15:01 UTC (permalink / raw)
To: vkoul, Frank.Li; +Cc: dmaengine, linux-kernel
pl330 uses two locks per transfer: the channel lock pch->lock and the
controller lock pl330->lock. My static analyzer reported that they can
be taken concurrently in opposite orders, leading to potential deadlocks.
The terminate/stop paths take pch->lock first and then pl330->lock, e.g.
pl330_terminate_all()
-> spin_lock_irqsave(&pch->lock, flags);
-> spin_lock(&pl330->lock);
and
pl330_pause()
-> spin_lock_irqsave(&pch->lock, flags);
-> spin_lock(&pl330->lock);
While on the other hand, the channel release path takes pl330->lock first
and then pch->lock in pl330_free_chan_resources():
pl330_free_chan_resources()
-> spin_lock_irqsave(&pl330->lock, flags);
-> pl330_release_channel(pch->thread);
-> dma_pl330_rqcb()
-> spin_lock_irqsave(&pch->lock, flags);
Freeing a channel (dma_release_channel() -> dma_chan_put() ->
pl330_free_chan_resources()) while another CPU is in pl330_terminate_all()
or pl330_pause() on the same channel is therefore an ABBA deadlock: the
one thread spins on pl330->lock that the other holds, and vice versa.
Inspecting the driver code suggests the rule that dma_pl330_rqcb() must
not be called with pl330->lock held: pl330_dotask() and the callback
drain loop in pl330_update() both releases pl330->lock before their
dma_pl330_rqcb() calls. Thus, pl330_release_channel() is expected
to follow the same manner.
Let pl330_release_channel() manage pl330->lock itself and keep the two
dma_pl330_rqcb() calls outside the critical section, and stop holding
pl330->lock around the call in pl330_free_chan_resources().
This was found by a static analyzer on Linux 7.3-rc4; it reported
DeadLock::AllLock (Certain) for
pl330_free_chan_resources() <-> pl330_terminate_all()
pl330_free_chan_resources() <-> pl330_pause().
Fixes: 91539eb1fda2 ("dmaengine: pl330: fix double lock")
Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
---
drivers/dma/pl330.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1805,16 +1805,32 @@
static void pl330_release_channel(struct pl330_thread *thrd)
{
+ struct pl330_dmac *pl330;
+ unsigned long flags;
+
if (!thrd || thrd->free)
return;
+ pl330 = thrd->dmac;
+
+ spin_lock_irqsave(&pl330->lock, flags);
_stop(thrd);
+ spin_unlock_irqrestore(&pl330->lock, flags);
+ /*
+ * dma_pl330_rqcb() takes the channel lock, which is acquired before
+ * pl330->lock on the terminate/pause/tx_status paths. Calling it with
+ * pl330->lock held would invert the lock order, so keep it outside the
+ * critical section - the same convention pl330_dotask() and
+ * pl330_update() already follow.
+ */
dma_pl330_rqcb(thrd->req[1 - thrd->lstenq].desc, PL330_ERR_ABORT);
dma_pl330_rqcb(thrd->req[thrd->lstenq].desc, PL330_ERR_ABORT);
+ spin_lock_irqsave(&pl330->lock, flags);
_free_event(thrd, thrd->ev);
thrd->free = true;
+ spin_unlock_irqrestore(&pl330->lock, flags);
}
/* Initialize the structure for PL330 configuration, that can be used
@@ -2358,9 +2374,15 @@
tasklet_kill(&pch->task);
pm_runtime_get_sync(pch->dmac->ddma.dev);
- spin_lock_irqsave(&pl330->lock, flags);
+ /*
+ * pl330_release_channel() takes pl330->lock itself and calls
+ * dma_pl330_rqcb(), which takes the channel lock. It must therefore
+ * not be called with pl330->lock held (see the comment there).
+ */
pl330_release_channel(pch->thread);
+
+ spin_lock_irqsave(&pl330->lock, flags);
pch->thread = NULL;
if (pch->cyclic)
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: pl330: Fix lock-order inversion in the channel release path
2026-10-07 15:01 [PATCH] dmaengine: pl330: Fix lock-order inversion in the channel release path Ginger Li
@ 2026-10-08 21:23 ` Frank Li
2026-10-12 3:15 ` Ginger
0 siblings, 1 reply; 4+ messages in thread
From: Frank Li @ 2026-10-08 21:23 UTC (permalink / raw)
To: Ginger Li; +Cc: vkoul, Frank.Li, dmaengine, linux-kernel
On Wed, Oct 07, 2026 at 11:01:04PM +0800, Ginger Li wrote:
> [You don't often get email from ginger.jzllee@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> pl330 uses two locks per transfer: the channel lock pch->lock and the
> controller lock pl330->lock. My static analyzer reported that they can
The static analyzer ....
> be taken concurrently in opposite orders, leading to potential deadlocks.
>
> The terminate/stop paths take pch->lock first and then pl330->lock, e.g.
> pl330_terminate_all()
> -> spin_lock_irqsave(&pch->lock, flags);
> -> spin_lock(&pl330->lock);
> and
> pl330_pause()
> -> spin_lock_irqsave(&pch->lock, flags);
> -> spin_lock(&pl330->lock);
>
> While on the other hand, the channel release path takes pl330->lock first
> and then pch->lock in pl330_free_chan_resources():
> pl330_free_chan_resources()
> -> spin_lock_irqsave(&pl330->lock, flags);
> -> pl330_release_channel(pch->thread);
> -> dma_pl330_rqcb()
> -> spin_lock_irqsave(&pch->lock, flags);
>
> Freeing a channel (dma_release_channel() -> dma_chan_put() ->
> pl330_free_chan_resources()) while another CPU is in pl330_terminate_all()
> or pl330_pause() on the same channel is therefore an ABBA deadlock: the
> one thread spins on pl330->lock that the other holds, and vice versa.
To here is enough. cut below message.
>
> Inspecting the driver code suggests the rule that dma_pl330_rqcb() must
> not be called with pl330->lock held: pl330_dotask() and the callback
> drain loop in pl330_update() both releases pl330->lock before their
> dma_pl330_rqcb() calls. Thus, pl330_release_channel() is expected
> to follow the same manner.
>
> Let pl330_release_channel() manage pl330->lock itself and keep the two
> dma_pl330_rqcb() calls outside the critical section, and stop holding
> pl330->lock around the call in pl330_free_chan_resources().
>
> This was found by a static analyzer on Linux 7.3-rc4; it reported
> DeadLock::AllLock (Certain) for
>
> pl330_free_chan_resources() <-> pl330_terminate_all()
> pl330_free_chan_resources() <-> pl330_pause().
>
> Fixes: 91539eb1fda2 ("dmaengine: pl330: fix double lock")
> Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
> ---
> drivers/dma/pl330.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -1805,16 +1805,32 @@
>
> static void pl330_release_channel(struct pl330_thread *thrd)
> {
> + struct pl330_dmac *pl330;
> + unsigned long flags;
> +
> if (!thrd || thrd->free)
> return;
>
> + pl330 = thrd->dmac;
> +
> + spin_lock_irqsave(&pl330->lock, flags);
> _stop(thrd);
> + spin_unlock_irqrestore(&pl330->lock, flags);
[1]
>
> + /*
> + * dma_pl330_rqcb() takes the channel lock, which is acquired before
> + * pl330->lock on the terminate/pause/tx_status paths. Calling it with
> + * pl330->lock held would invert the lock order, so keep it outside the
> + * critical section
> + * pl330_update() already follow.
> + */
> dma_pl330_rqcb(thrd->req[1 - thrd->lstenq].desc, PL330_ERR_ABORT);
> dma_pl330_rqcb(thrd->req[thrd->lstenq].desc, PL330_ERR_ABORT);
[2]
>
> + spin_lock_irqsave(&pl330->lock, flags);
> _free_event(thrd, thrd->ev);
> thrd->free = true;
> + spin_unlock_irqrestore(&pl330->lock, flags);
[3],
how do you make sure it is safe to do 1, 2 and 3 without pl330->lock?
Frank
> }
>
> /* Initialize the structure for PL330 configuration, that can be used
> @@ -2358,9 +2374,15 @@
> tasklet_kill(&pch->task);
>
> pm_runtime_get_sync(pch->dmac->ddma.dev);
> - spin_lock_irqsave(&pl330->lock, flags);
>
> + /*
> + * pl330_release_channel() takes pl330->lock itself and calls
> + * dma_pl330_rqcb(), which takes the channel lock. It must therefore
> + * not be called with pl330->lock held (see the comment there).
> + */
> pl330_release_channel(pch->thread);
> +
> + spin_lock_irqsave(&pl330->lock, flags);
> pch->thread = NULL;
>
> if (pch->cyclic)
> --
> 2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: pl330: Fix lock-order inversion in the channel release path
2026-10-08 21:23 ` Frank Li
@ 2026-10-12 3:15 ` Ginger
2026-10-12 17:00 ` Frank Li
0 siblings, 1 reply; 4+ messages in thread
From: Ginger @ 2026-10-12 3:15 UTC (permalink / raw)
To: Frank Li; +Cc: vkoul, Frank.Li, dmaengine, linux-kernel
On Fri, Oct 9, 2026 at 5:23 AM Frank Li <Frank.li@oss.nxp.com> wrote:
>
> On Wed, Oct 07, 2026 at 11:01:04PM +0800, Ginger Li wrote:
> > [You don't often get email from ginger.jzllee@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > pl330 uses two locks per transfer: the channel lock pch->lock and the
> > controller lock pl330->lock. My static analyzer reported that they can
>
> The static analyzer ....
>
> > be taken concurrently in opposite orders, leading to potential deadlocks.
> >
> > The terminate/stop paths take pch->lock first and then pl330->lock, e.g.
> > pl330_terminate_all()
> > -> spin_lock_irqsave(&pch->lock, flags);
> > -> spin_lock(&pl330->lock);
> > and
> > pl330_pause()
> > -> spin_lock_irqsave(&pch->lock, flags);
> > -> spin_lock(&pl330->lock);
> >
> > While on the other hand, the channel release path takes pl330->lock first
> > and then pch->lock in pl330_free_chan_resources():
> > pl330_free_chan_resources()
> > -> spin_lock_irqsave(&pl330->lock, flags);
> > -> pl330_release_channel(pch->thread);
> > -> dma_pl330_rqcb()
> > -> spin_lock_irqsave(&pch->lock, flags);
> >
> > Freeing a channel (dma_release_channel() -> dma_chan_put() ->
> > pl330_free_chan_resources()) while another CPU is in pl330_terminate_all()
> > or pl330_pause() on the same channel is therefore an ABBA deadlock: the
> > one thread spins on pl330->lock that the other holds, and vice versa.
>
> To here is enough. cut below message.
>
> >
> > Inspecting the driver code suggests the rule that dma_pl330_rqcb() must
> > not be called with pl330->lock held: pl330_dotask() and the callback
> > drain loop in pl330_update() both releases pl330->lock before their
> > dma_pl330_rqcb() calls. Thus, pl330_release_channel() is expected
> > to follow the same manner.
> >
> > Let pl330_release_channel() manage pl330->lock itself and keep the two
> > dma_pl330_rqcb() calls outside the critical section, and stop holding
> > pl330->lock around the call in pl330_free_chan_resources().
> >
> > This was found by a static analyzer on Linux 7.3-rc4; it reported
> > DeadLock::AllLock (Certain) for
> >
> > pl330_free_chan_resources() <-> pl330_terminate_all()
> > pl330_free_chan_resources() <-> pl330_pause().
> >
> > Fixes: 91539eb1fda2 ("dmaengine: pl330: fix double lock")
> > Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
> > ---
> > drivers/dma/pl330.c | 24 +++++++++++++++++++++++-
> > 1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> > --- a/drivers/dma/pl330.c
> > +++ b/drivers/dma/pl330.c
> > @@ -1805,16 +1805,32 @@
> >
> > static void pl330_release_channel(struct pl330_thread *thrd)
> > {
> > + struct pl330_dmac *pl330;
> > + unsigned long flags;
> > +
> > if (!thrd || thrd->free)
> > return;
> >
> > + pl330 = thrd->dmac;
> > +
> > + spin_lock_irqsave(&pl330->lock, flags);
> > _stop(thrd);
> > + spin_unlock_irqrestore(&pl330->lock, flags);
>
> [1]
> >
> > + /*
> > + * dma_pl330_rqcb() takes the channel lock, which is acquired before
> > + * pl330->lock on the terminate/pause/tx_status paths. Calling it with
> > + * pl330->lock held would invert the lock order, so keep it outside the
> > + * critical section
> > + * pl330_update() already follow.
> > + */
> > dma_pl330_rqcb(thrd->req[1 - thrd->lstenq].desc, PL330_ERR_ABORT);
> > dma_pl330_rqcb(thrd->req[thrd->lstenq].desc, PL330_ERR_ABORT);
> [2]
> >
> > + spin_lock_irqsave(&pl330->lock, flags);
> > _free_event(thrd, thrd->ev);
> > thrd->free = true;
> > + spin_unlock_irqrestore(&pl330->lock, flags);
>
> [3],
>
> how do you make sure it is safe to do 1, 2 and 3 without pl330->lock?
>
It seems that [2] is not strictly needed inside pl330->lock (e.g., pl330_dotask
releases the lock before calling dma_pl330_rqcb()). However, if [1] and [3] are
expected to run atomically, would the following change do? This fix keeps the
original structure but moves the dma_pl330_rqcb() calls outside the critical
section of pl330->lock.
Sincerely,
Ginger
---
drivers/dma/pl330.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1803,15 +1803,33 @@ static inline void _free_event(struct
pl330_thread *thrd, int ev)
pl330->events[ev] = -1;
}
-static void pl330_release_channel(struct pl330_thread *thrd)
+static void pl330_release_channel(struct pl330_thread *thrd,
+ struct dma_pl330_desc **done_a,
+ struct dma_pl330_desc **done_b)
{
+ *done_a = NULL;
+ *done_b = NULL;
+
if (!thrd || thrd->free)
return;
_stop(thrd);
- dma_pl330_rqcb(thrd->req[1 - thrd->lstenq].desc, PL330_ERR_ABORT);
- dma_pl330_rqcb(thrd->req[thrd->lstenq].desc, PL330_ERR_ABORT);
+ *done_a = thrd->req[1 - thrd->lstenq].desc;
+ *done_b = thrd->req[thrd->lstenq].desc;
_free_event(thrd, thrd->ev);
thrd->free = true;
@@ -1993,12 +2011,15 @@ static int pl330_add(struct pl330_dmac *pl330)
static int dmac_free_threads(struct pl330_dmac *pl330)
{
struct pl330_thread *thrd;
+ struct dma_pl330_desc *done_a, *done_b;
int i;
/* Release Channel threads */
for (i = 0; i < pl330->pcfg.num_chan; i++) {
thrd = &pl330->channels[i];
- pl330_release_channel(thrd);
+ pl330_release_channel(thrd, &done_a, &done_b);
+ dma_pl330_rqcb(done_a, PL330_ERR_ABORT);
+ dma_pl330_rqcb(done_b, PL330_ERR_ABORT);
}
/* Free memory */
@@ -2353,6 +2374,7 @@ static void pl330_free_chan_resources(struct
dma_chan *chan)
{
struct dma_pl330_chan *pch = to_pchan(chan);
struct pl330_dmac *pl330 = pch->dmac;
+ struct dma_pl330_desc *done_a, *done_b;
unsigned long flags;
tasklet_kill(&pch->task);
@@ -2360,13 +2382,17 @@ static void pl330_free_chan_resources(struct
dma_chan *chan)
pm_runtime_get_sync(pch->dmac->ddma.dev);
spin_lock_irqsave(&pl330->lock, flags);
- pl330_release_channel(pch->thread);
+ /* The two callbacks take pch->lock, so they run after the unlock below. */
+ pl330_release_channel(pch->thread, &done_a, &done_b);
pch->thread = NULL;
if (pch->cyclic)
list_splice_tail_init(&pch->work_list, &pch->dmac->desc_pool);
spin_unlock_irqrestore(&pl330->lock, flags);
+
+ dma_pl330_rqcb(done_a, PL330_ERR_ABORT);
+ dma_pl330_rqcb(done_b, PL330_ERR_ABORT);
pm_runtime_put_autosuspend(pch->dmac->ddma.dev);
pl330_unprep_slave_fifo(pch);
}
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: pl330: Fix lock-order inversion in the channel release path
2026-10-12 3:15 ` Ginger
@ 2026-10-12 17:00 ` Frank Li
0 siblings, 0 replies; 4+ messages in thread
From: Frank Li @ 2026-10-12 17:00 UTC (permalink / raw)
To: Ginger; +Cc: vkoul, Frank.Li, dmaengine, linux-kernel
On Mon, Oct 12, 2026 at 11:15:16AM +0800, Ginger wrote:
> On Fri, Oct 9, 2026 at 5:23 AM Frank Li <Frank.li@oss.nxp.com> wrote:
> >
> > On Wed, Oct 07, 2026 at 11:01:04PM +0800, Ginger Li wrote:
> > > [You don't often get email from ginger.jzllee@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > >
> > > pl330 uses two locks per transfer: the channel lock pch->lock and the
> > > controller lock pl330->lock. My static analyzer reported that they can
> >
> > The static analyzer ....
> >
> > > be taken concurrently in opposite orders, leading to potential deadlocks.
> > >
> > > The terminate/stop paths take pch->lock first and then pl330->lock, e.g.
> > > pl330_terminate_all()
> > > -> spin_lock_irqsave(&pch->lock, flags);
> > > -> spin_lock(&pl330->lock);
> > > and
> > > pl330_pause()
> > > -> spin_lock_irqsave(&pch->lock, flags);
> > > -> spin_lock(&pl330->lock);
> > >
> > > While on the other hand, the channel release path takes pl330->lock first
> > > and then pch->lock in pl330_free_chan_resources():
> > > pl330_free_chan_resources()
> > > -> spin_lock_irqsave(&pl330->lock, flags);
> > > -> pl330_release_channel(pch->thread);
> > > -> dma_pl330_rqcb()
> > > -> spin_lock_irqsave(&pch->lock, flags);
> > >
> > > Freeing a channel (dma_release_channel() -> dma_chan_put() ->
> > > pl330_free_chan_resources()) while another CPU is in pl330_terminate_all()
> > > or pl330_pause() on the same channel is therefore an ABBA deadlock: the
> > > one thread spins on pl330->lock that the other holds, and vice versa.
> >
> > To here is enough. cut below message.
> >
> > >
> > > Inspecting the driver code suggests the rule that dma_pl330_rqcb() must
> > > not be called with pl330->lock held: pl330_dotask() and the callback
> > > drain loop in pl330_update() both releases pl330->lock before their
> > > dma_pl330_rqcb() calls. Thus, pl330_release_channel() is expected
> > > to follow the same manner.
> > >
> > > Let pl330_release_channel() manage pl330->lock itself and keep the two
> > > dma_pl330_rqcb() calls outside the critical section, and stop holding
> > > pl330->lock around the call in pl330_free_chan_resources().
> > >
> > > This was found by a static analyzer on Linux 7.3-rc4; it reported
> > > DeadLock::AllLock (Certain) for
> > >
> > > pl330_free_chan_resources() <-> pl330_terminate_all()
> > > pl330_free_chan_resources() <-> pl330_pause().
> > >
> > > Fixes: 91539eb1fda2 ("dmaengine: pl330: fix double lock")
> > > Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
> > > ---
> > > drivers/dma/pl330.c | 24 +++++++++++++++++++++++-
> > > 1 file changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> > > --- a/drivers/dma/pl330.c
> > > +++ b/drivers/dma/pl330.c
> > > @@ -1805,16 +1805,32 @@
> > >
> > > static void pl330_release_channel(struct pl330_thread *thrd)
> > > {
> > > + struct pl330_dmac *pl330;
> > > + unsigned long flags;
> > > +
> > > if (!thrd || thrd->free)
> > > return;
> > >
> > > + pl330 = thrd->dmac;
> > > +
> > > + spin_lock_irqsave(&pl330->lock, flags);
> > > _stop(thrd);
> > > + spin_unlock_irqrestore(&pl330->lock, flags);
> >
> > [1]
> > >
> > > + /*
> > > + * dma_pl330_rqcb() takes the channel lock, which is acquired before
> > > + * pl330->lock on the terminate/pause/tx_status paths. Calling it with
> > > + * pl330->lock held would invert the lock order, so keep it outside the
> > > + * critical section
> > > + * pl330_update() already follow.
> > > + */
> > > dma_pl330_rqcb(thrd->req[1 - thrd->lstenq].desc, PL330_ERR_ABORT);
> > > dma_pl330_rqcb(thrd->req[thrd->lstenq].desc, PL330_ERR_ABORT);
> > [2]
> > >
> > > + spin_lock_irqsave(&pl330->lock, flags);
> > > _free_event(thrd, thrd->ev);
> > > thrd->free = true;
> > > + spin_unlock_irqrestore(&pl330->lock, flags);
> >
> > [3],
> >
> > how do you make sure it is safe to do 1, 2 and 3 without pl330->lock?
> >
>
> It seems that [2] is not strictly needed inside pl330->lock (e.g., pl330_dotask
> releases the lock before calling dma_pl330_rqcb()). However, if [1] and [3] are
> expected to run atomically, would the following change do? This fix keeps the
> original structure but moves the dma_pl330_rqcb() calls outside the critical
> section of pl330->lock.
I am not familar pl330 drivers. The key point here is that avoid introduce
new problem by resolve one exitings problems.
Frank
>
> Sincerely,
> Ginger
>
> ---
> drivers/dma/pl330.c | 36 +++++++++++++++++++++++++++++++-----
> 1 file changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -1803,15 +1803,33 @@ static inline void _free_event(struct
> pl330_thread *thrd, int ev)
> pl330->events[ev] = -1;
> }
>
> -static void pl330_release_channel(struct pl330_thread *thrd)
> +static void pl330_release_channel(struct pl330_thread *thrd,
> + struct dma_pl330_desc **done_a,
> + struct dma_pl330_desc **done_b)
> {
> + *done_a = NULL;
> + *done_b = NULL;
> +
> if (!thrd || thrd->free)
> return;
>
> _stop(thrd);
>
> - dma_pl330_rqcb(thrd->req[1 - thrd->lstenq].desc, PL330_ERR_ABORT);
> - dma_pl330_rqcb(thrd->req[thrd->lstenq].desc, PL330_ERR_ABORT);
> + *done_a = thrd->req[1 - thrd->lstenq].desc;
> + *done_b = thrd->req[thrd->lstenq].desc;
>
> _free_event(thrd, thrd->ev);
> thrd->free = true;
> @@ -1993,12 +2011,15 @@ static int pl330_add(struct pl330_dmac *pl330)
> static int dmac_free_threads(struct pl330_dmac *pl330)
> {
> struct pl330_thread *thrd;
> + struct dma_pl330_desc *done_a, *done_b;
> int i;
>
> /* Release Channel threads */
> for (i = 0; i < pl330->pcfg.num_chan; i++) {
> thrd = &pl330->channels[i];
> - pl330_release_channel(thrd);
> + pl330_release_channel(thrd, &done_a, &done_b);
> + dma_pl330_rqcb(done_a, PL330_ERR_ABORT);
> + dma_pl330_rqcb(done_b, PL330_ERR_ABORT);
> }
>
> /* Free memory */
> @@ -2353,6 +2374,7 @@ static void pl330_free_chan_resources(struct
> dma_chan *chan)
> {
> struct dma_pl330_chan *pch = to_pchan(chan);
> struct pl330_dmac *pl330 = pch->dmac;
> + struct dma_pl330_desc *done_a, *done_b;
> unsigned long flags;
>
> tasklet_kill(&pch->task);
> @@ -2360,13 +2382,17 @@ static void pl330_free_chan_resources(struct
> dma_chan *chan)
> pm_runtime_get_sync(pch->dmac->ddma.dev);
> spin_lock_irqsave(&pl330->lock, flags);
>
> - pl330_release_channel(pch->thread);
> + /* The two callbacks take pch->lock, so they run after the unlock below. */
> + pl330_release_channel(pch->thread, &done_a, &done_b);
> pch->thread = NULL;
>
> if (pch->cyclic)
> list_splice_tail_init(&pch->work_list, &pch->dmac->desc_pool);
>
> spin_unlock_irqrestore(&pl330->lock, flags);
> +
> + dma_pl330_rqcb(done_a, PL330_ERR_ABORT);
> + dma_pl330_rqcb(done_b, PL330_ERR_ABORT);
> pm_runtime_put_autosuspend(pch->dmac->ddma.dev);
> pl330_unprep_slave_fifo(pch);
> }
> --
> 2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-12 17:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07 15:01 [PATCH] dmaengine: pl330: Fix lock-order inversion in the channel release path Ginger Li
2026-10-08 21:23 ` Frank Li
2026-10-12 3:15 ` Ginger
2026-10-12 17:00 ` Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®