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 62F8BA0547; Thu, 9 Sep 2021 16:26:46 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DA060406B4; Thu, 9 Sep 2021 16:26:45 +0200 (CEST) Received: from new4-smtp.messagingengine.com (new4-smtp.messagingengine.com [66.111.4.230]) by mails.dpdk.org (Postfix) with ESMTP id 0F1D84003E for ; Thu, 9 Sep 2021 16:26:45 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailnew.nyi.internal (Postfix) with ESMTP id 8F5D7580807; Thu, 9 Sep 2021 10:26:44 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 09 Sep 2021 10:26:44 -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= x95FEJzj74DSevpi+It//CxJjSME0QNKR7cuQ3XVFuM=; b=qCrzgqY/DeD1r0yu tcXH3McX8t4jhd2GlmJSzCFT/0VyzjX64zNvMa1l3i8KWCQ+CVrc9EL4QwsqPaBZ sfE+n20CzoBmbp1ULyXJ3pM6mHicRJNE0/JMOeTSEPyKoLmtwY4PEkd9MRjvVJY3 GDKnl6P+vKDAGlf++KRFvSW78CVWWNn69KcpaPLpi+Bs+CjR/LnqO1WrBwjO3R+E uicR+FYZbgB1lmzvBbiTtDy9DV5FyQIPFCakSteAGkkk4NKQOz+Vri3W/XvnifX0 bahvTNhOwCMQHJ4ZQTcagbquwM/kWbyKBBoUtB59QVjh00wGNYlPxqP4T5grShrk t8P5Iw== 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=x95FEJzj74DSevpi+It//CxJjSME0QNKR7cuQ3XVF uM=; b=IrsYDt3U/mSB0uvB+sBWBELZMJO8SazMbDhMSPtAh/P1XbE4NoE0ysLCx uUsL1H7XPx/AXdqq7tMIMvlx7GPCYYvoxXlOGv7pTeE+gK7BSY+oKhHMjv9yDfkr caJktCjK0MPCRFqAvX5I+4TMmHNLNmO86ovqSycJEy3q7uTrWHoGq2EisE5ohq5g DUM68ZRJOx0hNU3TbvpeDBX07fIFqkbV9pu2kZWTgMXpVEmI5YM1YucDhpWeEO7i pNBmwwUF6zNWxPWOI1oyMnPC07LQkndabNbfQrmkEGxRGM/JQ9LbtTSfPmZDmbfu /4xOfymd129/yc32ftRrM5kgaN9Fg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudefledgjeejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 9 Sep 2021 10:26:41 -0400 (EDT) From: Thomas Monjalon To: Bruce Richardson , fengchengwen Cc: dev@dpdk.org, 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 16:26:40 +0200 Message-ID: <1860027.RmYFm2ALpc@thomas> In-Reply-To: <4108b879-a365-67db-72bb-1deb8440fa73@huawei.com> References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <4108b879-a365-67db-72bb-1deb8440fa73@huawei.com> 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 15:54, fengchengwen: > On 2021/9/9 20:45, Bruce Richardson wrote: > > On Thu, Sep 09, 2021 at 01:29:33PM +0200, Thomas Monjalon wrote: > >> 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. > > > > No, it doesn't work as you can't have wrap-around of the IDs once you use > > signed values - and that impacts both the end app and the internals of the > > drivers. Let's keep it as-is otherwise it will have massive impacts - > > including potential perf impacts. Not sure to understand what you mean. Please could you explain what does not work and what is the perf impact? I guess you want unsigned index for rings, then OK. For device ID however, I believe signed integer is useful. [...] > >>>>> +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_. > >> > > > > Well, any DPDK dma functions which are not in dma library should have the > > prefix of the library they are in e.g. rte_eal_dma_*, rte_pci_dma_* Quite often, we skip the eal_ prefix, that's why I was thinking about a possible namespace conflict. Anyway it could be managed. > > Therefore, I don't think name conflicts should be an issue, and I like > > having less typing to do in function names (and I believe Morten was > > strongly proposing this previously too) > > The dmadev is rather short, if change I prefer all public API with rte_dma_ prefix, > and don't have rte_dma_dev_ prefix for such start/stop/close. (ps: the rte_eth_ also > have rte_eth_dev_close which is painful for OCD). Yes OK for rte_dma_ prefix everywhere. > Also should the filename(e.g. rte_dmadev.h) and directory-name(lib/dmadev) also change ? I believe it's better to keep dmadev as name of the lib and filename, so it's consistent with other device classes. What are the other opinions?