Re: [PATCH v6 0/2] Make find_later_rq() choose a closer cpu in topology

[Joel Fernandes (Google)]

On Thu, Aug 17, 2017 at 6:25 PM, Byungchul Park <byungchul.park@xxxxxxx> wrote:
> On Mon, Aug 07, 2017 at 12:50:32PM +0900, Byungchul Park wrote:
>> When cpudl_find() returns any among free_cpus, the cpu might not be
>> closer than others, considering sched domain. For example:
>>
>>    this_cpu: 15
>>    free_cpus: 0, 1,..., 14 (== later_mask)
>>    best_cpu: 0
>>
>>    topology:
>>
>>    0 --+
>>        +--+
>>    1 --+  |
>>           +-- ... --+
>>    2 --+  |         |
>>        +--+         |
>>    3 --+            |
>>
>>    ...             ...
>>
>>    12 --+           |
>>         +--+        |
>>    13 --+  |        |
>>            +-- ... -+
>>    14 --+  |
>>         +--+
>>    15 --+
>>
>> In this case, it would be best to select 14 since it's a free cpu and
>> closest to 15(this_cpu). However, currently the code select 0(best_cpu)
>> even though that's just any among free_cpus. Fix it.
>
> Could you let me know your opinions about this?

Patch looks good to me, I would also add a comment ontop of
fallback_cpu (I think Steve mentioned similar thing at [1])

/*
 * fallback is the closest CPU in the closest SD incase
 * all domains are PREFER_SIBLING
 */
 if (fallback_cpu == -1)
     fallback_cpu = best_cpu;

And clarify this in the commit message.


thanks,

-Joel

[1] https://patchwork.kernel.org/patch/9884383/


>
>> Change from v5
>>    -. exclude two patches already picked up by peterz
>>       (sched/deadline: Make find_later_rq() choose a closer cpu in topology)
>>       (sched/deadline: Change return value of cpudl_find())
>>    -. apply what peterz fixed for 'prefer sibling', into deadline and rt
>>
>> Change from v4
>>    -. remove a patch that might cause huge lock contention
>>       (by spin lock(&cpudl.lock) in a hot path of scheduler)
>>
>> Change from v3
>>    -. rename closest_cpu to best_cpu so that it align with rt
>>    -. protect referring cpudl.elements with cpudl.lock
>>    -. change return value of cpudl_find() to bool
>>
>> Change from v2
>>    -. add support for SD_PREFER_SIBLING
>>
>> Change from v1
>>    -. clean up the patch
>>
>> Byungchul Park (2):
>>   sched/deadline: Add support for SD_PREFER_SIBLING on find_later_rq()
>>   sched/rt: Add support for SD_PREFER_SIBLING on find_lowest_rq()
>>
>>  kernel/sched/deadline.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
>>  kernel/sched/rt.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 87 insertions(+), 6 deletions(-)
>>
>> --
>> 1.9.1