From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (xvm-189-124.dc0.ghst.net [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3DA47A0524; Thu, 7 Jan 2021 13:37:44 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F11AF140F50; Thu, 7 Jan 2021 13:37:43 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mails.dpdk.org (Postfix) with ESMTP id 95E07140F4F for ; Thu, 7 Jan 2021 13:37:40 +0100 (CET) IronPort-SDR: CtRjJQfUNeXCwkeeXSOc5y28GjaYkJ4ZSvELNS6p3iu5JyFeXBl3kAv+Lk73Iuo8pSpTSGZd8M yCbepSf91f2Q== X-IronPort-AV: E=McAfee;i="6000,8403,9856"; a="176638896" X-IronPort-AV: E=Sophos;i="5.79,329,1602572400"; d="scan'208";a="176638896" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Jan 2021 04:37:39 -0800 IronPort-SDR: A6O+Pp1qPx/tf5DQ1KYLD2LleyDhr7MY6lCHDrHxzeI2B90k9FDsa5UJ7w6wYxVlJ5AyeEj6Dd Srp0jwDCdhLQ== X-IronPort-AV: E=Sophos;i="5.79,329,1602572400"; d="scan'208";a="351244603" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.5.128]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 07 Jan 2021 04:37:38 -0800 Date: Thu, 7 Jan 2021 12:37:35 +0000 From: Bruce Richardson To: Kumar Amber Cc: dev@dpdk.org Message-ID: <20210107123735.GA210@bricha3-MOBL.ger.corp.intel.com> References: <20210107050545.554653-1-kumar.amber@intel.com> <20210107122312.564619-1-kumar.amber@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210107122312.564619-1-kumar.amber@intel.com> Subject: Re: [dpdk-dev] [PATCH v7] raw/ioat: add secondary process 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 Thu, Jan 07, 2021 at 05:53:12PM +0530, Kumar Amber wrote: > Add support for secondary processes in ioat devices. The update > allocates a memzone for a primary process or returns it in a > secondary process. > > Signed-off-by: Kumar Amber > > --- > v5 > * add error check for memzone lookup > v6 > * fix compilation > v7 > * include dev ops for secondary > --- Two comments below. With those fixed, you can add my ack to v8. Acked-by: Bruce Richardson > drivers/raw/ioat/ioat_common.c | 22 ++++++++++++++++++++-- > drivers/raw/ioat/ioat_rawdev.c | 21 +++++++++++++++++++-- > 2 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/drivers/raw/ioat/ioat_common.c b/drivers/raw/ioat/ioat_common.c > index 142e171bc9..2428c8a5b6 100644 > --- a/drivers/raw/ioat/ioat_common.c > +++ b/drivers/raw/ioat/ioat_common.c > @@ -215,14 +215,32 @@ idxd_rawdev_create(const char *name, struct rte_device *dev, > goto cleanup; > } > > + /* Allocate memory for the primary process or else return the memory > + * of primary memzone for the secondary process. > + */ > snprintf(mz_name, sizeof(mz_name), "rawdev%u_private", rawdev->dev_id); > - mz = rte_memzone_reserve(mz_name, sizeof(struct idxd_rawdev), > - dev->numa_node, RTE_MEMZONE_IOVA_CONTIG); This line actually doesn't need to be changed, and in doing so, it's actually introduced a bug. The sizeof() call here, has "idxd_rawdev" while the replacement below is a copy-paste error using "ioat_rawdev". > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) { > + mz = rte_memzone_lookup(mz_name); > + if (mz == NULL) { > + IOAT_PMD_ERR("Unable lookup memzone for private data\n"); > + ret = -ENOMEM; > + goto cleanup; > + } > + rawdev->dev_private = mz->addr; > + rawdev->dev_ops = ops; > + rawdev->device = dev; > + return 0; > + } > + mz = rte_memzone_reserve(mz_name, > + sizeof(struct rte_ioat_rawdev), > + dev->numa_node, > + RTE_MEMZONE_IOVA_CONTIG); > if (mz == NULL) { > IOAT_PMD_ERR("Unable to reserve memzone for private data\n"); > ret = -ENOMEM; > goto cleanup; > } > + > rawdev->dev_private = mz->addr; > rawdev->dev_ops = ops; > rawdev->device = dev; > diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c > index 2c88b4369f..e152004219 100644 > --- a/drivers/raw/ioat/ioat_rawdev.c > +++ b/drivers/raw/ioat/ioat_rawdev.c > @@ -165,9 +165,26 @@ ioat_rawdev_create(const char *name, struct rte_pci_device *dev) > goto cleanup; > } > > + /* Allocate memory for the primary process or else return the memory > + * of primary memzone for the secondary process. > + */ > snprintf(mz_name, sizeof(mz_name), "rawdev%u_private", rawdev->dev_id); > - mz = rte_memzone_reserve(mz_name, sizeof(struct rte_ioat_rawdev), > - dev->device.numa_node, RTE_MEMZONE_IOVA_CONTIG); No bug introduced here, but again I think changing this line is unnecessary. > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) { > + mz = rte_memzone_lookup(mz_name); > + if (mz == NULL) { > + IOAT_PMD_ERR("Unable lookup memzone for private data\n"); > + ret = -ENOMEM; > + goto cleanup; > + } > + rawdev->dev_private = mz->addr; > + rawdev->dev_ops = &ioat_rawdev_ops; > + rawdev->device = &dev->device; > + return 0; > + } > + mz = rte_memzone_reserve(mz_name, > + sizeof(struct rte_ioat_rawdev), > + dev->device.numa_node, > + RTE_MEMZONE_IOVA_CONTIG); > if (mz == NULL) { > IOAT_PMD_ERR("Unable to reserve memzone for private data\n"); > ret = -ENOMEM; > -- > 2.25.1 >