From owner-svn-src-head@FreeBSD.ORG Sat Mar 23 16:20:46 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 05DDF6DF; Sat, 23 Mar 2013 16:20:46 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-qa0-f52.google.com (mail-qa0-f52.google.com [209.85.216.52]) by mx1.freebsd.org (Postfix) with ESMTP id 8D4FBBF4; Sat, 23 Mar 2013 16:20:45 +0000 (UTC) Received: by mail-qa0-f52.google.com with SMTP id bs12so781345qab.18 for ; Sat, 23 Mar 2013 09:20:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=LakhN2ZHNV7/3ZKN1IJfj2/JBdur2KiX8OHH+YWhuSc=; b=DIIFjVbjT+yqyKoe1GfNSSr0Oq2JHcS+yvsjgzYgUVsiwrTOBZJlv4ft78B7Dy6wbL etO0tSYU5mFLypWUW0k306HLmxgU0YApsgcN3jsbqyvlfXgr9tA1MFKOc/t8m1+htHVY e8bRDY9OR7Yc7ZD7y2dZOqA3Y+otwOdJuGMrEWuomJcwZqaWb5huncXO5k/N+8d68jpJ YM/G41XB8mPeNA+iiOwCQhwWe3fNyx0k9SM4jAhJwutyvYvD/1VJL+dR9mhWqsUO6jCf gefFUgiOiaV0LMBzj5IfVjcY7XUGLxprUDciSPio0sr61tGd7OeP4s+ETkKZaxltMcA5 tHXw== MIME-Version: 1.0 X-Received: by 10.49.25.202 with SMTP id e10mr1851252qeg.49.1364055638919; Sat, 23 Mar 2013 09:20:38 -0700 (PDT) Sender: mdf356@gmail.com Received: by 10.229.179.42 with HTTP; Sat, 23 Mar 2013 09:20:38 -0700 (PDT) In-Reply-To: <201303231511.r2NFBrrr074289@svn.freebsd.org> References: <201303231511.r2NFBrrr074289@svn.freebsd.org> Date: Sat, 23 Mar 2013 09:20:38 -0700 X-Google-Sender-Auth: p8rQpvr6sTykIL9cJEbPkzLD6P8 Message-ID: Subject: Re: svn commit: r248649 - in head: share/man/man9 sys/kern sys/sys From: mdf@FreeBSD.org To: Will Andrews Content-Type: text/plain; charset=ISO-8859-1 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 Mar 2013 16:20:46 -0000 On Sat, Mar 23, 2013 at 8:11 AM, Will Andrews 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