diff mbox series

[net-next,4/5] enic: Allocate arrays in enic struct based on VIC config

Message ID 20241022041707.27402-5-neescoba@cisco.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Use all the resources configured on VIC | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 5 maintainers not CCed: benve@cisco.com edumazet@google.com andrew+netdev@lunn.ch pabeni@redhat.com kuba@kernel.org
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 21 this patch: 21
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 211 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-10-23--12-00 (tests: 777)

Commit Message

Nelson Escobar (neescoba) Oct. 22, 2024, 4:17 a.m. UTC 
Allocate wq, rq, cq, intr, and napi arrays based on the number of
resources configured in the VIC.

Signed-off-by: Nelson Escobar <neescoba@cisco.com>
Signed-off-by: John Daley <johndale@cisco.com>
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
 drivers/net/ethernet/cisco/enic/enic.h      |  24 ++---
 drivers/net/ethernet/cisco/enic/enic_main.c | 102 ++++++++++++++++++--
 2 files changed, 105 insertions(+), 21 deletions(-)

Comments

Simon Horman Oct. 22, 2024, 4:36 p.m. UTC | #1 
On Mon, Oct 21, 2024 at 09:17:06PM -0700, Nelson Escobar wrote:
> Allocate wq, rq, cq, intr, and napi arrays based on the number of
> resources configured in the VIC.
> 
> Signed-off-by: Nelson Escobar <neescoba@cisco.com>
> Signed-off-by: John Daley <johndale@cisco.com>
> Signed-off-by: Satish Kharat <satishkh@cisco.com>

