[v6,9/9] interrupts: extend event list

Message ID 20211024200449.12024-10-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series make rte_intr_handle internal |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-unit-testing fail Testing issues
ci/Intel-compilation fail Compilation issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-x86_64-compile-testing fail Testing issues

Commit Message

David Marchand Oct. 24, 2021, 8:04 p.m. UTC
  From: Harman Kalra <hkalra@marvell.com>

Dynamically allocating the efds and elist array os intr_handle
structure, based on size provided by user. Eg size can be
MSIX interrupts supported by a PCI device.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v5:
- split from patch5,

---
 drivers/bus/pci/linux/pci_vfio.c       |   6 ++
 drivers/common/cnxk/roc_platform.h     |   1 +
 lib/eal/common/eal_common_interrupts.c | 119 ++++++++++++++++++++++++-
 lib/eal/common/eal_interrupts.h        |   5 +-
 4 files changed, 126 insertions(+), 5 deletions(-)
  

Comments

Dmitry Kozlyuk Oct. 25, 2021, 10:49 a.m. UTC | #1
Hi David,

With some nits below,
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

2021-10-24 22:04 (UTC+0200), David Marchand:
> From: Harman Kalra <hkalra@marvell.com>
> 
> Dynamically allocating the efds and elist array os intr_handle

Typo: "os" -> "of"

