Skip to content

Commit

Permalink
FreeBSD: Improve taskq wrapper
Browse files Browse the repository at this point in the history
 - 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().

Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
  • Loading branch information
amotin committed Oct 5, 2023
1 parent 2a6c621 commit 8bbcc3e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 37 deletions.
14 changes: 7 additions & 7 deletions include/os/freebsd/spl/sys/taskq.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
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;
uint_t tqent_type;
volatile uint_t tqent_rc;
} taskq_ent_t;

/*
Expand Down
60 changes: 30 additions & 30 deletions module/os/freebsd/spl/spl_taskq.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,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)
Expand Down Expand Up @@ -162,25 +162,25 @@ taskq_lookup(taskqid_t tqid)
{
taskq_ent_t *ent = NULL;

sx_xlock(TQIDHASHLOCK(tqid));
if (tqid == 0)
return (NULL);
sx_slock(TQIDHASHLOCK(tqid));
CK_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);
sx_xunlock(TQIDHASHLOCK(tqid));
Expand All @@ -192,13 +192,12 @@ 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);
sx_xunlock(TQIDHASHLOCK(tqid));
ent->tqent_registered = B_FALSE;
ent->tqent_id = 0;
}

static void
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand All @@ -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;
Expand All @@ -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);
}

Expand Down

0 comments on commit 8bbcc3e

Please sign in to comment.