Messages in this thread Patch in this message | | | Date | Thu, 2 Apr 2015 17:53:09 +0100 | From | Morten Rasmussen <> | Subject | Re: [PATCH v10 04/11] sched: Make sched entity usage tracking scale-invariant |
| |
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)
| |