linux-kernel - Re: [PATCH v3 5/7] sched: SIS_CORE to disable idle core search

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:   Thu, 4 Jul 2019 18:04:49 +0530
From:   Parth Shah <parth@...ux.ibm.com>
To:     Subhra Mazumdar <subhra.mazumdar@...cle.com>,
        linux-kernel@...r.kernel.org
Cc:     peterz@...radead.org, mingo@...hat.com, tglx@...utronix.de,
        steven.sistare@...cle.com, dhaval.giani@...cle.com,
        daniel.lezcano@...aro.org, vincent.guittot@...aro.org,
        viresh.kumar@...aro.org, tim.c.chen@...ux.intel.com,
        mgorman@...hsingularity.net
Subject: Re: [PATCH v3 5/7] sched: SIS_CORE to disable idle core search



On 7/2/19 2:07 AM, Subhra Mazumdar wrote:
> 
>>>> Also, systems like POWER9 has sd_llc as a pair of core only. So it
>>>> won't benefit from the limits and hence also hiding your code in select_idle_cpu
>>>> behind static keys will be much preferred.
>>> If it doesn't hurt then I don't see the point.
>>>
>> So these is the result from POWER9 system with your patches:
>> System configuration: 2 Socket, 44 cores, 176 CPUs
>>
>> Experiment setup:
>> ===========
>> => Setup 1:
>> - 44 tasks doing just while(1), this is to make select_idle_core return -1 most times
>> - perf bench sched messaging -g 1 -l 1000000
>> +-----------+--------+--------------+--------+
>> | Baseline  | stddev |    Patch     | stddev |
>> +-----------+--------+--------------+--------+
>> |       135 |   3.21 | 158(-17.03%) |   4.69 |
>> +-----------+--------+--------------+--------+
>>
>> => Setup 2:
>> - schbench -m44 -t 1
>> +=======+==========+=========+=========+==========+
>> | %ile  | Baseline | stddev  |  patch  |  stddev  |
>> +=======+==========+=========+=========+==========+
>> |    50 |       10 |    3.49 |      10 |     2.29 |
>> +-------+----------+---------+---------+----------+
>> |    95 |      467 |    4.47 |     469 |     0.81 |
>> +-------+----------+---------+---------+----------+
>> |    99 |      571 |   21.32 |     584 |    18.69 |
>> +-------+----------+---------+---------+----------+
>> |  99.5 |      629 |   30.05 |     641 |    20.95 |
>> +-------+----------+---------+---------+----------+
>> |  99.9 |      780 |   40.38 |     773 |     44.2 |
>> +-------+----------+---------+---------+----------+
>>
>> I guess it doesn't make much difference in schbench results but hackbench (perf bench)
>> seems to have an observable regression.
>>
>>
>> Best,
>> Parth
>>
> If POWER9 sd_llc has only 2 cores, the behavior shouldn't change much with
> the select_idle_cpu changes as the limits are 1 and 2 core. Previously the
> lower bound was 4 cpus and upper bound calculated by the prop. Now it is
> 1 core (4 cpus on SMT4) and upper bound 2 cores. Could it be the extra
> computation of cpumask_weight causing the regression rather than the
> sliding window itself (one way to check this would be hardcode 4 in place
> of topology_sibling_weight)? Or is it the L1 cache coherency? I am a bit
> suprised because SPARC SMT8 which has more cores in sd_llc and L1 cache per
> core showed improvement with Hackbench.
> 

Same experiment with hackbench and with perf analysis shows increase in L1
cache miss rate with these patches
(Lower is better)
                          Baseline(%)   Patch(%)   
 ----------------------- ------------- ----------- 
  Total Cache miss rate         17.01   19(-11%)   
  L1 icache miss rate            5.45   6.7(-22%) 



So is is possible for idle_cpu search to try checking target_cpu first and
then goto sliding window if not found.
Below diff works as expected in IBM POWER9 system and resolves the problem
of far wakeup upto large extent.

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ff2e9b5c3ac5..fae035ce1162 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6161,6 +6161,7 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t
        u64 time, cost;
        s64 delta;
        int cpu, limit, floor, target_tmp, nr = INT_MAX;
+       struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
 
        this_sd = rcu_dereference(*this_cpu_ptr(&sd_llc));
        if (!this_sd)
@@ -6198,16 +6199,22 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t
 
        time = local_clock();
 
-       for_each_cpu_wrap(cpu, sched_domain_span(sd), target_tmp) {
+       cpumask_and(cpus, sched_domain_span(sd), &p->cpus_allowed);
+       for_each_cpu_wrap(cpu, cpu_smt_mask(target), target) {
+               __cpumask_clear_cpu(cpu, cpus);
+               if (available_idle_cpu(cpu))
+                       goto idle_cpu_exit;
+       }
+
+       for_each_cpu_wrap(cpu, cpus, target_tmp) {
                per_cpu(next_cpu, target) = cpu;
                if (!--nr)
                        return -1;
-               if (!cpumask_test_cpu(cpu, &p->cpus_allowed))
-                       continue;
                if (available_idle_cpu(cpu))
                        break;
        }
 
+idle_cpu_exit:
        time = local_clock() - time;
        cost = this_sd->avg_scan_cost;
        delta = (s64)(time - cost) / 8;



Best,
Parth