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 7E3A2A034F; Mon, 11 Oct 2021 12:41:05 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4446940150; Mon, 11 Oct 2021 12:41:05 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 925CC40142 for ; Mon, 11 Oct 2021 12:41:03 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10133"; a="290339347" X-IronPort-AV: E=Sophos;i="5.85,364,1624345200"; d="scan'208";a="290339347" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Oct 2021 03:40:46 -0700 X-IronPort-AV: E=Sophos;i="5.85,364,1624345200"; d="scan'208";a="716330091" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.13.30]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 11 Oct 2021 03:40:42 -0700 Date: Mon, 11 Oct 2021 11:40:39 +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> <20211009093340.43237-1-fengchengwen@huawei.com> <20211009093340.43237-4-fengchengwen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211009093340.43237-4-fengchengwen@huawei.com> Subject: Re: [dpdk-dev] [PATCH v24 3/6] dmadev: add data plane API support 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, Oct 09, 2021 at 05:33:37PM +0800, Chengwen Feng wrote: > This patch add data plane API for dmadev. > A few initial comments inline. I'll work on rebasing my follow-up patchset to this, and let you know if I have any more feedback based on that. /Bruce > diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c > index a6a5680d2b..891ceeb988 100644 > --- a/lib/dmadev/rte_dmadev.c > +++ b/lib/dmadev/rte_dmadev.c > @@ -17,6 +17,7 @@ > > static int16_t dma_devices_max; > > +struct rte_dma_fp_object *rte_dma_fp_objs; While I think I like this approach of making more of the dmadev hidden, I think we need a better name for this. While there is the dev_private pointer in it, the struct is pretty much the datapath functions, so how about "rte_dma_funcs" as a name? > struct rte_dma_dev *rte_dma_devices; > > +/** > + * @internal > + * Fast-path dmadev functions and related data are hold in a flat array. > + * One entry per dmadev. > + * > + * On 64-bit systems contents of this structure occupy exactly two 64B lines. > + * On 32-bit systems contents of this structure fits into one 64B line. > + * > + * The 'dev_private' field was placed in the first cache line to optimize > + * performance because the PMD driver mainly depends on this field. > + */ > +struct rte_dma_fp_object { > + void *dev_private; /**< PMD-specific private data. */ > + rte_dma_copy_t copy; > + rte_dma_copy_sg_t copy_sg; > + rte_dma_fill_t fill; > + rte_dma_submit_t submit; > + rte_dma_completed_t completed; > + rte_dma_completed_status_t completed_status; > + void *reserved_cl0; > + /** Reserve space for future IO functions, while keeping data and > + * dev_ops pointers on the second cacheline. > + */ This comment is out of date. > + void *reserved_cl1[6]; > +} __rte_cache_aligned; Small suggestion: since there is no data at the end of the structure, rather than adding in padding arrays which need to be adjusted as we add fields into the struct, let's just change the "__rte_cache_aligned" macro to "__rte_aligned(128)". This will explicitly set the size to 128-bytes and allow us to remove the reserved fields - making it easier to add new pointers. > + > +extern struct rte_dma_fp_object *rte_dma_fp_objs; > + > +#endif /* RTE_DMADEV_CORE_H */