From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 69D59A0C52; Fri, 16 Jul 2021 11:50:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 55C4B4134C; Fri, 16 Jul 2021 11:50:33 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 6A72A40151 for ; Fri, 16 Jul 2021 11:50:31 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10046"; a="271821108" X-IronPort-AV: E=Sophos;i="5.84,244,1620716400"; d="scan'208";a="271821108" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Jul 2021 02:50:29 -0700 X-IronPort-AV: E=Sophos;i="5.84,244,1620716400"; d="scan'208";a="460696453" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.26.216]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 16 Jul 2021 02:50:25 -0700 Date: Fri, 16 Jul 2021 10:50:20 +0100 From: Bruce Richardson To: fengchengwen Cc: thomas@monjalon.net, ferruh.yigit@intel.com, jerinj@marvell.com, jerinjacobk@gmail.com, andrew.rybchenko@oktetlabs.ru, dev@dpdk.org, mb@smartsharesystems.com, nipun.gupta@nxp.com, hemant.agrawal@nxp.com, maxime.coquelin@redhat.com, honnappa.nagarahalli@arm.com, david.marchand@redhat.com, sburla@marvell.com, pkapoor@marvell.com, konstantin.ananyev@intel.com Message-ID: References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <1626363661-51103-1-git-send-email-fengchengwen@huawei.com> <0aa82990-566b-f471-129b-31fbc2410ecc@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0aa82990-566b-f471-129b-31fbc2410ecc@huawei.com> Subject: Re: [dpdk-dev] [PATCH v4] dmadev: introduce DMA device library X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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