update

bin/tests/task_test.c

@@ -10,17 +10,14 @@
 #include <isc/thread.h>
 
 mem_context_t mctx = NULL;
-os_mutex_t timer_lock;
-os_condition_t timer_wakeup;
 
 /*ARGSUSED*/
 static boolean_t
 my_callback(task_t __attribute__((unused)) task,
-	    void *arg,
 	    task_event_t __attribute__((unused)) event)
 {
 	int i, j;
-	char *name = arg;
+	char *name = event->arg;
 
 	j = 0;
 	for (i = 0; i < 100000000; i++)
@@ -33,10 +30,9 @@ my_callback(task_t __attribute__((unused)) task,
 /*ARGSUSED*/
 static boolean_t
 my_shutdown(task_t __attribute__((unused)) task,
-	    void *arg,
 	    task_event_t __attribute__((unused)) event)
 {
-	char *name = arg;
+	char *name = event->arg;
 
 	printf("shutdown %s\n", name);
 	return (TRUE);
@@ -45,50 +41,25 @@ my_shutdown(task_t __attribute__((unused)) task,
 /*ARGSUSED*/
 static boolean_t
 my_tick(task_t __attribute__((unused)) task,
-	void *arg,
 	task_event_t __attribute__((unused)) event)
 {
-	char *name = arg;
+	char *name = event->arg;
 
 	printf("tick %s\n", name);
 	return (FALSE);
 }
 
-/*ARGSUSED*/
-static boolean_t
-wakeup_timer(task_t __attribute__((unused)) task,
-	     void *arg,
-	     task_event_t __attribute__((unused)) event)
-{
-	printf("wakeup timer\n");
-	(void)os_condition_broadcast(&timer_wakeup);
-	return (FALSE);
-}
-
 void *
 simple_timer_run(void *arg) {
 	task_t task = arg;
 	task_event_t event;
 	int i;
-	struct timespec ts;
-	struct timeval tv;
-	struct timeval tv1;
-	boolean_t timeout;
 	
-	for (i = 0; i < 5; i++) {
-		(void)gettimeofday(&tv, NULL);
-		ts.tv_sec = tv.tv_sec + 5;
-		ts.tv_nsec = 0;
-		(void)os_mutex_lock(&timer_lock);
-		(void)os_condition_waituntil(&timer_wakeup, &timer_lock, &ts,
-					     &timeout);
-		(void)os_mutex_unlock(&timer_lock);
-		(void)gettimeofday(&tv1, NULL);
-		printf("slept %d secs\n", tv1.tv_sec - tv.tv_sec);
-		if (timeout)
-			printf("timer timeout\n");
+	for (i = 0; i < 10; i++) {
+		sleep(1);
 		printf("sending timer to %p\n", task);
-		event = task_event_allocate(mctx, 2, my_tick, NULL, sizeof *event);
+		event = task_event_allocate(mctx, 2, my_tick, "foo",
+					    sizeof *event);
 		INSIST(event != NULL);
 		(void)task_send_event(task, &event);
 	}
@@ -104,8 +75,6 @@ simple_timer_init(task_t task) {
 
 	task_clone = NULL;
 	task_attach(task, &task_clone);
-	(void)os_mutex_init(&timer_lock);
-	(void)os_condition_init(&timer_wakeup);
 	INSIST(os_thread_create(simple_timer_run, task_clone, &t));
 	(void)os_thread_detach(t);
 }
@@ -128,10 +97,10 @@ main(int argc, char *argv[]) {
 
 	INSIST(task_manager_create(mctx, workers, 0, &manager) == workers);
 
-	INSIST(task_create(manager, "1", my_shutdown, 0, &t1));
-	INSIST(task_create(manager, "2", my_shutdown, 0, &t2));
-	INSIST(task_create(manager, "3", my_shutdown, 0, &t3));
-	INSIST(task_create(manager, "4", my_shutdown, 0, &t4));
+	INSIST(task_create(manager, my_shutdown, "1", 0, &t1));
+	INSIST(task_create(manager, my_shutdown, "2", 0, &t2));
+	INSIST(task_create(manager, my_shutdown, "3", 0, &t3));
+	INSIST(task_create(manager, my_shutdown, "4", 0, &t4));
 
 	simple_timer_init(t1);
 	simple_timer_init(t2);
@@ -139,35 +108,33 @@ main(int argc, char *argv[]) {
 	printf("task 2 = %p\n", t2);
 	sleep(2);
 
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "1", sizeof *event);
 	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, wakeup_timer, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "1", sizeof *event);
 	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "1", sizeof *event);
 	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "1", sizeof *event);
 	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "1", sizeof *event);
 	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "1", sizeof *event);
 	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "1", sizeof *event);
 	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "1", sizeof *event);
 	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
