On Sat, Mar 23, 2013 at 9:45 AM, Will Andrews <w...@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, <m...@freebsd.org> wrote: >> >> On Sat, Mar 23, 2013 at 8:11 AM, Will Andrews <w...@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 > > _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
