Date: Tue, 28 Mar 2017 09:04:29 +0900
From: Byungchul Park <byungchul.park@lge.com>
To: Juri Lelli <juri.lelli@arm.com>
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: <20170328000429.GD21430@X58A-UD3R>
References: <1490235169-370-1-git-send-email-byungchul.park@lge.com>
 <20170327143343.GP10289@e106622-lin>
MIME-Version: 1.0
In-Reply-To: <20170327143343.GP10289@e106622-lin>
User-Agent: Mutt/1.5.21 (2010-09-15)
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2415
Lines: 62

On Mon, Mar 27, 2017 at 03:33:43PM +0100, Juri Lelli wrote:
> > 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.

Hi Juri,

Actually I changed the return value to be bool, but didn't include the
patch since it looks not that valuable. But I will add it if you also
think so. ;)

> 
> >  		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?

OK. I will rename it to best_cpu.

Thanks,
Byungchul