Re: [PATCH] sched: Support current clocksource handling in

Peter Zijlstra
2009-05-28T09:34:41Z

On Thu, 2009-05-28 at 18:19 +0900, Paul Mundt wrote:

> Ok, there were some ordering problems with the early platform code, but
> I've played with this a bit more and got it to the point where this now
> also works. I can live with this over the v3 version if people prefer
> this approach instead.
>
> --

> @@ -38,8 +39,7 @@
> */
> unsigned long long __attribute__((weak)) sched_clock(void)
> {
> - return (unsigned long long)(jiffies - INITIAL_JIFFIES)
> - * (NSEC_PER_SEC / HZ);
> + return cyc2ns(sched_clocksource, clocksource_read(sched_clocksource));
> }
>
> static __read_mostly int sched_clock_running;

> @@ -362,6 +363,9 @@ static struct clocksource *select_clocksource(void)
> if (next = curr_clocksource)
> return NULL;
>
> + if (next->flags & CLOCK_SOURCE_USE_FOR_SCHED_CLOCK)
> + sched_clocksource = next;
> +
> return next;
> }
>

That's a single assignment, vs two reads on use. Should we be worried
about the SMP race where we observe two different sched_clocksource
pointers on read?

I would suggest we write it as:

u64 __weak sched_clock(void)
{
struct clocksource *clock = ACCESS_ONCE(sched_clocksource);

return cyc2ns(clock, clocksource_read(clock));
}