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 736CAA0547; Thu, 9 Sep 2021 13:29:40 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 318B840041; Thu, 9 Sep 2021 13:29:40 +0200 (CEST) Received: from new3-smtp.messagingengine.com (new3-smtp.messagingengine.com [66.111.4.229]) by mails.dpdk.org (Postfix) with ESMTP id 8333D4003E for ; Thu, 9 Sep 2021 13:29:39 +0200 (CEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.nyi.internal (Postfix) with ESMTP id D7D22580ABB; Thu, 9 Sep 2021 07:29:38 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Thu, 09 Sep 2021 07:29:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm2; bh= As2SkrF+Izd455hD4DUyKCJNH2+khWTVl9g6WfwQvGk=; b=oH5h4DE3PeslsiOK HlHxDUrzMhLAgMxZz1qXymIidIBnUeumAoKWZ25jYLGmdi+sUpfv2UKTBwoiGHog 50GVarcd5/8IQggeo9yhIDZjKLSJmcqRi7MSnWk+wWEH/ujnkGjzFBP2zqDq3Ce5 TDZ6DoHBoeGI5yWoiDdq5VgPDt7/haW4AgTq81TYeW0PvEnXuCVb/6lbXGd5nVxt yXRk7sqP06xj5an3OV/L4BbeVRscDAkTJLrwq0IWx9ihexoN/3SIj5dOcOXebgn6 f/M2R1AHgUoXbfhA8eYE24r91oBFc+i6Ip4llu6W5NDz9uYIm89CAqRV5AUapdQp IwBrRQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=As2SkrF+Izd455hD4DUyKCJNH2+khWTVl9g6WfwQv Gk=; b=oOa84BpOlIyiAOSpuY/uWDuwsyUduWPIJcU2FuCZF34arVoJw+gb+CIkC F5DLvgWroAIn0p4FRlQxmdZ4HdyC/2NVn+oSo81jp0JhkC0OstbPvv3KW51DbR6/ 76miu0HIcxyZ2ekdnbQXN+or+MFPgjNLem6yR3tB7plbPuWZlagDZri1mFUnyFLi mRt7WpVSC8OKliPM2L84osOr6+dcBdIZt455EQWVrMBJInnYtzyqtL6jsN4wEnkL HImZsDG4WgcRRX+zVJ07yf8Z794q1Wl1unrRXjZ6wCXq6rxM+1CWbfyUggcoG/Gh Vl6IlwWuLGJW1iWOSi6Jh15desAog== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudefledggedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 9 Sep 2021 07:29:34 -0400 (EDT) From: Thomas Monjalon To: Bruce Richardson , Chengwen Feng Cc: 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, conor.walsh@intel.com, kevin.laatz@intel.com Date: Thu, 09 Sep 2021 13:29:33 +0200 Message-ID: <1912226.bRRTmGCQLz@thomas> In-Reply-To: References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <1855788.BnNMhiXu0z@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v21 1/7] dmadev: introduce DMA device library public APIs 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" 09/09/2021 13:18, Bruce Richardson: > On Thu, Sep 09, 2021 at 12:33:00PM +0200, Thomas Monjalon wrote: > > 07/09/2021 14:56, Chengwen Feng: > > > + * The first three APIs are used to submit the operation request to the virtual > > > + * DMA channel, if the submission is successful, an uint16_t ring_idx is > > > + * returned, otherwise a negative number is returned. > > > > unsigned or negative? looks weird. > > May be, but it works well. We could perhaps rephase to make it less weird > though: > "if the submission is successful, a positive ring_idx <= UINT16_MAX is > returned, otherwise a negative number is returned." I am advocating for int16_t, it makes a lot of things simpler. > > > + * > > > + * The last API was used to issue doorbell to hardware, and also there are flags > > > + * (@see RTE_DMA_OP_FLAG_SUBMIT) parameter of the first three APIs could do the > > > + * same work. > > > > I don't understand this sentence. > > You mean rte_dmadev_submit function? > > Why past tense "was"? > > Why having a redundant function? > > > > Just because there are two ways to do something does not mean that one of > them is redundant, as both may be more suitable for different situations. I agree. > When enqueuing a set of jobs to the device, having a separate submit > outside a loop makes for clearer code than having a check for the last > iteration inside the loop to set a special submit flag. However, for cases > where one item alone is to be submitted or there is a small set of jobs to > be submitted sequentially, having a submit flag provides a lower-overhead > way of doing the submission while still keeping the code clean. This kind of explanation may be missing in doxygen? > > > +bool > > > +rte_dmadev_is_valid_dev(uint16_t dev_id); > > > > I would suggest dropping the final "_dev" in the function name. > > > > The alternative, which I would support, is replacing "rte_dmadev" with > "rte_dma" across the API. This would then become "rte_dma_is_valid_dev" > which is clearer, since the dev is not part of the standard prefix. It also > would fit in with a possible future function of "rte_dma_is_valid_vchan" > for instance. Yes The question is whether it would make sense to reserver rte_dma_ prefix for some DMA functions which would be outside of dmadev lib? If you think that all DMA functions will be in dmadev, then yes we can shorten the prefix to rte_dma_. > > > +uint16_t > > > +rte_dmadev_count(void); > > > > It would be safer to name it rte_dmadev_count_avail > > in case we need new kind of device count later. > > > > If we change "dmadev" to "dma" this could then be > "rte_dma_count_avail_dev". Yes > > > +/** > > > + * A structure used to retrieve the information of a DMA device. > > > + */ > > > +struct rte_dmadev_info { > > > + struct rte_device *device; /**< Generic Device information. */ > > > > Please do not expose this. > > > > > + uint64_t dev_capa; /**< Device capabilities (RTE_DMADEV_CAPA_*). */ > > > + uint16_t max_vchans; > > > + /**< Maximum number of virtual DMA channels supported. */ > > > + uint16_t max_desc; > > > + /**< Maximum allowed number of virtual DMA channel descriptors. */ > > > + uint16_t min_desc; > > > + /**< Minimum allowed number of virtual DMA channel descriptors. */ > > > + uint16_t max_sges; > > > + /**< Maximum number of source or destination scatter-gather entry > > > + * supported. > > > + * If the device does not support COPY_SG capability, this value can be > > > + * zero. > > > + * If the device supports COPY_SG capability, then rte_dmadev_copy_sg() > > > + * parameter nb_src/nb_dst should not exceed this value. > > > + */ > > > + uint16_t nb_vchans; /**< Number of virtual DMA channel configured. */ > > > > What about adding NUMA node? > > > > /* Local NUMA memory ID. -1 if unknown. */ > > int16_t numa_node; > > > > That was omitted as it could be got through the device structure. If device > is removed, we need to ensure all fields needed from device, such as numa > node, are made available here. Ah yes, forgot about rte_device :) Yes please remove rte_device from this struct.