From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932523AbdJJPzP (ORCPT ); Tue, 10 Oct 2017 11:55:15 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:33699 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932488AbdJJPzL (ORCPT ); Tue, 10 Oct 2017 11:55:11 -0400 Subject: Re: [PATCH 2/3] sched/fair: Introduce scaled capacity awareness in select_idle_sibling code path To: Rohit Jain , linux-kernel@vger.kernel.org, eas-dev@lists.linaro.org References: <1507420114-12708-1-git-send-email-rohit.k.jain@oracle.com> <1507420114-12708-3-git-send-email-rohit.k.jain@oracle.com> Cc: peterz@infradead.org, mingo@redhat.com, joelaf@google.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, morten.rasmussen@arm.com From: Atish Patra Organization: Oracle Corporation Message-ID: <49cc651e-5cbc-2924-34aa-2bb271d0a0d6@oracle.com> Date: Tue, 10 Oct 2017 10:54:22 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1507420114-12708-3-git-send-email-rohit.k.jain@oracle.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Minor nit: version number missing On 10/07/2017 06:48 PM, Rohit Jain wrote: > While looking for CPUs to place running tasks on, the scheduler > completely ignores the capacity stolen away by RT/IRQ tasks. This patch > changes that behavior to also take the scaled capacity into account. > > Signed-off-by: Rohit Jain > --- > kernel/sched/fair.c | 37 ++++++++++++++++++++++++++++--------- > 1 file changed, 28 insertions(+), 9 deletions(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index eaede50..5b1f7b9 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -6004,7 +6004,7 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int > > for_each_cpu(cpu, cpu_smt_mask(core)) { > cpumask_clear_cpu(cpu, cpus); > - if (!idle_cpu(cpu)) > + if (!idle_cpu(cpu) || !full_capacity(cpu)) Do we need to skip the entire core just because 1st cpu in the core doesn't have full capacity ? Let's say that is the only idle core available. It will go and try to select_idle_cpu() to find the idlest cpu. Is it worth spending extra time to search an idle cpu with full capacity when there are idle cores available ? Regards, Atish > idle = false; > } > > @@ -6025,7 +6025,8 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int > */ > static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target) > { > - int cpu; > + int cpu, backup_cpu = -1; > + unsigned int backup_cap = 0; > > if (!static_branch_likely(&sched_smt_present)) > return -1; > @@ -6033,11 +6034,17 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t > for_each_cpu(cpu, cpu_smt_mask(target)) { > if (!cpumask_test_cpu(cpu, &p->cpus_allowed)) > continue; > - if (idle_cpu(cpu)) > - return cpu; > + if (idle_cpu(cpu)) { > + if (full_capacity(cpu)) > + return cpu; > + if (capacity_of(cpu) > backup_cap) { > + backup_cap = capacity_of(cpu); > + backup_cpu = cpu; > + } > + } > } > > - return -1; > + return backup_cpu; > } > > #else /* CONFIG_SCHED_SMT */ > @@ -6066,6 +6073,8 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t > u64 time, cost; > s64 delta; > int cpu, nr = INT_MAX; > + int backup_cpu = -1; > + unsigned int backup_cap = 0; > > this_sd = rcu_dereference(*this_cpu_ptr(&sd_llc)); > if (!this_sd) > @@ -6096,10 +6105,19 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t > return -1; > if (!cpumask_test_cpu(cpu, &p->cpus_allowed)) > continue; > - if (idle_cpu(cpu)) > - break; > + if (idle_cpu(cpu)) { > + if (full_capacity(cpu)) { > + backup_cpu = -1; > + break; > + } else if (capacity_of(cpu) > backup_cap) { > + backup_cap = capacity_of(cpu); > + backup_cpu = cpu; > + } > + } > } > > + if (backup_cpu >= 0) > + cpu = backup_cpu; > time = local_clock() - time; > cost = this_sd->avg_scan_cost; > delta = (s64)(time - cost) / 8; > @@ -6116,13 +6134,14 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target) > struct sched_domain *sd; > int i; > > - if (idle_cpu(target)) > + if (idle_cpu(target) && full_capacity(target)) > return target; > > /* > * If the previous cpu is cache affine and idle, don't be stupid. > */ > - if (prev != target && cpus_share_cache(prev, target) && idle_cpu(prev)) > + if (prev != target && cpus_share_cache(prev, target) && idle_cpu(prev) > + && full_capacity(prev)) > return prev; > > sd = rcu_dereference(per_cpu(sd_llc, target));