-	task_send_event(t1, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "2", sizeof *event);
 	task_send_event(t2, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "3", sizeof *event);
 	task_send_event(t3, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "4", sizeof *event);
 	task_send_event(t4, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "2", sizeof *event);
 	task_send_event(t2, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "3", sizeof *event);
 	task_send_event(t3, &event);
-	event = task_event_allocate(mctx, 1, my_callback, NULL, sizeof *event);
+	event = task_event_allocate(mctx, 1, my_callback, "4", sizeof *event);
 	task_send_event(t4, &event);
 
 	task_detach(&t1);

lib/isc/include/isc/task.h

@@ -30,8 +30,7 @@ typedef struct task_manager *		task_manager_t;
  */
 typedef int				task_eventtype_t;
 
-typedef boolean_t			(*task_action_t)(task_t, void *,
-							 task_event_t);
+typedef boolean_t			(*task_action_t)(task_t, task_event_t);
 
 /*
  * This structure is public because "subclassing" it may be useful when
@@ -46,6 +45,9 @@ struct task_event {
 	LINK(struct task_event)	link;
 };
 
+#define TASK_EVENT_NOP			0
+#define TASK_EVENT_SHUTDOWN		(-1)
+
 typedef LIST(struct task_event)		task_eventlist_t;
 
 task_event_t				task_event_allocate(mem_context_t,
@@ -61,8 +63,8 @@ void					task_event_free(task_event_t *);
  ***/
 
 boolean_t				task_create(task_manager_t,
-						    void *,
 						    task_action_t,
+						    void *,
 						    unsigned int,
 						    task_t *);
 void					task_attach(task_t, task_t *);

lib/isc/task.c

