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 76EFBA0C43; Mon, 6 Sep 2021 15:35:36 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3A9F6410EF; Mon, 6 Sep 2021 15:35:36 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mails.dpdk.org (Postfix) with ESMTP id 07962410ED for ; Mon, 6 Sep 2021 15:35:34 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10099"; a="219662053" X-IronPort-AV: E=Sophos;i="5.85,272,1624345200"; d="scan'208";a="219662053" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Sep 2021 06:35:34 -0700 X-IronPort-AV: E=Sophos;i="5.85,272,1624345200"; d="scan'208";a="537043337" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.3.77]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 06 Sep 2021 06:35:30 -0700 Date: Mon, 6 Sep 2021 14:35:26 +0100 From: Bruce Richardson To: Chengwen Feng 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, conor.walsh@intel.com, kevin.laatz@intel.com Message-ID: References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <20210904101027.43252-1-fengchengwen@huawei.com> <20210904101027.43252-3-fengchengwen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20210904101027.43252-3-fengchengwen@huawei.com> Subject: Re: [dpdk-dev] [PATCH v20 2/7] dmadev: introduce DMA device library internal header 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 Sat, Sep 04, 2021 at 06:10:22PM +0800, Chengwen Feng wrote: > This patch introduce DMA device library internal header, which contains > internal data types that are used by the DMA devices in order to expose > their ops to the class. > > Signed-off-by: Chengwen Feng > Acked-by: Bruce Richardson > Acked-by: Morten Brørup > Reviewed-by: Kevin Laatz > Reviewed-by: Conor Walsh > --- > +struct rte_dmadev { > + void *dev_private; > + /**< PMD-specific private data. > + * > + * - If is the primary process, after dmadev allocated by > + * rte_dmadev_pmd_allocate(), the PCI/SoC device probing should > + * initialize this field, and copy it's value to the 'dev_private' > + * field of 'struct rte_dmadev_data' which pointer by 'data' filed. > + * > + * - If is the secondary process, dmadev framework will initialize this > + * field by copy from 'dev_private' field of 'struct rte_dmadev_data' > + * which initialized by primary process. > + * > + * @note It's the primary process responsibility to deinitialize this > + * field after invoke rte_dmadev_pmd_release() in the PCI/SoC device > + * removing stage. > + */ > + rte_dmadev_copy_t copy; > + rte_dmadev_copy_sg_t copy_sg; > + rte_dmadev_fill_t fill; > + rte_dmadev_submit_t submit; > + rte_dmadev_completed_t completed; > + rte_dmadev_completed_status_t completed_status; > + void *reserved_ptr[7]; /**< Reserved for future IO function. */ This is new in this set, I think. I assume that 7 was chosen so that we have the "data" pointer and the "dev_ops" pointers on the second cacheline (if 64-byte CLs)? If so, I wonder if we can find a good way to express that in the code or in the comments? The simplest - and probably as clear as any - is to split this into "void *__reserved_cl0" and "void *__reserved_cl1[6]" to show that it is split across the two cachelines, with the latter having comment: "Reserve space for future IO functions, while keeping data and dev_ops pointers on the second cacheline" If we don't mind using a slightly different type the magic "6" could be changed to a computation: char __reserved_cl1[RTE_CACHELINE_SZ - sizeof(void *) * 2]; However, for simplicity, I think the magic 6 can be kept, and just split into reserved_cl0 and reserved_cl1 as I suggest above. /Bruce