Messages in this thread | | | Date | Mon, 6 Jan 2020 22:49:41 +0800 | From | Peng Liu <> | Subject | Re: [PATCH v2] sched/fair: fix sgc->{min,max}_capacity miscalculate |
| |
On Mon, Jan 06, 2020 at 10:25:49AM +0100, Dietmar Eggemann wrote: > On 04/01/2020 14:08, Peng Liu wrote: > > Could you add a hint that this is about the SD_OVERLAP path? Something > like 'Fix sgc->{min,max}_capacity calculation for SD_OVERLAP' > > > commit bf475ce0a3dd ("sched/fair: Add per-CPU min capacity to > > sched_group_capacity") introduced per-cpu min_capacity. > > > > commit e3d6d0cb66f2 ("sched/fair: Add sched_group per-CPU max capacity") > > introduced per-cpu max_capacity. > > > > Could we improve the description of the issue and the change a little > bit? Something like: > > In the SD_OVERLAP case, the local variable 'capacity' represents the sum > of CPU capacity of all CPUs in the first sched group (sg) of the sched > domain (sd). > > It is erroneously used to calculate sg's min and max CPU capacity. > To fix this use capacity_of(cpu) instead of 'capacity'. > > The code which achieves this via cpu_rq(cpu)->sd->groups->sgc->capacity > (for rq->sd != NULL) can be removed since it delivers the same value as > capacity_of(cpu) which is currently only used for the (!rq->sd) case > (see update_cpu_capacity()). > A sg of the lowest sd (rq->sd or sd->child == NULL) represents a single > CPU (and hence sg->sgc->capacity == capacity_of(cpu)).
Dietmar, thanks for your time. Indeed, it's better with a detailed description.
> > > Nit: Why not > > + capacity += cpu_cap; > + min_capacity = min(cpu_cap, min_capacity); > + max_capacity = max(cpu_cap, max_capacity); > > like in the !SD_OVERLAP path? > > > } > > } else { > > /* > >
| |