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;
Date:   Fri, 25 Oct 2019 13:41:08 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Parth Shah <parth@linux.ibm.com>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched/fair: Make sched-idle cpu selection consistent
 throughout
Message-ID: <20191025081108.6gaprbwm5fvokun6@vireshk-i7>
References: <5eba2fb4af9ebc7396101bb9bd6c8aa9c8af0710.1571899508.git.viresh.kumar@linaro.org>
 <7d3a1549-a99c-ae42-6074-8ed2ecd7074f@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7d3a1549-a99c-ae42-6074-8ed2ecd7074f@linux.ibm.com>
User-Agent: NeoMutt/20180716-391-311a52
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 25-10-19, 12:13, Parth Shah wrote:
> Hi Viresh,
> 
> On 10/24/19 12:15 PM, Viresh Kumar wrote:
> > There are instances where we keep searching for an idle CPU despite
> > having a sched-idle cpu already (in find_idlest_group_cpu(),
> > select_idle_smt() and select_idle_cpu() and then there are places where
> > we don't necessarily do that and return a sched-idle cpu as soon as we
> > find one (in select_idle_sibling()). This looks a bit inconsistent and
> > it may be worth having the same policy everywhere.
> > 
> > On the other hand, choosing a sched-idle cpu over a idle one shall be
> > beneficial from performance point of view as well, as we don't need to
> > get the cpu online from a deep idle state which is quite a time
> > consuming process and delays the scheduling of the newly wakeup task.
> > 
> > This patch tries to simplify code around sched-idle cpu selection and
> > make it consistent throughout.
> > 
> > FWIW, tests were done with the help of rt-app (8 SCHED_OTHER and 5
> > SCHED_IDLE tasks, not bound to any cpu) on ARM platform (octa-core), and
> > no significant difference in scheduling latency of SCHED_OTHER tasks was
> > found.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> 
> [...]
> 
> > @@ -5755,13 +5749,11 @@ static int select_idle_smt(struct task_struct *p, int target)
> >  	for_each_cpu(cpu, cpu_smt_mask(target)) {
> >  		if (!cpumask_test_cpu(cpu, p->cpus_ptr))
> >  			continue;
> > -		if (available_idle_cpu(cpu))
> > +		if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
> >  			return cpu;
> 
> I guess this is a correct approach, but just wondering what if we still
> keep searching for a sched_idle CPU even though we have found an
> available_idle CPU?

I do believe selecting a sched-idle CPU should almost always be better
(performance wise), unless we have a strong argument against it. And
anyway, the load balancer will get triggered at a later point of time
and will pull away these newly wakeup tasks to idle CPUs. The
advantage we get out of it is that the tasks get serviced a bit
earlier when they first get queued.

It is really up to the maintainers to see what kind of policy do we
want to adapt here and not a choice I can make :)

-- 
viresh