On Monday 26 March 2007 15:00, Mike Galbraith wrote:
> 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.
Works fine with jiffy only resolution. sched_clock just returns the change
when it happens. This leaves us with the accuracy of the previous code on
hardware that doesn't give higher resolution time from sched_clock.
> +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
I don't see a problem.
> + 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.)
As the comment says, the likelihood that both runqueues happen to be at the
same priority_level is very low so the exact position cannot be transposed in
my opinion. I'll see if I can simplify it though.
> +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.
I don't see quite why that's a problem. If there's memory of the last dequeue
and it enqueues at a different rotation it gets ignored. If it enqueues
during the same rotation then that memory proves useful for ensuring it
doesn't get a new full quota. Either way the array is always updated on
enqueue so it wont be trying to add it to the wrong runlist.
> try_to_wake_up() doesn't currently evaluate
> and set p->rotation (but should per design doc),
try_to_wake_up->activate_task->enqueue_task->recalc_task_prio which updates
p->rotation
> 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.
I prefer to use the array value as a memory in case it wakes up on the same
rotation and runqueue.
>
> -Mike
Thanks.
--
-ck
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
