svn commit: r248649 - in head: share/man/man9 sys/kern sys/sys
Will Andrews
will at firepipe.net
Sat Mar 23 16:45:31 UTC 2013
I agree about the name length, it is a bit obnoxious. However, it is also
descriptive. TQCB strikes me as perhaps a bit too far in the other
direction. How about TQ_CALLBACK_? Is there an existing (pseudo)
convention for callback names?
Thanks,
--Will.
On Sat, Mar 23, 2013 at 10:20 AM, <mdf at freebsd.org> wrote:
> On Sat, Mar 23, 2013 at 8:11 AM, Will Andrews <will at freebsd.org> wrote:
> > Author: will
> > Date: Sat Mar 23 15:11:53 2013
> > New Revision: 248649
> > URL: http://svnweb.freebsd.org/changeset/base/248649
> >
> > Log:
> > Extend taskqueue(9) to enable per-taskqueue callbacks.
> >
> > The scope of these callbacks is primarily to support actions that
> affect the
> > taskqueue's thread environments. They are entirely optional, and
> > consequently are introduced as a new API: taskqueue_set_callback().
> >
> > This interface allows the caller to specify that a taskqueue requires a
> > callback and optional context pointer for a given callback type.
> >
> > The callback types included in this commit can be used to register a
> > constructor and destructor for thread-local storage using osd(9). This
> > allows a particular taskqueue to define that its threads require a
> specific
> > type of TLS, without the need for a specially-orchestrated task-based
> > mechanism for startup and shutdown in order to accomplish it.
> >
> > Two callback types are supported at this point:
> >
> > - TASKQUEUE_CALLBACK_TYPE_INIT, called by every thread when it starts,
> prior
> > to processing any tasks.
> > - TASKQUEUE_CALLBACK_TYPE_SHUTDOWN, called by every thread when it
> exits,
> > after it has processed its last task but before the taskqueue is
> > reclaimed.
> >
> > While I'm here:
> >
> > - Add two new macros, TQ_ASSERT_LOCKED and TQ_ASSERT_UNLOCKED, and use
> them
> > in appropriate locations.
> > - Fix taskqueue.9 to mention taskqueue_start_threads(), which is a
> required
> > interface for all consumers of taskqueue(9).
> >
> > Reviewed by: kib (all), eadler (taskqueue.9), brd (taskqueue.9)
> > Approved by: ken (mentor)
> > Sponsored by: Spectra Logic
> > MFC after: 1 month
> >
> > Modified:
> > head/share/man/man9/taskqueue.9
> > head/sys/kern/subr_taskqueue.c
> > head/sys/sys/taskqueue.h
> >
> > Modified: head/share/man/man9/taskqueue.9
> >
> ==============================================================================
> > --- head/share/man/man9/taskqueue.9 Sat Mar 23 14:52:31 2013
> (r248648)
> > +++ head/share/man/man9/taskqueue.9 Sat Mar 23 15:11:53 2013
> (r248649)
> > @@ -53,12 +53,23 @@ struct task {
> > void *ta_context; /* argument for handler
> */
> > };
> >
> > +enum taskqueue_callback_type {
> > + TASKQUEUE_CALLBACK_TYPE_INIT,
> > + TASKQUEUE_CALLBACK_TYPE_SHUTDOWN,
> > +};
> > +
> > +typedef void (*taskqueue_callback_fn)(void *context);
> > +
> > struct timeout_task;
> > .Ed
> > .Ft struct taskqueue *
> > .Fn taskqueue_create "const char *name" "int mflags"
> "taskqueue_enqueue_fn enqueue" "void *context"
> > .Ft struct taskqueue *
> > .Fn taskqueue_create_fast "const char *name" "int mflags"
> "taskqueue_enqueue_fn enqueue" "void *context"
> > +.Ft int
> > +.Fn taskqueue_start_threads "struct taskqueue **tqp" "int count" "int
> pri" "const char *name" "..."
> > +.Ft void
> > +.Fn taskqueue_set_callback "struct taskqueue *queue" "enum
> taskqueue_callback_type cb_type" "taskqueue_callback_fn callback" "void
> *context"
> > .Ft void
> > .Fn taskqueue_free "struct taskqueue *queue"
> > .Ft int
> > @@ -127,6 +138,23 @@ should be used to free the memory used b
> > Any tasks that are on the queue will be executed at this time after
> > which the thread servicing the queue will be signaled that it should
> exit.
> > .Pp
> > +Once a taskqueue has been created, its threads should be started using
> > +.Fn taskqueue_start_threads .
> > +Callbacks may optionally be registered using
> > +.Fn taskqueue_set_callback .
> > +Currently, callbacks may be registered for the following purposes:
> > +.Bl -tag -width TASKQUEUE_CALLBACK_TYPE_SHUTDOWN
> > +.It Dv TASKQUEUE_CALLBACK_TYPE_INIT
> > +This callback is called by every thread in the taskqueue, before it
> executes
> > +any tasks.
> > +This callback must be set before the taskqueue's threads are started.
> > +.It Dv TASKQUEUE_CALLBACK_TYPE_SHUTDOWN
> > +This callback is called by every thread in the taskqueue, after it
> executes
> > +its last task.
> > +This callback will always be called before the taskqueue structure is
> > +reclaimed.
> > +.El
> > +.Pp
> > To add a task to the list of tasks queued on a taskqueue, call
> > .Fn taskqueue_enqueue
> > with pointers to the queue and task.
> >
> > Modified: head/sys/kern/subr_taskqueue.c
> >
> ==============================================================================
> > --- head/sys/kern/subr_taskqueue.c Sat Mar 23 14:52:31 2013
> (r248648)
> > +++ head/sys/kern/subr_taskqueue.c Sat Mar 23 15:11:53 2013
> (r248649)
> > @@ -63,6 +63,8 @@ struct taskqueue {
> > int tq_spin;
> > int tq_flags;
> > int tq_callouts;
> > + taskqueue_callback_fn tq_callbacks[TASKQUEUE_NUM_CALLBACKS];
> > + void *tq_cb_contexts[TASKQUEUE_NUM_CALLBACKS];
> > };
> >
> > #define TQ_FLAGS_ACTIVE (1 << 0)
> > @@ -78,6 +80,7 @@ struct taskqueue {
> > else \
> > mtx_lock(&(tq)->tq_mutex); \
> > } while (0)
> > +#define TQ_ASSERT_LOCKED(tq) mtx_assert(&(tq)->tq_mutex,
> MA_OWNED)
> >
> > #define TQ_UNLOCK(tq)
> \
> > do { \
> > @@ -86,6 +89,7 @@ struct taskqueue {
> > else \
> > mtx_unlock(&(tq)->tq_mutex); \
> > } while (0)
> > +#define TQ_ASSERT_UNLOCKED(tq) mtx_assert(&(tq)->tq_mutex,
> MA_NOTOWNED)
> >
> > void
> > _timeout_task_init(struct taskqueue *queue, struct timeout_task
> *timeout_task,
> > @@ -137,6 +141,23 @@ taskqueue_create(const char *name, int m
> > MTX_DEF, "taskqueue");
> > }
> >
> > +void
> > +taskqueue_set_callback(struct taskqueue *queue,
> > + enum taskqueue_callback_type cb_type, taskqueue_callback_fn
> callback,
> > + void *context)
> > +{
> > +
> > + KASSERT(((cb_type >= TASKQUEUE_CALLBACK_TYPE_MIN) &&
> > + (cb_type <= TASKQUEUE_CALLBACK_TYPE_MAX)),
> > + ("Callback type %d not valid, must be %d-%d", cb_type,
> > + TASKQUEUE_CALLBACK_TYPE_MIN, TASKQUEUE_CALLBACK_TYPE_MAX));
> > + KASSERT((queue->tq_callbacks[cb_type] == NULL),
> > + ("Re-initialization of taskqueue callback?"));
> > +
> > + queue->tq_callbacks[cb_type] = callback;
> > + queue->tq_cb_contexts[cb_type] = context;
> > +}
> > +
> > /*
> > * Signal a taskqueue thread to terminate.
> > */
> > @@ -293,7 +314,7 @@ taskqueue_run_locked(struct taskqueue *q
> > struct task *task;
> > int pending;
> >
> > - mtx_assert(&queue->tq_mutex, MA_OWNED);
> > + TQ_ASSERT_LOCKED(queue);
> > tb.tb_running = NULL;
> > TAILQ_INSERT_TAIL(&queue->tq_active, &tb, tb_link);
> >
> > @@ -332,7 +353,7 @@ task_is_running(struct taskqueue *queue,
> > {
> > struct taskqueue_busy *tb;
> >
> > - mtx_assert(&queue->tq_mutex, MA_OWNED);
> > + TQ_ASSERT_LOCKED(queue);
> > TAILQ_FOREACH(tb, &queue->tq_active, tb_link) {
> > if (tb->tb_running == task)
> > return (1);
> > @@ -489,6 +510,18 @@ taskqueue_start_threads(struct taskqueue
> > return (0);
> > }
> >
> > +static inline void
> > +taskqueue_run_callback(struct taskqueue *tq,
> > + enum taskqueue_callback_type cb_type)
> > +{
> > + taskqueue_callback_fn tq_callback;
> > +
> > + TQ_ASSERT_UNLOCKED(tq);
> > + tq_callback = tq->tq_callbacks[cb_type];
> > + if (tq_callback != NULL)
> > + tq_callback(tq->tq_cb_contexts[cb_type]);
> > +}
> > +
> > void
> > taskqueue_thread_loop(void *arg)
> > {
> > @@ -496,6 +529,7 @@ taskqueue_thread_loop(void *arg)
> >
> > tqp = arg;
> > tq = *tqp;
> > + taskqueue_run_callback(tq, TASKQUEUE_CALLBACK_TYPE_INIT);
> > TQ_LOCK(tq);
> > while ((tq->tq_flags & TQ_FLAGS_ACTIVE) != 0) {
> > taskqueue_run_locked(tq);
> > @@ -510,6 +544,15 @@ taskqueue_thread_loop(void *arg)
> > }
> > taskqueue_run_locked(tq);
> >
> > + /*
> > + * This thread is on its way out, so just drop the lock
> temporarily
> > + * in order to call the shutdown callback. This allows the
> callback
> > + * to look at the taskqueue, even just before it dies.
> > + */
> > + TQ_UNLOCK(tq);
> > + taskqueue_run_callback(tq, TASKQUEUE_CALLBACK_TYPE_SHUTDOWN);
> > + TQ_LOCK(tq);
> > +
> > /* rendezvous with thread that asked us to terminate */
> > tq->tq_tcount--;
> > wakeup_one(tq->tq_threads);
> > @@ -525,7 +568,7 @@ taskqueue_thread_enqueue(void *context)
> > tqp = context;
> > tq = *tqp;
> >
> > - mtx_assert(&tq->tq_mutex, MA_OWNED);
> > + TQ_ASSERT_LOCKED(tq);
> > wakeup_one(tq);
> > }
> >
> >
> > Modified: head/sys/sys/taskqueue.h
> >
> ==============================================================================
> > --- head/sys/sys/taskqueue.h Sat Mar 23 14:52:31 2013 (r248648)
> > +++ head/sys/sys/taskqueue.h Sat Mar 23 15:11:53 2013 (r248649)
> > @@ -47,6 +47,16 @@ struct timeout_task {
> > int f;
> > };
> >
> > +enum taskqueue_callback_type {
> > + TASKQUEUE_CALLBACK_TYPE_INIT,
> > + TASKQUEUE_CALLBACK_TYPE_SHUTDOWN,
> > +};
> > +#define TASKQUEUE_CALLBACK_TYPE_MIN
> TASKQUEUE_CALLBACK_TYPE_INIT
> > +#define TASKQUEUE_CALLBACK_TYPE_MAX
> TASKQUEUE_CALLBACK_TYPE_SHUTDOWN
> > +#define TASKQUEUE_NUM_CALLBACKS
> TASKQUEUE_CALLBACK_TYPE_MAX + 1
>
> This need parentheses to defensively guard against unexpected
> precedence of operators.
>
> While here, TASKQUEUE_CALLBACK_ is a very long prefix (19 characters!)
> Can this be named TQCB_ instead so it's not so verbose?
>
> Thanks,
> matthew
>
More information about the svn-src-all
mailing list