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

2017-08-17 Thread Byungchul Park
On Thu, Aug 17, 2017 at 09:51:34PM -0700, Joel Fernandes (Google) wrote:
> On Thu, Aug 17, 2017 at 6:25 PM, Byungchul Park  
> 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.

Right. I will add it.

Thank you very much,
Byungchul


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

2017-08-17 Thread Byungchul Park
On Thu, Aug 17, 2017 at 09:51:34PM -0700, Joel Fernandes (Google) wrote:
> On Thu, Aug 17, 2017 at 6:25 PM, Byungchul Park  
> 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.

Right. I will add it.

Thank you very much,
Byungchul


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

2017-08-17 Thread Joel Fernandes (Google)
On Thu, Aug 17, 2017 at 6:25 PM, Byungchul Park  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() 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


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

2017-08-17 Thread Joel Fernandes (Google)
On Thu, Aug 17, 2017 at 6:25 PM, Byungchul Park  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() 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


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

2017-08-17 Thread Byungchul Park
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?

> 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() 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


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

2017-08-17 Thread Byungchul Park
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?

> 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() 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


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

2017-08-06 Thread Byungchul Park
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.

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() 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



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

2017-08-06 Thread Byungchul Park
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.

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() 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