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 1B2B1A0548; Mon, 20 Sep 2021 15:31:56 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9093240DF7; Mon, 20 Sep 2021 15:31:55 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id E45A440DF5 for ; Mon, 20 Sep 2021 15:31:53 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10112"; a="223164655" X-IronPort-AV: E=Sophos;i="5.85,308,1624345200"; d="scan'208";a="223164655" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Sep 2021 06:31:52 -0700 X-IronPort-AV: E=Sophos;i="5.85,308,1624345200"; d="scan'208";a="511364084" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.17.156]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 20 Sep 2021 06:31:50 -0700 Date: Mon, 20 Sep 2021 14:31:47 +0100 From: Bruce Richardson To: Conor Walsh Cc: fengchengwen@huawei.com, jerinj@marvell.com, kevin.laatz@intel.com, dev@dpdk.org Message-ID: References: <20210827172550.1522362-1-conor.walsh@intel.com> <20210917154227.737554-1-conor.walsh@intel.com> <20210917154227.737554-3-conor.walsh@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210917154227.737554-3-conor.walsh@intel.com> Subject: Re: [dpdk-dev] [PATCH v4 02/11] dma/ioat: create dmadev instances on PCI probe 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 Fri, Sep 17, 2021 at 03:42:18PM +0000, Conor Walsh wrote: > When a suitable device is found during the PCI probe, create a dmadev > instance for each channel. Internal structures and HW definitions required > for device creation are also included. > > Signed-off-by: Conor Walsh > Reviewed-by: Kevin Laatz > --- > drivers/dma/ioat/ioat_dmadev.c | 119 ++++++++++++++++++++++++++++++- > drivers/dma/ioat/ioat_hw_defs.h | 45 ++++++++++++ > drivers/dma/ioat/ioat_internal.h | 24 +++++++ > 3 files changed, 186 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/ioat/ioat_dmadev.c b/drivers/dma/ioat/ioat_dmadev.c > index f3491d45b1..b815d30bcf 100644 > --- a/drivers/dma/ioat/ioat_dmadev.c > +++ b/drivers/dma/ioat/ioat_dmadev.c > @@ -4,6 +4,7 @@ > > +/* Destroy a DMA device. */ > +static int > +ioat_dmadev_destroy(const char *name) > +{ > + struct rte_dma_dev *dev; > + struct ioat_dmadev *ioat; > + int ret; > + > + if (!name) { > + IOAT_PMD_ERR("Invalid device name"); > + return -EINVAL; > + } > + > + dev = &rte_dma_devices[rte_dma_get_dev_id(name)]; > + if (!dev) { > + IOAT_PMD_ERR("Invalid device name (%s)", name); > + return -EINVAL; > + } > + I think you need to independently check the return value from rte_dma_get_dev_id, rather than assuming when it returns an error value the resultant index location will hold a null pointer. > + ioat = dev->dev_private; > + if (!ioat) { > + IOAT_PMD_ERR("Error getting dev_private"); > + return -EINVAL; > + } > + > + dev->dev_private = NULL; > + rte_free(ioat->desc_ring); > + > + ret = rte_dma_pmd_release(name); The rte_dma_pmd_allocate function reserves memory for the private data, so the release function should free that memory too. However, you have assigned private_data to NULL just above, so that probably won't work. > + if (ret) > + IOAT_PMD_DEBUG("Device cleanup failed"); > + > + return 0; > +} > +