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@xxxxxxxxx>
> > > Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> > > Tested-by: Swathi Kovvuri <swathi.kovvuri@xxxxxxxxx>
> > > ---
> > > 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
