Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <1db704b2e118b91c05ff37cd74c9adb818a4d033.camel@perches.com>
Subject: Re: [PATCH 09/11] sched: use pelt for scale_rt_capacity()
From:   Joe Perches <joe@perches.com>
To:     Ingo Molnar <mingo@kernel.org>,
        Vincent Guittot <vincent.guittot@linaro.org>
Cc:     peterz@infradead.org, linux-kernel@vger.kernel.org,
        rjw@rjwysocki.net, juri.lelli@redhat.com, dietmar.eggemann@arm.com,
        Morten.Rasmussen@arm.com, viresh.kumar@linaro.org,
        valentin.schneider@arm.com, patrick.bellasi@arm.com,
        joel@joelfernandes.org, daniel.lezcano@linaro.org,
        quentin.perret@arm.com, luca.abeni@santannapisa.it,
        claudio@evidence.eu.com, Ingo Molnar <mingo@redhat.com>
Date:   Sun, 15 Jul 2018 15:46:49 -0700
In-Reply-To: <20180715221538.GA23280@gmail.com>
References: <1530200714-4504-1-git-send-email-vincent.guittot@linaro.org>
         <1530200714-4504-10-git-send-email-vincent.guittot@linaro.org>
         <20180715221538.GA23280@gmail.com>
Content-Type: text/plain; charset="ISO-8859-1"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 2018-07-16 at 00:15 +0200, Ingo Molnar wrote:
> * Vincent Guittot <vincent.guittot@linaro.org> wrote:
> 
> > The utilization of the CPU by rt, dl and interrupts are now tracked with
> > PELT so we can use these metrics instead of rt_avg to evaluate the remaining
> > capacity available for cfs class.
> > 
> > scale_rt_capacity() behavior has been changed and now returns the remaining
> > capacity available for cfs instead of a scaling factor because rt, dl and
> > interrupt provide now absolute utilization value.
> > 
> > The same formula as schedutil is used:
> >   irq util_avg + (1 - irq util_avg / max capacity ) * /Sum rq util_avg
> > but the implementation is different because it doesn't return the same value
> > and doesn't benefit of the same optimization
[]
> 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:
[]
> diff --git 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)

Perhaps combine these two #if defined blocks into
a single block

>  	irq = READ_ONCE(rq->avg_irq.util_avg);