| Message ID | 1601257218-6606-3-git-send-email-tianfei.zhang@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Qi Zhang |
| Headers | show |
| Series | raw/ifpga/base: An improvement for multi-process | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
Hi, > -----Original Message----- > From: Zhang, Tianfei <tianfei.zhang@intel.com> > Sent: Monday, September 28, 2020 9:40 > To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Huang, Wei > <wei.huang@intel.com> > Cc: Zhang, Tianfei <tianfei.zhang@intel.com> > Subject: [PATCH v2 2/4] raw/ifpga/base: free resources when destroying > ifpga device > > From: Wei Huang <wei.huang@intel.com> > > Add two functions to complete the resources free work, one is > ifpga_adapter_destroy(), the other is ifpga_bus_uinit(). > Then call opae_adapter_destroy() in ifpga_rawdev_destroy(). > > Additional modifiction is removing opae_adapter_free() from > ifpga_rawdev_destroy() because opae adapter will be released in > rte_rawdev_pmd_release(). > > Signed-off-by: Wei Huang <wei.huang@intel.com> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com> > --- > drivers/raw/ifpga/base/ifpga_api.c | 12 ++++++++++++ > drivers/raw/ifpga/base/ifpga_enumerate.c | 16 ++++++++++++++++ > drivers/raw/ifpga/base/ifpga_enumerate.h | 1 + > drivers/raw/ifpga/ifpga_rawdev.c | 8 ++++++-- > 4 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/drivers/raw/ifpga/base/ifpga_api.c > b/drivers/raw/ifpga/base/ifpga_api.c > index 6dbd7159e..1ff57fa18 100644 > --- a/drivers/raw/ifpga/base/ifpga_api.c > +++ b/drivers/raw/ifpga/base/ifpga_api.c > @@ -330,8 +330,20 @@ static int ifpga_adapter_enumerate(struct > opae_adapter *adapter) > return -ENOMEM; > } > > +static void ifpga_adapter_destroy(struct opae_adapter *adapter) { > + struct ifpga_fme_hw *fme; > + > + if (adapter && adapter->mgr && adapter->mgr->data) { > + fme = (struct ifpga_fme_hw *)adapter->mgr->data; > + if (fme->parent) > + ifpga_bus_uinit(fme->parent); > + } > +} > + > struct opae_adapter_ops ifpga_adapter_ops = { > .enumerate = ifpga_adapter_enumerate, > + .destroy = ifpga_adapter_destroy, > }; > > /** > diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c > b/drivers/raw/ifpga/base/ifpga_enumerate.c > index b8846e373..48b8af458 100644 > --- a/drivers/raw/ifpga/base/ifpga_enumerate.c > +++ b/drivers/raw/ifpga/base/ifpga_enumerate.c > @@ -722,3 +722,19 @@ int ifpga_bus_init(struct ifpga_hw *hw) > > return 0; > } > + > +int ifpga_bus_uinit(struct ifpga_hw *hw) { > + int i; > + struct ifpga_port_hw *port; > + > + if (hw) { > + fme_hw_uinit(&hw->fme); > + for (i = 0; i < MAX_FPGA_PORT_NUM; i++) { > + port = &hw->port[i]; > + port_hw_uinit(port); > + } > + } > + > + return 0; > +} > diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.h > b/drivers/raw/ifpga/base/ifpga_enumerate.h > index 14131e320..95ed594cd 100644 > --- a/drivers/raw/ifpga/base/ifpga_enumerate.h > +++ b/drivers/raw/ifpga/base/ifpga_enumerate.h > @@ -6,6 +6,7 @@ > #define _IFPGA_ENUMERATE_H_ > > int ifpga_bus_init(struct ifpga_hw *hw); > +int ifpga_bus_uinit(struct ifpga_hw *hw); > int ifpga_bus_enumerate(struct ifpga_hw *hw); > > #endif /* _IFPGA_ENUMERATE_H_ */ > diff --git a/drivers/raw/ifpga/ifpga_rawdev.c > b/drivers/raw/ifpga/ifpga_rawdev.c > index 374a7ff1d..98b02b5fa 100644 > --- a/drivers/raw/ifpga/ifpga_rawdev.c > +++ b/drivers/raw/ifpga/ifpga_rawdev.c > @@ -1535,6 +1535,7 @@ ifpga_rawdev_destroy(struct rte_pci_device > *pci_dev) > char name[RTE_RAWDEV_NAME_MAX_LEN]; > struct opae_adapter *adapter; > struct opae_manager *mgr; > + struct ifpga_rawdev *dev; > > if (!pci_dev) { > IFPGA_RAWDEV_PMD_ERR("Invalid pci_dev of the device!"); > @@ -1554,6 +1555,9 @@ ifpga_rawdev_destroy(struct rte_pci_device > *pci_dev) > IFPGA_RAWDEV_PMD_ERR("Invalid device name (%s)", > name); > return -EINVAL; > } > + dev = ifpga_rawdev_get(rawdev); > + if (dev) > + dev->rawdev = NULL; > > adapter = ifpga_rawdev_get_priv(rawdev); > if (!adapter) > @@ -1564,11 +1568,11 @@ ifpga_rawdev_destroy(struct rte_pci_device > *pci_dev) > return -ENODEV; > > if (ifpga_unregister_msix_irq(IFPGA_FME_IRQ, 0, > - fme_interrupt_handler, mgr)) > + fme_interrupt_handler, mgr) < 0) > return -EINVAL; > > + opae_adapter_destroy(adapter); > opae_adapter_data_free(adapter->data); > - opae_adapter_free(adapter); > > /* rte_rawdev_close is called by pmd_release */ > ret = rte_rawdev_pmd_release(rawdev); > -- > 2.17.1 Acked-by: Rosen Xu <rosen.xu@intel.com>
> -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Xu, Rosen > Sent: Tuesday, September 29, 2020 9:43 AM > To: Zhang, Tianfei <tianfei.zhang@intel.com>; dev@dpdk.org; Huang, Wei > <wei.huang@intel.com> > Subject: Re: [dpdk-dev] [PATCH v2 2/4] raw/ifpga/base: free resources when > destroying ifpga device > > Hi, > > > -----Original Message----- > > From: Zhang, Tianfei <tianfei.zhang@intel.com> > > Sent: Monday, September 28, 2020 9:40 > > To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Huang, Wei > > <wei.huang@intel.com> > > Cc: Zhang, Tianfei <tianfei.zhang@intel.com> > > Subject: [PATCH v2 2/4] raw/ifpga/base: free resources when destroying > > ifpga device > > > > From: Wei Huang <wei.huang@intel.com> > > > > Add two functions to complete the resources free work, one is > > ifpga_adapter_destroy(), the other is ifpga_bus_uinit(). > > Then call opae_adapter_destroy() in ifpga_rawdev_destroy(). > > > > Additional modifiction is removing opae_adapter_free() from > > ifpga_rawdev_destroy() because opae adapter will be released in > > rte_rawdev_pmd_release(). > > > > Signed-off-by: Wei Huang <wei.huang@intel.com> > > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com> > > --- > > drivers/raw/ifpga/base/ifpga_api.c | 12 ++++++++++++ > > drivers/raw/ifpga/base/ifpga_enumerate.c | 16 ++++++++++++++++ > > drivers/raw/ifpga/base/ifpga_enumerate.h | 1 + > > drivers/raw/ifpga/ifpga_rawdev.c | 8 ++++++-- > > 4 files changed, 35 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/raw/ifpga/base/ifpga_api.c > > b/drivers/raw/ifpga/base/ifpga_api.c > > index 6dbd7159e..1ff57fa18 100644 > > --- a/drivers/raw/ifpga/base/ifpga_api.c > > +++ b/drivers/raw/ifpga/base/ifpga_api.c > > @@ -330,8 +330,20 @@ static int ifpga_adapter_enumerate(struct > > opae_adapter *adapter) > > return -ENOMEM; > > } > > > > +static void ifpga_adapter_destroy(struct opae_adapter *adapter) { > > + struct ifpga_fme_hw *fme; > > + > > + if (adapter && adapter->mgr && adapter->mgr->data) { > > + fme = (struct ifpga_fme_hw *)adapter->mgr->data; > > + if (fme->parent) > > + ifpga_bus_uinit(fme->parent); > > + } > > +} > > + > > struct opae_adapter_ops ifpga_adapter_ops = { > > .enumerate = ifpga_adapter_enumerate, > > + .destroy = ifpga_adapter_destroy, > > }; > > > > /** > > diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c > > b/drivers/raw/ifpga/base/ifpga_enumerate.c > > index b8846e373..48b8af458 100644 > > --- a/drivers/raw/ifpga/base/ifpga_enumerate.c > > +++ b/drivers/raw/ifpga/base/ifpga_enumerate.c > > @@ -722,3 +722,19 @@ int ifpga_bus_init(struct ifpga_hw *hw) > > > > return 0; > > } > > + > > +int ifpga_bus_uinit(struct ifpga_hw *hw) { > > + int i; > > + struct ifpga_port_hw *port; > > + > > + if (hw) { > > + fme_hw_uinit(&hw->fme); > > + for (i = 0; i < MAX_FPGA_PORT_NUM; i++) { > > + port = &hw->port[i]; > > + port_hw_uinit(port); > > + } > > + } > > + > > + return 0; > > +} > > diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.h > > b/drivers/raw/ifpga/base/ifpga_enumerate.h > > index 14131e320..95ed594cd 100644 > > --- a/drivers/raw/ifpga/base/ifpga_enumerate.h > > +++ b/drivers/raw/ifpga/base/ifpga_enumerate.h > > @@ -6,6 +6,7 @@ > > #define _IFPGA_ENUMERATE_H_ > > > > int ifpga_bus_init(struct ifpga_hw *hw); > > +int ifpga_bus_uinit(struct ifpga_hw *hw); > > int ifpga_bus_enumerate(struct ifpga_hw *hw); > > > > #endif /* _IFPGA_ENUMERATE_H_ */ > > diff --git a/drivers/raw/ifpga/ifpga_rawdev.c > > b/drivers/raw/ifpga/ifpga_rawdev.c > > index 374a7ff1d..98b02b5fa 100644 > > --- a/drivers/raw/ifpga/ifpga_rawdev.c > > +++ b/drivers/raw/ifpga/ifpga_rawdev.c > > @@ -1535,6 +1535,7 @@ ifpga_rawdev_destroy(struct rte_pci_device > > *pci_dev) > > char name[RTE_RAWDEV_NAME_MAX_LEN]; > > struct opae_adapter *adapter; > > struct opae_manager *mgr; > > + struct ifpga_rawdev *dev; > > > > if (!pci_dev) { > > IFPGA_RAWDEV_PMD_ERR("Invalid pci_dev of the device!"); @@ > -1554,6 > > +1555,9 @@ ifpga_rawdev_destroy(struct rte_pci_device > > *pci_dev) > > IFPGA_RAWDEV_PMD_ERR("Invalid device name (%s)", name); > > return -EINVAL; > > } > > + dev = ifpga_rawdev_get(rawdev); > > + if (dev) > > + dev->rawdev = NULL; > > > > adapter = ifpga_rawdev_get_priv(rawdev); > > if (!adapter) > > @@ -1564,11 +1568,11 @@ ifpga_rawdev_destroy(struct rte_pci_device > > *pci_dev) > > return -ENODEV; > > > > if (ifpga_unregister_msix_irq(IFPGA_FME_IRQ, 0, > > - fme_interrupt_handler, mgr)) > > + fme_interrupt_handler, mgr) < 0) > > return -EINVAL; > > > > + opae_adapter_destroy(adapter); > > opae_adapter_data_free(adapter->data); > > - opae_adapter_free(adapter); > > > > /* rte_rawdev_close is called by pmd_release */ > > ret = rte_rawdev_pmd_release(rawdev); > > -- > > 2.17.1 > > Acked-by: Rosen Xu <rosen.xu@intel.com> Applied to dpdk-next-net-intel. Thanks Qi
On 9/28/2020 2:40 AM, Tianfei zhang wrote: > From: Wei Huang <wei.huang@intel.com> > > Add two functions to complete the resources free work, one > is ifpga_adapter_destroy(), the other is ifpga_bus_uinit(). > Then call opae_adapter_destroy() in ifpga_rawdev_destroy(). > > Additional modifiction is removing opae_adapter_free() from s/modifiction/modification > ifpga_rawdev_destroy() because opae adapter will be released > in rte_rawdev_pmd_release(). I can see following call stack, rte_rawdev_pmd_release() rte_rawdev_close() ifpga_rawdev_close() In this path 'opae_adapter_free()' is not called, can you please confirm if opae adapter free is done. > > Signed-off-by: Wei Huang <wei.huang@intel.com> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com> <...>
> -----Original Message----- > From: Ferruh Yigit <ferruh.yigit@intel.com> > Sent: 2020年10月16日 2:57 > To: Zhang, Tianfei <tianfei.zhang@intel.com>; dev@dpdk.org; Xu, Rosen > <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com> > Subject: Re: [dpdk-dev] [PATCH v2 2/4] raw/ifpga/base: free resources when > destroying ifpga device > > On 9/28/2020 2:40 AM, Tianfei zhang wrote: > > From: Wei Huang <wei.huang@intel.com> > > > > Add two functions to complete the resources free work, one is > > ifpga_adapter_destroy(), the other is ifpga_bus_uinit(). > > Then call opae_adapter_destroy() in ifpga_rawdev_destroy(). > > > > Additional modifiction is removing opae_adapter_free() from > > s/modifiction/modification > > > ifpga_rawdev_destroy() because opae adapter will be released in > > rte_rawdev_pmd_release(). > > I can see following call stack, > > rte_rawdev_pmd_release() > rte_rawdev_close() > ifpga_rawdev_close() > > In this path 'opae_adapter_free()' is not called, can you please confirm if opae > adapter free is done. opae_adapter_free() function is use to free the adapter data struct. But in rawdev framework, the rte_rawdev_pmd_release() will release this adapter because the adapter will pointer to rawdev->dev_private when we create the rawdev in ifpga_rawdev_create() . And in rte_rawdev_pmd_release(), it will free the rawdev->dev_private. > > > > > Signed-off-by: Wei Huang <wei.huang@intel.com> > > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com> > > <...>
diff --git a/drivers/raw/ifpga/base/ifpga_api.c b/drivers/raw/ifpga/base/ifpga_api.c index 6dbd7159e..1ff57fa18 100644 --- a/drivers/raw/ifpga/base/ifpga_api.c +++ b/drivers/raw/ifpga/base/ifpga_api.c @@ -330,8 +330,20 @@ static int ifpga_adapter_enumerate(struct opae_adapter *adapter) return -ENOMEM; } +static void ifpga_adapter_destroy(struct opae_adapter *adapter) +{ + struct ifpga_fme_hw *fme; + + if (adapter && adapter->mgr && adapter->mgr->data) { + fme = (struct ifpga_fme_hw *)adapter->mgr->data; + if (fme->parent) + ifpga_bus_uinit(fme->parent); + } +} + struct opae_adapter_ops ifpga_adapter_ops = { .enumerate = ifpga_adapter_enumerate, + .destroy = ifpga_adapter_destroy, }; /** diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c index b8846e373..48b8af458 100644 --- a/drivers/raw/ifpga/base/ifpga_enumerate.c +++ b/drivers/raw/ifpga/base/ifpga_enumerate.c @@ -722,3 +722,19 @@ int ifpga_bus_init(struct ifpga_hw *hw) return 0; } + +int ifpga_bus_uinit(struct ifpga_hw *hw) +{ + int i; + struct ifpga_port_hw *port; + + if (hw) { + fme_hw_uinit(&hw->fme); + for (i = 0; i < MAX_FPGA_PORT_NUM; i++) { + port = &hw->port[i]; + port_hw_uinit(port); + } + } + + return 0; +} diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.h b/drivers/raw/ifpga/base/ifpga_enumerate.h index 14131e320..95ed594cd 100644 --- a/drivers/raw/ifpga/base/ifpga_enumerate.h +++ b/drivers/raw/ifpga/base/ifpga_enumerate.h @@ -6,6 +6,7 @@ #define _IFPGA_ENUMERATE_H_ int ifpga_bus_init(struct ifpga_hw *hw); +int ifpga_bus_uinit(struct ifpga_hw *hw); int ifpga_bus_enumerate(struct ifpga_hw *hw); #endif /* _IFPGA_ENUMERATE_H_ */ diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index 374a7ff1d..98b02b5fa 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -1535,6 +1535,7 @@ ifpga_rawdev_destroy(struct rte_pci_device *pci_dev) char name[RTE_RAWDEV_NAME_MAX_LEN]; struct opae_adapter *adapter; struct opae_manager *mgr; + struct ifpga_rawdev *dev; if (!pci_dev) { IFPGA_RAWDEV_PMD_ERR("Invalid pci_dev of the device!"); @@ -1554,6 +1555,9 @@ ifpga_rawdev_destroy(struct rte_pci_device *pci_dev) IFPGA_RAWDEV_PMD_ERR("Invalid device name (%s)", name); return -EINVAL; } + dev = ifpga_rawdev_get(rawdev); + if (dev) + dev->rawdev = NULL; adapter = ifpga_rawdev_get_priv(rawdev); if (!adapter) @@ -1564,11 +1568,11 @@ ifpga_rawdev_destroy(struct rte_pci_device *pci_dev) return -ENODEV; if (ifpga_unregister_msix_irq(IFPGA_FME_IRQ, 0, - fme_interrupt_handler, mgr)) + fme_interrupt_handler, mgr) < 0) return -EINVAL; + opae_adapter_destroy(adapter); opae_adapter_data_free(adapter->data); - opae_adapter_free(adapter); /* rte_rawdev_close is called by pmd_release */ ret = rte_rawdev_pmd_release(rawdev);