Messages in this thread | | | Subject | Re: [PATCH v3 1/3] sched/fair: Add asymmetric CPU capacity wakeup scan | From | Valentin Schneider <> | Date | Wed, 29 Jan 2020 12:10:55 +0000 |
| |
On 29/01/2020 11:04, Dietmar Eggemann wrote: >> + /* >> + * It would be silly to keep looping when we've found a CPU >> + * of highest available capacity. Just check that it's not been >> + * too pressured lately. >> + */ >> + if (rq->cpu_capacity_orig == READ_ONCE(rq->rd->max_cpu_capacity) && > > There is a similar check in check_misfit_status(). Common helper function?
Mright, and check_misfit_status() is missing the READ_ONCE(). That said...
> >> + !check_cpu_capacity(rq, sd)) >> + return cpu; > > I wonder how this special treatment of a big CPU behaves in (LITTLE, > medium, big) system like Pixel4 (Snapdragon 855): > > flame:/ $ cat /sys/devices/system/cpu/cpu*/cpu_capacity > > 261 > 261 > 261 > 261 > 871 > 871 > 871 > 1024 > > Or on legacy systems where the sd->imbalance_pct is 25% instead of 17%? >
... This is a very valid point. When I wrote this bit I had the good old big.LITTLE split in mind where there are big differences between the capacity values. As you point out, that's not so true with DynamIQ systems sporting > 2 capacity values. The issue here is that we could bail early picking a (slightly) pressured big (1024 capacity_orig) when there was a non-pressured idle medium (871 capacity orig).
It's borderline in this example - the threshold for a big to be seen as pressured by check_cpu_capacity(), assuming a flat topology with just an MC domain, is ~ 875. If we have e.g. mediums at 900 and bigs at 1024, this logic is broken.
So this is pretty much a case of my trying to be too clever for my own good, I'll remove that "fastpath" in v4. Thanks for pointing it out!
| |