Re: rSDl cpu scheduler version 0.34-test patch

[Mike Galbraith]

On Mon, 2007-03-26 at 11:00 +1000, Con Kolivas wrote:
> This is just for testing at the moment! The reason is the size of this patch.

(no testing done yet, but I have a couple comments)

> In the interest of evolution, I've taken the RSDL cpu scheduler and increased 
> the resolution of the task timekeeping to nanosecond resolution.

+	/* All the userspace visible cpu accounting is done here */
+	time_diff = now - p->last_ran;
...
+		/* cpu scheduler quota accounting is performed here */
+		if (p->policy != SCHED_FIFO)
+			p->time_slice -= time_diff;

If we still have any jiffies resolution clocks out there, this could be
a bit problematic.

+static inline void enqueue_pulled_task(struct rq *src_rq, struct rq *rq,
+				       struct task_struct *p)
+{
+	int queue_prio;
+
+	p->array = rq->active; <== set
+	if (!rt_task(p)) {
+		if (p->rotation == src_rq->prio_rotation) {
+			if (p->array == src_rq->expired) { <== evaluate
+				queue_expired(p, rq);
+				goto out_queue;
+			}
+			if (p->time_slice < 0)
+				task_new_array(p, rq);
+		} else
+			task_new_array(p, rq);
+	}
+	queue_prio = next_entitled_slot(p, rq);

(bug aside, this special function really shouldn't exist imho, because
there's nothing special going on.  we didn't need it before to do the
same thing, so we shouldn't need it now.)

+static void recalc_task_prio(struct task_struct *p, struct rq *rq)
+{
+	struct prio_array *array = rq->active;
+	int queue_prio;
+
+	if (p->rotation == rq->prio_rotation) {
+		if (p->array == array) {
+			if (p->time_slice > 0)
+				return;
+			p->time_slice = p->quota;
+		} else if (p->array == rq->expired) {
+			queue_expired(p, rq);
+			return;
+		} else
+			task_new_array(p, rq);
+	} else

Dequeueing a task still leaves a stale p->array laying around to be
possibly evaluated later.  try_to_wake_up() doesn't currently evaluate
and set p->rotation (but should per design doc), so when you get here, a
cross-cpu waking task won't continue it's rotation.  If it did evaluate
and set, recalc_task_prio() would evaluate the guaranteed to fail these
tests array pointer, so the task will still not continue it's rotation.
Stale pointers are evil.

	-Mike

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/