[PATCH v2] ARM: sched_clock: allow sched_clock to be selected at runtime

Nicolas Pitre nico at fluxnic.net
Mon Aug 15 20:00:45 EDT 2011


On Thu, 11 Aug 2011, Marc Zyngier wrote:

> sched_clock() is yet another blocker on the road to the single
> image. This patch implements an idea by Russell King:
> 
> http://www.spinics.net/lists/linux-omap/msg49561.html
> 
> Instead of asking the platform to implement both sched_clock()
> itself and the rollover callback, simply register a read()
> function, and let the ARM code care about sched_clock() itself,
> the conversion to ns and the rollover. sched_clock() uses
> this read() function as an indirection to the platform code.
> 
> This allow some simplifications and possibly some footprint gain
> when multiple platforms are compiled in. Among the drawbacks,
> the removal of the *_fixed_sched_clock optimization which could
> negatively impact some platforms (sa1100, tegra, versatile
> and omap).
> 
> Tested on 11MPCore, OMAP4 and Tegra.

[...]

> +unsigned long long notrace sched_clock(void)
> +{
> +	if (read_sched_clock) {
> +		u32 cyc = read_sched_clock();
> +		return cyc_to_sched_clock(&cd, cyc, sched_clock_mask);
> +	}
> +
> +	return (unsigned long long)(jiffies - INITIAL_JIFFIES)
> +					* (NSEC_PER_SEC / HZ);
> +}

This bothers me that this function has to include a conditional which 
will _always_ be evaluated as true except during early boot.  Why not 
simply initializing read_sched_clock with a jiffy_shed_clock function 
instead, and let it be overriden during boot?  This way the actual 
sched_clock() code will be more straight forward.

Of course there is the race where read_sched_clock might be updated 
before or after the corresponding clock data/mask are updated leading to 
funky results if you are lucky enough to call sched_clock() during that 
exact wrong moment.  But either we don't care (switching from a jiffy to 
a hardware clock base is going to cause quite a glitch already anyway), 
or simply disabling IRQs during initialization should prevent the race 
(presuming no other CPUs are running at that point).


Nicolas



More information about the linux-arm-kernel mailing list