[PATCH 0/4] dmatest: shared channel fixes and fatal errors option

[PATCH 0/4] dmatest: shared channel fixes and fatal errors option

[Nathan Lynch via B4 Relay]

Sending several fixes for issues in dmatest that arise when running
multiple threads per channel. I have encountered these issues while
testing the sdxi driver.

Patch 1 "Don't forcibly terminate channel in polled mode" is a resend of a
fix I sent a while back that didn't get picked up. [1]

Also included is a feature introducing a "fatal errors" parameter that
provides the option of panicking the kernel after a given number of errors
have occurred in a run.

Claude Code w/Opus 4.8 was used to investigate the problems fixed by
patches 2 and 3, and it assisted in writing the changes in patch 2-4.

[1] https://lore.kernel.org/dmaengine/20250408-dmaengine-dmatest-poll-mode-fix-v1-1-754a446a5363@amd.com/

Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
Nathan Lynch (4):
      dmaengine: dmatest: Don't forcibly terminate channel in polled mode
      dmaengine: dmatest: Suppress bogus failures when stopping a run
      dmaengine: dmatest: Terminate channel before freeing threads
      dmaengine: dmatest: Optionally make test errors fatal

 drivers/dma/dmatest.c | 67 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 11 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260722-dmatest-fixes-and-fatal-errors-3b3653d5ac9b

Best regards,
--  
Nathan Lynch <nathan.lynch@amd.com>

[PATCH 1/4] dmaengine: dmatest: Don't forcibly terminate channel in polled mode

[Nathan Lynch via B4 Relay]

From: Nathan Lynch <nathan.lynch@amd.com>

Invoking dmaengine_terminate_sync() against the channel after each
submitted descriptor is strange. It's benign when the test is
single-threaded, but disruptive when multiple threads are submitting
descriptors to the channel concurrently:

  # cd /sys/module/dmatest/parameters/
  # echo 1 > polled
  # echo 2 > threads_per_chan
  # echo dma0chan0 > channel
  # echo 1 > run
...
  dmatest: Added 2 threads using dma0chan0
  dmatest: Started 2 threads using dma0chan0
  xilinx-zynqmp-dma ffa80000.dma: dma_sync_wait: timeout!
  xilinx-zynqmp-dma ffa80000.dma: dma_sync_wait: timeout!
  dmatest: dma0chan0-copy0: result #3: 'test timed out' with
    src_off=0x487 dst_off=0xf8 len=0x171b (0)
  dmatest: dma0chan0-copy1: result #6: 'test timed out' with
    src_off=0x227d dst_off=0xf99 len=0xf7a (0)

Remove the call to dmaengine_terminate_sync() from the main thread
loop.

Fixes: fb9816f9d05f ("dmaengine: dmatest: Add support for completion polling")
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
 drivers/dma/dmatest.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 2ae3469397f3..60bc448f42ee 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -837,7 +837,6 @@ static int dmatest_func(void *data)
 
 		if (params->polled) {
 			status = dma_sync_wait(chan, cookie);
-			dmaengine_terminate_sync(chan);
 			if (status == DMA_COMPLETE)
 				done->done = true;
 		} else {

-- 
2.54.0

[PATCH 2/4] dmaengine: dmatest: Suppress bogus failures when stopping a run

[Nathan Lynch via B4 Relay]

From: Nathan Lynch <nathan.lynch@amd.com>

Writing 0 to the module 'run' attribute stops the test threads with
kthread_stop(), waking threads out of wait_event_freezable_timeout()
before the last transfer's completion callback has run. The stopping
thread samples the transfer status and then checks done->done at two
different instants, so the last transaction gets misreported two ways:

- "test timed out": the wait returns early with done->done still clear
  even though the transfer is simply still finishing — not a real
  timeout.

- "completion busy status": status is read as DMA_IN_PROGRESS a moment
  before the transfer completes and latches done->done, so the stale
  status is reported as a failure against a transfer that actually
  succeeded.

This is not a recent regression: the threads have used an interruptible,
freezable timeout wait since long before the wait queue was moved into
thread context by commit 6f6a23a213be ("dmaengine: dmatest: move callback
wait queue to thread context"), so a stop has always been able to wake
the wait before the final callback runs.

When kthread_should_stop() returns true, use dma_sync_wait() to settle
the pending transaction. Real errors still yield DMA_ERROR.

Assisted-by: Claude:claude-opus-4-8 [Claude-Code]
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
 drivers/dma/dmatest.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 60bc448f42ee..3267c7e86f5d 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -702,6 +702,7 @@ static int dmatest_func(void *data)
 		struct dmaengine_unmap_data *um;
 		dma_addr_t *dsts;
 		unsigned int len;
+		bool stopping;
 
 		total_tests++;
 
@@ -839,6 +840,7 @@ static int dmatest_func(void *data)
 			status = dma_sync_wait(chan, cookie);
 			if (status == DMA_COMPLETE)
 				done->done = true;
+			stopping = kthread_should_stop();
 		} else {
 			dma_async_issue_pending(chan);
 
@@ -846,14 +848,19 @@ static int dmatest_func(void *data)
 					done->done,
 					msecs_to_jiffies(params->timeout));
 
-			status = dma_async_is_tx_complete(chan, cookie, NULL,
-							  NULL);
+			stopping = kthread_should_stop();
+			status = stopping ?
+				dma_sync_wait(chan, cookie) :
+				dma_async_is_tx_complete(chan, cookie, NULL, NULL);
 		}
 
 		if (!done->done) {
-			result("test timed out", total_tests, src->off, dst->off,
-			       len, 0);
-			goto error_unmap_continue;
+			/* stopping: dma_sync_wait() let the transfer finish, not a timeout */
+			if (!stopping) {
+				result("test timed out", total_tests, src->off,
+				       dst->off, len, 0);
+				goto error_unmap_continue;
+			}
 		} else if (status != DMA_COMPLETE &&
 			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
 					 dev->cap_mask) &&

