Re: [PATCH v4 7/9] nvmet: pci-epf: Use dmaengine_prep_config_single_safe() API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Removing the mutex introduces a severe concurrency bug, leading to spurious channel-wide termination of DMA transfers and CPU starvation.
- [Low] The local variable `lock` is assigned but never used, resulting in a compiler warning.
--

commit 36bcfc6ae688edc18d4bf05e641f168d44b9db2c
Author: Frank Li <Frank.Li@nxp.com>

nvmet: pci-epf: Use dmaengine_prep_config_single_safe() API

This commit updates the driver to use the new
dmaengine_prep_config_single_safe() API, combining configuration and
descriptor preparation into a single call. It also removes the mutex lock
used during the transfer since the combined preparation is atomic.

> diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
> index 2afe8f4d0e461..04d8f48d69503 100644
> --- a/drivers/nvme/target/pci-epf.c
> +++ b/drivers/nvme/target/pci-epf.c
> @@ -388,22 +388,15 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
>  		return -EINVAL;
>  	}
>  
> -	mutex_lock(lock);
> -

Is the local variable lock still needed here? Since the mutex_lock() and
mutex_unlock() calls are removed, the earlier assignments of this variable
in the switch statement appear to be unused and might trigger a compiler
warning.

[ ... ]

> @@ -426,9 +419,6 @@ static int nvmet_pci_epf_dma_transfer(struct nvmet_pci_epf *nvme_epf,
>  unmap:
>  	dma_unmap_single(dma_dev, dma_addr, seg->length, dir);
>  
> -unlock:
> -	mutex_unlock(lock);
> -
>  	return ret;
>  }

Does removing this lock introduce concurrency issues with dma_sync_wait()
and the error handling path?

Looking at the remainder of nvmet_pci_epf_dma_transfer(), it calls:

    if (dma_sync_wait(chan, cookie) != DMA_COMPLETE) {
        dev_err(dev, "DMA transfer failed\n");
        ret = -EIO;
        dmaengine_terminate_sync(chan);
    }

If the mutex is removed, multiple threads could concurrently submit
transfers to the same DMA channel and enter dma_sync_wait(). Since
dma_sync_wait() polls for completion, could this lead to CPU starvation
from multiple threads busy-spinning concurrently?

Additionally, if one thread fails or times out in dma_sync_wait(), it
will call dmaengine_terminate_sync(chan). Will this indiscriminately
abort the in-flight transfers of all other concurrently executing threads
on the same channel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260506-dma_prep_config-v4-0-85b3d22babff@nxp.com?part=7