aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKrzysztof Galazka <krzysztof.galazka@intel.com>2024-07-05 11:00:13 +0000
committerWarner Losh <imp@FreeBSD.org>2025-01-24 21:08:12 +0000
commit3ad01642fe9e241124553f2f18fd365ffea5d20b (patch)
treee108646ede2267f7a2f652c8a36ab131efa47432
parentca363716fc492146978c8650b97862f94daab6bf (diff)
iflib(4): Replace admin taskqueue group with per-interface taskqueues
Using one taskqueue group with single thread to execute all admin tasks may lead to unexpected timeouts when long running task (e.g. handling a reset after FW update) for one interface prevents tasks from other interfaces being executed. Taskqueue group API doesn't let to dynamically add threads, and pre-allocating thread for each CPU as it's done for traffic queues would be a waste of resources on systems with small number of interfaces. Replace global taskqueue group for admin tasks with taskqueue allocated for each interface to allow independent execution. Signed-off-by: Krzysztof Galazka <krzysztof.galazka@intel.com> Reviewed by: imp, jhb Pull Request: https://github.com/freebsd/freebsd-src/pull/1336
-rw-r--r--sys/dev/bnxt/bnxt_en/bnxt.h2
-rw-r--r--sys/dev/bnxt/bnxt_en/if_bnxt.c10
-rw-r--r--sys/net/iflib.c77
-rw-r--r--sys/net/iflib.h6
4 files changed, 48 insertions, 47 deletions
diff --git a/sys/dev/bnxt/bnxt_en/bnxt.h b/sys/dev/bnxt/bnxt_en/bnxt.h
index c5fadeafa181..e4f866807070 100644
--- a/sys/dev/bnxt/bnxt_en/bnxt.h
+++ b/sys/dev/bnxt/bnxt_en/bnxt.h
@@ -1091,7 +1091,7 @@ struct bnxt_softc {
struct bnxt_cp_ring def_nq_ring;
struct iflib_dma_info def_cp_ring_mem;
struct iflib_dma_info def_nq_ring_mem;
- struct grouptask def_cp_task;
+ struct task def_cp_task;
int db_size;
int legacy_db_size;
struct bnxt_doorbell_ops db_ops;
diff --git a/sys/dev/bnxt/bnxt_en/if_bnxt.c b/sys/dev/bnxt/bnxt_en/if_bnxt.c
index 00d37f5e0151..efc467a043f9 100644
--- a/sys/dev/bnxt/bnxt_en/if_bnxt.c
+++ b/sys/dev/bnxt/bnxt_en/if_bnxt.c
@@ -229,7 +229,7 @@ static void bnxt_clear_ids(struct bnxt_softc *softc);
static void inline bnxt_do_enable_intr(struct bnxt_cp_ring *cpr);
static void inline bnxt_do_disable_intr(struct bnxt_cp_ring *cpr);
static void bnxt_mark_cpr_invalid(struct bnxt_cp_ring *cpr);
-static void bnxt_def_cp_task(void *context);
+static void bnxt_def_cp_task(void *context, int pending);
static void bnxt_handle_async_event(struct bnxt_softc *softc,
struct cmpl_base *cmpl);
static uint64_t bnxt_get_baudrate(struct bnxt_link_info *link);
@@ -2384,8 +2384,7 @@ bnxt_attach_pre(if_ctx_t ctx)
&softc->def_cp_ring_mem, 0);
softc->def_cp_ring.ring.vaddr = softc->def_cp_ring_mem.idi_vaddr;
softc->def_cp_ring.ring.paddr = softc->def_cp_ring_mem.idi_paddr;
- iflib_config_gtask_init(ctx, &softc->def_cp_task, bnxt_def_cp_task,
- "dflt_cp");
+ iflib_config_task_init(ctx, &softc->def_cp_task, bnxt_def_cp_task);
rc = bnxt_init_sysctl_ctx(softc);
if (rc)
@@ -2512,7 +2511,6 @@ bnxt_detach(if_ctx_t ctx)
bnxt_free_ctx_mem(softc);
bnxt_clear_ids(softc);
iflib_irq_free(ctx, &softc->def_cp_ring.irq);
- iflib_config_gtask_deinit(&softc->def_cp_task);
/* We need to free() these here... */
for (i = softc->nrxqsets-1; i>=0; i--) {
if (BNXT_CHIP_P5(softc))
@@ -4348,7 +4346,7 @@ bnxt_handle_def_cp(void *arg)
struct bnxt_softc *softc = arg;
softc->db_ops.bnxt_db_rx_cq(&softc->def_cp_ring, 0);
- GROUPTASK_ENQUEUE(&softc->def_cp_task);
+ iflib_config_task_enqueue(softc->ctx, &softc->def_cp_task);
return FILTER_HANDLED;
}
@@ -4608,7 +4606,7 @@ async_event_process_exit:
}
static void
-bnxt_def_cp_task(void *context)
+bnxt_def_cp_task(void *context, int pending)
{
if_ctx_t ctx = context;
struct bnxt_softc *softc = iflib_get_softc(ctx);
diff --git a/sys/net/iflib.c b/sys/net/iflib.c
index 6644197f7d5b..1ae2207c9d13 100644
--- a/sys/net/iflib.c
+++ b/sys/net/iflib.c
@@ -178,8 +178,9 @@ struct iflib_ctx {
struct resource *ifc_msix_mem;
struct if_irq ifc_legacy_irq;
- struct grouptask ifc_admin_task;
- struct grouptask ifc_vflr_task;
+ struct task ifc_admin_task;
+ struct task ifc_vflr_task;
+ struct taskqueue *ifc_tq;
struct iflib_filter_info ifc_filter_info;
struct ifmedia ifc_media;
struct ifmedia *ifc_mediap;
@@ -1640,7 +1641,7 @@ static int
iflib_fast_intr_ctx(void *arg)
{
iflib_filter_info_t info = arg;
- struct grouptask *gtask = info->ifi_task;
+ if_ctx_t ctx = info->ifi_ctx;
int result;
DBG_COUNTER_INC(fast_intrs);
@@ -1650,8 +1651,7 @@ iflib_fast_intr_ctx(void *arg)
return (result);
}
- if (gtask->gt_taskqueue != NULL)
- GROUPTASK_ENQUEUE(gtask);
+ taskqueue_enqueue(ctx->ifc_tq, &ctx->ifc_admin_task);
return (FILTER_HANDLED);
}
@@ -4176,7 +4176,7 @@ skip_rxeof:
}
static void
-_task_fn_admin(void *context)
+_task_fn_admin(void *context, int pending)
{
if_ctx_t ctx = context;
if_softc_ctx_t sctx = &ctx->ifc_softc_ctx;
@@ -4227,7 +4227,7 @@ _task_fn_admin(void *context)
}
static void
-_task_fn_iov(void *context)
+_task_fn_iov(void *context, int pending)
{
if_ctx_t ctx = context;
@@ -5200,6 +5200,7 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct
kobj_method_t *kobj_method;
int err, msix, rid;
int num_txd, num_rxd;
+ char namebuf[TASKQUEUE_NAMELEN];
ctx = malloc(sizeof(*ctx), M_IFLIB, M_WAITOK | M_ZERO);
@@ -5290,10 +5291,25 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct
scctx->isc_rss_table_size = 64;
scctx->isc_rss_table_mask = scctx->isc_rss_table_size - 1;
- GROUPTASK_INIT(&ctx->ifc_admin_task, 0, _task_fn_admin, ctx);
- /* XXX format name */
- taskqgroup_attach(qgroup_if_config_tqg, &ctx->ifc_admin_task, ctx,
- NULL, NULL, "admin");
+ /* Create and start admin taskqueue */
+ snprintf(namebuf, TASKQUEUE_NAMELEN, "if_%s_tq", device_get_nameunit(dev));
+ ctx->ifc_tq = taskqueue_create_fast(namebuf, M_NOWAIT,
+ taskqueue_thread_enqueue, &ctx->ifc_tq);
+ if (ctx->ifc_tq == NULL) {
+ device_printf(dev, "Unable to create admin taskqueue\n");
+ return (ENOMEM);
+ }
+
+ err = taskqueue_start_threads(&ctx->ifc_tq, 1, PI_NET, "%s", namebuf);
+ if (err) {
+ device_printf(dev,
+ "Unable to start admin taskqueue threads error: %d\n",
+ err);
+ taskqueue_free(ctx->ifc_tq);
+ return (err);
+ }
+
+ TASK_INIT(&ctx->ifc_admin_task, 0, _task_fn_admin, ctx);
/* Set up cpu set. If it fails, use the set of all CPUs. */
if (bus_get_cpus(dev, INTR_CPUS, sizeof(ctx->ifc_cpus), &ctx->ifc_cpus) != 0) {
@@ -5430,6 +5446,7 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct
fail_detach:
ether_ifdetach(ctx->ifc_ifp);
fail_queues:
+ taskqueue_free(ctx->ifc_tq);
iflib_tqg_detach(ctx);
iflib_tx_structures_free(ctx);
iflib_rx_structures_free(ctx);
@@ -5508,6 +5525,9 @@ iflib_device_deregister(if_ctx_t ctx)
IFDI_QUEUES_FREE(ctx);
CTX_UNLOCK(ctx);
+ taskqueue_free(ctx->ifc_tq);
+ ctx->ifc_tq = NULL;
+
/* ether_ifdetach calls if_qflush - lock must be destroy afterwards*/
iflib_free_intr_mem(ctx);
@@ -5545,11 +5565,6 @@ iflib_tqg_detach(if_ctx_t ctx)
if (rxq->ifr_task.gt_uniq != NULL)
taskqgroup_detach(tqg, &rxq->ifr_task);
}
- tqg = qgroup_if_config_tqg;
- if (ctx->ifc_admin_task.gt_uniq != NULL)
- taskqgroup_detach(tqg, &ctx->ifc_admin_task);
- if (ctx->ifc_vflr_task.gt_uniq != NULL)
- taskqgroup_detach(tqg, &ctx->ifc_vflr_task);
}
static void
@@ -6324,9 +6339,7 @@ iflib_irq_alloc_generic(if_ctx_t ctx, if_irq_t irq, int rid,
q = ctx;
tqrid = -1;
info = &ctx->ifc_filter_info;
- gtask = &ctx->ifc_admin_task;
- tqg = qgroup_if_config_tqg;
- fn = _task_fn_admin;
+ gtask = NULL;
intr_fast = iflib_fast_intr_ctx;
break;
default:
@@ -6388,12 +6401,8 @@ iflib_softirq_alloc_generic(if_ctx_t ctx, if_irq_t irq, iflib_intr_type_t type,
NET_GROUPTASK_INIT(gtask, 0, fn, q);
break;
case IFLIB_INTR_IOV:
- q = ctx;
- gtask = &ctx->ifc_vflr_task;
- tqg = qgroup_if_config_tqg;
- fn = _task_fn_iov;
- GROUPTASK_INIT(gtask, 0, fn, q);
- break;
+ TASK_INIT(&ctx->ifc_vflr_task, 0, _task_fn_iov, ctx);
+ return;
default:
panic("unknown net intr type");
}
@@ -6483,15 +6492,14 @@ void
iflib_admin_intr_deferred(if_ctx_t ctx)
{
- MPASS(ctx->ifc_admin_task.gt_taskqueue != NULL);
- GROUPTASK_ENQUEUE(&ctx->ifc_admin_task);
+ taskqueue_enqueue(ctx->ifc_tq, &ctx->ifc_admin_task);
}
void
iflib_iov_intr_deferred(if_ctx_t ctx)
{
- GROUPTASK_ENQUEUE(&ctx->ifc_vflr_task);
+ taskqueue_enqueue(ctx->ifc_tq, &ctx->ifc_vflr_task);
}
void
@@ -6503,20 +6511,15 @@ iflib_io_tqg_attach(struct grouptask *gt, void *uniq, int cpu, const char *name)
}
void
-iflib_config_gtask_init(void *ctx, struct grouptask *gtask, gtask_fn_t *fn,
- const char *name)
+iflib_config_task_init(if_ctx_t ctx, struct task *config_task, task_fn_t *fn)
{
-
- GROUPTASK_INIT(gtask, 0, fn, ctx);
- taskqgroup_attach(qgroup_if_config_tqg, gtask, gtask, NULL, NULL,
- name);
+ TASK_INIT(config_task, 0, fn, ctx);
}
void
-iflib_config_gtask_deinit(struct grouptask *gtask)
+iflib_config_task_enqueue(if_ctx_t ctx, struct task *config_task)
{
-
- taskqgroup_detach(qgroup_if_config_tqg, gtask);
+ taskqueue_enqueue(ctx->ifc_tq, config_task);
}
void
diff --git a/sys/net/iflib.h b/sys/net/iflib.h
index e3d76fbd3c01..3817445228d0 100644
--- a/sys/net/iflib.h
+++ b/sys/net/iflib.h
@@ -470,9 +470,9 @@ void iflib_irq_free(if_ctx_t ctx, if_irq_t irq);
void iflib_io_tqg_attach(struct grouptask *gt, void *uniq, int cpu,
const char *name);
-void iflib_config_gtask_init(void *ctx, struct grouptask *gtask,
- gtask_fn_t *fn, const char *name);
-void iflib_config_gtask_deinit(struct grouptask *gtask);
+void iflib_config_task_init(if_ctx_t ctx, struct task *config_task,
+ task_fn_t *fn);
+void iflib_config_task_enqueue(if_ctx_t ctx, struct task *config_task);
void iflib_tx_intr_deferred(if_ctx_t ctx, int txqid);
void iflib_rx_intr_deferred(if_ctx_t ctx, int rxqid);