linux-kernel - Re: [PATCH 1/3] sched/fair: Introduce scaled capacity awareness in find_idlest

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]

Date:   Mon, 25 Sep 2017 23:59:43 -0700
From:   Joel Fernandes <joelaf@...gle.com>
To:     Rohit Jain <rohit.k.jain@...cle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>, eas-dev@...ts.linaro.org,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Atish Patra <atish.patra@...cle.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Morten Rasmussen <morten.rasmussen@....com>
Subject: Re: [PATCH 1/3] sched/fair: Introduce scaled capacity awareness in
 find_idlest_cpu code path

Hi Rohit,

On Mon, Sep 25, 2017 at 9:40 PM, Rohit Jain <rohit.k.jain@...cle.com> wrote:
> On 09/25/2017 07:51 PM, joelaf wrote:
[...]
>>
>> On 09/25/2017 05:02 PM, Rohit Jain wrote:
>>>
>>> While looking for idle CPUs for a waking task, we should also account
>>> for the delays caused due to the bandwidth reduction by RT/IRQ tasks.
>>>
>>> This patch does that by trying to find a higher capacity CPU with
>>> minimum wake up latency.
>>>
>>>
>>> Signed-off-by: Rohit Jain <rohit.k.jain@...cle.com>
>>> ---
>>>   kernel/sched/fair.c | 27 ++++++++++++++++++++++++---
>>>   1 file changed, 24 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index eca6a57..afb701f 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -5590,6 +5590,11 @@ static unsigned long capacity_orig_of(int cpu)
>>>         return cpu_rq(cpu)->cpu_capacity_orig;
>>>   }
>>>   +static inline bool full_capacity(int cpu)
>>> +{
>>> +       return (capacity_of(cpu) >= (capacity_orig_of(cpu)*819 >> 10));
>>
>> Wouldn't 768 be better for multiplication? gcc converts the expression to
>> shifts and adds then.
>
>
> While 768 is easier to convert to shifts and adds, 819/1024 gets you
> very close to 80% which is what I was trying to achieve.

Yeah I guess if its not too hard, you could check if 768 gets you a
similar result but I would defer to the maintainers on what they are
Ok with.

>>> +}
>>> +
>>>   static unsigned long cpu_avg_load_per_task(int cpu)
>>>   {
>>>         struct rq *rq = cpu_rq(cpu);
>>> @@ -5916,8 +5921,10 @@ find_idlest_cpu(struct sched_group *group, struct
>>> task_struct *p, int this_cpu)
>>>         unsigned long load, min_load = ULONG_MAX;
>>>         unsigned int min_exit_latency = UINT_MAX;
>>>         u64 latest_idle_timestamp = 0;
>>> +       unsigned int backup_cap = 0;
>>>         int least_loaded_cpu = this_cpu;
>>>         int shallowest_idle_cpu = -1;
>>> +       int shallowest_idle_cpu_backup = -1;
>>>         int i;
>>>         /* Check if we have any choice: */
>>> @@ -5937,7 +5944,12 @@ find_idlest_cpu(struct sched_group *group, struct
>>> task_struct *p, int this_cpu)
>>>                                  */
>>>                                 min_exit_latency = idle->exit_latency;
>>>                                 latest_idle_timestamp = rq->idle_stamp;
>>> -                               shallowest_idle_cpu = i;
>>> +                               if (full_capacity(i)) {
>>> +                                       shallowest_idle_cpu = i;
>>> +                               } else if (capacity_of(i) > backup_cap) {
>>> +                                       shallowest_idle_cpu_backup = i;
>>> +                                       backup_cap = capacity_of(i);
>>> +                               }
>>
>> I'm a bit skeptical about this - if the CPU is idle, then is it likely
>> that the capacity of the CPU is reduced due to RT pressure?
>
>
> What has idleness got to do with RT pressure?
>
> This is an instantaneous view where the scheduler is looking to place
> threads. In this case, if we know historically the capacity of the CPU
> is reduced (due to RT/IRQ/Thermal Throttling or whatever it may be) we
> should avoid that CPU if we have a choice.

Yeah Ok, that's a fair point, I don't dispute this fact. I was just
trying to understand your patch.

thanks,

- Joel

[...]