From 57a04f35f4e97d5f63079620dab3493832a851af Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Thu, 2 Jan 2014 02:07:01 +0000 Subject: [PATCH] sched: kill task->dead. The three possible states of a task are determined by the ->status and ->dead fields of struct task: status >= 0, !dead: running status >= 0, dead: dead (about to be removed from the task list) status < 0: zombie (dead, but not yet reaped, -status is an error code) This commit encodes the first two states as two non-negative numbers, so that the three states become status == TS_RUNNING: running status == TS_DEAD: dead status < 0: zombie This allows to remove ->dead which improves readability somewhat. --- sched.c | 68 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/sched.c b/sched.c index 1280e07e..13993b13 100644 --- a/sched.c +++ b/sched.c @@ -18,19 +18,34 @@ #include "time.h" #include "error.h" +/** + * The possible states of a task. + * + * In addition to the states listed here, a task may also enter zombie state. + * This happens when its ->post_select function returns negative, the ->status + * field is then set to this return value. Such tasks are not scheduled any + * more (i.e. ->pre_select() and ->post_select() are no longer called), but + * they stay on the scheduler task list until \ref task_reap() or + * \ref sched_shutdown() is called. + */ +enum task_status { + /** Task has been reaped and may be removed from the task list. */ + TS_DEAD, + /** Task is active. */ + TS_RUNNING, +}; + struct task { /** A copy of the task name supplied when the task was registered. */ char *name; /** Copied during task_register(). */ struct task_info info; - /** Whether this task is active (>=0) or in error state (<0). */ + /* TS_RUNNING, TS_DEAD, or zombie (negative value). */ int status; /** Position of the task in the task list of the scheduler. */ struct list_head node; /** If less than zero, the task was notified by another task. */ int notification; - /** True if task is in error state and exit status has been queried. */ - bool dead; }; static struct timeval now_struct; @@ -67,14 +82,16 @@ static void unlink_and_free_task(struct task *t) //#define SCHED_DEBUG 1 static inline void call_post_select(struct sched *s, struct task *t) { + int ret; + #ifndef SCHED_DEBUG - t->status = t->info.post_select(s, t); + ret = t->info.post_select(s, t); #else struct timeval t1, t2, diff; unsigned long pst; clock_get_realtime(&t1); - t->status = t->info.post_select(s, t); + ret = t->info.post_select(s, t); clock_get_realtime(&t2); tv_diff(&t1, &t2, &diff); pst = tv2ms(&diff); @@ -82,6 +99,7 @@ static inline void call_post_select(struct sched *s, struct task *t) PARA_WARNING_LOG("%s: post_select time: %lums\n", t->name, pst); #endif + t->status = ret < 0? ret : TS_RUNNING; } static unsigned sched_post_select(struct sched *s) @@ -90,15 +108,14 @@ static unsigned sched_post_select(struct sched *s) unsigned num_running_tasks = 0; list_for_each_entry_safe(t, tmp, &s->task_list, node) { - if (t->status < 0) { - if (t->dead) /* task has been reaped */ - unlink_and_free_task(t); - continue; + if (t->status == TS_DEAD) /* task has been reaped */ + unlink_and_free_task(t); + else if (t->status == TS_RUNNING) { + call_post_select(s, t); /* sets t->status */ + t->notification = 0; + if (t->status == TS_RUNNING) + num_running_tasks++; } - call_post_select(s, t); - t->notification = 0; - if (t->status >= 0) - num_running_tasks++; } return num_running_tasks; } @@ -175,6 +192,7 @@ again: int task_reap(struct task **tptr) { struct task *t; + int ret; if (!tptr) return 0; @@ -183,19 +201,19 @@ int task_reap(struct task **tptr) return 0; if (t->status >= 0) return 0; - if (t->dead) /* will be freed in sched_post_select() */ - return 0; + ret = t->status; /* * With list_for_each_entry_safe() it is only safe to remove the * _current_ list item. Since we are being called from the loop in * schedule() via some task's ->post_select() function, freeing the * given task here would result in use-after-free bugs in schedule(). - * So we only set t->dead which tells schedule() to free the task in - * the next iteration of its loop. + * So we only set the task status to TS_DEAD which tells schedule() to + * free the task in the next iteration of its loop. */ - t->dead = true; + t->status = TS_DEAD; + *tptr = NULL; - return t->status; + return ret; } /** @@ -210,7 +228,7 @@ void sched_shutdown(struct sched *s) struct task *t, *tmp; list_for_each_entry_safe(t, tmp, &s->task_list, node) { - if (t->status >= 0) + if (t->status == TS_RUNNING) /* The task list should contain only terminated tasks. */ PARA_WARNING_LOG("shutting down running task %s\n", t->name); @@ -239,8 +257,7 @@ struct task *task_register(struct task_info *info, struct sched *s) t->info = *info; t->name = para_strdup(info->name); t->notification = 0; - t->status = 0; - t->dead = false; + t->status = TS_RUNNING; list_add_tail(&t->node, &s->task_list); return t; } @@ -276,7 +293,8 @@ char *get_task_list(struct sched *s) list_for_each_entry_safe(t, tmp, &s->task_list, node) { char *tmp_msg; tmp_msg = make_message("%s%p\t%s\t%s\n", msg? msg : "", t, - t->status < 0? (t->dead? "dead" : "zombie") : "running", + t->status == TS_DEAD? "dead" : + (t->status == TS_RUNNING? "running" : "zombie"), t->name); free(msg); msg = tmp_msg; @@ -340,9 +358,9 @@ int task_status(const struct task *t) { if (!t) return 0; - if (t->dead) + if (t->status == TS_DEAD) /* pretend dead tasks don't exist */ return 0; - if (t->status >= 0) + if (t->status == TS_RUNNING) return 1; return t->status; } -- 2.39.2