Hi Ingo,
On Mon, 16 Jul 2018 at 00:15, Ingo Molnar <mi...@kernel.org> wrote:
>
>
> * Vincent Guittot <vincent.guit...@linaro.org> wrote:
>
> > @@ -7550,39 +7550,36 @@ static inline int get_sd_load_idx(struct
> > sched_domain *sd,
> > static unsigned long scale_rt_capacity(int cpu)
> > {
> > struct rq *rq = cpu_rq(cpu);
> > - u64 total, used, age_stamp, avg;
> > - s64 delta;
> > -
> > - /*
> > - * Since we're reading these variables without serialization make sure
> > - * we read them once before doing sanity checks on them.
> > - */
> > - age_stamp = READ_ONCE(rq->age_stamp);
> > - avg = READ_ONCE(rq->rt_avg);
> > - delta = __rq_clock_broken(rq) - age_stamp;
> > + unsigned long max = arch_scale_cpu_capacity(NULL, cpu);
> > + unsigned long used, irq, free;
> >
> > - if (unlikely(delta < 0))
> > - delta = 0;
> > +#if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
> > defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
> > + irq = READ_ONCE(rq->avg_irq.util_avg);
> >
> > - total = sched_avg_period() + delta;
> > + if (unlikely(irq >= max))
> > + return 1;
> > +#endif
>
> Note that 'irq' is unused outside that macro block, resulting in a new
> warning on
> defconfig builds:
>
> CC kernel/sched/fair.o
> kernel/sched/fair.c: In function ‘scale_rt_capacity’:
> kernel/sched/fair.c:7553:22: warning: unused variable ‘irq’
> [-Wunused-variable]
> unsigned long used, irq, free;
> ^~~
>
> I have applied the delta fix below for simplicity, but what we really want is
> a
> cleanup of that function to eliminate the #ifdefs. One solution would be to
> factor
> out the 'irq' utilization value into a helper inline, and double check that
> if the
> configs are off the compiler does the right thing and eliminates this identity
> transformation for the irq==0 case:
>
> free *= (max - irq);
> free /= max;
>
> If the compiler refuses to optimize this away (due to the zero and overflow
> cases), try to find something more clever?
Thanks for the fix.
I'm off for now and will look at your proposal above once back
Regards,
Vincent
>
> Thanks,
>
> Ingo
>
> kernel/sched/fair.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e3221db0511a..d5f7d521e448 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7550,7 +7550,10 @@ static unsigned long scale_rt_capacity(int cpu)
> {
> struct rq *rq = cpu_rq(cpu);
> unsigned long max = arch_scale_cpu_capacity(NULL, cpu);
> - unsigned long used, irq, free;
> + unsigned long used, free;
> +#if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
> defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
> + unsigned long irq;
> +#endif
>
> #if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
> defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
> irq = READ_ONCE(rq->avg_irq.util_avg);