-- 
2.54.0

[PATCH 3/4] dmaengine: dmatest: Terminate channel before freeing threads

[Nathan Lynch via B4 Relay]

From: Nathan Lynch <nathan.lynch@amd.com>

On a channel shared by multiple threads, the completion callbacks run from
one tasklet, and dmaengine_terminate_sync() flushes it. Freeing a thread
before that flush can leave a pending callback pointing at freed memory
(dmatest_callback() reads thread->done).

Stop all channel threads and then flush the channel before freeing the
threads.

Fixes: 6f6a23a213be ("dmaengine: dmatest: move callback wait queue to thread context")
Assisted-by: Claude:claude-opus-4-8 [Claude-Code]
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
 drivers/dma/dmatest.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 3267c7e86f5d..0d7752c2451a 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -959,18 +959,23 @@ static void dmatest_cleanup_channel(struct dmatest_chan *dtc)
 	struct dmatest_thread	*_thread;
 	int			ret;
 
-	list_for_each_entry_safe(thread, _thread, &dtc->threads, node) {
+	/* stop all threads first */
+	list_for_each_entry(thread, &dtc->threads, node) {
 		ret = kthread_stop(thread->task);
 		pr_debug("thread %s exited with status %d\n",
 			 thread->task->comm, ret);
+	}
+
+	/* flush the channel (e.g. vchan_synchronize() -> tasklet_kill()) */
+	dmaengine_terminate_sync(dtc->chan);
+
+	/* now it is safe to free the thread memory */
+	list_for_each_entry_safe(thread, _thread, &dtc->threads, node) {
 		list_del(&thread->node);
 		put_task_struct(thread->task);
 		kfree(thread);
 	}
 
-	/* terminate all transfers on specified channels */
-	dmaengine_terminate_sync(dtc->chan);
-
 	kfree(dtc);
 }
 

-- 
2.54.0

[PATCH 4/4] dmaengine: dmatest: Optionally make test errors fatal

[Nathan Lynch via B4 Relay]

From: Nathan Lynch <nathan.lynch@amd.com>

For automated testing it is useful to have dmatest halt the machine when
errors occur rather than logging them and continuing, so that the system
state at the point of failure can be captured instead of allowing the run
to carry on.

Add a 'fatal_errors' module parameter, defaulting to 0. When set to a
non-zero value N, panic the kernel once N test errors have accumulated in
a run; the count is reset at the start of each run. The panic is triggered
after the existing result() line is logged, so the specific error (data
mismatch, mapping/prep/submit error, timeout, or bad completion status) is
recorded before the crash.

Assisted-by: Claude:claude-opus-4-8 [Claude-Code]
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
 drivers/dma/dmatest.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 0d7752c2451a..a070ce6f2fa9 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -7,6 +7,7 @@
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/atomic.h>
 #include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
@@ -92,6 +93,11 @@ static bool polled;
 module_param(polled, bool, 0644);
 MODULE_PARM_DESC(polled, "Use polling for completion instead of interrupts");
 
+static unsigned int fatal_errors;
+module_param(fatal_errors, uint, 0644);
+MODULE_PARM_DESC(fatal_errors,
+		"Panic after this many test errors in a run; 0 = never (default: 0)");
+
 /**
  * struct dmatest_params - test parameters.
  * @nobounce:		prevent using swiotlb buffer
@@ -109,6 +115,7 @@ MODULE_PARM_DESC(polled, "Use polling for completion instead of interrupts");
  * @alignment:		custom data address alignment taken as 2^alignment
  * @transfer_size:	custom transfer size in bytes
  * @polled:		use polling for completion instead of interrupts
+ * @fatal_errors:	panic after this many test errors in a run (0 = never)
  */
 struct dmatest_params {
 	bool		nobounce;
@@ -126,6 +133,7 @@ struct dmatest_params {
 	int		alignment;
 	unsigned int	transfer_size;
 	bool		polled;
+	unsigned int	fatal_errors;
 };
 
 /**
@@ -133,6 +141,7 @@ struct dmatest_params {
  * @params:		test parameters
  * @channels:		channels under test
  * @nr_channels:	number of channels under test
+ * @error_count:	test errors in the current run (for fatal_errors)
  * @lock:		access protection to the fields of this structure
  * @did_init:		module has been initialized completely
  * @last_error:		test has faced configuration issues
@@ -144,6 +153,7 @@ struct dmatest_info {
 	/* Internal state */
 	struct list_head	channels;
 	unsigned int		nr_channels;
+	atomic_t		error_count;
 	int			last_error;
 	struct mutex		lock;
 	bool			did_init;
@@ -487,6 +497,24 @@ static void dbg_result(const char *err, unsigned int n, unsigned int src_off,
 		dbg_result(err, n, src_off, dst_off, len, data);\
 })
 
