On Wed, Mar 25, 2015 at 05:33:09PM +0000, Peter Zijlstra wrote:
On Tue, Mar 24, 2015 at 11:00:57AM +0100, Vincent Guittot wrote:
On 23 March 2015 at 14:19, Peter Zijlstra peterz@infradead.org wrote:
On Fri, Feb 27, 2015 at 04:54:07PM +0100, Vincent Guittot wrote:
unsigned long scale_freq = arch_scale_freq_capacity(NULL, cpu);
sa->running_avg_sum += delta_w * scale_freq
>> SCHED_CAPACITY_SHIFT;
so the only thing that could be improved is somehow making this multiplication go away when the arch doesn't implement the function.
But I'm not sure how to do that without #ifdef.
Maybe a little something like so then... that should make the compiler get rid of those multiplications unless the arch needs them.
yes, it removes useless multiplication when not used by an arch. It also adds a constraint on the arch side which have to define arch_scale_freq_capacity like below:
#define arch_scale_freq_capacity xxx_arch_scale_freq_capacity with xxx_arch_scale_freq_capacity an architecture specific function
Yeah, but it not being weak should make that a compile time warn/fail, which should be pretty easy to deal with.
Could you enlighten me a bit about how to define the arch specific implementation without getting into trouble? I'm failing miserably :(
I thought the arm arch-specific topology.h file was a good place to put the define as it get included in sched.h, so I did a:
#define arch_scale_freq_capacity arm_arch_scale_freq_capacity
However, I have to put a function prototype in the same (or some other included) header file to avoid doing an implicit function definition. arch_scale_freq_capacity() takes a struct sched_domain pointer, so I have to include linux/sched.h which leads to circular dependency between linux/sched.h and topology.h.
The only way out I can think of to create (or find) a new arch-specific header file that can include linux/sched.h and be included in kernel/sched/sched.h and have the define and prototype in there.
I must be missing something?
We can drop the sched_domain pointer as we don't use it, but I'm going to do the same trick for arch_scale_cpu_capacity() as well which does require the sd pointer.
Finally, is introducing an ARCH_HAS_SCALE_FREQ_CAPACITY or similar a complete no go?
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 91c6736..5707fb7 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -1388,12 +1388,14 @@ static inline int hrtick_enabled(struct rq *rq) #ifdef CONFIG_SMP extern void sched_avg_update(struct rq *rq);
-#ifndef arch_scale_freq_capacity +#ifndef ARCH_HAS_SCALE_FREQ_CAPACITY static __always_inline unsigned long arch_scale_freq_capacity(struct sched_domain *sd, int cpu) { return SCHED_CAPACITY_SCALE; } +#else +extern unsigned long arch_scale_freq_capacity(struct sched_domain *sd, int cpu); #endif
static inline void sched_rt_avg_update(struct rq *rq, u64 rt_delta)