Re: [PATCH v2 06/13] sched: Store maximum per-cpu capacity in root domain

[Morten Rasmussen]

On Thu, Jul 14, 2016 at 03:25:36PM +0200, Vincent Guittot wrote:
> On 13 July 2016 at 18:37, Morten Rasmussen <morten.rasmussen@xxxxxxx> wrote:
> > On Wed, Jul 13, 2016 at 02:48:24PM +0100, Dietmar Eggemann wrote:
> >> On 13/07/16 13:40, Vincent Guittot wrote:
> >> > On 22 June 2016 at 19:03, Morten Rasmussen <morten.rasmussen@xxxxxxx> wrote:
> >> >> From: Dietmar Eggemann <dietmar.eggemann@xxxxxxx>
> >> >>
> >> >> To be able to compare the capacity of the target cpu with the highest
> >> >> available cpu capacity, store the maximum per-cpu capacity in the root
> >> >> domain.
> >> >
> >> > I thought that the capacity of all CPUS were built so the highest
> >> > capacity of the CPU of the system is 1024  for big LITTLE system . So
> >> > this patch doesn't seem necessary for big.LITTLE system
> >>
> >> The asymmetric cpu capacity support currently only has an effect on arm
> >> big.LITTLE (32bit) using the existing 'struct cpu_efficiency
> >> table_efficiency[]' based approach.
> >
> > True for this patch set, but longer term and if you use the preview
> > branch mentioned in the cover letter Vincent is right. The idea is that
> > the highest capacity anywhere should be 1024.
> >
> > If we fix the arch/arm/kernel/topology.c code at the same time we could
> > kill this patch.
> >
> > However, even further down the road we might need it (or something
> > similar) anyway due to the thermal framework. At some point we would
> > like to adjust the max capacity based any OPP constraints imposed by the
> > thermal framework. In extreme cases big cpus might be capped so hard
> > that they effectively have smaller capacity than little. I don't think
> > it makes sense to re-normalize everything to the highest available
> > capacity to ensure that there is always a cpu with capacity = 1024 in
> > the system, instead we must be able to cope with scenarios where max
> > capacity is smaller than 1024.
> 
> Yes we will have to found a solution for thermal mitigation but i
> don't know if a rd->max_cpu_capacity would the best solution

Agreed, I'm pretty sure that the current form isn't sufficient.

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

Morten