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
This commit is contained in:
Alexander Motin 2023-10-13 13:41:11 -04:00 committed by GitHub
parent 8a74070128
commit 380c25f640
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 46 additions and 46 deletions

View File

@ -30,9 +30,9 @@
#include <sys/types.h> #include <sys/types.h>
#include <sys/proc.h> #include <sys/proc.h>
#include <sys/queue.h>
#include <sys/taskqueue.h> #include <sys/taskqueue.h>
#include <sys/thread.h> #include <sys/thread.h>
#include <sys/ck.h>
#ifdef __cplusplus #ifdef __cplusplus
extern "C" { extern "C" {
@ -48,16 +48,16 @@ typedef uintptr_t taskqid_t;
typedef void (task_func_t)(void *); typedef void (task_func_t)(void *);
typedef struct taskq_ent { typedef struct taskq_ent {
union {
struct task tqent_task; struct task tqent_task;
struct timeout_task tqent_timeout_task; struct timeout_task tqent_timeout_task;
};
task_func_t *tqent_func; task_func_t *tqent_func;
void *tqent_arg; void *tqent_arg;
taskqid_t tqent_id; taskqid_t tqent_id;
CK_LIST_ENTRY(taskq_ent) tqent_hash; LIST_ENTRY(taskq_ent) tqent_hash;
uint8_t tqent_type; uint_t tqent_type;
uint8_t tqent_registered; volatile uint_t tqent_rc;
uint8_t tqent_cancelled;
volatile uint32_t tqent_rc;
} taskq_ent_t; } taskq_ent_t;
/* /*

View File

@ -30,8 +30,6 @@
__FBSDID("$FreeBSD$"); __FBSDID("$FreeBSD$");
#include <sys/param.h> #include <sys/param.h>
#include <sys/ck.h>
#include <sys/epoch.h>
#include <sys/kernel.h> #include <sys/kernel.h>
#include <sys/kmem.h> #include <sys/kmem.h>
#include <sys/lock.h> #include <sys/lock.h>
@ -70,7 +68,7 @@ extern int uma_align_cache;
static MALLOC_DEFINE(M_TASKQ, "taskq", "taskq structures"); 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 tqenthash;
static unsigned long tqenthashlock; static unsigned long tqenthashlock;
static struct sx *tqenthashtbl_lock; static struct sx *tqenthashtbl_lock;
@ -80,8 +78,8 @@ static taskqid_t tqidnext;
#define TQIDHASH(tqid) (&tqenthashtbl[(tqid) & tqenthash]) #define TQIDHASH(tqid) (&tqenthashtbl[(tqid) & tqenthash])
#define TQIDHASHLOCK(tqid) (&tqenthashtbl_lock[((tqid) & tqenthashlock)]) #define TQIDHASHLOCK(tqid) (&tqenthashtbl_lock[((tqid) & tqenthashlock)])
#define NORMAL_TASK 0
#define TIMEOUT_TASK 1 #define TIMEOUT_TASK 1
#define NORMAL_TASK 2
static void static void
system_taskq_init(void *arg) system_taskq_init(void *arg)
@ -121,7 +119,7 @@ system_taskq_fini(void *arg)
for (i = 0; i < tqenthashlock + 1; i++) for (i = 0; i < tqenthashlock + 1; i++)
sx_destroy(&tqenthashtbl_lock[i]); sx_destroy(&tqenthashtbl_lock[i]);
for (i = 0; i < tqenthash + 1; 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_lock, M_TASKQ);
free(tqenthashtbl, M_TASKQ); free(tqenthashtbl, M_TASKQ);
} }
@ -162,27 +160,27 @@ taskq_lookup(taskqid_t tqid)
{ {
taskq_ent_t *ent = NULL; taskq_ent_t *ent = NULL;
sx_xlock(TQIDHASHLOCK(tqid)); if (tqid == 0)
CK_LIST_FOREACH(ent, TQIDHASH(tqid), tqent_hash) { return (NULL);
sx_slock(TQIDHASHLOCK(tqid));
LIST_FOREACH(ent, TQIDHASH(tqid), tqent_hash) {
if (ent->tqent_id == tqid) if (ent->tqent_id == tqid)
break; break;
} }
if (ent != NULL) if (ent != NULL)
refcount_acquire(&ent->tqent_rc); refcount_acquire(&ent->tqent_rc);
sx_xunlock(TQIDHASHLOCK(tqid)); sx_sunlock(TQIDHASHLOCK(tqid));
return (ent); return (ent);
} }
static taskqid_t static taskqid_t
taskq_insert(taskq_ent_t *ent) taskq_insert(taskq_ent_t *ent)
{ {
taskqid_t tqid; taskqid_t tqid = __taskq_genid();
tqid = __taskq_genid();
ent->tqent_id = tqid; ent->tqent_id = tqid;
ent->tqent_registered = B_TRUE;
sx_xlock(TQIDHASHLOCK(tqid)); 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)); sx_xunlock(TQIDHASHLOCK(tqid));
return (tqid); return (tqid);
} }
@ -192,13 +190,14 @@ taskq_remove(taskq_ent_t *ent)
{ {
taskqid_t tqid = ent->tqent_id; taskqid_t tqid = ent->tqent_id;
if (!ent->tqent_registered) if (tqid == 0)
return; return;
sx_xlock(TQIDHASHLOCK(tqid)); 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)); sx_xunlock(TQIDHASHLOCK(tqid));
ent->tqent_registered = B_FALSE;
} }
static void static void
@ -285,21 +284,22 @@ taskq_cancel_id(taskq_t *tq, taskqid_t tid)
int rc; int rc;
taskq_ent_t *ent; taskq_ent_t *ent;
if (tid == 0)
return (0);
if ((ent = taskq_lookup(tid)) == NULL) if ((ent = taskq_lookup(tid)) == NULL)
return (0); return (0);
ent->tqent_cancelled = B_TRUE; if (ent->tqent_type == NORMAL_TASK) {
if (ent->tqent_type == TIMEOUT_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, rc = taskqueue_cancel_timeout(tq->tq_queue,
&ent->tqent_timeout_task, &pend); &ent->tqent_timeout_task, &pend);
} else
rc = taskqueue_cancel(tq->tq_queue, &ent->tqent_task, &pend);
if (rc == EBUSY) { if (rc == EBUSY) {
taskqueue_drain(tq->tq_queue, &ent->tqent_task); taskqueue_drain_timeout(tq->tq_queue,
} else if (pend) { &ent->tqent_timeout_task);
}
}
if (pend) {
/* /*
* Tasks normally free themselves when run, but here the task * Tasks normally free themselves when run, but here the task
* was cancelled so it did not free itself. * was cancelled so it did not free itself.
@ -312,11 +312,12 @@ taskq_cancel_id(taskq_t *tq, taskqid_t tid)
} }
static void static void
taskq_run(void *arg, int pending __unused) taskq_run(void *arg, int pending)
{ {
taskq_ent_t *task = arg; taskq_ent_t *task = arg;
if (!task->tqent_cancelled) if (pending == 0)
return;
task->tqent_func(task->tqent_arg); task->tqent_func(task->tqent_arg);
taskq_free(task); 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_func = func;
task->tqent_arg = arg; task->tqent_arg = arg;
task->tqent_type = TIMEOUT_TASK; task->tqent_type = TIMEOUT_TASK;
task->tqent_cancelled = B_FALSE;
refcount_init(&task->tqent_rc, 1); refcount_init(&task->tqent_rc, 1);
tqid = taskq_insert(task); tqid = taskq_insert(task);
TIMEOUT_TASK_INIT(tq->tq_queue, &task->tqent_timeout_task, 0, 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); refcount_init(&task->tqent_rc, 1);
task->tqent_func = func; task->tqent_func = func;
task->tqent_arg = arg; task->tqent_arg = arg;
task->tqent_cancelled = B_FALSE;
task->tqent_type = NORMAL_TASK; task->tqent_type = NORMAL_TASK;
tqid = taskq_insert(task); tqid = taskq_insert(task);
TASK_INIT(&task->tqent_task, prio, taskq_run, 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 static void
taskq_run_ent(void *arg, int pending __unused) taskq_run_ent(void *arg, int pending)
{ {
taskq_ent_t *task = arg; taskq_ent_t *task = arg;
if (pending == 0)
return;
task->tqent_func(task->tqent_arg); 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. * can go at the front of the queue.
*/ */
prio = !!(flags & TQ_FRONT); prio = !!(flags & TQ_FRONT);
task->tqent_cancelled = B_FALSE;
task->tqent_registered = B_FALSE;
task->tqent_id = 0; task->tqent_id = 0;
task->tqent_func = func; task->tqent_func = func;
task->tqent_arg = arg; task->tqent_arg = arg;
@ -427,12 +426,13 @@ taskq_wait_id(taskq_t *tq, taskqid_t tid)
{ {
taskq_ent_t *ent; taskq_ent_t *ent;
if (tid == 0)
return;
if ((ent = taskq_lookup(tid)) == NULL) if ((ent = taskq_lookup(tid)) == NULL)
return; return;
if (ent->tqent_type == NORMAL_TASK)
taskqueue_drain(tq->tq_queue, &ent->tqent_task); taskqueue_drain(tq->tq_queue, &ent->tqent_task);
else
taskqueue_drain_timeout(tq->tq_queue, &ent->tqent_timeout_task);
taskq_free(ent); taskq_free(ent);
} }