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

On 7/22/2026 10:57 AM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Removing dmaengine_terminate_sync(chan) from the polled mode path causes the test to unmap DMA buffers while the hardware might still be actively processing the queued descriptor.
> 
> Pre-existing issues:
> - [High] Test data buffers are freed before pending DMA transfers are terminated at the end of the test thread, creating a race window where active DMA hardware can corrupt freed slab memory.
> --
> 
> --- Patch [1]: [PATCH 1/4] dmaengine: dmatest: Don't forcibly terminate channel in polled mode ---
> commit ff3488aa5c4eb22b5aa3631448708f7560e37c84
> Author: Nathan Lynch <[email protected]>
> 
> dmaengine: dmatest: Don't forcibly terminate channel in polled mode
> 
> This commit removes the call to dmaengine_terminate_sync() from the
> main thread loop for polled transfers. This prevents disruptive
> terminations when multiple threads are concurrently submitting descriptors.
> 
>> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
>> index 2ae3469397f37..60bc448f42eeb 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)
> 
> [Severity: High]
> By removing the termination here, does this code leave the hardware running
> if dma_sync_wait() times out?
> 
> If dma_sync_wait() returns DMA_ERROR on a timeout, done->done remains false.
> The loop then eventually branches to error_unmap_continue without terminating
> the pending transfer.
> 
> This error path calls dmaengine_unmap_put(), unmapping the buffers while the
> hardware might still be actively processing the queued descriptor. Could this
> lead to active DMA memory corruption?

Yes, but this change simply makes the synchronous path through the test
loop consistent with the async path. dmatest treats the buffers
associated with timed-out transactions as safe to release. Perhaps that
should change, but it's out of scope.


> [Severity: High]
> This is a pre-existing issue, but does dmatest_func() free test buffers
> before terminating pending DMA transfers during thread cleanup?
> 
> Looking at the error handling and exit path at the end of the test thread:
> 
> dmatest_func() {
>     ...
>     err_dst:
>         dmatest_free_test_data(dst);
>     err_src:
>         dmatest_free_test_data(src);
>     ...
>     err_thread_type:
>         dmaengine_terminate_sync(chan);
>     ...
> }
> 
> If a transfer had timed out and was left running, could the DMA hardware
> execute the pending descriptor in the window between freeing the slab
> memory and terminating the channel?

Yes, but see above.

> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260722-dmatest-fixes-and-fatal-errors-v1-0-8287709987ea@amd.com?part=1