All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Nathan Lynch" <nathan.lynch@amd.com>
Cc: Frank.Li@kernel.org, dmaengine@vger.kernel.org, vkoul@kernel.org
Subject: Re: [PATCH 1/4] dmaengine: dmatest: Don't forcibly terminate channel in polled mode
Date: Wed, 22 Jul 2026 15:57:50 +0000	[thread overview]
Message-ID: <20260722155751.289771F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260722-dmatest-fixes-and-fatal-errors-v1-1-8287709987ea@amd.com>

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 <nathan.lynch@amd.com>

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?

[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?

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

  reply	other threads:[~2026-07-22 15:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 15:47 [PATCH 0/4] dmatest: shared channel fixes and fatal errors option Nathan Lynch via B4 Relay
2026-07-22 15:47 ` Nathan Lynch
2026-07-22 15:47 ` [PATCH 1/4] dmaengine: dmatest: Don't forcibly terminate channel in polled mode Nathan Lynch via B4 Relay
2026-07-22 15:47   ` Nathan Lynch
2026-07-22 15:57   ` sashiko-bot [this message]
2026-07-22 15:47 ` [PATCH 2/4] dmaengine: dmatest: Suppress bogus failures when stopping a run Nathan Lynch via B4 Relay
2026-07-22 15:47   ` Nathan Lynch
2026-07-22 16:09   ` sashiko-bot
2026-07-22 15:47 ` [PATCH 3/4] dmaengine: dmatest: Terminate channel before freeing threads Nathan Lynch via B4 Relay
2026-07-22 15:47   ` Nathan Lynch
2026-07-22 16:23   ` sashiko-bot
2026-07-22 15:47 ` [PATCH 4/4] dmaengine: dmatest: Optionally make test errors fatal Nathan Lynch via B4 Relay
2026-07-22 15:47   ` Nathan Lynch
2026-07-22 16:33   ` sashiko-bot
2026-07-22 19:58   ` Tycho Andersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260722155751.289771F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=nathan.lynch@amd.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.