Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 16 Jun 2019 14:53:53 +0000 (UTC)
From:      Marius Strobl <marius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r349109 - in stable/12/sys: kern net sys
Message-ID:  <201906161453.x5GErrjx022910@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: marius
Date: Sun Jun 16 14:53:53 2019
New Revision: 349109
URL: https://svnweb.freebsd.org/changeset/base/349109

Log:
  MFC: r344062 (partial)
  
  - For diff reduction, bring in as much of the taskqgroup_attach{,_cpu}(9)
    fix from r344062 as possible without breaking KBI, which in turn means
    that these functions still don't properly work across architectures in
    stable/12 (in theory, compat shims should be possible but result in a
    PITA to maintain). However, e. g. the static iflib_irq_set_affinity()
    now also takes the irq as an if_irq_t instead of an int.
  - Move the gtaskqueue_enqueue_fn typedef from <sys/gtaskqueue.h> to
    the gtaskqueue implementation as it's only used and needed there.
  - Change the GTASK_INIT macro to use "gtask" rather than "task" as
    argument given that it actually operates on a struct gtask rather
    than a struct task.
  - Let subr_gtaskqueue.c consistently use __func__ to print functions
    names.

Modified:
  stable/12/sys/kern/subr_gtaskqueue.c
  stable/12/sys/net/iflib.c
  stable/12/sys/sys/_task.h
  stable/12/sys/sys/gtaskqueue.h
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/kern/subr_gtaskqueue.c
==============================================================================
--- stable/12/sys/kern/subr_gtaskqueue.c	Sun Jun 16 13:51:45 2019	(r349108)
+++ stable/12/sys/kern/subr_gtaskqueue.c	Sun Jun 16 14:53:53 2019	(r349109)
@@ -64,6 +64,8 @@ struct gtaskqueue_busy {
 
 static struct gtask * const TB_DRAIN_WAITER = (struct gtask *)0x1;
 
+typedef void (*gtaskqueue_enqueue_fn)(void *context);
+
 struct gtaskqueue {
 	STAILQ_HEAD(, gtask)	tq_queue;
 	gtaskqueue_enqueue_fn	tq_enqueue;
@@ -681,7 +683,7 @@ taskqgroup_find(struct taskqgroup *qgroup, void *uniq)
 		}
 	}
 	if (idx == -1)
-		panic("taskqgroup_find: Failed to pick a qid.");
+		panic("%s: failed to pick a qid.", __func__);
 
 	return (idx);
 }
@@ -734,7 +736,8 @@ taskqgroup_attach(struct taskqgroup *qgroup, struct gr
 		mtx_unlock(&qgroup->tqg_lock);
 		error = intr_setaffinity(irq, CPU_WHICH_IRQ, &mask);
 		if (error)
-			printf("%s: setaffinity failed for %s: %d\n", __func__, gtask->gt_name, error);
+			printf("%s: binding interrupt failed for %s: %d\n",
+			    __func__, gtask->gt_name, error);
 	} else
 		mtx_unlock(&qgroup->tqg_lock);
 }
@@ -756,13 +759,12 @@ taskqgroup_attach_deferred(struct taskqgroup *qgroup, 
 		error = intr_setaffinity(gtask->gt_irq, CPU_WHICH_IRQ, &mask);
 		mtx_lock(&qgroup->tqg_lock);
 		if (error)
-			printf("%s: %s setaffinity failed: %d\n", __func__, gtask->gt_name, error);
+			printf("%s: binding interrupt failed for %s: %d\n",
+			    __func__, gtask->gt_name, error);
 
 	}
 	qgroup->tqg_queue[qid].tgc_cnt++;
-
-	LIST_INSERT_HEAD(&qgroup->tqg_queue[qid].tgc_tasks, gtask,
-			 gt_list);
+	LIST_INSERT_HEAD(&qgroup->tqg_queue[qid].tgc_tasks, gtask, gt_list);
 	MPASS(qgroup->tqg_queue[qid].tgc_taskq != NULL);
 	gtask->gt_taskqueue = qgroup->tqg_queue[qid].tgc_taskq;
 	mtx_unlock(&qgroup->tqg_lock);
