DPDK patches and discussions
 help / color / mirror / Atom feed
From: Hyong Youb Kim <hyonkim@cisco.com>
To: David Marchand <david.marchand@redhat.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>
Cc: dev@dpdk.org, John Daley <johndale@cisco.com>,
	Hyong Youb Kim <hyonkim@cisco.com>
Subject: [dpdk-dev] [RFC PATCH] vfio: avoid re-installing irq handler
Date: Mon, 15 Jul 2019 08:58:12 -0700	[thread overview]
Message-ID: <20190715155812.10124-1-hyonkim@cisco.com> (raw)

A rough patch for the approach mentioned earlier. It is only for
discussion.
http://mails.dpdk.org/archives/dev/2019-July/138113.html

To try this out, first revert the following then apply.
commit 89aac60e0be9 ("vfio: fix interrupts race condition")

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 .../common/include/rte_eal_interrupts.h       |  1 +
 lib/librte_eal/linux/eal/eal_interrupts.c     | 68 ++++++++++++++++++-
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..d509967d3 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -76,6 +76,7 @@ struct rte_intr_handle {
 	enum rte_intr_handle_type type;  /**< handle type */
 	uint32_t max_intr;             /**< max interrupt requested */
 	uint32_t nb_efd;               /**< number of available efd(event fd) */
+	uint8_t vfio_efd_assigned;
 	uint8_t efd_counter_size;      /**< size of efd counter, used for vdev */
 	int efds[RTE_MAX_RXTX_INTR_VEC_ID];  /**< intr vectors/efds mapping */
 	struct rte_epoll_event elist[RTE_MAX_RXTX_INTR_VEC_ID];
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 79ad5e8d7..5b03388dd 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -107,9 +107,21 @@ static pthread_t intr_thread;
 #define MSIX_IRQ_SET_BUF_LEN (sizeof(struct vfio_irq_set) + \
 			      sizeof(int) * (RTE_MAX_RXTX_INTR_VEC_ID + 1))
 
-/* enable legacy (INTx) interrupts */
+#ifndef __INTEL_COMPILER
+#pragma GCC diagnostic ignored "-Wcast-qual"
+#endif
+
+static void
+set_vfio_efd_assigned(const struct rte_intr_handle *intr_handle, uint8_t v) {
+	((struct rte_intr_handle *)intr_handle)->vfio_efd_assigned = v;
+}
+
+#ifndef __INTEL_COMPILER
+#pragma GCC diagnostic error "-Wcast-qual"
+#endif
+
 static int
-vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
+vfio_assign_efd_intx(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	char irq_set_buf[IRQ_SET_BUF_LEN];
 	int len, ret;
@@ -134,8 +146,36 @@ vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
 						intr_handle->fd);
 		return -1;
 	}
+	/*
+	 * In the kernel, irq has been set up, its handler installed (request_irq),
+	 * and efd is assigned to it.  Remember this so we can avoid
+	 * re-doing irq setup later.
+	 */
+	set_vfio_efd_assigned(intr_handle, 1);
+	return 0;
+}
+
+/* enable legacy (INTx) interrupts */
+static int
+vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
+	struct vfio_irq_set *irq_set;
+	char irq_set_buf[IRQ_SET_BUF_LEN];
+	int len, ret;
+
+	/*
+	 * Assign efd to irq only if it's not been done, to avoid
+	 * re-installing irq handler in the kernel.
+	 */
+	if (!intr_handle->vfio_efd_assigned) {
+		ret = vfio_assign_efd_intx(intr_handle);
+		if (ret)
+			return ret;
+	}
+
+	len = sizeof(irq_set_buf);
 
 	/* unmask INTx after enabling */
+	irq_set = (struct vfio_irq_set *) irq_set_buf;
 	memset(irq_set, 0, len);
 	len = sizeof(struct vfio_irq_set);
 	irq_set->argsz = len;
@@ -194,6 +234,12 @@ vfio_disable_intx(const struct rte_intr_handle *intr_handle) {
 			"Error disabling INTx interrupts for fd %d\n", intr_handle->fd);
 		return -1;
 	}
+	/*
+	 * Irq handler's been removed in the kernel. The next time we
+	 * enable intr_handle, we need to assign efd to irq. Remember
+	 * that.
+	 */
+	set_vfio_efd_assigned(intr_handle, 0);
 	return 0;
 }
 
@@ -205,6 +251,13 @@ vfio_enable_msi(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	int *fd_ptr;
 
+	/*
+	 * vfio does not support MSI masking/unmaking. So if irq has
+	 * been set up previously, nothing to do.
+	 */
+	if (intr_handle->vfio_efd_assigned)
+		return 0;
+
 	len = sizeof(irq_set_buf);
 
 	irq_set = (struct vfio_irq_set *) irq_set_buf;
@@ -223,6 +276,7 @@ vfio_enable_msi(const struct rte_intr_handle *intr_handle) {
 						intr_handle->fd);
 		return -1;
 	}
+	set_vfio_efd_assigned(intr_handle, 1);
 	return 0;
 }
 
@@ -248,6 +302,7 @@ vfio_disable_msi(const struct rte_intr_handle *intr_handle) {
 		RTE_LOG(ERR, EAL,
 			"Error disabling MSI interrupts for fd %d\n", intr_handle->fd);
 
+	set_vfio_efd_assigned(intr_handle, 0);
 	return ret;
 }
 
@@ -259,6 +314,13 @@ vfio_enable_msix(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	int *fd_ptr;
 
+	/*
+	 * vfio does not support MSI-X masking/unmaking. So if irq has
+	 * been set up previously, nothing to do.
+	 */
+	if (intr_handle->vfio_efd_assigned)
+		return 0;
+
 	len = sizeof(irq_set_buf);
 
 	irq_set = (struct vfio_irq_set *) irq_set_buf;
@@ -284,6 +346,7 @@ vfio_enable_msix(const struct rte_intr_handle *intr_handle) {
 		return -1;
 	}
 
+	set_vfio_efd_assigned(intr_handle, 1);
 	return 0;
 }
 
@@ -309,6 +372,7 @@ vfio_disable_msix(const struct rte_intr_handle *intr_handle) {
 		RTE_LOG(ERR, EAL,
 			"Error disabling MSI-X interrupts for fd %d\n", intr_handle->fd);
 
+	set_vfio_efd_assigned(intr_handle, 0);
 	return ret;
 }
 
-- 
2.22.0


             reply	other threads:[~2019-07-15 15:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-15 15:58 Hyong Youb Kim [this message]
2019-07-15 16:50 Jerin Jacob Kollanukkaran
2019-07-16  5:58 ` Hyong Youb Kim (hyonkim)
2019-07-16  6:47   ` Jerin Jacob Kollanukkaran
2019-07-16  7:49     ` Hyong Youb Kim (hyonkim)
2019-07-16  9:56       ` Jerin Jacob Kollanukkaran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190715155812.10124-1-hyonkim@cisco.com \
    --to=hyonkim@cisco.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jerinj@marvell.com \
    --cc=johndale@cisco.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).