[v3,3/5] raw/ifpga: unregister interrupt in ifpga close function
Checks
Commit Message
In original implementation, interrupts are unregistered in
ifpga_rawdev_destroy function. If application want to release
the ifpga driver resource by calling rte_rawdev_pmd_release,
interrupt resource will not be released.
Now interrupt unregistration is moved from ifpga destroy function
to ifpga close function, when rte_rawdev_pmd_release is called,
rte_rawdev_close will be called, then interrupts are unregistered.
Signed-off-by: Wei Huang <wei.huang@intel.com>
---
v2: update commit log
---
drivers/raw/ifpga/ifpga_rawdev.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
Comments
Hi Wei,
It's a little bit hard to find the connections between functions you changed and raw API.
Could you add more explanations about the Function Call in the functions you mentioned below?
Thanks,
Rosen
> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: Thursday, May 26, 2022 11:33
> 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 v3 3/5] raw/ifpga: unregister interrupt in ifpga close function
>
> In original implementation, interrupts are unregistered in
> ifpga_rawdev_destroy function. If application want to release the ifpga
> driver resource by calling rte_rawdev_pmd_release, interrupt resource will
> not be released.
> Now interrupt unregistration is moved from ifpga destroy function to ifpga
> close function, when rte_rawdev_pmd_release is called, rte_rawdev_close
> will be called, then interrupts are unregistered.
>
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
> v2: update commit log
> ---
> 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
> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: Thursday, May 26, 2022 11:33 AM
> 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 v3 3/5] raw/ifpga: unregister interrupt in ifpga close function
>
> In original implementation, interrupts are unregistered in ifpga_rawdev_destroy
> function. If application want to release the ifpga driver resource by calling
> rte_rawdev_pmd_release, interrupt resource will not be released.
> Now interrupt unregistration is moved from ifpga destroy function to ifpga close
> function, when rte_rawdev_pmd_release is called, rte_rawdev_close will be
> called, then interrupts are unregistered.
>
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
> v2: update commit log
> ---
> 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);
> --
Hi Wei,
It looks good for me, you can add:
Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
@@ -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);