linux-kernel - Re: [PATCH v3 1/3] sched/fair: Add asymmetric CPU capacity wakeup scan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <40bfa77b-b695-5f53-848a-b72b67b33d69@arm.com>
Date:   Wed, 29 Jan 2020 12:10:55 +0000
From:   Valentin Schneider <valentin.schneider@....com>
To:     Dietmar Eggemann <dietmar.eggemann@....com>,
        linux-kernel@...r.kernel.org
Cc:     mingo@...hat.com, peterz@...radead.org, vincent.guittot@...aro.org,
        morten.rasmussen@....com, qperret@...gle.com,
        adharmap@...eaurora.org
Subject: Re: [PATCH v3 1/3] sched/fair: Add asymmetric CPU capacity wakeup
 scan

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!