* [PATCH v2 1/4] dmaengine: dw: Prevent tx-status calling DMA-desc callback
2026-10-22 18:52 [PATCH v2 0/4] dmaengine: dw: serialize completion and termination James Hilliard
@ 2026-10-22 18:52 ` James Hilliard
2026-10-22 18:52 ` [PATCH v2 2/4] dmaengine: dw: Fix XFER bit set, but channel not idle error James Hilliard
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: James Hilliard @ 2026-10-22 18:52 UTC (permalink / raw)
To: Viresh Kumar, Andy Shevchenko, Vinod Koul, Frank Li
Cc: Serge Semin, Greg Kroah-Hartman, Jiri Slaby, Ilpo Järvinen,
dmaengine, linux-serial, linux-kernel, Sashiko, James Hilliard,
stable
From: Serge Semin <fancer.lancer@gmail.com>
The dmaengine_tx_status() method implemented in the DW DMAC driver is
responsible for not just DMA-transfer status getting, but may also cause
the transfer finalization with the Tx-descriptors callback invocation.
This makes the simple DMA-transfer status getting being much more complex
than it seems with a wider room for possible bugs.
In particular a deadlock has been discovered in the DW 8250 UART device
driver interacting with the DW DMA controller channels. Here is the
call-trace causing the deadlock:
serial8250_handle_irq()
uart_port_lock_irqsave(port); ----------------------+
handle_rx_dma() |
serial8250_rx_dma_flush() |
__dma_rx_complete() |
dmaengine_tx_status() |
dwc_scan_descriptors() |
dwc_complete_all() |
dwc_descriptor_complete() |
dmaengine_desc_callback_invoke() |
cb->callback(cb->callback_param); |
|| |
dma_rx_complete(); |
uart_port_lock_irqsave(port); ----+ <- Deadlock!
So in case if the DMA-engine finished working at some point before the
serial8250_rx_dma_flush() invocation and the respective tasklet hasn't
been executed yet to finalize the DMA transfer, then calling the
dmaengine_tx_status() will cause the DMA-descriptors status update and the
Tx-descriptor callback invocation.
Generalizing the case up: if the dmaengine_tx_status() method callee and
the Tx-descriptor callback refer to the related critical section, then
calling dmaengine_tx_status() from the Tx-descriptor callback will
inevitably cause a deadlock around the guarding lock as it happens in the
Serial 8250 DMA implementation above. (Note the deadlock doesn't happen
very often, but can be eventually discovered if the being received data
size is greater than the Rx DMA-buffer size defined in the 8250_dma.c
driver. In my case reducing the Rx DMA-buffer size increased the deadlock
probability.)
Alas there is no obvious way to prevent the deadlock by fixing the
8250-port drivers because the UART-port lock must be held for the entire
port IRQ handling procedure. Thus the best way to fix the discovered
problem (and prevent similar ones in the drivers using the DW DMAC device
channels) is to simplify the DMA-transfer status getter by removing the
Tx-descriptors state update from there and making the function to serve
just one purpose - calculate the DMA-transfer residue and return the
transfer status. The DMA-transfer status update will be performed in the
bottom-half procedure only.
Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Cc: stable@vger.kernel.org
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changelog RFC:
- Instead of just dropping the dwc_scan_descriptors() method invocation
calculate the residue in the Tx-status getter.
---
drivers/dma/dw/core.c | 90 ++++++++++++++++++++++++++++-----------------------
1 file changed, 50 insertions(+), 40 deletions(-)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index dd75f97a33b3..af1871646eb9 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -39,6 +39,8 @@
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES)
+static u32 dwc_get_hard_llp_desc_residue(struct dw_dma_chan *dwc, struct dw_desc *desc);
+
/*----------------------------------------------------------------------*/
static struct device *chan2dev(struct dma_chan *chan)
@@ -297,14 +299,12 @@ static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
{
- dma_addr_t llp;
struct dw_desc *desc, *_desc;
struct dw_desc *child;
u32 status_xfer;
unsigned long flags;
spin_lock_irqsave(&dwc->lock, flags);
- llp = channel_readl(dwc, LLP);
status_xfer = dma_readl(dw, RAW.XFER);
if (status_xfer & dwc->mask) {
@@ -358,41 +358,16 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
return;
}
- dev_vdbg(chan2dev(&dwc->chan), "%s: llp=%pad\n", __func__, &llp);
+ dev_vdbg(chan2dev(&dwc->chan), "%s: hard LLP mode\n", __func__);
list_for_each_entry_safe(desc, _desc, &dwc->active_list, desc_node) {
- /* Initial residue value */
- desc->residue = desc->total_len;
-
- /* Check first descriptors addr */
- if (desc->txd.phys == DWC_LLP_LOC(llp)) {
- spin_unlock_irqrestore(&dwc->lock, flags);
- return;
- }
-
- /* Check first descriptors llp */
- if (lli_read(desc, llp) == llp) {
- /* This one is currently in progress */
- desc->residue -= dwc_get_sent(dwc);
+ desc->residue = dwc_get_hard_llp_desc_residue(dwc, desc);
+ if (desc->residue) {
spin_unlock_irqrestore(&dwc->lock, flags);
return;
}
- desc->residue -= desc->len;
- list_for_each_entry(child, &desc->tx_list, desc_node) {
- if (lli_read(child, llp) == llp) {
- /* Currently in progress */
- desc->residue -= dwc_get_sent(dwc);
- spin_unlock_irqrestore(&dwc->lock, flags);
- return;
- }
- desc->residue -= child->len;
- }
-
- /*
- * No descriptors so far seem to be in progress, i.e.
- * this one must be done.
- */
+ /* No data left to be send. Finalize the transfer then */
spin_unlock_irqrestore(&dwc->lock, flags);
dwc_descriptor_complete(dwc, desc, true);
spin_lock_irqsave(&dwc->lock, flags);
@@ -976,6 +951,45 @@ static struct dw_desc *dwc_find_desc(struct dw_dma_chan *dwc, dma_cookie_t c)
return NULL;
}
+static u32 dwc_get_soft_llp_desc_residue(struct dw_dma_chan *dwc, struct dw_desc *desc)
+{
+ u32 residue = desc->residue;
+
+ if (residue)
+ residue -= dwc_get_sent(dwc);
+
+ return residue;
+}
+
+static u32 dwc_get_hard_llp_desc_residue(struct dw_dma_chan *dwc, struct dw_desc *desc)
+{
+ u32 residue = desc->total_len;
+ struct dw_desc *child;
+ dma_addr_t llp;
+
+ llp = channel_readl(dwc, LLP);
+
+ /* Check first descriptor for been pending to be fetched by DMAC */
+ if (desc->txd.phys == DWC_LLP_LOC(llp))
+ return residue;
+
+ /* Check first descriptor LLP to see if it's currently in-progress */
+ if (lli_read(desc, llp) == llp)
+ return residue - dwc_get_sent(dwc);
+
+ /* Check subordinate LLPs to find the currently in-progress desc */
+ residue -= desc->len;
+ list_for_each_entry(child, &desc->tx_list, desc_node) {
+ if (lli_read(child, llp) == llp)
+ return residue - dwc_get_sent(dwc);
+
+ residue -= child->len;
+ }
+
+ /* Shall return zero if no in-progress desc found */
+ return residue;
+}
+
static u32 dwc_get_residue_and_status(struct dw_dma_chan *dwc, dma_cookie_t cookie,
enum dma_status *status)
{
@@ -988,9 +1002,11 @@ static u32 dwc_get_residue_and_status(struct dw_dma_chan *dwc, dma_cookie_t cook
desc = dwc_find_desc(dwc, cookie);
if (desc) {
if (desc == dwc_first_active(dwc)) {
- residue = desc->residue;
- if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags) && residue)
- residue -= dwc_get_sent(dwc);
+ if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags))
+ residue = dwc_get_soft_llp_desc_residue(dwc, desc);
+ else
+ residue = dwc_get_hard_llp_desc_residue(dwc, desc);
+
if (test_bit(DW_DMA_IS_PAUSED, &dwc->flags))
*status = DMA_PAUSED;
} else {
@@ -1012,12 +1028,6 @@ dwc_tx_status(struct dma_chan *chan,
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
enum dma_status ret;
- ret = dma_cookie_status(chan, cookie, txstate);
- if (ret == DMA_COMPLETE)
- return ret;
-
- dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
-
ret = dma_cookie_status(chan, cookie, txstate);
if (ret == DMA_COMPLETE)
return ret;
--
2.53.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 2/4] dmaengine: dw: Fix XFER bit set, but channel not idle error
2026-10-22 18:52 [PATCH v2 0/4] dmaengine: dw: serialize completion and termination James Hilliard
2026-10-22 18:52 ` [PATCH v2 1/4] dmaengine: dw: Prevent tx-status calling DMA-desc callback James Hilliard
@ 2026-10-22 18:52 ` James Hilliard
2026-10-22 18:52 ` [PATCH v2 3/4] dmaengine: dw: serialize error recovery with channel termination James Hilliard
2026-10-22 18:52 ` [PATCH v2 4/4] dmaengine: dw: synchronize completion callbacks James Hilliard
3 siblings, 0 replies; 5+ messages in thread
From: James Hilliard @ 2026-10-22 18:52 UTC (permalink / raw)
To: Viresh Kumar, Andy Shevchenko, Vinod Koul, Frank Li
Cc: Serge Semin, Greg Kroah-Hartman, Jiri Slaby, Ilpo Järvinen,
dmaengine, linux-serial, linux-kernel, Sashiko, James Hilliard,
stable
From: Serge Semin <fancer.lancer@gmail.com>
If a client driver gets to use the DW DMAC engine device tougher
than usual, with occasional DMA-transfers termination and restart, then
the next error can be randomly spotted in the system log:
> dma dma0chan0: BUG: XFER bit set, but channel not idle!
For instance that happens in case of the 8250 UART port driver handling
the looped back high-speed traffic (in my case > 1.5Mbaud) by means of the
DMA-engine interface.
The error happens due to the two-staged nature of the DW DMAC IRQs
handling procedure and due to the critical section break in the meantime.
In particular in case if the DMA-transfer is terminated and restarted:
1. after the IRQ-handler submitted the tasklet but before the tasklet
started handling the DMA-descriptors in dwc_scan_descriptors();
2. after the XFER completion flag was detected in the
dwc_scan_descriptors() method, but before the dwc_complete_all() method
is called
the error denoted above is printed due to the overlap of the last transfer
completion and the new transfer execution stages.
There are two places need to be altered in order to fix the problem.
1. Clear the IRQs in the dwc_chan_disable() method. That will prevent the
dwc_scan_descriptors() method call in case if the DMA-transfer is
restarted in the middle of the two-staged IRQs-handling procedure.
2. Move the dwc_complete_all() code to being executed inseparably (in the
same atomic section) from the DMA-descriptors scanning procedure. That
will prevent the DMA-transfer restarts after the DMA-transfer completion
was spotted but before the actual completion is executed.
Fixes: 69cea5a00d31 ("dmaengine/dw_dmac: Replace spin_lock* with irqsave variants and enable submission from callback")
Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Cc: stable@vger.kernel.org
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/dw/core.c | 54 ++++++++++++++++++++++++---------------------------
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index af1871646eb9..fbc46cbfe259 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -143,6 +143,12 @@ static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc)
channel_clear_bit(dw, CH_EN, dwc->mask);
while (dma_readl(dw, CH_EN) & dwc->mask)
cpu_relax();
+
+ dma_writel(dw, CLEAR.XFER, dwc->mask);
+ dma_writel(dw, CLEAR.BLOCK, dwc->mask);
+ dma_writel(dw, CLEAR.SRC_TRAN, dwc->mask);
+ dma_writel(dw, CLEAR.DST_TRAN, dwc->mask);
+ dma_writel(dw, CLEAR.ERROR, dwc->mask);
}
/*----------------------------------------------------------------------*/
@@ -259,34 +265,6 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
dmaengine_desc_callback_invoke(&cb, NULL);
}
-static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
-{
- struct dw_desc *desc, *_desc;
- LIST_HEAD(list);
- unsigned long flags;
-
- spin_lock_irqsave(&dwc->lock, flags);
- if (dma_readl(dw, CH_EN) & dwc->mask) {
- dev_err(chan2dev(&dwc->chan),
- "BUG: XFER bit set, but channel not idle!\n");
-
- /* Try to continue after resetting the channel... */
- dwc_chan_disable(dw, dwc);
- }
-
- /*
- * Submit queued descriptors ASAP, i.e. before we go through
- * the completed ones.
- */
- list_splice_init(&dwc->active_list, &list);
- dwc_dostart_first_queued(dwc);
-
- spin_unlock_irqrestore(&dwc->lock, flags);
-
- list_for_each_entry_safe(desc, _desc, &list, desc_node)
- dwc_descriptor_complete(dwc, desc, true);
-}
-
/* Returns how many bytes were already received from source */
static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
{
@@ -303,6 +281,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
struct dw_desc *child;
u32 status_xfer;
unsigned long flags;
+ LIST_HEAD(list);
spin_lock_irqsave(&dwc->lock, flags);
status_xfer = dma_readl(dw, RAW.XFER);
@@ -341,9 +320,26 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
}
+ /*
+ * No more active descriptors left to handle. So submit the
+ * queued descriptors and finish up the already handled ones.
+ */
+ if (dma_readl(dw, CH_EN) & dwc->mask) {
+ dev_err(chan2dev(&dwc->chan),
+ "BUG: XFER bit set, but channel not idle!\n");
+
+ /* Try to continue after resetting the channel... */
+ dwc_chan_disable(dw, dwc);
+ }
+
+ list_splice_init(&dwc->active_list, &list);
+ dwc_dostart_first_queued(dwc);
+
spin_unlock_irqrestore(&dwc->lock, flags);
- dwc_complete_all(dw, dwc);
+ list_for_each_entry_safe(desc, _desc, &list, desc_node)
+ dwc_descriptor_complete(dwc, desc, true);
+
return;
}
--
2.53.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 3/4] dmaengine: dw: serialize error recovery with channel termination
2026-10-22 18:52 [PATCH v2 0/4] dmaengine: dw: serialize completion and termination James Hilliard
2026-10-22 18:52 ` [PATCH v2 1/4] dmaengine: dw: Prevent tx-status calling DMA-desc callback James Hilliard
2026-10-22 18:52 ` [PATCH v2 2/4] dmaengine: dw: Fix XFER bit set, but channel not idle error James Hilliard
@ 2026-10-22 18:52 ` James Hilliard
2026-10-22 18:52 ` [PATCH v2 4/4] dmaengine: dw: synchronize completion callbacks James Hilliard
3 siblings, 0 replies; 5+ messages in thread
From: James Hilliard @ 2026-10-22 18:52 UTC (permalink / raw)
To: Viresh Kumar, Andy Shevchenko, Vinod Koul, Frank Li
Cc: Serge Semin, Greg Kroah-Hartman, Jiri Slaby, Ilpo Järvinen,
dmaengine, linux-serial, linux-kernel, Sashiko, James Hilliard,
stable
The tasklet samples RAW.ERROR before taking the channel lock. A
concurrent termination can empty the active list before dwc_handle_error()
uses that cached error to select a descriptor. The list head is then
interpreted as a descriptor. Even with an active descriptor, moving
queue.next unconditionally can put an empty queue's head on the active
list and pass it to dwc_dostart().
Recheck the error status under the channel lock and stop the channel
before selecting its failed descriptor. Only one transaction is issued
at a time, so take that descriptor directly instead of scanning for
completions first. The scan can otherwise complete it and start queued
work before the error handler has identified the failed transaction.
Detach the failed descriptor under the same lock and use the queued-start
helper, which leaves an empty queue alone. Clear the software-LLP flag
before restarting so a failed multiblock transfer does not prevent the
next transaction from starting. Keep the completion callback outside the
lock, with the detached descriptor owned by the tasklet.
Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Fixes: f336e42f73d9 ("dw_dmac: Move single descriptor from dwc->queue to dwc->active_list in dwc_complete_all")
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/dw/core.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index fbc46cbfe259..1df028970c4a 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -395,10 +395,17 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
struct dw_desc *child;
unsigned long flags;
- dwc_scan_descriptors(dw, dwc);
-
spin_lock_irqsave(&dwc->lock, flags);
+ /* Termination may have cleared the error since the tasklet sampled it. */
+ if (!(dma_readl(dw, RAW.ERROR) & dwc->mask))
+ goto unlock;
+
+ dwc_chan_disable(dw, dwc);
+ clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
+ if (list_empty(&dwc->active_list))
+ goto unlock;
+
/*
* The descriptor currently at the head of the active list is
* borked. Since we don't have any way to report errors, we'll
@@ -406,12 +413,7 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
*/
bad_desc = dwc_first_active(dwc);
list_del_init(&bad_desc->desc_node);
- list_move(dwc->queue.next, dwc->active_list.prev);
-
- /* Clear the error flag and try to restart the controller */
- dma_writel(dw, CLEAR.ERROR, dwc->mask);
- if (!list_empty(&dwc->active_list))
- dwc_dostart(dwc, dwc_first_active(dwc));
+ dwc_dostart_first_queued(dwc);
/*
* WARN may seem harsh, but since this only happens
@@ -430,6 +432,10 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
/* Pretend the descriptor completed successfully */
dwc_descriptor_complete(dwc, bad_desc, true);
+ return;
+
+unlock:
+ spin_unlock_irqrestore(&dwc->lock, flags);
}
static void dw_dma_tasklet(struct tasklet_struct *t)
--
2.53.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 4/4] dmaengine: dw: synchronize completion callbacks
2026-10-22 18:52 [PATCH v2 0/4] dmaengine: dw: serialize completion and termination James Hilliard
` (2 preceding siblings ...)
2026-10-22 18:52 ` [PATCH v2 3/4] dmaengine: dw: serialize error recovery with channel termination James Hilliard
@ 2026-10-22 18:52 ` James Hilliard
3 siblings, 0 replies; 5+ messages in thread
From: James Hilliard @ 2026-10-22 18:52 UTC (permalink / raw)
To: Viresh Kumar, Andy Shevchenko, Vinod Koul, Frank Li
Cc: Serge Semin, Greg Kroah-Hartman, Jiri Slaby, Ilpo Järvinen,
dmaengine, linux-serial, linux-kernel, Sashiko, James Hilliard,
stable
The controller tasklet can remove a descriptor from active_list and
invoke its callback outside the channel lock. Terminating the channel
does not wait for that callback, and there is no device_synchronize
hook. A client can therefore reuse callback state or release its buffers
after dmaengine_synchronize() while an old callback still uses them.
Disable and re-enable the controller tasklet from device_synchronize,
outside the channel lock. Disabling waits for an executing tasklet,
including descriptors it already removed from the channel lists. After
termination, a later tasklet run cannot acquire another descriptor from
the terminated channel. Clients must continue to exclude new submissions
on that channel until synchronization finishes.
The tasklet is shared, so other channels may schedule it while this wait
is in progress. Preserve those pending runs: tasklet_kill() temporarily
owns the scheduling bit and can consume a concurrent scheduling request,
leaving controller interrupts masked. The disable/enable pair leaves the
scheduling bit and pending tasklet queue intact. Other channels' hardware
transfers continue, though their callbacks may be briefly deferred.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-180707@kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/dw/core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 1df028970c4a..9e5e526f7672 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -942,6 +942,19 @@ static int dwc_terminate_all(struct dma_chan *chan)
return 0;
}
+static void dwc_synchronize(struct dma_chan *chan)
+{
+ struct dw_dma *dw = to_dw_dma(chan->device);
+
+ /*
+ * Termination removes the channel's descriptors from the lists, but a
+ * running tasklet may still own a completed descriptor and its callback.
+ * Wait for that run without losing schedules from the other channels.
+ */
+ tasklet_disable(&dw->tasklet);
+ tasklet_enable(&dw->tasklet);
+}
+
static struct dw_desc *dwc_find_desc(struct dw_dma_chan *dwc, dma_cookie_t c)
{
struct dw_desc *desc;
@@ -1333,6 +1346,7 @@ int do_dma_probe(struct dw_dma_chip *chip)
dw->dma.device_pause = dwc_pause;
dw->dma.device_resume = dwc_resume;
dw->dma.device_terminate_all = dwc_terminate_all;
+ dw->dma.device_synchronize = dwc_synchronize;
dw->dma.device_tx_status = dwc_tx_status;
dw->dma.device_issue_pending = dwc_issue_pending;
--
2.53.0
^ permalink raw reply [flat|nested] 5+ messages in thread