Re: [PATCH] dmaengine: cookie bypass for out of order completion

[Vinod Koul <vkoul@kernel.org>]

Hi Dave,

On 30-04-20, 11:35, Dave Jiang wrote:
> The cookie tracking in dmaengine expects all submissions completed in

Correct and that is a *very* fundamental assumption of the cookie
management. Modifying this will cause impact to other as well..

> order. Some DMA devices like Intel DSA can complete submissions out of
> order, especially if configured with a work queue sharing multiple DMA
> engines. Add a status DMA_OUT_OF_ORDER that tx_status can be returned for

We should add this as a capability in dmaengine. How else would users
know if they can expect out of order completion..

> those DMA devices. The user should use callbacks to track the completion
> rather than the DMA cookie. This would address the issue of dmatest
> complaining that descriptors are "busy" when the cookie count goes
> backwards due to out of order completion.

Can we add some documentation for this behaviour as well

> 
> Reported-by: Swathi Kovvuri <swathi.kovvuri@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Tested-by: Swathi Kovvuri <swathi.kovvuri@intel.com>
> ---
>  drivers/dma/dmatest.c     |    3 ++-
>  drivers/dma/idxd/dma.c    |    2 +-
>  include/linux/dmaengine.h |    1 +
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index a2cadfa2e6d7..60a4a9cec3c8 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -821,7 +821,8 @@ static int dmatest_func(void *data)
>  			result("test timed out", total_tests, src->off, dst->off,
>  			       len, 0);
>  			goto error_unmap_continue;
> -		} else if (status != DMA_COMPLETE) {
> +		} else if (status != DMA_COMPLETE &&
> +			   status != DMA_OUT_OF_ORDER) {
>  			result(status == DMA_ERROR ?
>  			       "completion error status" :
>  			       "completion busy status", total_tests, src->off,
> diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
> index c64c1429d160..3f54826abc12 100644
> --- a/drivers/dma/idxd/dma.c
> +++ b/drivers/dma/idxd/dma.c
> @@ -133,7 +133,7 @@ static enum dma_status idxd_dma_tx_status(struct dma_chan *dma_chan,
>  					  dma_cookie_t cookie,
>  					  struct dma_tx_state *txstate)
>  {
> -	return dma_cookie_status(dma_chan, cookie, txstate);
> +	return DMA_OUT_OF_ORDER;

So you are returning out of order always?

>  }
>  
>  /*
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 21065c04c4ac..a0c130131e45 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -39,6 +39,7 @@ enum dma_status {
>  	DMA_IN_PROGRESS,
>  	DMA_PAUSED,
>  	DMA_ERROR,
> +	DMA_OUT_OF_ORDER,
>  };
>  
>  /**

-- 
~Vinod