Date: Sun, 14 May 2000 11:31:32 -0600 From: Chuck Paterson <cp@bsdi.com> To: Doug Rabson <dfr@nlsystems.com> Cc: Arun Sharma <adsharma@sharmas.dhs.org>, freebsd-arch@freebsd.org Subject: Re: A new api for asynchronous task execution Message-ID: <200005141731.LAA04793@berserker.bsdi.com>
next in thread | raw e-mail | index | archive | help
While writing up this I noticed a trivial bug
taskqueue_enqueue(struct taskqueue *queue, struct task *task)
{
int s = splhigh();
/*
* Count multiple enqueues.
*/
if (task->pending) {
task->pending++;
return; <---- really need a splx before this
}
}
}I agree that the queue should have a mutex to protect itself. Since we
}haven't finalised the api for mutex locking (we will almost certainly use
}the BSD/OS one), I can't add anything yet but I will when the new SMP
}work starts.
FYI, the current BSD/OS mutex's have a string associated
with them, ie there name. This name is used for debugging (checking
lock order) and perhaps for blowing locks open at panic time.
Mutex's which aren't part of a group of a unique name, an example
of this might be "ipqlock". For mutex's which are all really the
same they share the same name such as "vnode interlock". There is
also the issue of what flags are passed in to initialize the mutex,
and for that matter flags used in acquiring the mutex. I guess I
should for now just point out some of the issues.
1) It would be useful to have meaningful name associated with
a mutex when initializing the mutex associated with a taskqueue.
2) There are flags needed with initializing the mutex associated
with a taskqueue.
3) When acquiring the mutex associated with a queue it wants to
in general not be of type M_SPIN, but sometimes it will.
4) As proposed now the mutex protecting the queue often does not
want to be held while the enqueueing function is called. This is
particularly true if the enqueueing function may cause an immediate
context switch.
5) Even the taskqueue_run really wants to just know what
type of mutex is present.
In some ways the implementation of taskqueue_enqueue() is
not as important to get right from the start as the calling sequence
to it. There are almost certainly going to be lots more users of
the swi taskqueue than anything else and changing the few actual
implementation of the enqueuer function and init function seem like
not a big deal.
I have included just some sketches of how you could deal
with some of this. Just something to think about
The following is very slightly slower because the type of
mutex is not known at compile times, but it makes it possible
to not have to totally change to model.
Chuck
struct taskqueue {
STAILQ_HEAD(, task) queue;
mtx_t mtx;
int mtx_flags;
void (*enqueue)(struct taskqueue *queue);
};
void
taskqueue_init(struct taskqueue *queue,
void (*enqueue)(struct taskqueue *queue), char *name, int flags)
{
STAILQ_INIT(&queue->queue);
queue->enqueue = enqueue;
queue->mtx_flags = flags & TQ_SPIN ? M_SPIN, M_DEF;
mtx_init(&queue->mtx, name, queueu->mtx_flags;
}
void
taskqueue_enqueue(struct taskqueue *queue, struct task *task)
{
mtx_enter(&queue->mtx, queue->mtx_flags);
/*
* Count multiple enqueues.
*/
if (task->pending) {
task->pending++;
mtx_exit(&queue->mtx, queue->mtx_flags);
return;
}
STAILQ_INSERT_TAIL(&queue->queue, task, link);
task->pending = 1;
mtx_exit(&queue->mtx, queue->mtx_flags);
if (queue->enqueue)
queue->enqueue(queue);
}
void
taskqueue_run(struct taskqueue *queue)
{
int s;
struct task *task;
int pending;
mtx_enter(&queue->mtx, queue->mtx_flags);
while (STAILQ_FIRST(&queue->queue)) {
/*
* Carefully remove the first task from the queue and
* zero its pending count.
*/
task = STAILQ_FIRST(&queue->queue);
STAILQ_REMOVE_HEAD(&queue->queue, link);
pending = task->pending;
task->pending = 0;
mtx_exit(&queue->mtx, queue->mtx_flags);
task->func(task->arg, pending);
mtx_enter(&queue->mtx, queue->mtx_flags);
}
mtx_exit(&queue->mtx, queue->mtx_flags);
}
/*
* The following will probably work, but cheating
* on locking probably ought not to be done until it has been
* shown that in the particular path that the performance
* is worth the potential risk.
*/
void
taskqueue_run(struct taskqueue *queue)
{
int s;
struct task *task;
int pending;
while (STAILQ_FIRST(&queue->queue)) {
mtx_enter(&queue->mtx, queue->mtx_flags);
/*
* Carefully remove the first task from the queue and
* zero its pending count.
*/
task = STAILQ_FIRST(&queue->queue);
STAILQ_REMOVE_HEAD(&queue->queue, link);
pending = task->pending;
task->pending = 0;
mtx_exit(&queue->mtx, queue->mtx_flags);
task->func(task->arg, pending);
}
}
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200005141731.LAA04793>