Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 24 Feb 2016 00:42:44 +0000 (UTC)
From:      Eric Joyner <erj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r295946 - head/sys/dev/ixl
Message-ID:  <201602240042.u1O0giUN099192@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: erj
Date: Wed Feb 24 00:42:43 2016
New Revision: 295946
URL: https://svnweb.freebsd.org/changeset/base/295946

Log:
  ixl(4): Fix potential driver interrupt setup issues and startup crash.
  
  - Limit queue autoconfiguration to 8 queues to prevent the driver from
    requesting a large number of MSI-X vectors at boot.
  
  - Fix potential kernel panic that occurs when the driver loads and cannot
    get all requested MSIX vectors. Instead, attach() will fail with an error.
    - Move taskqueue setup to later in attach() to prevent having to free
      taskqueues if some other error in attach() occurs.
  
  Differential Revision:	 https://reviews.freebsd.org/D5205
  MFC after:	1 month
  Tested by:	jeffrey.e.pieper@intel.com
  Sponsored by:	Intel Corporation

Modified:
  head/sys/dev/ixl/if_ixl.c

Modified: head/sys/dev/ixl/if_ixl.c
==============================================================================
--- head/sys/dev/ixl/if_ixl.c	Wed Feb 24 00:39:36 2016	(r295945)
+++ head/sys/dev/ixl/if_ixl.c	Wed Feb 24 00:42:43 2016	(r295946)
@@ -115,6 +115,8 @@ static int	ixl_init_msix(struct ixl_pf *
 static void	ixl_configure_msix(struct ixl_pf *);
 static void	ixl_configure_itr(struct ixl_pf *);
 static void	ixl_configure_legacy(struct ixl_pf *);
+static void	ixl_init_taskqueues(struct ixl_pf *);
+static void	ixl_free_taskqueues(struct ixl_pf *);
 static void	ixl_free_pci_resources(struct ixl_pf *);
 static void	ixl_local_timer(void *);
 static int	ixl_setup_interface(device_t, struct ixl_vsi *);
@@ -642,7 +644,7 @@ ixl_attach(device_t dev)
 	else
 		error = ixl_assign_vsi_legacy(pf);
 	if (error) 
-		goto err_late;
+		goto err_mac_hmc;
 
 	if (((hw->aq.fw_maj_ver == 4) && (hw->aq.fw_min_ver < 33)) ||
 	    (hw->aq.fw_maj_ver < 4)) {
@@ -667,7 +669,7 @@ ixl_attach(device_t dev)
 	error = ixl_switch_config(pf);
 	if (error) {
 		device_printf(dev, "Initial switch config failed: %d\n", error);
-		goto err_mac_hmc;
+		goto err_late;
 	}
 
 	/* Limit phy interrupts to link and modules failure */
@@ -680,6 +682,9 @@ ixl_attach(device_t dev)
 	bus = ixl_get_bus_info(hw, dev);
 	i40e_set_pci_config_data(hw, bus);
 
+	/* Initialize taskqueues */
+	ixl_init_taskqueues(pf);
+
 	/* Initialize statistics */
 	ixl_pf_reset_stats(pf);
 	ixl_update_stats_counters(pf);
@@ -748,7 +753,6 @@ ixl_detach(device_t dev)
 	struct ixl_pf		*pf = device_get_softc(dev);
 	struct i40e_hw		*hw = &pf->hw;
 	struct ixl_vsi		*vsi = &pf->vsi;
-	struct ixl_queue	*que = vsi->queues;
 	i40e_status		status;
 #ifdef PCI_IOV
 	int			error;
@@ -777,13 +781,7 @@ ixl_detach(device_t dev)
 		IXL_PF_UNLOCK(pf);
 	}
 
-	for (int i = 0; i < vsi->num_queues; i++, que++) {
-		if (que->tq) {
-			taskqueue_drain(que->tq, &que->task);
-			taskqueue_drain(que->tq, &que->tx_task);
-			taskqueue_free(que->tq);
-		}
-	}
+	ixl_free_taskqueues(pf);
 
 	/* Shutdown LAN HMC */
 	status = i40e_shutdown_lan_hmc(hw);
@@ -1990,6 +1988,58 @@ ixl_assign_vsi_legacy(struct ixl_pf *pf)
 	return (0);
 }
 
