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().
 - Switch from CK_LIST to regular LIST.

Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Mateusz Guzik <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15356
  • Loading branch information
@amotin @tonyhutter
amotin authored and tonyhutter committed Nov 7, 2023
1 parent 459c99f commit b76724a
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 46 deletions.
18 changes: 9 additions & 9 deletions include/os/freebsd/spl/sys/taskq.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand All @@ -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;

/*
Expand Down
74 changes: 37 additions & 37 deletions module/os/freebsd/spl/spl_taskq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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
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 b76724a

Please sign in to comment.