@@ -53,9 +53,8 @@ struct task {
 	unsigned int			references;
 	task_eventlist_t			events;
 	unsigned int			quantum;
-	boolean_t			shutdown_pending;
-	task_action_t			shutdown_action;
-	void *				arg;
+	boolean_t			enqueue_allowed;
+	task_event_t			shutdown_event;
 	/* Locked by task manager lock. */
 	LINK(struct task)		link;
 	LINK(struct task)		ready_link;
@@ -83,6 +82,56 @@ struct task_manager {
 #define DEFAULT_DEFAULT_QUANTUM	5
 #define FINISHED(m)		((m)->exiting && EMPTY((m)->tasks))
 
+
+/***
+ *** Events.
+ ***/
+
+static inline task_event_t
+event_allocate(mem_context_t mctx, task_eventtype_t type,
+	       task_action_t action, void *arg, size_t size)
+{
+	task_event_t event;
+
+	event = mem_get(mctx, size);
+	if (event == NULL)
+		return (NULL);
+	event->mctx = mctx;
+	event->size = size;
+	event->type = type;
+	event->action = action;
+	event->arg = arg;
+
+	return (event);
+}
+
+task_event_t
+task_event_allocate(mem_context_t mctx, task_eventtype_t type,
+		    task_action_t action, void *arg, size_t size)
+{
+	if (size < sizeof (struct task_event))
+		return (NULL);
+	if (type < 0)
+		return (NULL);
+	if (action == NULL)
+		return (NULL);
+
+	return (event_allocate(mctx, type, action, arg, size));
+}
+
+void
+task_event_free(task_event_t *eventp) {
+	task_event_t event;
+	
+	REQUIRE(eventp != NULL);
+	event = *eventp;
+	REQUIRE(event != NULL);
+
+	mem_put(event->mctx, event, event->size);
+
+	*eventp = NULL;
+}
+
 /***
  *** Tasks.
  ***/
@@ -107,14 +156,15 @@ task_free(task_t task) {
 	}
 	UNLOCK(&manager->lock);
 	(void)os_mutex_destroy(&task->lock);
+	if (task->shutdown_event != NULL)
+		task_event_free(&task->shutdown_event);
 	task->magic = 0;
 	mem_put(manager->mctx, task, sizeof *task);
 }
 
 boolean_t
-task_create(task_manager_t manager, void *arg,
-	    task_action_t shutdown_action, unsigned int quantum,
-	    task_t *taskp)
+task_create(task_manager_t manager, task_action_t shutdown_action,
+	    void *shutdown_arg, unsigned int quantum, task_t *taskp)
 {
 	task_t task;
 
@@ -135,9 +185,17 @@ task_create(task_manager_t manager, void *arg,
 	task->references = 1;
 	INIT_LIST(task->events);
 	task->quantum = quantum;
-	task->shutdown_pending = FALSE;
-	task->arg = arg;
-	task->shutdown_action = shutdown_action;
+	task->enqueue_allowed = TRUE;
+	task->shutdown_event = event_allocate(manager->mctx,
+					      TASK_EVENT_SHUTDOWN,
+					      shutdown_action,
+					      shutdown_arg,
+					      sizeof *task->shutdown_event);
+	if (task->shutdown_event == NULL) {
+		(void)os_mutex_destroy(&task->lock);
+		mem_put(manager->mctx, task, sizeof *task);
+		return (FALSE);
+	}
 	INIT_LINK(task, link);
 	INIT_LINK(task, ready_link);
 
@@ -204,6 +262,7 @@ task_send_event(task_t task, task_event_t *eventp) {
 	REQUIRE(eventp != NULL);
 	event = *eventp;
 	REQUIRE(event != NULL);
+	REQUIRE(event->type >= 0);
 
 	XTRACE("sending");
 	/*
@@ -212,7 +271,7 @@ task_send_event(task_t task, task_event_t *eventp) {
 	 * some processing is deferred until after a lock is released.
 	 */
 	LOCK(&task->lock);
-	if (task->state != task_state_shutdown && !task->shutdown_pending) {
+	if (task->enqueue_allowed) {
 		if (task->state == task_state_idle) {
 			was_idle = TRUE;
 			INSIST(EMPTY(task->events));
@@ -289,7 +348,7 @@ task_shutdown(task_t task) {
 	 */
 
 	LOCK(&task->lock);
-	if (task->state != task_state_shutdown && !task->shutdown_pending) {
+	if (task->enqueue_allowed) {
 		if (task->state == task_state_idle) {
 			was_idle = TRUE;
 			INSIST(EMPTY(task->events));
@@ -297,7 +356,10 @@ task_shutdown(task_t task) {
 		}
 		INSIST(task->state == task_state_ready ||
 		       task->state == task_state_running);
-		task->shutdown_pending = TRUE;
+		INSIST(task->shutdown_event != NULL);
+		ENQUEUE(task->events, task->shutdown_event, link);
+		task->shutdown_event = NULL;
+		task->enqueue_allowed = FALSE;
 	} else
 		discard = TRUE;
 	UNLOCK(&task->lock);
@@ -419,10 +481,9 @@ void *task_manager_run(void *uap) {
 			boolean_t done = FALSE;
 			boolean_t requeue = FALSE;
 			boolean_t wants_shutdown;
+			boolean_t is_shutdown;
 			boolean_t free_task = FALSE;
-			void *arg;
-			task_action_t action;
-			task_event_t	event;
+			task_event_t event;
 			task_eventlist_t remaining_events;
 			boolean_t discard_remaining = FALSE;
 
@@ -439,52 +500,36 @@ void *task_manager_run(void *uap) {
 			LOCK(&task->lock);
 			task->state = task_state_running;
 			while (!done) {
-				INSIST(task->shutdown_pending ||
-				       !EMPTY(task->events));
-				if (task->shutdown_pending &&
-				    EMPTY(task->events)) {
-					event = NULL;
-					action = task->shutdown_action;
-				} else {
-					event = HEAD(task->events);
-					action = event->action;
-					DEQUEUE(task->events, event, link);
-				}
-				arg = task->arg;
+				INSIST(!EMPTY(task->events));
+				event = HEAD(task->events);
+				DEQUEUE(task->events, event, link);
 				UNLOCK(&task->lock);
 
+				if (event->type == TASK_EVENT_SHUTDOWN)
+					is_shutdown = TRUE;
+				else
+					is_shutdown = FALSE;
+
 				/*
 				 * Execute the event action.
 				 */
 				XTRACE("execute action");
-				if (action != NULL)
-					wants_shutdown = (*action)(task,
-								   arg,
-								   event);
+				if (event->action != NULL)
+					wants_shutdown =
+						(event->action)(task, event);
 				else
 					wants_shutdown = FALSE;
 				dispatch_count++;
-
-				/*
-				 * If this wasn't a shutdown event, we
-				 * need to free it.
-				 *
-				 * Also, if we've delivered the shutdown
-				 * event to the task, then we are going
-				 * to shut it down no matter what the task
-				 * callback returned.
-				 */
-				if (event != NULL)
-					task_event_free(&event);
-				else
-					wants_shutdown = TRUE;
+				
+				task_event_free(&event);
 
 				LOCK(&task->lock);
-				if (wants_shutdown) {
+				if (wants_shutdown || is_shutdown) {
 					/*
-					 * The task has either had the
-					 * shutdown event sent to it, or
-					 * an event action requested shutdown.
+					 * The event action has either
+					 * requested shutdown, or the event
+					 * we just executed was the shutdown
+					 * event.
 					 *
 					 * Since no more events can be
 					 * delivered to the task, we purge
@@ -502,10 +547,9 @@ void *task_manager_run(void *uap) {
 					if (task->references == 0)
 						free_task = TRUE;
 					task->state = task_state_shutdown;
-					task->shutdown_pending = FALSE;
+					task->enqueue_allowed = FALSE;
 					done = TRUE;
-				} else if (EMPTY(task->events) &&
-					   !task->shutdown_pending) {
+				} else if (EMPTY(task->events)) {
 					/*
 					 * Nothing else to do for this task.
 					 * Put it to sleep.
@@ -690,16 +734,25 @@ task_manager_destroy(task_manager_t *managerp) {
 	manager->exiting = TRUE;
 
 	/*
-	 * Post a shutdown event to every task.
+	 * Post the shutdown event to every task (if it hasn't already been
+	 * posted).
 	 */
 	for (task = HEAD(manager->tasks);
 	     task != NULL;
 	     task = NEXT(task, link)) {
 		LOCK(&task->lock);
-		task->shutdown_pending = TRUE;
-		if (task->state == task_state_idle) {
-			task->state = task_state_ready;
-			ENQUEUE(manager->ready_tasks, task, ready_link);
+		if (task->enqueue_allowed) {
+			INSIST(task->shutdown_event != NULL);
+			ENQUEUE(task->events, task->shutdown_event, link);
+			task->shutdown_event = NULL;
+			if (task->state == task_state_idle) {
+				task->state = task_state_ready;
+				ENQUEUE(manager->ready_tasks, task,
+					ready_link);
+			}
+			INSIST(task->state == task_state_ready ||
+			       task->state == task_state_running);
+			task->enqueue_allowed = FALSE;
 		}
 		UNLOCK(&task->lock);
 	}
@@ -723,45 +776,3 @@ task_manager_destroy(task_manager_t *managerp) {
 
 	*managerp = NULL;
 }
-
-
-/***
- *** Events.
- ***/
-
-task_event_t
-task_event_allocate(mem_context_t mctx, task_eventtype_t type,
-		    task_action_t action, void *arg, size_t size)
-{
-	task_event_t event;
-
-	if (size < sizeof *event)
-		return (NULL);
-	if (type < 0)
-		return (NULL);
-	if (action == NULL)
-		return (NULL);
-	event = mem_get(mctx, size);
-	if (event == NULL)
-		return (NULL);
-	event->mctx = mctx;
-	event->size = size;
-	event->type = type;
-	event->action = action;
-	event->arg = arg;
-
-	return (event);
-}
-
-void
-task_event_free(task_event_t *eventp) {
-	task_event_t event;
-	
-	REQUIRE(eventp != NULL);
-	event = *eventp;
-	REQUIRE(event != NULL);
-
-	mem_put(event->mctx, event, event->size);
-
-	*eventp = NULL;
-}