svn commit: r248649 - in head: share/man/man9 sys/kern sys/sys
Will Andrews
will at firepipe.net
Sun Mar 24 17:35:20 UTC 2013
BTW, the TQ_ prefix is only used in subr_taskqueue.c; it is not used in
taskqueue.h. It could appear unrelated when viewed in taskqueue(9)
consumers. There are also other very long names in taskqueue.h, so I'm not
sure it's really worth renaming the callback types.
Thanks,
--Will.
On Sat, Mar 23, 2013 at 12:36 PM, <mdf at freebsd.org> wrote:
> On Sat, Mar 23, 2013 at 9:45 AM, Will Andrews <will at firepipe.net> wrote:
> > 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?
>
> I'm not sure FreeBSD has anything standard, but at $WORK we use CB or
> cb frequently as an abbreviation for callback. TASKQUEUE -> TQ still
> removes 7 letters, which is a start. :-)
>
> Thanks,
> matthew
>
>
> > 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