+static void
+ixl_init_taskqueues(struct ixl_pf *pf)
+{
+       struct ixl_vsi *vsi = &pf->vsi;
+       struct ixl_queue *que = vsi->queues;
+       device_t dev = pf->dev;
+
+       /* Tasklet for Admin Queue */
+       TASK_INIT(&pf->adminq, 0, ixl_do_adminq, pf);
+#ifdef PCI_IOV
+       /* VFLR Tasklet */
+       TASK_INIT(&pf->vflr_task, 0, ixl_handle_vflr, pf);
+#endif
+
+       /* Create and start PF taskqueue */
+       pf->tq = taskqueue_create_fast("ixl_adm", M_NOWAIT,
+           taskqueue_thread_enqueue, &pf->tq);
+       taskqueue_start_threads(&pf->tq, 1, PI_NET, "%s adminq",
+           device_get_nameunit(dev));
+
+       /* Create queue tasks and start queue taskqueues */
+       for (int i = 0; i < vsi->num_queues; i++, que++) {
+               TASK_INIT(&que->tx_task, 0, ixl_deferred_mq_start, que);
+               TASK_INIT(&que->task, 0, ixl_handle_que, que);
+               que->tq = taskqueue_create_fast("ixl_que", M_NOWAIT,
+                   taskqueue_thread_enqueue, &que->tq);
+#ifdef RSS
+               CPU_SETOF(cpu_id, &cpu_mask);
+               taskqueue_start_threads_cpuset(&que->tq, 1, PI_NET,
+                   &cpu_mask, "%s (bucket %d)",
+                   device_get_nameunit(dev), cpu_id);
+#else
+               taskqueue_start_threads(&que->tq, 1, PI_NET,
+                   "%s (que %d)", device_get_nameunit(dev), que->me);
+#endif
+       }
+
+}
+
+static void
+ixl_free_taskqueues(struct ixl_pf *pf)
+{
+       struct ixl_vsi          *vsi = &pf->vsi;
+       struct ixl_queue        *que = vsi->queues;
+
+       if (pf->tq)
+               taskqueue_free(pf->tq);
+       for (int i = 0; i < vsi->num_queues; i++, que++) {
+               if (que->tq)
+                       taskqueue_free(que->tq);
+       }
+}
 
 /*********************************************************************
  *
@@ -2028,17 +2078,6 @@ ixl_assign_vsi_msix(struct ixl_pf *pf)
 	}
 	bus_describe_intr(dev, pf->res, pf->tag, "aq");
 	pf->admvec = vector;
-	/* Tasklet for Admin Queue */
-	TASK_INIT(&pf->adminq, 0, ixl_do_adminq, pf);
-
-#ifdef PCI_IOV
-	TASK_INIT(&pf->vflr_task, 0, ixl_handle_vflr, pf);
-#endif
-
-	pf->tq = taskqueue_create_fast("ixl_adm", M_NOWAIT,
-	    taskqueue_thread_enqueue, &pf->tq);
-	taskqueue_start_threads(&pf->tq, 1, PI_NET, "%s adminq",
-	    device_get_nameunit(pf->dev));
 	++vector;
 
 	/* Now set up the stations */
@@ -2069,19 +2108,6 @@ ixl_assign_vsi_msix(struct ixl_pf *pf)
 #endif
 		bus_bind_intr(dev, que->res, cpu_id);
 		que->msix = vector;
-		TASK_INIT(&que->tx_task, 0, ixl_deferred_mq_start, que);
-		TASK_INIT(&que->task, 0, ixl_handle_que, que);
-		que->tq = taskqueue_create_fast("ixl_que", M_NOWAIT,
-		    taskqueue_thread_enqueue, &que->tq);
-#ifdef RSS
-		CPU_SETOF(cpu_id, &cpu_mask);
-		taskqueue_start_threads_cpuset(&que->tq, 1, PI_NET,
-		    &cpu_mask, "%s (bucket %d)",
-		    device_get_nameunit(dev), cpu_id);
-#else
-		taskqueue_start_threads(&que->tq, 1, PI_NET,
-		    "%s que", device_get_nameunit(dev));
-#endif
 	}
 
 	return (0);
@@ -2144,9 +2170,15 @@ ixl_init_msix(struct ixl_pf *pf)
 	/* Figure out a reasonable auto config value */
 	queues = (mp_ncpus > (available - 1)) ? (available - 1) : mp_ncpus;
 
-	/* Override with hardcoded value if sane */
+	/* Override with hardcoded value if it's less than autoconfig count */
 	if ((ixl_max_queues != 0) && (ixl_max_queues <= queues)) 
 		queues = ixl_max_queues;
+	else if ((ixl_max_queues != 0) && (ixl_max_queues > queues))
+		device_printf(dev, "ixl_max_queues > # of cpus, using "
+		    "autoconfig amount...\n");
+	/* Or limit maximum auto-configured queues to 8 */
+	else if ((ixl_max_queues == 0) && (queues > 8))
+		queues = 8;
 
 #ifdef  RSS
 	/* If we're doing RSS, clamp at the number of RSS buckets */

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201602240042.u1O0giUN099192>