Date: Fri, 24 Jan 2025 21:07:38 GMT From: Warner Losh <imp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 3ad01642fe9e - main - iflib(4): Replace admin taskqueue group with per-interface taskqueues Message-ID: <202501242107.50OL7cdA039517@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=3ad01642fe9e241124553f2f18fd365ffea5d20b commit 3ad01642fe9e241124553f2f18fd365ffea5d20b Author: Krzysztof Galazka <krzysztof.galazka@intel.com> AuthorDate: 2024-07-05 11:00:13 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2025-01-24 21:08:12 +0000 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 --- sys/dev/bnxt/bnxt_en/bnxt.h | 2 +- sys/dev/bnxt/bnxt_en/if_bnxt.c | 10 +++--- sys/net/iflib.c | 77 ++++++++++++++++++++++-------------------- sys/net/iflib.h | 6 ++-- 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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202501242107.50OL7cdA039517>