Re: [dpdk-dev] [PATCH v4] dmadev: introduce DMA device library

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Jul 16, 2021 at 11:04:30AM +0800, fengchengwen wrote:
> On 2021/7/16 0:33, Bruce Richardson wrote:
> > On Fri, Jul 16, 2021 at 12:04:33AM +0800, fengchengwen wrote:
> >> @burce, jerin  Some unmodified review comments are returned here:
> >>
> 
> [snip]
> 
> > 
> >> 2.  COMMENT: > + * @see struct rte_dmadev_info::dev_capa
> >>> + */
> >> Drop this flag as unnecessary. All devices either always provide ordering
> >> guarantee - in which case it's a no-op - or else support the flag.
> >>
> >> REPLY: I prefer define it, it could let user know whether support fence.
> >>
> > I don't see it that way. The flag is pointless because the application
> > can't use it to make any decisions. If two operations require ordering the
> > application must use the fence flag because if the device doesn't guarantee
> > ordering it's necessary, and if the device does guarantee ordering it's
> > better and easier to just specify the flag than to put in code branches.
> > Having this as a capability is just going to confuse the user - better to
> > just say that if you need ordering, put in a fence.
> > 
> 
> If driver don't support fence, and application set the fence flag, What's
> driving behavior like? return error or implement fence at driver layer ?
> 
The simple matter is that all devices must support ordering. Therefore all
devices must support the fence flag. If a device does all jobs in-order
anyway, then fence flag can be ignored, while if jobs are done in parallel
or out-of-order, then fence flag must be respected. A driver should never
return error just because a fence flag is set.

> If expose the fence capability to application, then application could decide
> which option to use. e.g. commit the operations before the fence and make sure it completes,
> or use another collaborative approach.
> 
> I think in this manner, the driver implementation can be simplified.
> 
What you are describing is not a fence capability, but a limitation of the
hardware to enforce ordering. Even if we don't have fence, there is no
guarantee that one burst of jobs committed will complete before the next is
started as some HW can do batches in parallel in some circumstances.

As far as I know, all currently proposed HW using this API either works
in-order or supports fencing, so this capability flag is unnecessary. I
suggest we omit it until it is needed - at which point we can re-open the
discussion based on a concrete usecase.

If you *really* want to have the flag, I suggest:
* have one for fencing doesn't order, i.e. fence flag is ignored and the
  device does not work in-order
* make it clear in the documentation that even if fencing doesn't order
  it's still not an error to put in a fence flag - it will just be ignored.

/Bruce