Let's warn on interrupt handle that still reference what looks like a valid file descriptor.
This will likely raise false positives, but it should help limiting FD leaks on the long term. Add missing reset to -1 in drivers, and document code. Signed-off-by: David Marchand <[email protected]> --- Changes since RFC v2: - split code comments added by patch 3 in the RFC v2, - added a check on leftover FD in a interrupt handle, --- app/test/test_interrupts.c | 5 ++++- drivers/bus/cdx/cdx_vfio.c | 4 ++++ drivers/bus/pci/linux/pci_vfio.c | 2 ++ drivers/common/mlx5/linux/mlx5_common_os.c | 6 +++++- drivers/net/mana/mana.c | 1 + drivers/net/mlx4/mlx4_intr.c | 1 + drivers/net/sxe2/sxe2_irq.c | 5 ++++- drivers/net/tap/rte_eth_tap.c | 1 + drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 ++ drivers/raw/cnxk_gpio/cnxk_gpio.c | 3 +++ drivers/vdpa/mlx5/mlx5_vdpa_event.c | 1 + drivers/vdpa/mlx5/mlx5_vdpa_virtq.c | 1 + lib/eal/common/eal_common_interrupts.c | 6 ++++++ 13 files changed, 35 insertions(+), 3 deletions(-) diff --git a/app/test/test_interrupts.c b/app/test/test_interrupts.c index 67f1e6429a..a737a9cd51 100644 --- a/app/test/test_interrupts.c +++ b/app/test/test_interrupts.c @@ -130,8 +130,11 @@ test_interrupt_deinit(void) { int i; - for (i = 0; i < TEST_INTERRUPT_HANDLE_MAX; i++) + for (i = 0; i < TEST_INTERRUPT_HANDLE_MAX; i++) { + rte_intr_fd_set(intr_handles[i], -1); + rte_intr_dev_fd_set(intr_handles[i], -1); rte_intr_instance_free(intr_handles[i]); + } close(pfds.pipefd[0]); close(pfds.pipefd[1]); diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c index 8d67058bfe..0c1b263a85 100644 --- a/drivers/bus/cdx/cdx_vfio.c +++ b/drivers/bus/cdx/cdx_vfio.c @@ -116,6 +116,8 @@ cdx_vfio_unmap_resource_primary(struct rte_cdx_device *dev) vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle); if (vfio_dev_fd < 0) return -1; + /* vfio_dev_fd is owned by VFIO, only clear reference here. */ + rte_intr_dev_fd_set(dev->intr_handle, -1); ret = rte_vfio_release_device(RTE_CDX_BUS_DEVICES_PATH, dev->device.name, vfio_dev_fd); @@ -150,6 +152,8 @@ cdx_vfio_unmap_resource_secondary(struct rte_cdx_device *dev) vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle); if (vfio_dev_fd < 0) return -1; + /* vfio_dev_fd is owned by VFIO, only clear reference here. */ + rte_intr_dev_fd_set(dev->intr_handle, -1); ret = rte_vfio_release_device(RTE_CDX_BUS_DEVICES_PATH, dev->device.name, vfio_dev_fd); diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c index 84c338c68e..b5f05e9f9f 100644 --- a/drivers/bus/pci/linux/pci_vfio.c +++ b/drivers/bus/pci/linux/pci_vfio.c @@ -356,6 +356,7 @@ pci_vfio_enable_notifier(struct rte_pci_device *dev, int vfio_dev_fd) error: rte_intr_fd_close(dev->vfio_req_intr_handle); rte_intr_type_set(dev->vfio_req_intr_handle, RTE_INTR_HANDLE_UNKNOWN); + /* vfio_dev_fd is managed by VFIO layer, only clear reference here. */ rte_intr_dev_fd_set(dev->vfio_req_intr_handle, -1); return -1; @@ -383,6 +384,7 @@ pci_vfio_disable_notifier(struct rte_pci_device *dev) rte_intr_fd_close(dev->vfio_req_intr_handle); rte_intr_type_set(dev->vfio_req_intr_handle, RTE_INTR_HANDLE_UNKNOWN); + /* vfio_dev_fd is managed by VFIO layer, only clear reference here. */ rte_intr_dev_fd_set(dev->vfio_req_intr_handle, -1); return 0; diff --git a/drivers/common/mlx5/linux/mlx5_common_os.c b/drivers/common/mlx5/linux/mlx5_common_os.c index 3e9cd86062..09d37f32cb 100644 --- a/drivers/common/mlx5/linux/mlx5_common_os.c +++ b/drivers/common/mlx5/linux/mlx5_common_os.c @@ -1108,6 +1108,7 @@ mlx5_os_interrupt_handler_create(int mode, bool set_fd_nonblock, int fd, } return tmp_intr_handle; err: + rte_intr_fd_set(tmp_intr_handle, -1); rte_intr_instance_free(tmp_intr_handle); return NULL; } @@ -1181,8 +1182,11 @@ void mlx5_os_interrupt_handler_destroy(struct rte_intr_handle *intr_handle, rte_intr_callback_fn cb, void *cb_arg) { - if (rte_intr_fd_get(intr_handle) >= 0) + if (rte_intr_fd_get(intr_handle) >= 0) { mlx5_intr_callback_unregister(intr_handle, cb, cb_arg); + /* fd is not owned by the driver, only clear reference here. */ + rte_intr_fd_set(intr_handle, -1); + } rte_intr_instance_free(intr_handle); } diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c index 1864ba2a2b..f51598ea68 100644 --- a/drivers/net/mana/mana.c +++ b/drivers/net/mana/mana.c @@ -1964,6 +1964,7 @@ mana_intr_install(struct rte_eth_dev *eth_dev, struct mana_priv *priv) mana_intr_handler, priv); if (ret) { DRV_LOG(ERR, "Failed to register intr callback"); + /* fd is owned by ibverbs, only clear reference here. */ rte_intr_fd_set(priv->intr_handle, -1); goto free_intr; } diff --git a/drivers/net/mlx4/mlx4_intr.c b/drivers/net/mlx4/mlx4_intr.c index 01057482ec..c278dd5ecc 100644 --- a/drivers/net/mlx4/mlx4_intr.c +++ b/drivers/net/mlx4/mlx4_intr.c @@ -265,6 +265,7 @@ mlx4_intr_uninstall(struct mlx4_priv *priv) (void (*)(void *)) mlx4_interrupt_handler, priv); + /* fd is owned by ibverbs, only clear reference here. */ if (rte_intr_fd_set(priv->intr_handle, -1)) return -rte_errno; } diff --git a/drivers/net/sxe2/sxe2_irq.c b/drivers/net/sxe2/sxe2_irq.c index 3306504761..c30a26d94e 100644 --- a/drivers/net/sxe2/sxe2_irq.c +++ b/drivers/net/sxe2/sxe2_irq.c @@ -377,8 +377,11 @@ static void sxe2_intr_handler_destroy(struct rte_intr_handle *intr_handle, if (!intr_handle) return; - if (rte_intr_fd_get(intr_handle) >= 0) + if (rte_intr_fd_get(intr_handle) >= 0) { (void)rte_intr_callback_unregister(intr_handle, cb, cb_arg); + /* fd is not owned by the driver, only clear reference here. */ + rte_intr_fd_set(intr_handle, -1); + } rte_intr_instance_free(intr_handle); } diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c index 13114edba5..693be653ba 100644 --- a/drivers/net/tap/rte_eth_tap.c +++ b/drivers/net/tap/rte_eth_tap.c @@ -1947,6 +1947,7 @@ tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set) if (rte_intr_fd_get(pmd->intr_handle) >= 0) { tap_nl_final(rte_intr_fd_get(pmd->intr_handle)); + /* fd is not owned by the driver, only clear reference here. */ rte_intr_fd_set(pmd->intr_handle, -1); } diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index f3df73c1f0..f89c871beb 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -858,6 +858,8 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev) { struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->hw.port_id]; + /* fd is not owned by the driver, only clear reference here. */ + rte_intr_fd_set(eth_dev->intr_handle, -1); rte_intr_instance_free(eth_dev->intr_handle); eth_dev->intr_handle = NULL; diff --git a/drivers/raw/cnxk_gpio/cnxk_gpio.c b/drivers/raw/cnxk_gpio/cnxk_gpio.c index 0549e326f9..2b5ea0bf9f 100644 --- a/drivers/raw/cnxk_gpio/cnxk_gpio.c +++ b/drivers/raw/cnxk_gpio/cnxk_gpio.c @@ -442,6 +442,8 @@ cnxk_gpio_unregister_irq(struct cnxk_gpio *gpio) if (ret) return ret; + /* fd is owned by gpio, only clear reference here. */ + rte_intr_fd_set(gpio->intr.intr_handle, -1); rte_intr_instance_free(gpio->intr.intr_handle); gpio->intr.intr_handle = NULL; @@ -635,6 +637,7 @@ cnxk_gpio_register_irq_compat(struct cnxk_gpio *gpio, struct cnxk_gpio_irq *irq, return 0; out: + rte_intr_fd_set(intr_handle, -1); rte_intr_instance_free(intr_handle); return ret; diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_event.c b/drivers/vdpa/mlx5/mlx5_vdpa_event.c index dee2ccc23d..d18cf7d7e5 100644 --- a/drivers/vdpa/mlx5/mlx5_vdpa_event.c +++ b/drivers/vdpa/mlx5/mlx5_vdpa_event.c @@ -487,6 +487,7 @@ mlx5_vdpa_err_event_unset(struct mlx5_vdpa_priv *priv) rte_pause(); } } + /* fd is owned by err_chnl, only clear reference here. */ rte_intr_fd_set(priv->err_intr_handle, -1); if (priv->err_chnl) { #ifdef HAVE_IBV_DEVX_EVENT diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c b/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c index 093cdd08d2..9cad7e1df4 100644 --- a/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c +++ b/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c @@ -96,6 +96,7 @@ mlx5_vdpa_virtq_unregister_intr_handle(struct mlx5_vdpa_virtq *virtq) pthread_mutex_lock(&virtq->virtq_lock); } } + /* fd is owned by vhost, only clear reference here. */ (void)rte_intr_fd_set(virtq->intr_handle, -1); } rte_intr_instance_free(virtq->intr_handle); diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c index 8176eb089a..6368ef1771 100644 --- a/lib/eal/common/eal_common_interrupts.c +++ b/lib/eal/common/eal_common_interrupts.c @@ -187,6 +187,12 @@ void rte_intr_instance_free(struct rte_intr_handle *intr_handle) { if (intr_handle == NULL) return; + if (rte_intr_fd_get(intr_handle) >= 0) + EAL_LOG(NOTICE, "Some interrupt handle is leaking a FD: %d", + rte_intr_fd_get(intr_handle)); + if (rte_intr_dev_fd_get(intr_handle) >= 0) + EAL_LOG(NOTICE, "Some interrupt handle is leaking a device FD: %d", + rte_intr_dev_fd_get(intr_handle)); if (RTE_INTR_INSTANCE_USES_RTE_MEMORY(intr_handle->alloc_flags)) { rte_free(intr_handle->efds); rte_free(intr_handle->elist); -- 2.54.0
