lkml.org 
[lkml]   [2004]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] CPU time clock support in clock_* syscalls
Roland McGrath wrote:

> /*
> + * This is called on clock ticks and on context switches.
> + * Bank in p->sched_time the ns elapsed since the last tick or switch.
> + */
> +static void update_cpu_clock(task_t *p, runqueue_t *rq,
> + unsigned long long now)
> +{
> + unsigned long long last = max(p->timestamp, rq->timestamp_last_tick);
> + p->sched_time += now - last;
> +}

This looks wrong. But update_cpu_clock is never called from another
CPU. In which case you don't need to worry about timestamp_last_tick.

> +
> +/*
> + * Return current->sched_time plus any more ns on the sched_clock
> + * that have not yet been banked.
> + */
> +unsigned long long current_sched_time(const task_t *tsk)
> +{
> + unsigned long long ns;
> + local_irq_disable();
> + ns = max(tsk->timestamp, task_rq(tsk)->timestamp_last_tick);
> + ns = tsk->sched_time + (sched_clock() - ns);
> + local_irq_enable();
> + return ns;
> +}
> +

This doesn't perform the timestamp_last_tick "normalisation" properly
either.

It also seems to conveniently ignore locking when reading those values
off another CPU. Not a big deal for dynamic load calculations, but I'm
not so sure about your usage...?

Lastly, even when using timestamp_last_tick correctly, I think sched_clock
will still drift around slightly, especially if a task switches CPUs a lot
(but not restricted to moving CPUs). Again this is something that I
haven't thought about much because it is not a big deal for current usage.

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

\
 
 \ /
  Last update: 2005-03-22 14:06    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site