> structure, based on size provided by user. Eg size can be
> MSIX interrupts supported by a PCI device.
> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v5:
> - split from patch5,
> 
> ---
[...]
> diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c
> index 3285c4335f..7feb9da8fa 100644
> --- a/lib/eal/common/eal_common_interrupts.c
> +++ b/lib/eal/common/eal_common_interrupts.c
[...]
>  int rte_intr_fd_set(struct rte_intr_handle *intr_handle, int fd)
> @@ -239,6 +330,12 @@ int rte_intr_efds_index_get(const struct rte_intr_handle *intr_handle,
>  {
>  	CHECK_VALID_INTR_HANDLE(intr_handle);
>  
> +	if (intr_handle->efds == NULL) {
> +		RTE_LOG(ERR, EAL, "Event fd list not allocated\n");
> +		rte_errno = EFAULT;
> +		goto fail;
> +	}
> +

Here and below:
The check for `nb_intr` will already catch not allocated `efds`,
because `nb_intr` is necessarily 0 in this case.

>  	if (index >= intr_handle->nb_intr) {
>  		RTE_LOG(ERR, EAL, "Invalid index %d, max limit %d\n", index,
>  			intr_handle->nb_intr);
> @@ -256,6 +353,12 @@ int rte_intr_efds_index_set(struct rte_intr_handle *intr_handle,
>  {
>  	CHECK_VALID_INTR_HANDLE(intr_handle);
>  
> +	if (intr_handle->efds == NULL) {
> +		RTE_LOG(ERR, EAL, "Event fd list not allocated\n");
> +		rte_errno = EFAULT;
> +		goto fail;
> +	}
> +
>  	if (index >= intr_handle->nb_intr) {
>  		RTE_LOG(ERR, EAL, "Invalid size %d, max limit %d\n", index,
>  			intr_handle->nb_intr);
> @@ -275,6 +378,12 @@ struct rte_epoll_event *rte_intr_elist_index_get(
>  {
>  	CHECK_VALID_INTR_HANDLE(intr_handle);
>  
> +	if (intr_handle->elist == NULL) {
> +		RTE_LOG(ERR, EAL, "Event list not allocated\n");
> +		rte_errno = EFAULT;
> +		goto fail;
> +	}
> +
>  	if (index >= intr_handle->nb_intr) {
>  		RTE_LOG(ERR, EAL, "Invalid index %d, max limit %d\n", index,
>  			intr_handle->nb_intr);
> @@ -292,6 +401,12 @@ int rte_intr_elist_index_set(struct rte_intr_handle *intr_handle,
>  {
>  	CHECK_VALID_INTR_HANDLE(intr_handle);
>  
> +	if (intr_handle->elist == NULL) {
> +		RTE_LOG(ERR, EAL, "Event list not allocated\n");
> +		rte_errno = EFAULT;
> +		goto fail;
> +	}
> +
>  	if (index >= intr_handle->nb_intr) {
>  		RTE_LOG(ERR, EAL, "Invalid index %d, max limit %d\n", index,
>  			intr_handle->nb_intr);
[...]
  
David Marchand Oct. 25, 2021, 11:11 a.m. UTC | #2
On Mon, Oct 25, 2021 at 12:49 PM Dmitry Kozlyuk
<dmitry.kozliuk@gmail.com> wrote:
> > diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c
> > index 3285c4335f..7feb9da8fa 100644
> > --- a/lib/eal/common/eal_common_interrupts.c
> > +++ b/lib/eal/common/eal_common_interrupts.c
> [...]
> >  int rte_intr_fd_set(struct rte_intr_handle *intr_handle, int fd)
> > @@ -239,6 +330,12 @@ int rte_intr_efds_index_get(const struct rte_intr_handle *intr_handle,
> >  {
> >       CHECK_VALID_INTR_HANDLE(intr_handle);
> >
> > +     if (intr_handle->efds == NULL) {
> > +             RTE_LOG(ERR, EAL, "Event fd list not allocated\n");
> > +             rte_errno = EFAULT;
> > +             goto fail;
> > +     }
> > +
>
> Here and below:
> The check for `nb_intr` will already catch not allocated `efds`,
> because `nb_intr` is necessarily 0 in this case.

+1.
Thanks Dmitry.
  

Patch

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 7b2f8296c5..f622e7f8e6 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -266,6 +266,12 @@  pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
 			return -1;
 		}
 
+		/* Reallocate the efds and elist fields of intr_handle based
+		 * on PCI device MSIX size.
+		 */
+		if (rte_intr_event_list_update(dev->intr_handle, irq.count))
+			return -1;
+
 		/* if this vector cannot be used with eventfd, fail if we explicitly
 		 * specified interrupt type, otherwise continue */
 		if ((irq.flags & VFIO_IRQ_INFO_EVENTFD) == 0) {
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index 60227b72d0..5da23fe5f8 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -121,6 +121,7 @@ 
 #define plt_intr_instance_alloc		rte_intr_instance_alloc
 #define plt_intr_instance_dup		rte_intr_instance_dup
 #define plt_intr_instance_free		rte_intr_instance_free
+#define plt_intr_event_list_update	rte_intr_event_list_update
 #define plt_intr_max_intr_get		rte_intr_max_intr_get
 #define plt_intr_max_intr_set		rte_intr_max_intr_set
 #define plt_intr_nb_efd_get		rte_intr_nb_efd_get
diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c
index 3285c4335f..7feb9da8fa 100644
--- a/lib/eal/common/eal_common_interrupts.c
+++ b/lib/eal/common/eal_common_interrupts.c
@@ -53,10 +53,46 @@  struct rte_intr_handle *rte_intr_instance_alloc(uint32_t flags)
 		return NULL;
 	}
 
+	if (uses_rte_memory) {
+		intr_handle->efds = rte_zmalloc(NULL,
+			RTE_MAX_RXTX_INTR_VEC_ID * sizeof(int), 0);
+	} else {
+		intr_handle->efds = calloc(RTE_MAX_RXTX_INTR_VEC_ID,
+			sizeof(int));
+	}
+	if (intr_handle->efds == NULL) {
+		RTE_LOG(ERR, EAL, "Fail to allocate event fd list\n");
+		rte_errno = ENOMEM;
+		goto fail;
+	}
+
+	if (uses_rte_memory) {
+		intr_handle->elist = rte_zmalloc(NULL,
+			RTE_MAX_RXTX_INTR_VEC_ID * sizeof(struct rte_epoll_event),
+			0);
+	} else {
+		intr_handle->elist = calloc(RTE_MAX_RXTX_INTR_VEC_ID,
+			sizeof(struct rte_epoll_event));
+	}
+	if (intr_handle->elist == NULL) {
+		RTE_LOG(ERR, EAL, "fail to allocate event fd list\n");
+		rte_errno = ENOMEM;
+		goto fail;
+	}
+
 	intr_handle->alloc_flags = flags;
 	intr_handle->nb_intr = RTE_MAX_RXTX_INTR_VEC_ID;
 
 	return intr_handle;
+fail:
+	if (uses_rte_memory) {
+		rte_free(intr_handle->efds);
+		rte_free(intr_handle);
+	} else {
+		free(intr_handle->efds);
+		free(intr_handle);
+	}
+	return NULL;
 }
 
 struct rte_intr_handle *rte_intr_instance_dup(const struct rte_intr_handle *src)
@@ -83,14 +119,69 @@  struct rte_intr_handle *rte_intr_instance_dup(const struct rte_intr_handle *src)
 	return intr_handle;
 }
 
+int rte_intr_event_list_update(struct rte_intr_handle *intr_handle, int size)
+{
+	struct rte_epoll_event *tmp_elist;
+	bool uses_rte_memory;
+	int *tmp_efds;
+
+	CHECK_VALID_INTR_HANDLE(intr_handle);
+
+	if (size == 0) {
+		RTE_LOG(ERR, EAL, "Size can't be zero\n");
+		rte_errno = EINVAL;
+		goto fail;
+	}
+
+	uses_rte_memory =
+		RTE_INTR_INSTANCE_USES_RTE_MEMORY(intr_handle->alloc_flags);
+	if (uses_rte_memory) {
+		tmp_efds = rte_realloc(intr_handle->efds, size * sizeof(int),
+			0);
+	} else {
+		tmp_efds = realloc(intr_handle->efds, size * sizeof(int));
+	}
+	if (tmp_efds == NULL) {
+		RTE_LOG(ERR, EAL, "Failed to realloc the efds list\n");
+		rte_errno = ENOMEM;
+		goto fail;
+	}
+	intr_handle->efds = tmp_efds;
+
+	if (uses_rte_memory) {
+		tmp_elist = rte_realloc(intr_handle->elist,
+			size * sizeof(struct rte_epoll_event), 0);
+	} else {
+		tmp_elist = realloc(intr_handle->elist,
+			size * sizeof(struct rte_epoll_event));
+	}
+	if (tmp_elist == NULL) {
+		RTE_LOG(ERR, EAL, "Failed to realloc the event list\n");
+		rte_errno = ENOMEM;
+		goto fail;
+	}
+	intr_handle->elist = tmp_elist;
+
+	intr_handle->nb_intr = size;
+
+	return 0;
+fail:
+	return -rte_errno;
+}
+
 void rte_intr_instance_free(struct rte_intr_handle *intr_handle)
 {
 	if (intr_handle == NULL)
 		return;
-	if (RTE_INTR_INSTANCE_USES_RTE_MEMORY(intr_handle->alloc_flags))
+	if (RTE_INTR_INSTANCE_USES_RTE_MEMORY(intr_handle->alloc_flags)) {
+		rte_free(intr_handle->efds);
+		rte_free(intr_handle->elist);
 		rte_free(intr_handle);
-	else
+	} else {
+		free(intr_handle->efds);
+		free(intr_handle->elist);
 		free(intr_handle);
+	}
 }
 
 int rte_intr_fd_set(struct rte_intr_handle *intr_handle, int fd)
@@ -239,6 +330,12 @@  int rte_intr_efds_index_get(const struct rte_intr_handle *intr_handle,
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
+	if (intr_handle->efds == NULL) {
+		RTE_LOG(ERR, EAL, "Event fd list not allocated\n");
+		rte_errno = EFAULT;
+		goto fail;
+	}
+
 	if (index >= intr_handle->nb_intr) {
 		RTE_LOG(ERR, EAL, "Invalid index %d, max limit %d\n", index,
 			intr_handle->nb_intr);
@@ -256,6 +353,12 @@  int rte_intr_efds_index_set(struct rte_intr_handle *intr_handle,
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
+	if (intr_handle->efds == NULL) {
+		RTE_LOG(ERR, EAL, "Event fd list not allocated\n");
+		rte_errno = EFAULT;
+		goto fail;
+	}
+
 	if (index >= intr_handle->nb_intr) {
 		RTE_LOG(ERR, EAL, "Invalid size %d, max limit %d\n", index,
 			intr_handle->nb_intr);
@@ -275,6 +378,12 @@  struct rte_epoll_event *rte_intr_elist_index_get(
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
+	if (intr_handle->elist == NULL) {
+		RTE_LOG(ERR, EAL, "Event list not allocated\n");
+		rte_errno = EFAULT;
+		goto fail;
+	}
+
 	if (index >= intr_handle->nb_intr) {
 		RTE_LOG(ERR, EAL, "Invalid index %d, max limit %d\n", index,
 			intr_handle->nb_intr);
@@ -292,6 +401,12 @@  int rte_intr_elist_index_set(struct rte_intr_handle *intr_handle,
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
+	if (intr_handle->elist == NULL) {
+		RTE_LOG(ERR, EAL, "Event list not allocated\n");
+		rte_errno = EFAULT;
+		goto fail;
+	}
+
 	if (index >= intr_handle->nb_intr) {
 		RTE_LOG(ERR, EAL, "Invalid index %d, max limit %d\n", index,
 			intr_handle->nb_intr);
diff --git a/lib/eal/common/eal_interrupts.h b/lib/eal/common/eal_interrupts.h
index 1a4e5573b2..482781b862 100644
--- a/lib/eal/common/eal_interrupts.h
+++ b/lib/eal/common/eal_interrupts.h
@@ -21,9 +21,8 @@  struct rte_intr_handle {
 	uint8_t efd_counter_size;      /**< size of efd counter, used for vdev */
 	uint16_t nb_intr;
 		/**< Max vector count, default RTE_MAX_RXTX_INTR_VEC_ID */
-	int efds[RTE_MAX_RXTX_INTR_VEC_ID];  /**< intr vectors/efds mapping */
-	struct rte_epoll_event elist[RTE_MAX_RXTX_INTR_VEC_ID];
-				       /**< intr vector epoll event */
+	int *efds;  /**< intr vectors/efds mapping */
+	struct rte_epoll_event *elist; /**< intr vector epoll event */
 	uint16_t vec_list_size;
 	int *intr_vec;                 /**< intr vector number array */
 };