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

diff --git a/include/os/freebsd/spl/sys/taskq.h b/include/os/freebsd/spl/sys/taskq.h
index 30579b391711b8165f01e21f194c897c5873d7ac..b23a939b3aa717a31c8d760b2560c3444dc2c08d 100644 (file)
--- 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 ba22c77b69c33ce9ed0cf12546a41fb17a3fdfde..daefe3559538e84a4c2a5782819aca728c6ea7bd 100644 (file)
--- 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);
 }