+/*
+ * Record a test error whose details were just logged via result(). When
+ * fatal_errors is set, panic once that many errors have accumulated in the
+ * current run.
+ */
+static void dmatest_note_error(struct dmatest_info *info)
+{
+	unsigned int errors;
+
+	if (!info->params.fatal_errors)
+		return;
+
+	errors = atomic_inc_return(&info->error_count);
+	if (errors >= info->params.fatal_errors)
+		panic("dmatest: fatal_errors=%u and %u error(s) encountered\n",
+		      info->params.fatal_errors, errors);
+}
+
 static unsigned long long dmatest_persec(s64 runtime, unsigned int val)
 {
 	unsigned long long per_sec = 1000000;
@@ -583,7 +611,7 @@ static int dmatest_func(void *data)
 	struct dma_device	*dev;
 	struct device		*dma_dev;
 	unsigned int		error_count;
-	unsigned int		failed_tests = 0;
+	unsigned int		failed_tests = 0;	/* for this thread */
 	unsigned int		total_tests = 0;
 	dma_cookie_t		cookie;
 	enum dma_status		status;
@@ -755,6 +783,7 @@ static int dmatest_func(void *data)
 			failed_tests++;
 			result("unmap data NULL", total_tests,
 			       src->off, dst->off, len, ret);
+			dmatest_note_error(info);
 			continue;
 		}
 
@@ -910,6 +939,7 @@ static int dmatest_func(void *data)
 			result("data error", total_tests, src->off, dst->off,
 			       len, error_count);
 			failed_tests++;
+			dmatest_note_error(info);
 		} else {
 			verbose_result("test passed", total_tests, src->off,
 				       dst->off, len, 0);
@@ -920,6 +950,7 @@ static int dmatest_func(void *data)
 error_unmap_continue:
 		dmaengine_unmap_put(um);
 		failed_tests++;
+		dmatest_note_error(info);
 	}
 	ktime = ktime_sub(ktime_get(), ktime);
 	ktime = ktime_sub(ktime, comparetime);
@@ -1135,6 +1166,7 @@ static void add_threaded_test(struct dmatest_info *info)
 	params->alignment = alignment;
 	params->transfer_size = transfer_size;
 	params->polled = polled;
+	params->fatal_errors = fatal_errors;
 
 	request_channels(info, DMA_MEMCPY);
 	request_channels(info, DMA_MEMSET);
@@ -1147,6 +1179,8 @@ static void run_pending_tests(struct dmatest_info *info)
 	struct dmatest_chan *dtc;
 	unsigned int thread_count = 0;
 
+	atomic_set(&info->error_count, 0);
+
 	list_for_each_entry(dtc, &info->channels, node) {
 		struct dmatest_thread *thread;
 

-- 
2.54.0

Re: [PATCH 4/4] dmaengine: dmatest: Optionally make test errors fatal

[Tycho Andersen]

On Wed, Jul 22, 2026 at 10:47:49AM -0500, Nathan Lynch via B4 Relay wrote:
> From: Nathan Lynch <nathan.lynch@amd.com>
> 
> For automated testing it is useful to have dmatest halt the machine when
> errors occur rather than logging them and continuing, so that the system
> state at the point of failure can be captured instead of allowing the run
> to carry on.
> 
> Add a 'fatal_errors' module parameter, defaulting to 0. When set to a
> non-zero value N, panic the kernel once N test errors have accumulated in
> a run; the count is reset at the start of each run. The panic is triggered
> after the existing result() line is logged, so the specific error (data
> mismatch, mapping/prep/submit error, timeout, or bad completion status) is
> recorded before the crash.
> 
> Assisted-by: Claude:claude-opus-4-8 [Claude-Code]
> Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>

Reviewed-by: Tycho Andersen (AMD) <tycho@kernel.org>

but,

> @@ -583,7 +611,7 @@ static int dmatest_func(void *data)
>  	struct dma_device	*dev;
>  	struct device		*dma_dev;
>  	unsigned int		error_count;
> -	unsigned int		failed_tests = 0;
> +	unsigned int		failed_tests = 0;	/* for this thread */
>  	unsigned int		total_tests = 0;
>  	dma_cookie_t		cookie;
>  	enum dma_status		status;

I would probably drop this hunk if you re-spin. It's on the thread's
stack, so it seems like an "obvious" comment.

Tycho