Message-ID: <20160715114615.GG21816@e105550-lin.cambridge.arm.com>
Date:	Fri, 15 Jul 2016 12:46:17 +0100
From:	Morten Rasmussen <morten.rasmussen@....com>
To:	Vincent Guittot <vincent.guittot@...aro.org>
Cc:	Dietmar Eggemann <dietmar.eggemann@....com>,
	Peter Zijlstra <peterz@...radead.org>,
	"mingo@...hat.com" <mingo@...hat.com>,
	Yuyang Du <yuyang.du@...el.com>, mgalbraith@...e.de,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 06/13] sched: Store maximum per-cpu capacity in root
 domain

On Thu, Jul 14, 2016 at 04:15:20PM +0100, Morten Rasmussen wrote:
> On Thu, Jul 14, 2016 at 03:25:36PM +0200, Vincent Guittot wrote:
> > On 13 July 2016 at 18:37, Morten Rasmussen <morten.rasmussen@....com> wrote:
> > > Also, for SMT max capacity is less than 1024 already. No?
> > 
> > Yes, it is. I haven't looked in details but i think that we could use
> > a capacity of 1024 for SMT with changes that have been done on how to
> > evaluate if a sched_group is overloaded or not.
> 
> Changing SMT is a bit more invasive that I had hoped for for this patch
> set. I will see if we can make it work with the current SMT capacities.
> 
> > 
> > > But we may be able to cater for this in wake_cap() somehow. I can have a
> > > look if Vincent doesn't like this patch.
> > 
> > IMO, rd->max_cpu_capacity field doesn't seem to be required for now .
> 
> No problem. I will try to get rid of it. I will drop the "arm:" patches
> as well as they would have to be extended to guarantee a max capacity of
> 1024 and we most likely will have to change it again when Juri's DT
> solution hopefully gets merged.

I have had a closer look at wake_cap() again. Getting rid of
rd->max_cpu_capacity isn't as easy as I thought.

The fundamental problem is that all we have in wake_cap() is the waking
cpu and previous cpu ids which isn't sufficient to determine whether we
have an asymmetric capacity system or not. A capacity <1024 can either a
little cpu or an SMT thread. We need a third piece of information, which
can be either the highest cpu capacity available in the cpu, or a
flag/variable/function telling us whether we are on an SMT system.

I see the following solutions to the problem:

1. Have a system-wide max_cpu_capacity (as proposed in this patch) which
can let us detect SMT systems as max_cpu_capacity < 1024 implies SMT.

2. Change SMT thread capacity to 1024 so we implicitly know that max
capacity is always 1024. As said above, this is a very invasive change
as it would mean that we no longer distinguish between SMP and SMT.
smt_gain and SD_SHARE_CPUCAPACITY would no longer have any effect and
can be ripped out. I would prefer not create a dependency on such a
massive change. We can do the experiment afterwards if needed.

3. Detect SMT in wake_cap(). This requires access to the sched_domain
hierarchy as the SD_SHARE_CPUCAPACITY is the only way to detect SMT,
AFAIK, apart from looping through the capacities of all cpus in the
system basically computing max_cpu_capacity each time.
wake_cap() is currently called before rcu_read_lock() that gives us
access to the sched_domain hierarchy. I would have to postpone the
wake_cap() call to being inside the lock and introduce another lookup in
the sched_domain hierarchy which would be executed on every wake-up on
all systems. IMHO, that is a bit ugly.

I don't really like any of the solutions, but of those three I would go
for the current solution (1) as it is very minimal both in the amount of
code touched/affected and overhead. We can kill it later if we have a
better one, no problem for me.

Do you see any alternatives?

Powered by blists - more mailing lists