@@ -770,7 +772,7 @@ taskqgroup_attach_deferred(struct taskqgroup *qgroup, 
 
 int
 taskqgroup_attach_cpu(struct taskqgroup *qgroup, struct grouptask *gtask,
-	void *uniq, int cpu, int irq, const char *name)
+    void *uniq, int cpu, int irq, const char *name)
 {
 	cpuset_t mask;
 	int i, qid, error;
@@ -805,7 +807,8 @@ taskqgroup_attach_cpu(struct taskqgroup *qgroup, struc
 	if (irq != -1 && tqg_smp_started) {
 		error = intr_setaffinity(irq, CPU_WHICH_IRQ, &mask);
 		if (error)
-			printf("%s: setaffinity failed: %d\n", __func__, error);
+			printf("%s: binding interrupt failed for %s: %d\n",
+			    __func__, gtask->gt_name, error);
 	}
 	return (0);
 }
@@ -843,7 +846,8 @@ taskqgroup_attach_cpu_deferred(struct taskqgroup *qgro
 	if (irq != -1) {
 		error = intr_setaffinity(irq, CPU_WHICH_IRQ, &mask);
 		if (error)
-			printf("%s: setaffinity failed: %d\n", __func__, error);
+			printf("%s: binding interrupt failed for %s: %d\n",
+			    __func__, gtask->gt_name, error);
 	}
 	return (0);
 }
@@ -859,7 +863,7 @@ taskqgroup_detach(struct taskqgroup *qgroup, struct gr
 		if (qgroup->tqg_queue[i].tgc_taskq == gtask->gt_taskqueue)
 			break;
 	if (i == qgroup->tqg_cnt)
-		panic("taskqgroup_detach: task %s not in group\n", gtask->gt_name);
+		panic("%s: task %s not in group", __func__, gtask->gt_name);
 	qgroup->tqg_queue[i].tgc_cnt--;
 	LIST_REMOVE(gtask, gt_list);
 	mtx_unlock(&qgroup->tqg_lock);
@@ -882,8 +886,7 @@ taskqgroup_binder(void *ctx)
 	thread_unlock(curthread);
 
 	if (error)
-		printf("%s: setaffinity failed: %d\n", __func__,
-		    error);
+		printf("%s: binding curthread failed: %d\n", __func__, error);
 	free(gtask, M_DEVBUF);
 }
 
@@ -1051,7 +1054,7 @@ taskqgroup_destroy(struct taskqgroup *qgroup)
 
 void
 taskqgroup_config_gtask_init(void *ctx, struct grouptask *gtask, gtask_fn_t *fn,
-	const char *name)
+    const char *name)
 {
 
 	GROUPTASK_INIT(gtask, 0, fn, ctx);
@@ -1061,5 +1064,6 @@ taskqgroup_config_gtask_init(void *ctx, struct groupta
 void
 taskqgroup_config_gtask_deinit(struct grouptask *gtask)
 {
+
 	taskqgroup_detach(qgroup_config, gtask);
 }

Modified: stable/12/sys/net/iflib.c
==============================================================================
--- stable/12/sys/net/iflib.c	Sun Jun 16 13:51:45 2019	(r349108)
+++ stable/12/sys/net/iflib.c	Sun Jun 16 14:53:53 2019	(r349109)
@@ -4593,7 +4593,8 @@ iflib_device_register(device_t dev, void *sc, if_share
 
 	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, -1, "admin");
+	taskqgroup_attach(qgroup_if_config_tqg, &ctx->ifc_admin_task, ctx,
+	    -1, "admin");
 
 	/* 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) {
@@ -4858,7 +4859,8 @@ iflib_pseudo_register(device_t dev, if_shared_ctx_t sc
 
 	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, -1, "admin");
+	taskqgroup_attach(qgroup_if_config_tqg, &ctx->ifc_admin_task, ctx,
+	    -1, "admin");
 
 	/* XXX --- can support > 1 -- but keep it simple for now */
 	scctx->isc_intr = IFLIB_INTR_LEGACY;
@@ -5754,11 +5756,14 @@ get_core_offset(if_ctx_t ctx, iflib_intr_type_t type, 
 
 /* Just to avoid copy/paste */
 static inline int
-iflib_irq_set_affinity(if_ctx_t ctx, int irq, iflib_intr_type_t type, int qid,
-    struct grouptask *gtask, struct taskqgroup *tqg, void *uniq, const char *name)
+iflib_irq_set_affinity(if_ctx_t ctx, if_irq_t irq, iflib_intr_type_t type,
+    int qid, struct grouptask *gtask, struct taskqgroup *tqg, void *uniq,
+    const char *name)
 {
+	device_t dev;
 	int co, cpuid, err, tid;
 
+	dev = ctx->ifc_dev;
 	co = ctx->ifc_sysctl_core_offset;
 	if (ctx->ifc_sysctl_separate_txrx && type == IFLIB_INTR_TX)
 		co += ctx->ifc_softc_ctx.isc_nrxqsets;
@@ -5766,9 +5771,10 @@ iflib_irq_set_affinity(if_ctx_t ctx, int irq, iflib_in
 	tid = get_core_offset(ctx, type, qid);
 	MPASS(tid >= 0);
 	cpuid = find_close_core(cpuid, tid);
-	err = taskqgroup_attach_cpu(tqg, gtask, uniq, cpuid, irq, name);
+	err = taskqgroup_attach_cpu(tqg, gtask, uniq, cpuid,
+	    rman_get_start(irq->ii_res), name);
 	if (err) {
-		device_printf(ctx->ifc_dev, "taskqgroup_attach_cpu failed %d\n", err);
+		device_printf(dev, "taskqgroup_attach_cpu failed %d\n", err);
 		return (err);
 	}
 #ifdef notyet
@@ -5783,6 +5789,7 @@ iflib_irq_alloc_generic(if_ctx_t ctx, if_irq_t irq, in
 			iflib_intr_type_t type, driver_filter_t *filter,
 			void *filter_arg, int qid, const char *name)
 {
+	device_t dev;
 	struct grouptask *gtask;
 	struct taskqgroup *tqg;
 	iflib_filter_info_t info;
@@ -5842,20 +5849,23 @@ iflib_irq_alloc_generic(if_ctx_t ctx, if_irq_t irq, in
 	info->ifi_task = gtask;
 	info->ifi_ctx = q;
 
+	dev = ctx->ifc_dev;
 	err = _iflib_irq_alloc(ctx, irq, rid, intr_fast, NULL, info,  name);
 	if (err != 0) {
-		device_printf(ctx->ifc_dev, "_iflib_irq_alloc failed %d\n", err);
+		device_printf(dev, "_iflib_irq_alloc failed %d\n", err);
 		return (err);
 	}
 	if (type == IFLIB_INTR_ADMIN)
 		return (0);
 
 	if (tqrid != -1) {
-		err = iflib_irq_set_affinity(ctx, rman_get_start(irq->ii_res), type, qid, gtask, tqg, q, name);
+		err = iflib_irq_set_affinity(ctx, irq, type, qid, gtask, tqg,
+		    q, name);
 		if (err)
 			return (err);
 	} else {
-		taskqgroup_attach(tqg, gtask, q, rman_get_start(irq->ii_res), name);
+		taskqgroup_attach(tqg, gtask, q, rman_get_start(irq->ii_res),
+		    name);
 	}
 
 	return (0);
@@ -5868,7 +5878,6 @@ iflib_softirq_alloc_generic(if_ctx_t ctx, if_irq_t irq
 	struct taskqgroup *tqg;
 	gtask_fn_t *fn;
 	void *q;
-	int irq_num = -1;
 	int err;
 
 	switch (type) {
@@ -5877,16 +5886,12 @@ iflib_softirq_alloc_generic(if_ctx_t ctx, if_irq_t irq
 		gtask = &ctx->ifc_txqs[qid].ift_task;
 		tqg = qgroup_if_io_tqg;
 		fn = _task_fn_tx;
-		if (irq != NULL)
-			irq_num = rman_get_start(irq->ii_res);
 		break;
 	case IFLIB_INTR_RX:
 		q = &ctx->ifc_rxqs[qid];
 		gtask = &ctx->ifc_rxqs[qid].ifr_task;
 		tqg = qgroup_if_io_tqg;
 		fn = _task_fn_rx;
-		if (irq != NULL)
-			irq_num = rman_get_start(irq->ii_res);
 		break;
 	case IFLIB_INTR_IOV:
 		q = ctx;
@@ -5898,14 +5903,15 @@ iflib_softirq_alloc_generic(if_ctx_t ctx, if_irq_t irq
 		panic("unknown net intr type");
 	}
 	GROUPTASK_INIT(gtask, 0, fn, q);
-	if (irq_num != -1) {
-		err = iflib_irq_set_affinity(ctx, irq_num, type, qid, gtask, tqg, q, name);
+	if (irq != NULL) {
+		err = iflib_irq_set_affinity(ctx, irq, type, qid, gtask, tqg,
+		    q, name);
 		if (err)
-			taskqgroup_attach(tqg, gtask, q, irq_num, name);
+			taskqgroup_attach(tqg, gtask, q,
+			    rman_get_start(irq->ii_res), name);
+	} else {
+		taskqgroup_attach(tqg, gtask, q, -1, name);
 	}
-	else {
-		taskqgroup_attach(tqg, gtask, q, irq_num, name);
-	}
 }
 
 void
@@ -5954,7 +5960,8 @@ iflib_legacy_setup(if_ctx_t ctx, driver_filter_t filte
 	taskqgroup_attach(tqg, gtask, q, rman_get_start(irq->ii_res), name);
 
 	GROUPTASK_INIT(&txq->ift_task, 0, _task_fn_tx, txq);
-	taskqgroup_attach(qgroup_if_io_tqg, &txq->ift_task, txq, rman_get_start(irq->ii_res), "tx");
+	taskqgroup_attach(qgroup_if_io_tqg, &txq->ift_task, txq,
+	    rman_get_start(irq->ii_res), "tx");
 	return (0);
 }
 

Modified: stable/12/sys/sys/_task.h
==============================================================================
--- stable/12/sys/sys/_task.h	Sun Jun 16 13:51:45 2019	(r349108)
+++ stable/12/sys/sys/_task.h	Sun Jun 16 14:53:53 2019	(r349109)
@@ -39,8 +39,8 @@
  * field of struct task and the second argument is a count of how many
  * times the task was enqueued before the call to taskqueue_run().
  *
- * List of locks	 
- * (c)	const after init	 
+ * List of locks
+ * (c)	const after init
  * (q)	taskqueue lock
  */
 typedef void task_fn_t(void *context, int pending);

Modified: stable/12/sys/sys/gtaskqueue.h
==============================================================================
--- stable/12/sys/sys/gtaskqueue.h	Sun Jun 16 13:51:45 2019	(r349108)
+++ stable/12/sys/sys/gtaskqueue.h	Sun Jun 16 14:53:53 2019	(r349109)
@@ -38,7 +38,6 @@
 #endif
 
 struct gtaskqueue;
-typedef void (*gtaskqueue_enqueue_fn)(void *context);
 
 /*
  * Taskqueue groups.  Manages dynamic thread groups and irq binding for
@@ -55,28 +54,29 @@ void	gtaskqueue_drain_all(struct gtaskqueue *queue);
 void	grouptask_block(struct grouptask *grouptask);
 void	grouptask_unblock(struct grouptask *grouptask);
 int	grouptaskqueue_enqueue(struct gtaskqueue *queue, struct gtask *task);
+
 void	taskqgroup_attach(struct taskqgroup *qgroup, struct grouptask *grptask,
 	    void *uniq, int irq, const char *name);
-int		taskqgroup_attach_cpu(struct taskqgroup *qgroup, struct grouptask *grptask,
-		void *uniq, int cpu, int irq, const char *name);
+int	taskqgroup_attach_cpu(struct taskqgroup *qgroup,
+	    struct grouptask *grptask, void *uniq, int cpu, int irq,
+	    const char *name);
 void	taskqgroup_detach(struct taskqgroup *qgroup, struct grouptask *gtask);
 struct taskqgroup *taskqgroup_create(const char *name);
 void	taskqgroup_destroy(struct taskqgroup *qgroup);
 int	taskqgroup_adjust(struct taskqgroup *qgroup, int cnt, int stride);
-void	taskqgroup_config_gtask_init(void *ctx, struct grouptask *gtask, gtask_fn_t *fn,
-		const char *name);
+void	taskqgroup_config_gtask_init(void *ctx, struct grouptask *gtask,
+	    gtask_fn_t *fn, const char *name);
 void	taskqgroup_config_gtask_deinit(struct grouptask *gtask);
 
 #define TASK_ENQUEUED			0x1
 #define TASK_SKIP_WAKEUP		0x2
 #define TASK_NOENQUEUE			0x4
 
-
-#define GTASK_INIT(task, flags, priority, func, context) do {	\
-	(task)->ta_flags = flags;				\
-	(task)->ta_priority = (priority);		\
-	(task)->ta_func = (func);			\
-	(task)->ta_context = (context);			\
+#define	GTASK_INIT(gtask, flags, priority, func, context) do {	\
+	(gtask)->ta_flags = flags;				\
+	(gtask)->ta_priority = (priority);			\
+	(gtask)->ta_func = (func);				\
+	(gtask)->ta_context = (context);			\
 } while (0)
 
 #define	GROUPTASK_INIT(gtask, priority, func, context)	\



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