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

[Parth Shah]

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