[PATCH] dmaengine: xilinx_dma: Fix CPU stall in xilinx_dma_poll_timeout

[PATCH] dmaengine: xilinx_dma: Fix CPU stall in xilinx_dma_poll_timeout

[Alex Bereza]

Currently when calling xilinx_dma_poll_timeout with delay_us=0 and a
condition that is never fulfilled, the CPU busy-waits for prolonged time
and the timeout triggers only with a massive delay causing a CPU stall.

This happens due to a huge underestimation of wall clock time in
poll_timeout_us_atomic. Commit 7349a69cf312 ("iopoll: Do not use
timekeeping in read_poll_timeout_atomic()") changed the behavior to no
longer use ktime_get at the expense of underestimation of wall clock
time which appears to be very large for delay_us=0. Instead of timing
out after approximately XILINX_DMA_LOOP_COUNT microseconds, the timeout
takes XILINX_DMA_LOOP_COUNT * 1000 * (time that the overhead of the for
loop in poll_timeout_us_atomic takes) which is in the range of several
minutes for XILINX_DMA_LOOP_COUNT=1000000. Fix this by using a non-zero
value for delay_us. Use delay_us=10 to keep the delay in the hot path of
starting DMA transfers minimal but still avoid CPU stalls in case of
unexpected hardware failures.

One-off measurement with delay_us=0 causes the cpu to busy wait around 7
minutes in the timeout case. After applying this patch with delay_us=10
the measured timeout was 1053428 microseconds which is roughly
equivalent to the expected 1000000 microseconds specified in
XILINX_DMA_POLL_TIMEOUT_US.

Rename XILINX_DMA_LOOP_COUNT to XILINX_DMA_POLL_TIMEOUT_US because the
former is incorrect. It is a timeout value for polling various register
bits in microseconds. It is not a loop count. Add a constant
XILINX_DMA_POLL_DELAY_US for delay_us value.

Fixes: 7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")
Signed-off-by: Alex Bereza <alex@bereza.email>
---
Hi, in addition to this patch I also have a question: what is the point
of atomically polling for the HALTED or IDLE bit in the stop_transfer
functions? Does device_terminate_all really need to be callable from
atomic context? If not, one could switch to polling non-atomically and
avoid burning CPU cycles.

As this is my first patch, please feel free to point me in the right
direction if I am missing anything.
---
 drivers/dma/xilinx/xilinx_dma.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 02a05f215614..8556c357b665 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -165,8 +165,10 @@
 #define XILINX_DMA_FLUSH_MM2S		2
 #define XILINX_DMA_FLUSH_BOTH		1
 
-/* Delay loop counter to prevent hardware failure */
-#define XILINX_DMA_LOOP_COUNT		1000000
+/* Timeout for polling various registers */
+#define XILINX_DMA_POLL_TIMEOUT_US		1000000
+/* Delay between polls (avoid a delay of 0 to prevent CPU stalls) */
+#define XILINX_DMA_POLL_DELAY_US		10
 
 /* AXI DMA Specific Registers/Offsets */
 #define XILINX_DMA_REG_SRCDSTADDR	0x18
@@ -1332,8 +1334,9 @@ static int xilinx_dma_stop_transfer(struct xilinx_dma_chan *chan)
 
 	/* Wait for the hardware to halt */
 	return xilinx_dma_poll_timeout(chan, XILINX_DMA_REG_DMASR, val,
-				       val & XILINX_DMA_DMASR_HALTED, 0,
-				       XILINX_DMA_LOOP_COUNT);
+				       val & XILINX_DMA_DMASR_HALTED,
+				       XILINX_DMA_POLL_DELAY_US,
+				       XILINX_DMA_POLL_TIMEOUT_US);
 }
 
 /**
@@ -1347,8 +1350,9 @@ static int xilinx_cdma_stop_transfer(struct xilinx_dma_chan *chan)
 	u32 val;
 
 	return xilinx_dma_poll_timeout(chan, XILINX_DMA_REG_DMASR, val,
-				       val & XILINX_DMA_DMASR_IDLE, 0,
-				       XILINX_DMA_LOOP_COUNT);
+				       val & XILINX_DMA_DMASR_IDLE,
+				       XILINX_DMA_POLL_DELAY_US,
+				       XILINX_DMA_POLL_TIMEOUT_US);
 }
 
 /**
@@ -1364,8 +1368,9 @@ static void xilinx_dma_start(struct xilinx_dma_chan *chan)
 
 	/* Wait for the hardware to start */
 	err = xilinx_dma_poll_timeout(chan, XILINX_DMA_REG_DMASR, val,
-				      !(val & XILINX_DMA_DMASR_HALTED), 0,
-				      XILINX_DMA_LOOP_COUNT);
+				      !(val & XILINX_DMA_DMASR_HALTED),
+				      XILINX_DMA_POLL_DELAY_US,
+				      XILINX_DMA_POLL_TIMEOUT_US);
 
 	if (err) {
 		dev_err(chan->dev, "Cannot start channel %p: %x\n",
@@ -1780,8 +1785,9 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
 
 	/* Wait for the hardware to finish reset */
 	err = xilinx_dma_poll_timeout(chan, XILINX_DMA_REG_DMACR, tmp,
-				      !(tmp & XILINX_DMA_DMACR_RESET), 0,
-				      XILINX_DMA_LOOP_COUNT);
+				      !(tmp & XILINX_DMA_DMACR_RESET),
+				      XILINX_DMA_POLL_DELAY_US,
+				      XILINX_DMA_POLL_TIMEOUT_US);
 
 	if (err) {
 		dev_err(chan->dev, "reset timeout, cr %x, sr %x\n",

---
base-commit: b7560798466a07d9c3fb011698e92c335ab28baf
change-id: 20260330-fix-atomic-poll-timeout-regression-4f4e3baf3fd7

Best regards,
--  
Alex Bereza <alex@bereza.email>

Re: [PATCH] dmaengine: xilinx_dma: Fix CPU stall in xilinx_dma_poll_timeout

[Gupta, Suraj]

On 3/31/2026 10:14 PM, Alex Bereza wrote:
> [You don't often get email from alex@bereza.email. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> Currently when calling xilinx_dma_poll_timeout with delay_us=0 and a
> condition that is never fulfilled, the CPU busy-waits for prolonged time
> and the timeout triggers only with a massive delay causing a CPU stall.
> 
> This happens due to a huge underestimation of wall clock time in
> poll_timeout_us_atomic. Commit 7349a69cf312 ("iopoll: Do not use
> timekeeping in read_poll_timeout_atomic()") changed the behavior to no
> longer use ktime_get at the expense of underestimation of wall clock
> time which appears to be very large for delay_us=0. Instead of timing
> out after approximately XILINX_DMA_LOOP_COUNT microseconds, the timeout
> takes XILINX_DMA_LOOP_COUNT * 1000 * (time that the overhead of the for
> loop in poll_timeout_us_atomic takes) which is in the range of several
> minutes for XILINX_DMA_LOOP_COUNT=1000000. Fix this by using a non-zero
> value for delay_us. Use delay_us=10 to keep the delay in the hot path of
> starting DMA transfers minimal but still avoid CPU stalls in case of
> unexpected hardware failures.
> 
> One-off measurement with delay_us=0 causes the cpu to busy wait around 7
> minutes in the timeout case. After applying this patch with delay_us=10
> the measured timeout was 1053428 microseconds which is roughly
> equivalent to the expected 1000000 microseconds specified in
> XILINX_DMA_POLL_TIMEOUT_US.
> 
> Rename XILINX_DMA_LOOP_COUNT to XILINX_DMA_POLL_TIMEOUT_US because the
> former is incorrect. It is a timeout value for polling various register
> bits in microseconds. It is not a loop count. Add a constant
> XILINX_DMA_POLL_DELAY_US for delay_us value.

Please split this change in a new patch.

> 
> Fixes: 7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")

This patch doesn't fixes anything in iopoll, please use correct fixes tag.

> Signed-off-by: Alex Bereza <alex@bereza.email>
> ---
> Hi, in addition to this patch I also have a question: what is the point
> of atomically polling for the HALTED or IDLE bit in the stop_transfer
> functions? Does device_terminate_all really need to be callable from
> atomic context? If not, one could switch to polling non-atomically and
> avoid burning CPU cycles.
> 

dmaengine_terminate_async(), which directly calls device_terminate_all 
can be called from atomic context.

Regards,
Suraj

> As this is my first patch, please feel free to point me in the right
> direction if I am missing anything.
> ---
>   drivers/dma/xilinx/xilinx_dma.c | 26 ++++++++++++++++----------
>   1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 02a05f215614..8556c357b665 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -165,8 +165,10 @@
>   #define XILINX_DMA_FLUSH_MM2S          2
>   #define XILINX_DMA_FLUSH_BOTH          1
> 
> -/* Delay loop counter to prevent hardware failure */
> -#define XILINX_DMA_LOOP_COUNT          1000000
> +/* Timeout for polling various registers */
> +#define XILINX_DMA_POLL_TIMEOUT_US             1000000
> +/* Delay between polls (avoid a delay of 0 to prevent CPU stalls) */
> +#define XILINX_DMA_POLL_DELAY_US               10
> 
>   /* AXI DMA Specific Registers/Offsets */
>   #define XILINX_DMA_REG_SRCDSTADDR      0x18
> @@ -1332,8 +1334,9 @@ static int xilinx_dma_stop_transfer(struct xilinx_dma_chan *chan)
> 
>          /* Wait for the hardware to halt */
>          return xilinx_dma_poll_timeout(chan, XILINX_DMA_REG_DMASR, val,
> -                                      val & XILINX_DMA_DMASR_HALTED, 0,
> -                                      XILINX_DMA_LOOP_COUNT);
> +                                      val & XILINX_DMA_DMASR_HALTED,
> +                                      XILINX_DMA_POLL_DELAY_US,
> +                                      XILINX_DMA_POLL_TIMEOUT_US);
>   }
> 
>   /**
> @@ -1347,8 +1350,9 @@ static int xilinx_cdma_stop_transfer(struct xilinx_dma_chan *chan)
>          u32 val;
> 
>          return xilinx_dma_poll_timeout(chan, XILINX_DMA_REG_DMASR, val,
> -                                      val & XILINX_DMA_DMASR_IDLE, 0,
> -                                      XILINX_DMA_LOOP_COUNT);
> +                                      val & XILINX_DMA_DMASR_IDLE,
> +                                      XILINX_DMA_POLL_DELAY_US,
> +                                      XILINX_DMA_POLL_TIMEOUT_US);
>   }
> 
>   /**
> @@ -1364,8 +1368,9 @@ static void xilinx_dma_start(struct xilinx_dma_chan *chan)
> 
>          /* Wait for the hardware to start */
>          err = xilinx_dma_poll_timeout(chan, XILINX_DMA_REG_DMASR, val,
> -                                     !(val & XILINX_DMA_DMASR_HALTED), 0,
> -                                     XILINX_DMA_LOOP_COUNT);
> +                                     !(val & XILINX_DMA_DMASR_HALTED),
> +                                     XILINX_DMA_POLL_DELAY_US,
> +                                     XILINX_DMA_POLL_TIMEOUT_US);
> 
>          if (err) {
>                  dev_err(chan->dev, "Cannot start channel %p: %x\n",
> @@ -1780,8 +1785,9 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
> 
>          /* Wait for the hardware to finish reset */
>          err = xilinx_dma_poll_timeout(chan, XILINX_DMA_REG_DMACR, tmp,
> -                                     !(tmp & XILINX_DMA_DMACR_RESET), 0,
> -                                     XILINX_DMA_LOOP_COUNT);
> +                                     !(tmp & XILINX_DMA_DMACR_RESET),
> +                                     XILINX_DMA_POLL_DELAY_US,
> +                                     XILINX_DMA_POLL_TIMEOUT_US);
> 
>          if (err) {
>                  dev_err(chan->dev, "reset timeout, cr %x, sr %x\n",
> 
> ---
> base-commit: b7560798466a07d9c3fb011698e92c335ab28baf
> change-id: 20260330-fix-atomic-poll-timeout-regression-4f4e3baf3fd7
> 
> Best regards,
> --
> Alex Bereza <alex@bereza.email>
> 
> 

Re: [PATCH] dmaengine: xilinx_dma: Fix CPU stall in xilinx_dma_poll_timeout

[Alex Bereza]

On Wed Apr 1, 2026 at 7:23 AM CEST, Suraj Gupta wrote:

>> Rename XILINX_DMA_LOOP_COUNT to XILINX_DMA_POLL_TIMEOUT_US because the
>> former is incorrect. It is a timeout value for polling various register
>> bits in microseconds. It is not a loop count. Add a constant
>> XILINX_DMA_POLL_DELAY_US for delay_us value.
>
> Please split this change in a new patch.

Ok, will send a v2.

>> Fixes: 7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")
>
> This patch doesn't fixes anything in iopoll, please use correct fixes tag.

Ok, but I'm not sure what would be the correct fixes tag then? I though I need to reference
7349a69cf312 in fixes tag because this is the actual change that surfaced the CPU stall issue that I
want to fix in this driver. I'm fixing the call sites of xilinx_dma_poll_timeout but they were added
in different commits. Should I add all of them? That would be the following then:

Fixes: 9495f2648287 ("dmaengine: xilinx_vdma: Use readl_poll_timeout instead of do while loop's")
Fixes: 676f9c26c330 ("dmaengine: xilinx: fix device_terminate_all() callback for AXI CDMA")

Three call sites with delay_us=0 were first introduced by 9495f2648287, then 676f9c26c330 added the
fourth call site when introducing xilinx_cdma_stop_transfer (probably copy paste from
xilinx_dma_stop_transfer). Would adding these two fixes tags be correct?

>> Hi, in addition to this patch I also have a question: what is the point
>> of atomically polling for the HALTED or IDLE bit in the stop_transfer
>> functions? Does device_terminate_all really need to be callable from
>> atomic context? If not, one could switch to polling non-atomically and
>> avoid burning CPU cycles.
>>
>
> dmaengine_terminate_async(), which directly calls device_terminate_all
> can be called from atomic context.

Right, thanks! Just for my understanding: I still think there is potential for improvement, because
from my understanding it would be beneficial to do the waiting for the bits in the status register
and the freeing of descriptors in xilinx_dma_synchronize. Do I understand correctly that this is
currently not possible due to how the DMA engine API is structured? To make this possible I think
the deprecated dmaengine_terminate_all would have to be removed and all users of this API would have
to be adapted accordingly, correct? So this would be a patch of much larger scope than xilinx_dma
driver alone.

Re: [PATCH] dmaengine: xilinx_dma: Fix CPU stall in xilinx_dma_poll_timeout

[Geert Uytterhoeven]

Hi Alex,

On Wed, 1 Apr 2026 at 10:27, Alex Bereza <alex@bereza.email> wrote:
> On Wed Apr 1, 2026 at 7:23 AM CEST, Suraj Gupta wrote:
> >> Rename XILINX_DMA_LOOP_COUNT to XILINX_DMA_POLL_TIMEOUT_US because the
> >> former is incorrect. It is a timeout value for polling various register
> >> bits in microseconds. It is not a loop count. Add a constant
> >> XILINX_DMA_POLL_DELAY_US for delay_us value.
> >
> > Please split this change in a new patch.
>
> Ok, will send a v2.
>
> >> Fixes: 7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")
> >
> > This patch doesn't fixes anything in iopoll, please use correct fixes tag.

Fixes-tag are also used as guidelines, to indicate which patches
are also needed when backporting something.  I.e. if 7349a69cf312 is
ever backported, any other commits that contain "Fixes: 7349a69cf312"
should be backported, too.  So having this Fixes-tag, in addition to
another xilinx_dma-specific one, sounds fine to me.

> Ok, but I'm not sure what would be the correct fixes tag then? I though I need to reference
> 7349a69cf312 in fixes tag because this is the actual change that surfaced the CPU stall issue that I
> want to fix in this driver. I'm fixing the call sites of xilinx_dma_poll_timeout but they were added
> in different commits. Should I add all of them? That would be the following then:
>
> Fixes: 9495f2648287 ("dmaengine: xilinx_vdma: Use readl_poll_timeout instead of do while loop's")
> Fixes: 676f9c26c330 ("dmaengine: xilinx: fix device_terminate_all() callback for AXI CDMA")

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dmaengine: xilinx_dma: Fix CPU stall in xilinx_dma_poll_timeout

[Gupta, Suraj]

On 4/1/2026 1:57 PM, Alex Bereza wrote:
> [You don't often get email from alex@bereza.email. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Wed Apr 1, 2026 at 7:23 AM CEST, Suraj Gupta wrote:
> 
>>> Rename XILINX_DMA_LOOP_COUNT to XILINX_DMA_POLL_TIMEOUT_US because the
>>> former is incorrect. It is a timeout value for polling various register
>>> bits in microseconds. It is not a loop count. Add a constant
>>> XILINX_DMA_POLL_DELAY_US for delay_us value.
>>
>> Please split this change in a new patch.
> 
> Ok, will send a v2.
> 
>>> Fixes: 7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")
>>
>> This patch doesn't fixes anything in iopoll, please use correct fixes tag.
> 
> Ok, but I'm not sure what would be the correct fixes tag then? I though I need to reference
> 7349a69cf312 in fixes tag because this is the actual change that surfaced the CPU stall issue that I
> want to fix in this driver. I'm fixing the call sites of xilinx_dma_poll_timeout but they were added
> in different commits. Should I add all of them? That would be the following then:
> 
> Fixes: 9495f2648287 ("dmaengine: xilinx_vdma: Use readl_poll_timeout instead of do while loop's")
> Fixes: 676f9c26c330 ("dmaengine: xilinx: fix device_terminate_all() callback for AXI CDMA")
> 
> Three call sites with delay_us=0 were first introduced by 9495f2648287, then 676f9c26c330 added the
> fourth call site when introducing xilinx_cdma_stop_transfer (probably copy paste from
> xilinx_dma_stop_transfer). Would adding these two fixes tags be correct?
> 
>>> Hi, in addition to this patch I also have a question: what is the point
>>> of atomically polling for the HALTED or IDLE bit in the stop_transfer
>>> functions? Does device_terminate_all really need to be callable from
>>> atomic context? If not, one could switch to polling non-atomically and
>>> avoid burning CPU cycles.
>>>
>>
>> dmaengine_terminate_async(), which directly calls device_terminate_all
>> can be called from atomic context.
> 
> Right, thanks! Just for my understanding: I still think there is potential for improvement, because
> from my understanding it would be beneficial to do the waiting for the bits in the status register
> and the freeing of descriptors in xilinx_dma_synchronize. Do I understand correctly that this is
> currently not possible due to how the DMA engine API is structured? To make this possible I think
> the deprecated dmaengine_terminate_all would have to be removed and all users of this API would have
> to be adapted accordingly, correct? So this would be a patch of much larger scope than xilinx_dma
> driver alone.

Yes, your understanding of xilinx_dma_synchronize()and proposed changes 
looks correct.

Regards,
Suraj

Re: [PATCH] dmaengine: xilinx_dma: Fix CPU stall in xilinx_dma_poll_timeout

[Alex Bereza]

Hi Suraj,

On Wed Apr 1, 2026 at 1:15 PM CEST, Suraj Gupta wrote:

>>>> Hi, in addition to this patch I also have a question: what is the point
>>>> of atomically polling for the HALTED or IDLE bit in the stop_transfer
>>>> functions? Does device_terminate_all really need to be callable from
>>>> atomic context? If not, one could switch to polling non-atomically and
>>>> avoid burning CPU cycles.
>>>>
>>>
>>> dmaengine_terminate_async(), which directly calls device_terminate_all
>>> can be called from atomic context.
>>
>> Right, thanks! Just for my understanding: I still think there is potential for improvement, because
>> from my understanding it would be beneficial to do the waiting for the bits in the status register
>> and the freeing of descriptors in xilinx_dma_synchronize. Do I understand correctly that this is
>> currently not possible due to how the DMA engine API is structured? To make this possible I think
>> the deprecated dmaengine_terminate_all would have to be removed and all users of this API would have
>> to be adapted accordingly, correct? So this would be a patch of much larger scope than xilinx_dma
>> driver alone.
>
> Yes, your understanding of xilinx_dma_synchronize()and proposed changes
> looks correct.

Thank you for the feedback on this. It is really helpful since I'm quite new to
writing patches for the kernel. I was thinking about whether I can improve the
xilinx_dma driver in this regard, but given the large scope of changing the
whole DMA engine API and all its users unfortunately this task is too big for
me.

BR
Alex