From: Andre Noll <maan@systemlinux.org>
Date: Thu, 2 Jan 2014 02:07:01 +0000 (+0000)
Subject: sched: kill task->dead.
X-Git-Tag: v0.5.3~8^2~3
X-Git-Url: http://git.tue.mpg.de/?a=commitdiff_plain;h=57a04f35f4e97d5f63079620dab3493832a851af;p=paraslash.git

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.
---

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;
 }