lkml.org 
[lkml]   [2019]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [RFC v5 4/6] sched/fair: Tune task wake-up logic to pack small background tasks on fewer cores
From
Date


On 10/9/19 5:04 PM, Vincent Guittot wrote:
> On Wed, 9 Oct 2019 at 11:23, Parth Shah <parth@linux.ibm.com> wrote:
>>
>>
>>
>> On 10/8/19 6:58 PM, Hillf Danton wrote:
>>>
>>> On Mon, 7 Oct 2019 14:00:49 +0530 Parth Shah wrote:
>>>> +/*
>>>> + * Try to find a non idle core in the system based on few heuristics:
>>>> + * - Keep track of overutilized (>80% util) and busy (>12.5% util) CPUs
>>>> + * - If none CPUs are busy then do not select the core for task packing
>>>> + * - If atleast one CPU is busy then do task packing unless overutilized CPUs
>>>> + * count is < busy/2 CPU count
>>>> + * - Always select idle CPU for task packing
>>>> + */
>>>> +static int select_non_idle_core(struct task_struct *p, int prev_cpu, int target)
>>>> +{
>>>> + struct cpumask *cpus = this_cpu_cpumask_var_ptr(turbo_sched_mask);
>>>> + int iter_cpu, sibling;
>>>> +
>>>> + cpumask_and(cpus, cpu_online_mask, p->cpus_ptr);
>>>> +
>>>> + for_each_cpu_wrap(iter_cpu, cpus, prev_cpu) {
>>>> + int idle_cpu_count = 0, non_idle_cpu_count = 0;
>>>> + int overutil_cpu_count = 0;
>>>> + int busy_cpu_count = 0;
>>>> + int best_cpu = iter_cpu;
>>>> +
>>>> + for_each_cpu(sibling, cpu_smt_mask(iter_cpu)) {
>>>> + __cpumask_clear_cpu(sibling, cpus);
>>>> + if (idle_cpu(iter_cpu)) {
>>>
>>> Would you please elaborate the reasons that the iter cpu is checked idle
>>> more than once for finding a busy core?
>>>
>>
>> Thanks for looking at the patches.
>> Could you please point me out where iter_cpu is checked more than once?
>
> I think that point is that you have a sibling that there is
> for_each_cpu(sibling, cpu_smt_mask(iter_cpu) but you never use sibling
> in the loop except for clearing it on the cpumask cpus
> All the tests are done with iter_cpu so you will test several time
> iter_cpus but never the other sibling
> Should you use sibling instead ?
>

oh got it. it was unintentional here, my bad.
good find

I did s/iter_cpu/sibling/ at required places:

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d4a1b6474338..a75c2b382771 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6001,14 +6001,14 @@ static int select_non_idle_core(struct task_struct
*p, int prev_cpu, int target)

for_each_cpu(sibling, cpu_smt_mask(iter_cpu)) {
__cpumask_clear_cpu(sibling, cpus);
- if (idle_cpu(iter_cpu)) {
+ if (idle_cpu(sibling)) {
idle_cpu_count++;
- best_cpu = iter_cpu;
+ best_cpu = sibling;
} else {
non_idle_cpu_count++;
- if (cpu_overutilized(iter_cpu))
+ if (cpu_overutilized(sibling))
overutil_cpu_count++;
- if (is_cpu_busy(cpu_util(iter_cpu)))
+ if (is_cpu_busy(cpu_util(sibling)))
busy_cpu_count++;
}
}
and the took the results again to see functionality changes.
Results are still within the bounds with maximum of 15% gain in performance
and <2% of regression.

Frequency benefit of TurboSched w.r.t. CFS
+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+
20 +-+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +-+
| Frequency benefit in % |
| ** |
15 +-+ * * ** ****** +-+
| * * ************ |
| ** * * ************ * |
10 +-+ ** * * ************ * +-+
| ** * * ************ * * **** |
| **** * * ************ * * **** |
5 +-+ ****** * * ************ * * ****** +-+
| ****** * * ************ * * ********** |
0 +-******** * * ************ * * ************ * * * ********** * * * **+
| |
| |
-5 +-+ +-+
| + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + |
+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+
0 1 2 3 4 5 6 7 8 91011 1213141516171819 2021222324252627 28293031
No. of workload threads
Performance benefit of TurboSched w.r.t. CFS
20 +-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+
| + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + |
| Performance benefit in % |
15 +-+ ** +-+
| ** |
| ******** * |
10 +-+ ******** * ** +-+
| ******** * * ** |
| ******** * * ** |
5 +-+ ********** * * ****** +-+
| ********** * * ********** |
| ************ * * ********** * ** |
0 +-******** * * ************ * * ************ * * * ********** * * * **+
| ******** * |
| |
-5 +-+ +-+
| + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + |
+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+-+-+-+--+-+-+-+-+
0 1 2 3 4 5 6 7 8 91011 1213141516171819 2021222324252627 28293031
No. of workload threads
Thanks,
Parth

>
>>
>>>> + idle_cpu_count++;
>>>> + best_cpu = iter_cpu;
>>>> + } else {
>>>> + non_idle_cpu_count++;
>>>> + if (cpu_overutilized(iter_cpu))
>>>> + overutil_cpu_count++;
>>>> + if (is_cpu_busy(cpu_util(iter_cpu)))
>>>> + busy_cpu_count++;
>>>> + }
>>>> + }
>>>> +
>>>
>>

\
 
 \ /
  Last update: 2019-10-09 18:56    [W:0.219 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site