From b76724ae478a7c2f73693b39d8009101efb54995 Mon Sep 17 00:00:00 2001
From: Alexander Motin <mav@FreeBSD.org>
Date: Fri, 13 Oct 2023 13:41:11 -0400
Subject: [PATCH] FreeBSD: Improve taskq wrapper

- Group tqent_task and tqent_timeout_task into a union.  They are
never used same time. This shrinks taskq_ent_t from 192 to 160 bytes.
 - Remove tqent_registered.  Use tqent_id != 0 instead.
 - Remove tqent_cancelled.  Use taskqueue pending counter instead.
 - Change tqent_type into uint_t.  We don't need to pack it any more.
 - Change tqent_rc into uint_t, matching refcount(9).
 - Take shared locks in taskq_lookup().
 - Call proper taskqueue_drain_timeout() for TIMEOUT_TASK in
taskq_cancel_id() and taskq_wait_id().
 - Switch from CK_LIST to regular LIST.

Reviewed-by: Allan Jude <allan@klarasystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes #15356
---
 include/os/freebsd/spl/sys/taskq.h | 18 ++++----
 module/os/freebsd/spl/spl_taskq.c  | 74 +++++++++++++++---------------
 2 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/include/os/freebsd/spl/sys/taskq.h b/include/os/freebsd/spl/sys/taskq.h
index 30579b3917..b23a939b3a 100644
--- a/include/os/freebsd/spl/sys/taskq.h
+++ b/include/os/freebsd/spl/sys/taskq.h
@@ -30,9 +30,9 @@
 
 #include <sys/types.h>
 #include <sys/proc.h>
+#include <sys/queue.h>
 #include <sys/taskqueue.h>
 #include <sys/thread.h>
