[RFC 0/3] fix bogus interrupt FD leak warnings

[RFC 0/3] fix bogus interrupt FD leak warnings

[Stephen Hemminger]

Running testpmd with an i40e device on 26.11-rc1 prints a set of FD
leak warnings on port close and on exit:

  Closing ports...
  EAL: Some interrupt handle is leaking a FD: 19
  EAL: Some interrupt handle is leaking a device FD: 18
  Port 0 is closed
  ...
  Bye...
  EAL: Some interrupt handle is leaking a FD: 20
  EAL: Some interrupt handle is leaking a device FD: 18
  EAL: Some interrupt handle is leaking a device FD: 18
  EAL: Some interrupt handle is leaking a FD: 7
  EAL: Some interrupt handle is leaking a FD: 7

The warning is new in 26.11 (commit d770ee9caed0, "interrupts: warn on
leaked FDs"). It is doing its job - one of these is a real leak - but
most of what it reports here is not, and the noise will get reported as
a regression.

There are three distinct causes.

The interrupt source created by rte_intr_callback_register() keeps a
copy of the caller's handle. The copy carries the caller's fd and
dev_fd, but does not own them, so freeing the source complains about
fds that the caller still holds and will close itself.

The VFIO device fd is closed by dev_vfio_release_device() on unmap,
but is left behind in the interrupt handle, so freeing the handle later
reports an fd that is already closed. The VFIO request handle holds the
same fd and has the same problem.

The alarm timerfd is a real leak: it is never closed on EAL cleanup.
FreeBSD was already fixed for this; Linux was not.

Patches 1 and 2 address the bogus warnings, patch 3 the actual leak.
With all three applied the same testpmd run shuts down cleanly with no
warnings.

Open questions:

- Patch 1 clears the fds in the source's copy of the handle. Clearing
  them at dup time, or not duplicating the fds into the source at all,
  may be the better fix.
- Patch 2 resets dev_fd at the two unmap sites. Doing it inside
  dev_vfio_release_device(), next to the close, would cover any other
  caller.

Stephen Hemminger (3):
  eal/linux: do not warn when freeing interrupt source copy
  bus/pci: reset VFIO device FD after release
  eal/linux: close alarm timerfd on cleanup

 drivers/bus/pci/linux/pci_vfio.c |  3 +++
 lib/eal/linux/eal_alarm.c        |  1 +
 lib/eal/linux/eal_interrupts.c   | 18 +++++++++++++++---
 3 files changed, 19 insertions(+), 3 deletions(-)

--
2.51.0

Stephen Hemminger (3):
  eal/linux: do not warn when freeing interrupt source copy
  bus/pci: reset VFIO device FD after release
  eal/linux: close alarm timerfd on cleanup

 drivers/bus/pci/linux/pci_vfio.c |  3 +++
 lib/eal/linux/eal_alarm.c        |  1 +
 lib/eal/linux/eal_interrupts.c   | 18 +++++++++++++++---
 3 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.53.0

[RFC 1/3] eal/linux: do not warn when freeing interrupt source copy

[Stephen Hemminger]

The interrupt source keeps a copy of the caller's handle made with
rte_intr_instance_dup(). The copy carries the caller's fd and dev_fd,
which it does not own. Freeing it on callback unregister now logs a
bogus FD leak warning.

Clear the fds in the copy before freeing it.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/linux/eal_interrupts.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index 4cfaeba7fe..d4752a2b03 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -479,6 +479,18 @@ uio_intr_enable(const struct rte_intr_handle *intr_handle)
 	return 0;
 }
 
+/*
+ * The source holds a copy of the caller's handle; the fds are owned
+ * by the caller, so drop them before freeing the copy.
+ */
+static void
+eal_intr_source_free_handle(struct rte_intr_source *src)
+{
+	rte_intr_fd_set(src->intr_handle, -1);
+	rte_intr_dev_fd_set(src->intr_handle, -1);
+	rte_intr_instance_free(src->intr_handle);
+}
+
 RTE_EXPORT_SYMBOL(rte_intr_callback_register)
 int
 rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
@@ -666,7 +678,7 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 		/* all callbacks for that source are removed. */
 		if (TAILQ_EMPTY(&src->callbacks)) {
 			TAILQ_REMOVE(&intr_sources, src, next);
-			rte_intr_instance_free(src->intr_handle);
+			eal_intr_source_free_handle(src);
 			free(src);
 		}
 	}
@@ -922,7 +934,7 @@ eal_intr_source_remove_and_free(struct rte_intr_source *src)
 	}
 
 	/* Free the interrupt source */
-	rte_intr_instance_free(src->intr_handle);
+	eal_intr_source_free_handle(src);
 	free(src);
 }
 
@@ -1067,7 +1079,7 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 		/* all callbacks for that source are removed. */
 		if (TAILQ_EMPTY(&src->callbacks)) {
 			TAILQ_REMOVE(&intr_sources, src, next);
-			rte_intr_instance_free(src->intr_handle);
+			eal_intr_source_free_handle(src);
 			free(src);
 		}
 
-- 
2.53.0

[RFC 2/3] bus/pci: reset VFIO device FD after release

[Stephen Hemminger]

The VFIO device fd is closed by dev_vfio_release_device() but is left
in the interrupt handle, so freeing the handle on cleanup logs a bogus
FD leak warning.

Mark dev_fd invalid after release. In secondary process the request
handle also holds the device fd, reset it too.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/bus/pci/linux/pci_vfio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index fb35bc1d49..7ee8fe809a 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -1103,6 +1103,7 @@ pci_vfio_unmap_resource_primary(struct rte_pci_device *dev)
 		PCI_LOG(ERR, "Cannot release VFIO device");
 		return ret;
 	}
+	rte_intr_dev_fd_set(dev->intr_handle, -1);
 
 	vfio_res_list =
 		RTE_TAILQ_CAST(dev_vfio_tailq.head, mapped_pci_res_list);
@@ -1142,6 +1143,8 @@ pci_vfio_unmap_resource_secondary(struct rte_pci_device *dev)
 		PCI_LOG(ERR, "Cannot release VFIO device");
 		return ret;
 	}
+	rte_intr_dev_fd_set(dev->intr_handle, -1);
+	rte_intr_dev_fd_set(dev->vfio_req_intr_handle, -1);
 
 	vfio_res_list =
 		RTE_TAILQ_CAST(dev_vfio_tailq.head, mapped_pci_res_list);
-- 
2.53.0

[RFC 3/3] eal/linux: close alarm timerfd on cleanup

[Stephen Hemminger]

The timerfd used for alarms is never closed on EAL cleanup.
FreeBSD was fixed to close it, do the same on Linux.

Fixes: 90b13ab8d4f7 ("alarm: remove direct access to interrupt handle")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/linux/eal_alarm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c
index a1433eb867..5f8655ea11 100644
--- a/lib/eal/linux/eal_alarm.c
+++ b/lib/eal/linux/eal_alarm.c
@@ -61,6 +61,7 @@ rte_eal_alarm_cleanup(void)
 	int ret = rte_intr_callback_unregister_sync(intr_handle,
 						eal_alarm_callback, (void *)-1);
 	if (ret >= 0) {
+		rte_intr_fd_close(intr_handle);
 		rte_intr_instance_free(intr_handle);
 		intr_handle = NULL;
 		handler_registered = 0;
-- 
2.53.0