[v4,3/5] raw/ifpga: unregister interrupt in ifpga close function

Message ID 1653640433-5066-4-git-send-email-wei.huang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Support OFS card |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Wei Huang May 27, 2022, 8:33 a.m. UTC
  There is an API rte_pmd_ifpga_cleanup provided by ifpga driver to
free the software resource used by ifpga card. The function call
of rte_pmd_ifpga_cleanup is list below.
rte_pmd_ifpga_cleanup()
  ifpga_rawdev_cleanup()
     rte_rawdev_pmd_release()
       rte_rawdev_close()
         ifpga_rawdev_close()

The interrupts are unregistered in ifpga_rawdev_destroy instead of
ifpga_rawdev_close function, so rte_pmd_ifpga_cleanup cannot free
interrupt resource as expected.

To fix such issue, interrupt unregistration is moved from
ifpga_rawdev_destroy to ifpga_rawdev_close function. The change of
function call of ifpga_rawdev_destroy is as below.
ifpga_rawdev_destroy()
  ifpga_unregister_msix_irq()  // removed
  rte_rawdev_pmd_release()
    rte_rawdev_close()
      ifpga_rawdev_close()

Fixes: e0a1aafe2af9 ("raw/ifpga: introduce IRQ functions")
Cc: stable@dpdk.org

Signed-off-by: Wei Huang <wei.huang@intel.com>
Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
v2: update commit log with Tianfei's comment
---
v3: update commit log with more explanations about the function call
---
 drivers/raw/ifpga/ifpga_rawdev.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)
  

Comments