-#include <sys/ck.h>
 
 #ifdef	__cplusplus
 extern "C" {
@@ -48,16 +48,16 @@ typedef uintptr_t taskqid_t;
 typedef void (task_func_t)(void *);
 
 typedef struct taskq_ent {
-	struct task	 tqent_task;
-	struct timeout_task tqent_timeout_task;
+	union {
+		struct task	 tqent_task;
+		struct timeout_task tqent_timeout_task;
+	};
 	task_func_t	*tqent_func;
 	void		*tqent_arg;
-	taskqid_t tqent_id;
-	CK_LIST_ENTRY(taskq_ent) tqent_hash;
-	uint8_t tqent_type;
-	uint8_t tqent_registered;
-	uint8_t tqent_cancelled;
-	volatile uint32_t tqent_rc;
+	taskqid_t	 tqent_id;
+	LIST_ENTRY(taskq_ent) tqent_hash;
+	uint_t		 tqent_type;
+	volatile uint_t	 tqent_rc;
 } taskq_ent_t;
 
 /*
diff --git a/module/os/freebsd/spl/spl_taskq.c b/module/os/freebsd/spl/spl_taskq.c
index ba22c77b69..daefe35595 100644
--- a/module/os/freebsd/spl/spl_taskq.c
+++ b/module/os/freebsd/spl/spl_taskq.c
@@ -30,8 +30,6 @@
 __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
-#include <sys/ck.h>
-#include <sys/epoch.h>
 #include <sys/kernel.h>
 #include <sys/kmem.h>
 #include <sys/lock.h>
@@ -70,7 +68,7 @@ extern int uma_align_cache;
 
 static MALLOC_DEFINE(M_TASKQ, "taskq", "taskq structures");
 
-static CK_LIST_HEAD(tqenthashhead, taskq_ent) *tqenthashtbl;
+static LIST_HEAD(tqenthashhead, taskq_ent) *tqenthashtbl;
 static unsigned long tqenthash;
 static unsigned long tqenthashlock;
 static struct sx *tqenthashtbl_lock;
@@ -80,8 +78,8 @@ static taskqid_t tqidnext;
 #define	TQIDHASH(tqid) (&tqenthashtbl[(tqid) & tqenthash])
 #define	TQIDHASHLOCK(tqid) (&tqenthashtbl_lock[((tqid) & tqenthashlock)])
 
+#define	NORMAL_TASK 0
 #define	TIMEOUT_TASK 1
-#define	NORMAL_TASK 2
 
 static void
 system_taskq_init(void *arg)
@@ -121,7 +119,7 @@ system_taskq_fini(void *arg)
 	for (i = 0; i < tqenthashlock + 1; i++)
 		sx_destroy(&tqenthashtbl_lock[i]);
 	for (i = 0; i < tqenthash + 1; i++)
-		VERIFY(CK_LIST_EMPTY(&tqenthashtbl[i]));
+		VERIFY(LIST_EMPTY(&tqenthashtbl[i]));
 	free(tqenthashtbl_lock, M_TASKQ);
 	free(tqenthashtbl, M_TASKQ);
 }
@@ -162,27 +160,27 @@ taskq_lookup(taskqid_t tqid)
 {
 	taskq_ent_t *ent = NULL;
 
-	sx_xlock(TQIDHASHLOCK(tqid));
-	CK_LIST_FOREACH(ent, TQIDHASH(tqid), tqent_hash) {
+	if (tqid == 0)
+		return (NULL);
+	sx_slock(TQIDHASHLOCK(tqid));
+	LIST_FOREACH(ent, TQIDHASH(tqid), tqent_hash) {
 		if (ent->tqent_id == tqid)
 			break;
 	}
 	if (ent != NULL)
 		refcount_acquire(&ent->tqent_rc);
-	sx_xunlock(TQIDHASHLOCK(tqid));
+	sx_sunlock(TQIDHASHLOCK(tqid));
 	return (ent);
 }
 
 static taskqid_t
 taskq_insert(taskq_ent_t *ent)
 {
-	taskqid_t tqid;
+	taskqid_t tqid = __taskq_genid();
 
-	tqid = __taskq_genid();
 	ent->tqent_id = tqid;
-	ent->tqent_registered = B_TRUE;
 	sx_xlock(TQIDHASHLOCK(tqid));
-	CK_LIST_INSERT_HEAD(TQIDHASH(tqid), ent, tqent_hash);
+	LIST_INSERT_HEAD(TQIDHASH(tqid), ent, tqent_hash);
 	sx_xunlock(TQIDHASHLOCK(tqid));
 	return (tqid);
 }
@@ -192,13 +190,14 @@ taskq_remove(taskq_ent_t *ent)
 {
 	taskqid_t tqid = ent->tqent_id;
 
-	if (!ent->tqent_registered)
+	if (tqid == 0)
 		return;
-
 	sx_xlock(TQIDHASHLOCK(tqid));
-	CK_LIST_REMOVE(ent, tqent_hash);
+	if (ent->tqent_id != 0) {
+		LIST_REMOVE(ent, tqent_hash);
+		ent->tqent_id = 0;
+	}
 	sx_xunlock(TQIDHASHLOCK(tqid));
-	ent->tqent_registered = B_FALSE;
 }
 
 static void
@@ -285,21 +284,22 @@ taskq_cancel_id(taskq_t *tq, taskqid_t tid)
 	int rc;
 	taskq_ent_t *ent;
 
-	if (tid == 0)
-		return (0);
-
 	if ((ent = taskq_lookup(tid)) == NULL)
 		return (0);
 
-	ent->tqent_cancelled = B_TRUE;
-	if (ent->tqent_type == TIMEOUT_TASK) {
+	if (ent->tqent_type == NORMAL_TASK) {
+		rc = taskqueue_cancel(tq->tq_queue, &ent->tqent_task, &pend);
+		if (rc == EBUSY)
+			taskqueue_drain(tq->tq_queue, &ent->tqent_task);
+	} else {
 		rc = taskqueue_cancel_timeout(tq->tq_queue,
 		    &ent->tqent_timeout_task, &pend);
-	} else
-		rc = taskqueue_cancel(tq->tq_queue, &ent->tqent_task, &pend);
-	if (rc == EBUSY) {
-		taskqueue_drain(tq->tq_queue, &ent->tqent_task);
-	} else if (pend) {
+		if (rc == EBUSY) {
+			taskqueue_drain_timeout(tq->tq_queue,
+			    &ent->tqent_timeout_task);
+		}
+	}
+	if (pend) {
 		/*
 		 * Tasks normally free themselves when run, but here the task
 		 * was cancelled so it did not free itself.
@@ -312,12 +312,13 @@ taskq_cancel_id(taskq_t *tq, taskqid_t tid)
 }
 
 static void
-taskq_run(void *arg, int pending __unused)
+taskq_run(void *arg, int pending)
 {
 	taskq_ent_t *task = arg;
 
-	if (!task->tqent_cancelled)
-		task->tqent_func(task->tqent_arg);
+	if (pending == 0)
+		return;
+	task->tqent_func(task->tqent_arg);
 	taskq_free(task);
 }
 
@@ -345,7 +346,6 @@ taskq_dispatch_delay(taskq_t *tq, task_func_t func, void *arg,
 	task->tqent_func = func;
 	task->tqent_arg = arg;
 	task->tqent_type = TIMEOUT_TASK;
-	task->tqent_cancelled = B_FALSE;
 	refcount_init(&task->tqent_rc, 1);
 	tqid = taskq_insert(task);
 	TIMEOUT_TASK_INIT(tq->tq_queue, &task->tqent_timeout_task, 0,
@@ -379,7 +379,6 @@ taskq_dispatch(taskq_t *tq, task_func_t func, void *arg, uint_t flags)
 	refcount_init(&task->tqent_rc, 1);
 	task->tqent_func = func;
 	task->tqent_arg = arg;
-	task->tqent_cancelled = B_FALSE;
 	task->tqent_type = NORMAL_TASK;
 	tqid = taskq_insert(task);
 	TASK_INIT(&task->tqent_task, prio, taskq_run, task);
@@ -388,10 +387,12 @@ taskq_dispatch(taskq_t *tq, task_func_t func, void *arg, uint_t flags)
 }
 
 static void
-taskq_run_ent(void *arg, int pending __unused)
+taskq_run_ent(void *arg, int pending)
 {
 	taskq_ent_t *task = arg;
 
+	if (pending == 0)
+		return;
 	task->tqent_func(task->tqent_arg);
 }
 
@@ -406,8 +407,6 @@ taskq_dispatch_ent(taskq_t *tq, task_func_t func, void *arg, uint32_t flags,
 	 * can go at the front of the queue.
 	 */
 	prio = !!(flags & TQ_FRONT);
-	task->tqent_cancelled = B_FALSE;
-	task->tqent_registered = B_FALSE;
 	task->tqent_id = 0;
 	task->tqent_func = func;
 	task->tqent_arg = arg;
@@ -427,12 +426,13 @@ taskq_wait_id(taskq_t *tq, taskqid_t tid)
 {
 	taskq_ent_t *ent;
 
-	if (tid == 0)
-		return;
 	if ((ent = taskq_lookup(tid)) == NULL)
 		return;
 
-	taskqueue_drain(tq->tq_queue, &ent->tqent_task);
+	if (ent->tqent_type == NORMAL_TASK)
+		taskqueue_drain(tq->tq_queue, &ent->tqent_task);
+	else
+		taskqueue_drain_timeout(tq->tq_queue, &ent->tqent_timeout_task);
 	taskq_free(ent);
 }