On Fri, Jul 16, 2021 at 3:20 PM Bruce Richardson <bruce.richard...@intel.com> wrote: > > 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.
Since we have only two class of devices 1) By default fence is there between transfers 2) Need to explicitly add a fence. Since there is no device that does not support the fence. IMO, As Burce suggested we don't need a fence flag. In class (1) devices, for fastpath functions, RTE_DMA_OP_FLAG_FENCE will be NOP. > > /Bruce