Xu, Rosen June 7, 2022, 6:03 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: Friday, May 27, 2022 16:34
> To: dev@dpdk.org; thomas@monjalon.net; nipun.gupta@nxp.com;
> hemant.agrawal@nxp.com
> Cc: stable@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Tianfei
> <tianfei.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Huang, Wei
> <wei.huang@intel.com>
> Subject: [PATCH v4 3/5] raw/ifpga: unregister interrupt in ifpga close function
> 
> There is an API rte_pmd_ifpga_cleanup provided by ifpga driver to free the
> software resource used by ifpga card. The function call of
> rte_pmd_ifpga_cleanup is list below.
> rte_pmd_ifpga_cleanup()
>   ifpga_rawdev_cleanup()
>      rte_rawdev_pmd_release()
>        rte_rawdev_close()
>          ifpga_rawdev_close()
> 
> The interrupts are unregistered in ifpga_rawdev_destroy instead of
> ifpga_rawdev_close function, so rte_pmd_ifpga_cleanup cannot free
> interrupt resource as expected.
> 
> To fix such issue, interrupt unregistration is moved from
> ifpga_rawdev_destroy to ifpga_rawdev_close function. The change of
> function call of ifpga_rawdev_destroy is as below.
> ifpga_rawdev_destroy()
>   ifpga_unregister_msix_irq()  // removed
>   rte_rawdev_pmd_release()
>     rte_rawdev_close()
>       ifpga_rawdev_close()
> 
> Fixes: e0a1aafe2af9 ("raw/ifpga: introduce IRQ functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
> ---
> v2: update commit log with Tianfei's comment
> ---
> v3: update commit log with more explanations about the function call
> ---
>  drivers/raw/ifpga/ifpga_rawdev.c | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c
> b/drivers/raw/ifpga/ifpga_rawdev.c
> index fe3fc43..94df56c 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -78,6 +78,7 @@ static int set_surprise_link_check_aer(  static int
> ifpga_pci_find_next_ext_capability(unsigned int fd,
>  					      int start, uint32_t cap);
>  static int ifpga_pci_find_ext_capability(unsigned int fd, uint32_t cap);
> +static void fme_interrupt_handler(void *param);
> 
>  struct ifpga_rawdev *
>  ifpga_rawdev_get(const struct rte_rawdev *rawdev) @@ -740,8 +741,9 @@
> static int set_surprise_link_check_aer(  {
>  	struct ifpga_rawdev *ifpga_rdev = NULL;
>  	struct opae_adapter *adapter;
> +	struct opae_manager *mgr;
>  	char *vdev_name = NULL;
> -	int i = 0;
> +	int i, ret = 0;
> 
>  	if (dev) {
>  		ifpga_rdev = ifpga_rawdev_get(dev);
> @@ -756,12 +758,19 @@ static int set_surprise_link_check_aer(
>  		}
>  		adapter = ifpga_rawdev_get_priv(dev);
>  		if (adapter) {
> +			mgr = opae_adapter_get_mgr(adapter);
> +			if (ifpga_rdev && mgr) {
> +				if (ifpga_unregister_msix_irq(ifpga_rdev,
> +					IFPGA_FME_IRQ, 0,
> +					fme_interrupt_handler, mgr) < 0)
> +					ret = -EINVAL;
> +			}
>  			opae_adapter_destroy(adapter);
>  			opae_adapter_data_free(adapter->data);
>  		}
>  	}
> 
> -	return dev ? 0:1;
> +	return ret;
>  }
> 
>  static int
> @@ -1629,9 +1638,6 @@ static int fme_clean_fme_error(struct
> opae_manager *mgr)
>  	int ret;
>  	struct rte_rawdev *rawdev;
>  	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!");
> @@ -1651,19 +1657,6 @@ static int fme_clean_fme_error(struct
> opae_manager *mgr)
>  		IFPGA_RAWDEV_PMD_ERR("Invalid device name (%s)",
> name);
>  		return -EINVAL;
>  	}
> -	dev = ifpga_rawdev_get(rawdev);
> -
> -	adapter = ifpga_rawdev_get_priv(rawdev);
> -	if (!adapter)
> -		return -ENODEV;
> -
> -	mgr = opae_adapter_get_mgr(adapter);
> -	if (!mgr)
> -		return -ENODEV;
> -
> -	if (ifpga_unregister_msix_irq(dev, IFPGA_FME_IRQ, 0,
> -				fme_interrupt_handler, mgr) < 0)
> -		return -EINVAL;
> 
>  	/* rte_rawdev_close is called by pmd_release */
>  	ret = rte_rawdev_pmd_release(rawdev);
> --
> 1.8.3.1

Reviewed-by: Rosen Xu <rosen.xu@intel.com>
  

Patch

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index fe3fc43..94df56c 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -78,6 +78,7 @@  static int set_surprise_link_check_aer(
 static int ifpga_pci_find_next_ext_capability(unsigned int fd,
 					      int start, uint32_t cap);
 static int ifpga_pci_find_ext_capability(unsigned int fd, uint32_t cap);
+static void fme_interrupt_handler(void *param);
 
 struct ifpga_rawdev *
 ifpga_rawdev_get(const struct rte_rawdev *rawdev)
@@ -740,8 +741,9 @@  static int set_surprise_link_check_aer(
 {
 	struct ifpga_rawdev *ifpga_rdev = NULL;
 	struct opae_adapter *adapter;
+	struct opae_manager *mgr;
 	char *vdev_name = NULL;
-	int i = 0;
+	int i, ret = 0;
 
 	if (dev) {
 		ifpga_rdev = ifpga_rawdev_get(dev);
@@ -756,12 +758,19 @@  static int set_surprise_link_check_aer(
 		}
 		adapter = ifpga_rawdev_get_priv(dev);
 		if (adapter) {
+			mgr = opae_adapter_get_mgr(adapter);
+			if (ifpga_rdev && mgr) {
+				if (ifpga_unregister_msix_irq(ifpga_rdev,
+					IFPGA_FME_IRQ, 0,
+					fme_interrupt_handler, mgr) < 0)
+					ret = -EINVAL;
+			}
 			opae_adapter_destroy(adapter);
 			opae_adapter_data_free(adapter->data);
 		}
 	}
 
-	return dev ? 0:1;
+	return ret;
 }
 
 static int
@@ -1629,9 +1638,6 @@  static int fme_clean_fme_error(struct opae_manager *mgr)
 	int ret;
 	struct rte_rawdev *rawdev;
 	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!");
@@ -1651,19 +1657,6 @@  static int fme_clean_fme_error(struct opae_manager *mgr)
 		IFPGA_RAWDEV_PMD_ERR("Invalid device name (%s)", name);
 		return -EINVAL;
 	}
-	dev = ifpga_rawdev_get(rawdev);
-
-	adapter = ifpga_rawdev_get_priv(rawdev);
-	if (!adapter)
-		return -ENODEV;
-
-	mgr = opae_adapter_get_mgr(adapter);
-	if (!mgr)
-		return -ENODEV;
-
-	if (ifpga_unregister_msix_irq(dev, IFPGA_FME_IRQ, 0,
-				fme_interrupt_handler, mgr) < 0)
-		return -EINVAL;
 
 	/* rte_rawdev_close is called by pmd_release */
 	ret = rte_rawdev_pmd_release(rawdev);