Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753328AbdC0OeO (ORCPT ); Mon, 27 Mar 2017 10:34:14 -0400 Received: from foss.arm.com ([217.140.101.70]:35928 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752827AbdC0OeF (ORCPT ); Mon, 27 Mar 2017 10:34:05 -0400 Date: Mon, 27 Mar 2017 15:33:43 +0100 From: Juri Lelli To: Byungchul Park Cc: peterz@infradead.org, mingo@kernel.org, linux-kernel@vger.kernel.org, juri.lelli@gmail.com, rostedt@goodmis.org, kernel-team@lge.com Subject: Re: [PATCH v3 1/3] sched/deadline: Make find_later_rq() choose a closer cpu in topology Message-ID: <20170327143343.GP10289@e106622-lin> References: <1490235169-370-1-git-send-email-byungchul.park@lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1490235169-370-1-git-send-email-byungchul.park@lge.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3027 Lines: 92 Hi, On 23/03/17 11:12, Byungchul Park wrote: > When cpudl_find() returns any among free_cpus, the cpu might not be > closer than others, considering sched domain. For example: > > this_cpu: 15 > free_cpus: 0, 1,..., 14 (== later_mask) > best_cpu: 0 > > topology: > > 0 --+ > +--+ > 1 --+ | > +-- ... --+ > 2 --+ | | > +--+ | > 3 --+ | > > ... ... > > 12 --+ | > +--+ | > 13 --+ | | > +-- ... -+ > 14 --+ | > +--+ > 15 --+ > > In this case, it would be best to select 14 since it's a free cpu and > closest to 15(this_cpu). However, currently the code select 0(best_cpu) > even though that's just any among free_cpus. Fix it. > > Signed-off-by: Byungchul Park > --- > kernel/sched/deadline.c | 29 +++++++++++++++-------------- > 1 file changed, 15 insertions(+), 14 deletions(-) > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index a2ce590..49c93b9 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -1324,7 +1324,7 @@ static int find_later_rq(struct task_struct *task) > struct sched_domain *sd; > struct cpumask *later_mask = this_cpu_cpumask_var_ptr(local_cpu_mask_dl); > int this_cpu = smp_processor_id(); > - int best_cpu, cpu = task_cpu(task); > + int cpu = task_cpu(task); > > /* Make sure the mask is initialized first */ > if (unlikely(!later_mask)) > @@ -1337,17 +1337,14 @@ static int find_later_rq(struct task_struct *task) > * We have to consider system topology and task affinity > * first, then we can look for a suitable cpu. > */ > - best_cpu = cpudl_find(&task_rq(task)->rd->cpudl, > - task, later_mask); > - if (best_cpu == -1) > + if (cpudl_find(&task_rq(task)->rd->cpudl, task, later_mask) == -1) It seems that with this we loose the last user of the current return value of cpudl_find() (heap maximum). I guess we want to change the return value to be (int)bool, as in rt, so that we can simplify this and the conditions in check_preempt_equal_dl. > return -1; > > /* > - * If we are here, some target has been found, > - * the most suitable of which is cached in best_cpu. > - * This is, among the runqueues where the current tasks > - * have later deadlines than the task's one, the rq > - * with the latest possible one. > + * If we are here, some targets have been found, including > + * the most suitable which is, among the runqueues where the > + * current tasks have later deadlines than the task's one, the > + * rq with the latest possible one. > * > * Now we check how well this matches with task's > * affinity and system topology. > @@ -1367,6 +1364,7 @@ static int find_later_rq(struct task_struct *task) > rcu_read_lock(); > for_each_domain(cpu, sd) { > if (sd->flags & SD_WAKE_AFFINE) { > + int closest_cpu; Can we still call this best_cpu, so that we are aligned with rt? Thanks, - Juri