Reviewed-by: Simon Horman <horms@kernel.org>
Kalesh Anakkur Purayil Oct. 22, 2024, 4:57 p.m. UTC | #2 
On Tue, Oct 22, 2024 at 9:49 AM Nelson Escobar <neescoba@cisco.com> wrote:
>
> Allocate wq, rq, cq, intr, and napi arrays based on the number of
> resources configured in the VIC.
>
> Signed-off-by: Nelson Escobar <neescoba@cisco.com>
> Signed-off-by: John Daley <johndale@cisco.com>
> Signed-off-by: Satish Kharat <satishkh@cisco.com>
> ---
>  drivers/net/ethernet/cisco/enic/enic.h      |  24 ++---
>  drivers/net/ethernet/cisco/enic/enic_main.c | 102 ++++++++++++++++++--
>  2 files changed, 105 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
> index 1f32413a8f7c..cfb4667953de 100644
> --- a/drivers/net/ethernet/cisco/enic/enic.h
> +++ b/drivers/net/ethernet/cisco/enic/enic.h
> @@ -23,10 +23,8 @@
>
>  #define ENIC_BARS_MAX          6
>
> -#define ENIC_WQ_MAX            8
> -#define ENIC_RQ_MAX            8
> -#define ENIC_CQ_MAX            (ENIC_WQ_MAX + ENIC_RQ_MAX)
> -#define ENIC_INTR_MAX          (ENIC_CQ_MAX + 2)
> +#define ENIC_WQ_MAX            256
> +#define ENIC_RQ_MAX            256
>
>  #define ENIC_WQ_NAPI_BUDGET    256
>
> @@ -184,8 +182,8 @@ struct enic {
>         struct work_struct reset;
>         struct work_struct tx_hang_reset;
>         struct work_struct change_mtu_work;
> -       struct msix_entry msix_entry[ENIC_INTR_MAX];
> -       struct enic_msix_entry msix[ENIC_INTR_MAX];
> +       struct msix_entry *msix_entry;
> +       struct enic_msix_entry *msix;
>         u32 msg_enable;
>         spinlock_t devcmd_lock;
>         u8 mac_addr[ETH_ALEN];
> @@ -204,28 +202,24 @@ struct enic {
>         bool enic_api_busy;
>         struct enic_port_profile *pp;
>
> -       /* work queue cache line section */
> -       ____cacheline_aligned struct enic_wq wq[ENIC_WQ_MAX];
> +       struct enic_wq *wq;
>         unsigned int wq_avail;
>         unsigned int wq_count;
>         u16 loop_enable;
>         u16 loop_tag;
>
> -       /* receive queue cache line section */
> -       ____cacheline_aligned struct enic_rq rq[ENIC_RQ_MAX];
> +       struct enic_rq *rq;
>         unsigned int rq_avail;
>         unsigned int rq_count;
>         struct vxlan_offload vxlan;
> -       struct napi_struct napi[ENIC_RQ_MAX + ENIC_WQ_MAX];
> +       struct napi_struct *napi;
>
> -       /* interrupt resource cache line section */
> -       ____cacheline_aligned struct vnic_intr intr[ENIC_INTR_MAX];
> +       struct vnic_intr *intr;
>         unsigned int intr_avail;
>         unsigned int intr_count;
>         u32 __iomem *legacy_pba;                /* memory-mapped */
>
> -       /* completion queue cache line section */
> -       ____cacheline_aligned struct vnic_cq cq[ENIC_CQ_MAX];
> +       struct vnic_cq *cq;
>         unsigned int cq_avail;
>         unsigned int cq_count;
>         struct enic_rfs_flw_tbl rfs_h;
> diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
> index eb00058b6c68..a5d607be66b7 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_main.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_main.c
> @@ -940,7 +940,7 @@ static void enic_get_stats(struct net_device *netdev,
>         net_stats->rx_errors = stats->rx.rx_errors;
>         net_stats->multicast = stats->rx.rx_multicast_frames_ok;
>
> -       for (i = 0; i < ENIC_RQ_MAX; i++) {
> +       for (i = 0; i < enic->rq_count; i++) {
>                 struct enic_rq_stats *rqs = &enic->rq[i].stats;
>
>                 if (!enic->rq[i].vrq.ctrl)
> @@ -1792,7 +1792,7 @@ static void enic_free_intr(struct enic *enic)
>                 free_irq(enic->pdev->irq, enic);
>                 break;
>         case VNIC_DEV_INTR_MODE_MSIX:
> -               for (i = 0; i < ARRAY_SIZE(enic->msix); i++)
> +               for (i = 0; i < enic->intr_count; i++)
>                         if (enic->msix[i].requested)
>                                 free_irq(enic->msix_entry[i].vector,
>                                         enic->msix[i].devid);
> @@ -1859,7 +1859,7 @@ static int enic_request_intr(struct enic *enic)
>                 enic->msix[intr].isr = enic_isr_msix_notify;
>                 enic->msix[intr].devid = enic;
>
> -               for (i = 0; i < ARRAY_SIZE(enic->msix); i++)
> +               for (i = 0; i < enic->intr_count; i++)
>                         enic->msix[i].requested = 0;
>
>                 for (i = 0; i < enic->intr_count; i++) {
> @@ -2456,8 +2456,7 @@ static int enic_set_intr_mode(struct enic *enic)
>          * (the last INTR is used for notifications)
>          */
>
> -       BUG_ON(ARRAY_SIZE(enic->msix_entry) < n + m + 2);
> -       for (i = 0; i < n + m + 2; i++)
> +       for (i = 0; i < enic->intr_avail; i++)
>                 enic->msix_entry[i].entry = i;
>
>         /* Use multiple RQs if RSS is enabled
> @@ -2674,6 +2673,89 @@ static const struct netdev_stat_ops enic_netdev_stat_ops = {
>         .get_base_stats         = enic_get_base_stats,
>  };
>
> +static void enic_free_enic_resources(struct enic *enic)
> +{
> +       kfree(enic->wq);
> +       enic->wq = NULL;
> +
> +       kfree(enic->rq);
> +       enic->rq = NULL;
> +
> +       kfree(enic->cq);
> +       enic->cq = NULL;
> +
> +       kfree(enic->napi);
> +       enic->napi = NULL;
> +
> +       kfree(enic->msix_entry);
> +       enic->msix_entry = NULL;
> +
> +       kfree(enic->msix);
> +       enic->msix = NULL;
> +
> +       kfree(enic->intr);
> +       enic->intr = NULL;
> +}
> +
> +static int enic_alloc_enic_resources(struct enic *enic)
> +{
> +       int ret;
[Kalesh] There is no need for a local variable here. You can return
-ENOMEM in the error path directly.
> +
> +       enic->wq = NULL;
> +       enic->rq = NULL;
> +       enic->cq = NULL;
> +       enic->napi = NULL;
> +       enic->msix_entry = NULL;
> +       enic->msix = NULL;
> +       enic->intr = NULL;
[Kalesh] Looks like the above NULL pointer assignments are redundant.
all those fields will be 0 anyway.
> +
> +       enic->wq = kcalloc(enic->wq_avail, sizeof(struct enic_wq), GFP_KERNEL);
> +       if (!enic->wq) {
> +               ret = -ENOMEM;
> +               goto free_queues;
> +       }
> +       enic->rq = kcalloc(enic->rq_avail, sizeof(struct enic_rq), GFP_KERNEL);
> +       if (!enic->rq) {
> +               ret = -ENOMEM;
> +               goto free_queues;
> +       }
> +       enic->cq = kcalloc(enic->cq_avail, sizeof(struct vnic_cq), GFP_KERNEL);
> +       if (!enic->cq) {
> +               ret = -ENOMEM;
> +               goto free_queues;
> +       }
> +       enic->napi = kcalloc(enic->wq_avail + enic->rq_avail,
> +                            sizeof(struct napi_struct), GFP_KERNEL);
> +       if (!enic->napi) {
> +               ret = -ENOMEM;
> +               goto free_queues;
> +       }
> +       enic->msix_entry = kcalloc(enic->intr_avail, sizeof(struct msix_entry),
> +                                  GFP_KERNEL);
> +       if (!enic->msix_entry) {
> +               ret = -ENOMEM;
> +               goto free_queues;
> +       }
> +       enic->msix = kcalloc(enic->intr_avail, sizeof(struct enic_msix_entry),
> +                            GFP_KERNEL);
> +       if (!enic->msix) {
> +               ret = -ENOMEM;
> +               goto free_queues;
> +       }
> +       enic->intr = kcalloc(enic->intr_avail, sizeof(struct vnic_intr),
> +                            GFP_KERNEL);
> +       if (!enic->intr) {
> +               ret = -ENOMEM;
> +               goto free_queues;
> +       }
> +
> +       return 0;
> +
> +free_queues:
> +       enic_free_enic_resources(enic);
> +       return ret;
> +}
> +
>  static void enic_dev_deinit(struct enic *enic)
>  {
>         unsigned int i;
> @@ -2691,6 +2773,7 @@ static void enic_dev_deinit(struct enic *enic)
>         enic_free_vnic_resources(enic);
>         enic_clear_intr_mode(enic);
>         enic_free_affinity_hint(enic);
> +       enic_free_enic_resources(enic);
>  }
>
>  static void enic_kdump_kernel_config(struct enic *enic)
> @@ -2734,6 +2817,12 @@ static int enic_dev_init(struct enic *enic)
>
>         enic_get_res_counts(enic);
>
> +       err = enic_alloc_enic_resources(enic);
> +       if (err) {
> +               dev_err(dev, "Failed to allocate queues, aborting\n");
[Kalesh] You can drop this error log in case you want as memory
allocation failure will be logged by OOM.
> +               return err;
> +       }
> +
>         /* modify resource count if we are in kdump_kernel
>          */
>         enic_kdump_kernel_config(enic);
> @@ -2746,7 +2835,7 @@ static int enic_dev_init(struct enic *enic)
>         if (err) {
>                 dev_err(dev, "Failed to set intr mode based on resource "
>                         "counts and system capabilities, aborting\n");
> -               return err;
> +               goto err_out_free_vnic_resources;
>         }
>
>         /* Allocate and configure vNIC resources
> @@ -2788,6 +2877,7 @@ static int enic_dev_init(struct enic *enic)
>         enic_free_affinity_hint(enic);
>         enic_clear_intr_mode(enic);
>         enic_free_vnic_resources(enic);
> +       enic_free_enic_resources(enic);
>
>         return err;
>  }
> --
> 2.35.2
>
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 1f32413a8f7c..cfb4667953de 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -23,10 +23,8 @@ 
 
 #define ENIC_BARS_MAX		6
 
-#define ENIC_WQ_MAX		8
-#define ENIC_RQ_MAX		8
-#define ENIC_CQ_MAX		(ENIC_WQ_MAX + ENIC_RQ_MAX)
-#define ENIC_INTR_MAX		(ENIC_CQ_MAX + 2)
+#define ENIC_WQ_MAX		256
+#define ENIC_RQ_MAX		256
 
 #define ENIC_WQ_NAPI_BUDGET	256
 
@@ -184,8 +182,8 @@  struct enic {
 	struct work_struct reset;
 	struct work_struct tx_hang_reset;
 	struct work_struct change_mtu_work;
-	struct msix_entry msix_entry[ENIC_INTR_MAX];
-	struct enic_msix_entry msix[ENIC_INTR_MAX];
+	struct msix_entry *msix_entry;
+	struct enic_msix_entry *msix;
 	u32 msg_enable;
 	spinlock_t devcmd_lock;
 	u8 mac_addr[ETH_ALEN];
@@ -204,28 +202,24 @@  struct enic {
 	bool enic_api_busy;
 	struct enic_port_profile *pp;
 
-	/* work queue cache line section */
-	____cacheline_aligned struct enic_wq wq[ENIC_WQ_MAX];
+	struct enic_wq *wq;
 	unsigned int wq_avail;
 	unsigned int wq_count;
 	u16 loop_enable;
 	u16 loop_tag;
 
-	/* receive queue cache line section */
-	____cacheline_aligned struct enic_rq rq[ENIC_RQ_MAX];
+	struct enic_rq *rq;
 	unsigned int rq_avail;
 	unsigned int rq_count;
 	struct vxlan_offload vxlan;
-	struct napi_struct napi[ENIC_RQ_MAX + ENIC_WQ_MAX];
+	struct napi_struct *napi;
 
-	/* interrupt resource cache line section */
-	____cacheline_aligned struct vnic_intr intr[ENIC_INTR_MAX];
+	struct vnic_intr *intr;
 	unsigned int intr_avail;
 	unsigned int intr_count;
 	u32 __iomem *legacy_pba;		/* memory-mapped */
 
-	/* completion queue cache line section */
-	____cacheline_aligned struct vnic_cq cq[ENIC_CQ_MAX];
+	struct vnic_cq *cq;
 	unsigned int cq_avail;
 	unsigned int cq_count;
 	struct enic_rfs_flw_tbl rfs_h;
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index eb00058b6c68..a5d607be66b7 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -940,7 +940,7 @@  static void enic_get_stats(struct net_device *netdev,
 	net_stats->rx_errors = stats->rx.rx_errors;
 	net_stats->multicast = stats->rx.rx_multicast_frames_ok;
 
-	for (i = 0; i < ENIC_RQ_MAX; i++) {
+	for (i = 0; i < enic->rq_count; i++) {
 		struct enic_rq_stats *rqs = &enic->rq[i].stats;
 
 		if (!enic->rq[i].vrq.ctrl)
@@ -1792,7 +1792,7 @@  static void enic_free_intr(struct enic *enic)
 		free_irq(enic->pdev->irq, enic);
 		break;
 	case VNIC_DEV_INTR_MODE_MSIX:
-		for (i = 0; i < ARRAY_SIZE(enic->msix); i++)
+		for (i = 0; i < enic->intr_count; i++)
 			if (enic->msix[i].requested)
 				free_irq(enic->msix_entry[i].vector,
 					enic->msix[i].devid);
@@ -1859,7 +1859,7 @@  static int enic_request_intr(struct enic *enic)
 		enic->msix[intr].isr = enic_isr_msix_notify;
 		enic->msix[intr].devid = enic;
 
-		for (i = 0; i < ARRAY_SIZE(enic->msix); i++)
+		for (i = 0; i < enic->intr_count; i++)
 			enic->msix[i].requested = 0;
 
 		for (i = 0; i < enic->intr_count; i++) {
@@ -2456,8 +2456,7 @@  static int enic_set_intr_mode(struct enic *enic)
 	 * (the last INTR is used for notifications)
 	 */
 
-	BUG_ON(ARRAY_SIZE(enic->msix_entry) < n + m + 2);
-	for (i = 0; i < n + m + 2; i++)
+	for (i = 0; i < enic->intr_avail; i++)
 		enic->msix_entry[i].entry = i;
 
 	/* Use multiple RQs if RSS is enabled
@@ -2674,6 +2673,89 @@  static const struct netdev_stat_ops enic_netdev_stat_ops = {
 	.get_base_stats		= enic_get_base_stats,
 };
 
+static void enic_free_enic_resources(struct enic *enic)
+{
+	kfree(enic->wq);
+	enic->wq = NULL;
+
+	kfree(enic->rq);
+	enic->rq = NULL;
+
+	kfree(enic->cq);
+	enic->cq = NULL;
+
+	kfree(enic->napi);
+	enic->napi = NULL;
+
+	kfree(enic->msix_entry);
+	enic->msix_entry = NULL;
+
+	kfree(enic->msix);
+	enic->msix = NULL;
+
+	kfree(enic->intr);
+	enic->intr = NULL;
+}
+
+static int enic_alloc_enic_resources(struct enic *enic)
+{
+	int ret;
+
+	enic->wq = NULL;
+	enic->rq = NULL;
+	enic->cq = NULL;
+	enic->napi = NULL;
+	enic->msix_entry = NULL;
+	enic->msix = NULL;
+	enic->intr = NULL;
+
+	enic->wq = kcalloc(enic->wq_avail, sizeof(struct enic_wq), GFP_KERNEL);
+	if (!enic->wq) {
+		ret = -ENOMEM;
+		goto free_queues;
+	}
+	enic->rq = kcalloc(enic->rq_avail, sizeof(struct enic_rq), GFP_KERNEL);
+	if (!enic->rq) {
+		ret = -ENOMEM;
+		goto free_queues;
+	}
+	enic->cq = kcalloc(enic->cq_avail, sizeof(struct vnic_cq), GFP_KERNEL);
+	if (!enic->cq) {
+		ret = -ENOMEM;
+		goto free_queues;
+	}
+	enic->napi = kcalloc(enic->wq_avail + enic->rq_avail,
+			     sizeof(struct napi_struct), GFP_KERNEL);
+	if (!enic->napi) {
+		ret = -ENOMEM;
+		goto free_queues;
+	}
+	enic->msix_entry = kcalloc(enic->intr_avail, sizeof(struct msix_entry),
+				   GFP_KERNEL);
+	if (!enic->msix_entry) {
+		ret = -ENOMEM;
+		goto free_queues;
+	}
+	enic->msix = kcalloc(enic->intr_avail, sizeof(struct enic_msix_entry),
+			     GFP_KERNEL);
+	if (!enic->msix) {
+		ret = -ENOMEM;
+		goto free_queues;
+	}
+	enic->intr = kcalloc(enic->intr_avail, sizeof(struct vnic_intr),
+			     GFP_KERNEL);
+	if (!enic->intr) {
+		ret = -ENOMEM;
+		goto free_queues;
+	}
+
+	return 0;
+
+free_queues:
+	enic_free_enic_resources(enic);
+	return ret;
+}
+
 static void enic_dev_deinit(struct enic *enic)
 {
 	unsigned int i;
@@ -2691,6 +2773,7 @@  static void enic_dev_deinit(struct enic *enic)
 	enic_free_vnic_resources(enic);
 	enic_clear_intr_mode(enic);
 	enic_free_affinity_hint(enic);
+	enic_free_enic_resources(enic);
 }
 
 static void enic_kdump_kernel_config(struct enic *enic)
@@ -2734,6 +2817,12 @@  static int enic_dev_init(struct enic *enic)
 
 	enic_get_res_counts(enic);
 
+	err = enic_alloc_enic_resources(enic);
+	if (err) {
+		dev_err(dev, "Failed to allocate queues, aborting\n");
+		return err;
+	}
+
 	/* modify resource count if we are in kdump_kernel
 	 */
 	enic_kdump_kernel_config(enic);
@@ -2746,7 +2835,7 @@  static int enic_dev_init(struct enic *enic)
 	if (err) {
 		dev_err(dev, "Failed to set intr mode based on resource "
 			"counts and system capabilities, aborting\n");
-		return err;
+		goto err_out_free_vnic_resources;
 	}
 
 	/* Allocate and configure vNIC resources
@@ -2788,6 +2877,7 @@  static int enic_dev_init(struct enic *enic)
 	enic_free_affinity_hint(enic);
 	enic_clear_intr_mode(enic);
 	enic_free_vnic_resources(enic);
+	enic_free_enic_resources(enic);
 
 	return err;
 }