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

[Vinod Koul <vkoul@kernel.org>]

Hi Dave,

On 04-05-20, 07:59, Dave Jiang wrote:
> On 5/3/2020 10:39 PM, Vinod Koul wrote:
> > 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..
> 
> The current modification only impacts drivers that opt out of this. So all
> existing or future driver does not return the out of order flag should
> remain unimpacted.
> 
> > 
> > > 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..
> 
> Hmmm...this is more an attribute of the hardware rather than a capability
> that a user would request right? Should we add a new function that would
> provide an avenue for users to query the device on such attributes and
> others like channel depth or SGL max size?

Well we have capabilities of hw reported so that users knows. This
should also be a capability reported

> > > 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
> 
> sure thing.
> 
> > 
> > > 
> > > 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?
> 
> Yes. The hardware does not gaurantee in order processing at all. The only
> way to do so is to submit a batched operation with fence set on every
> descriptor in the batch.
> 
> > 
> > >   }
> > >   /*
> > > 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