Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-11 Thread Preeti U Murthy 
On 09/10/2014 07:20 PM, Peter Zijlstra wrote:
> On Sat, Aug 30, 2014 at 10:37:40PM +0530, Preeti U Murthy wrote:
>>> -   if ((sd->flags & SD_SHARE_CPUCAPACITY) && weight > 1) {
>>> -   if (sched_feat(ARCH_CAPACITY))
>>
>> Aren't you missing this check above? I understand that it is not
>> crucial, but that would also mean removing ARCH_CAPACITY sched_feat
>> altogether, wouldn't it?
> 
> Yes he's missing that, I added the below bit on top.
> 
> So the argument last time:
> lkml.kernel.org/r/20140709105721.gt19...@twins.programming.kicks-ass.net
> was that you cannot put sched_feat(ARCH_CAPACITY) inside a weak arch_*
> function. The test has to be outside, seeing how it needs to decide to
> call the arch function at all (or revert to the default implementation).
> 
> ---
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5700,7 +5700,7 @@ unsigned long __weak arch_scale_freq_cap
>   return default_scale_capacity(sd, cpu);
>  }
> 
> -unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
> cpu)
> +static unsigned long default_scale_cpu_capacity(struct sched_domain *sd, int 
> cpu)
>  {
>   if ((sd->flags & SD_SHARE_CPUCAPACITY) && (sd->span_weight > 1))
>   return sd->smt_gain / sd->span_weight;
> @@ -5708,6 +5708,11 @@ unsigned long __weak arch_scale_cpu_capa
>   return SCHED_CAPACITY_SCALE;
>  }
> 
> +unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
> cpu)
> +{
> + return default_scale_cpu_capacity(sd, cpu);
> +}
> +
>  static unsigned long scale_rt_capacity(int cpu)
>  {
>   struct rq *rq = cpu_rq(cpu);
> @@ -5747,7 +5752,10 @@ static void update_cpu_capacity(struct s
>   unsigned long capacity = SCHED_CAPACITY_SCALE;
>   struct sched_group *sdg = sd->groups;
> 
> - capacity *= arch_scale_cpu_capacity(sd, cpu);
> + if (sched_feat(ARCH_CAPACITY))
> + capacity *= arch_scale_cpu_capacity(sd, cpu);
> + else
> + capacity *= default_scale_cpu_capacity(sd, cpu);
> 
>   capacity >>= SCHED_CAPACITY_SHIFT;
> 
> 
Alright, I see now. Thanks!

Regards
Preeti U Murthy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-11 Thread Preeti U Murthy 
On 09/10/2014 07:20 PM, Peter Zijlstra wrote:
 On Sat, Aug 30, 2014 at 10:37:40PM +0530, Preeti U Murthy wrote:
 -   if ((sd-flags  SD_SHARE_CPUCAPACITY)  weight  1) {
 -   if (sched_feat(ARCH_CAPACITY))

 Aren't you missing this check above? I understand that it is not
 crucial, but that would also mean removing ARCH_CAPACITY sched_feat
 altogether, wouldn't it?
 
 Yes he's missing that, I added the below bit on top.
 
 So the argument last time:
 lkml.kernel.org/r/20140709105721.gt19...@twins.programming.kicks-ass.net
 was that you cannot put sched_feat(ARCH_CAPACITY) inside a weak arch_*
 function. The test has to be outside, seeing how it needs to decide to
 call the arch function at all (or revert to the default implementation).
 
 ---
 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -5700,7 +5700,7 @@ unsigned long __weak arch_scale_freq_cap
   return default_scale_capacity(sd, cpu);
  }
 
 -unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
 cpu)
 +static unsigned long default_scale_cpu_capacity(struct sched_domain *sd, int 
 cpu)
  {
   if ((sd-flags  SD_SHARE_CPUCAPACITY)  (sd-span_weight  1))
   return sd-smt_gain / sd-span_weight;
 @@ -5708,6 +5708,11 @@ unsigned long __weak arch_scale_cpu_capa
   return SCHED_CAPACITY_SCALE;
  }
 
 +unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
 cpu)
 +{
 + return default_scale_cpu_capacity(sd, cpu);
 +}
 +
  static unsigned long scale_rt_capacity(int cpu)
  {
   struct rq *rq = cpu_rq(cpu);
 @@ -5747,7 +5752,10 @@ static void update_cpu_capacity(struct s
   unsigned long capacity = SCHED_CAPACITY_SCALE;
   struct sched_group *sdg = sd-groups;
 
 - capacity *= arch_scale_cpu_capacity(sd, cpu);
 + if (sched_feat(ARCH_CAPACITY))
 + capacity *= arch_scale_cpu_capacity(sd, cpu);
 + else
 + capacity *= default_scale_cpu_capacity(sd, cpu);
 
   capacity = SCHED_CAPACITY_SHIFT;
 
 
Alright, I see now. Thanks!

Regards
Preeti U Murthy

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-10 Thread Vincent Guittot 
On 10 September 2014 15:50, Peter Zijlstra  wrote:
> On Sat, Aug 30, 2014 at 10:37:40PM +0530, Preeti U Murthy wrote:
>> > -   if ((sd->flags & SD_SHARE_CPUCAPACITY) && weight > 1) {
>> > -   if (sched_feat(ARCH_CAPACITY))
>>
>> Aren't you missing this check above? I understand that it is not
>> crucial, but that would also mean removing ARCH_CAPACITY sched_feat
>> altogether, wouldn't it?
>
> Yes he's missing that, I added the below bit on top.

FWIW, your changes are fine for me

>
> So the argument last time:
> lkml.kernel.org/r/20140709105721.gt19...@twins.programming.kicks-ass.net
> was that you cannot put sched_feat(ARCH_CAPACITY) inside a weak arch_*
> function. The test has to be outside, seeing how it needs to decide to
> call the arch function at all (or revert to the default implementation).
>
> ---
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5700,7 +5700,7 @@ unsigned long __weak arch_scale_freq_cap
> return default_scale_capacity(sd, cpu);
>  }
>
> -unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
> cpu)
> +static unsigned long default_scale_cpu_capacity(struct sched_domain *sd, int 
> cpu)
>  {
> if ((sd->flags & SD_SHARE_CPUCAPACITY) && (sd->span_weight > 1))
> return sd->smt_gain / sd->span_weight;
> @@ -5708,6 +5708,11 @@ unsigned long __weak arch_scale_cpu_capa
> return SCHED_CAPACITY_SCALE;
>  }
>
> +unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
> cpu)
> +{
> +   return default_scale_cpu_capacity(sd, cpu);
> +}
> +
>  static unsigned long scale_rt_capacity(int cpu)
>  {
> struct rq *rq = cpu_rq(cpu);
> @@ -5747,7 +5752,10 @@ static void update_cpu_capacity(struct s
> unsigned long capacity = SCHED_CAPACITY_SCALE;
> struct sched_group *sdg = sd->groups;
>
> -   capacity *= arch_scale_cpu_capacity(sd, cpu);
> +   if (sched_feat(ARCH_CAPACITY))
> +   capacity *= arch_scale_cpu_capacity(sd, cpu);
> +   else
> +   capacity *= default_scale_cpu_capacity(sd, cpu);
>
> capacity >>= SCHED_CAPACITY_SHIFT;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-10 Thread Peter Zijlstra 
On Sat, Aug 30, 2014 at 10:37:40PM +0530, Preeti U Murthy wrote:
> > -   if ((sd->flags & SD_SHARE_CPUCAPACITY) && weight > 1) {
> > -   if (sched_feat(ARCH_CAPACITY))
> 
> Aren't you missing this check above? I understand that it is not
> crucial, but that would also mean removing ARCH_CAPACITY sched_feat
> altogether, wouldn't it?

Yes he's missing that, I added the below bit on top.

So the argument last time:
lkml.kernel.org/r/20140709105721.gt19...@twins.programming.kicks-ass.net
was that you cannot put sched_feat(ARCH_CAPACITY) inside a weak arch_*
function. The test has to be outside, seeing how it needs to decide to
call the arch function at all (or revert to the default implementation).

---
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5700,7 +5700,7 @@ unsigned long __weak arch_scale_freq_cap
return default_scale_capacity(sd, cpu);
 }
 
-unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
+static unsigned long default_scale_cpu_capacity(struct sched_domain *sd, int 
cpu)
 {
if ((sd->flags & SD_SHARE_CPUCAPACITY) && (sd->span_weight > 1))
return sd->smt_gain / sd->span_weight;
@@ -5708,6 +5708,11 @@ unsigned long __weak arch_scale_cpu_capa
return SCHED_CAPACITY_SCALE;
 }
 
+unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
+{
+   return default_scale_cpu_capacity(sd, cpu);
+}
+
 static unsigned long scale_rt_capacity(int cpu)
 {
struct rq *rq = cpu_rq(cpu);
@@ -5747,7 +5752,10 @@ static void update_cpu_capacity(struct s
unsigned long capacity = SCHED_CAPACITY_SCALE;
struct sched_group *sdg = sd->groups;
 
-   capacity *= arch_scale_cpu_capacity(sd, cpu);
+   if (sched_feat(ARCH_CAPACITY))
+   capacity *= arch_scale_cpu_capacity(sd, cpu);
+   else
+   capacity *= default_scale_cpu_capacity(sd, cpu);
 
capacity >>= SCHED_CAPACITY_SHIFT;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-10 Thread Peter Zijlstra 
On Sat, Aug 30, 2014 at 10:37:40PM +0530, Preeti U Murthy wrote:
  -   if ((sd-flags  SD_SHARE_CPUCAPACITY)  weight  1) {
  -   if (sched_feat(ARCH_CAPACITY))
 
 Aren't you missing this check above? I understand that it is not
 crucial, but that would also mean removing ARCH_CAPACITY sched_feat
 altogether, wouldn't it?

Yes he's missing that, I added the below bit on top.

So the argument last time:
lkml.kernel.org/r/20140709105721.gt19...@twins.programming.kicks-ass.net
was that you cannot put sched_feat(ARCH_CAPACITY) inside a weak arch_*
function. The test has to be outside, seeing how it needs to decide to
call the arch function at all (or revert to the default implementation).

---
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5700,7 +5700,7 @@ unsigned long __weak arch_scale_freq_cap
return default_scale_capacity(sd, cpu);
 }
 
-unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
+static unsigned long default_scale_cpu_capacity(struct sched_domain *sd, int 
cpu)
 {
if ((sd-flags  SD_SHARE_CPUCAPACITY)  (sd-span_weight  1))
return sd-smt_gain / sd-span_weight;
@@ -5708,6 +5708,11 @@ unsigned long __weak arch_scale_cpu_capa
return SCHED_CAPACITY_SCALE;
 }
 
+unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
+{
+   return default_scale_cpu_capacity(sd, cpu);
+}
+
 static unsigned long scale_rt_capacity(int cpu)
 {
struct rq *rq = cpu_rq(cpu);
@@ -5747,7 +5752,10 @@ static void update_cpu_capacity(struct s
unsigned long capacity = SCHED_CAPACITY_SCALE;
struct sched_group *sdg = sd-groups;
 
-   capacity *= arch_scale_cpu_capacity(sd, cpu);
+   if (sched_feat(ARCH_CAPACITY))
+   capacity *= arch_scale_cpu_capacity(sd, cpu);
+   else
+   capacity *= default_scale_cpu_capacity(sd, cpu);
 
capacity = SCHED_CAPACITY_SHIFT;
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-10 Thread Vincent Guittot 
On 10 September 2014 15:50, Peter Zijlstra pet...@infradead.org wrote:
 On Sat, Aug 30, 2014 at 10:37:40PM +0530, Preeti U Murthy wrote:
  -   if ((sd-flags  SD_SHARE_CPUCAPACITY)  weight  1) {
  -   if (sched_feat(ARCH_CAPACITY))

 Aren't you missing this check above? I understand that it is not
 crucial, but that would also mean removing ARCH_CAPACITY sched_feat
 altogether, wouldn't it?

 Yes he's missing that, I added the below bit on top.

FWIW, your changes are fine for me


 So the argument last time:
 lkml.kernel.org/r/20140709105721.gt19...@twins.programming.kicks-ass.net
 was that you cannot put sched_feat(ARCH_CAPACITY) inside a weak arch_*
 function. The test has to be outside, seeing how it needs to decide to
 call the arch function at all (or revert to the default implementation).

 ---
 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -5700,7 +5700,7 @@ unsigned long __weak arch_scale_freq_cap
 return default_scale_capacity(sd, cpu);
  }

 -unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
 cpu)
 +static unsigned long default_scale_cpu_capacity(struct sched_domain *sd, int 
 cpu)
  {
 if ((sd-flags  SD_SHARE_CPUCAPACITY)  (sd-span_weight  1))
 return sd-smt_gain / sd-span_weight;
 @@ -5708,6 +5708,11 @@ unsigned long __weak arch_scale_cpu_capa
 return SCHED_CAPACITY_SCALE;
  }

 +unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
 cpu)
 +{
 +   return default_scale_cpu_capacity(sd, cpu);
 +}
 +
  static unsigned long scale_rt_capacity(int cpu)
  {
 struct rq *rq = cpu_rq(cpu);
 @@ -5747,7 +5752,10 @@ static void update_cpu_capacity(struct s
 unsigned long capacity = SCHED_CAPACITY_SCALE;
 struct sched_group *sdg = sd-groups;

 -   capacity *= arch_scale_cpu_capacity(sd, cpu);
 +   if (sched_feat(ARCH_CAPACITY))
 +   capacity *= arch_scale_cpu_capacity(sd, cpu);
 +   else
 +   capacity *= default_scale_cpu_capacity(sd, cpu);

 capacity = SCHED_CAPACITY_SHIFT;

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-03 Thread Preeti U Murthy 
On 09/01/2014 01:35 PM, Vincent Guittot wrote:
> On 30 August 2014 19:07, Preeti U Murthy  wrote:
>> Hi Vincent,
>>
>> On 08/26/2014 04:36 PM, Vincent Guittot wrote:
>>> capacity_orig is only changed for system with a SMT sched_domain level in 
>>> order
>>
>> I think I had asked this before, but why only capacity_orig? The
>> capacity of a group is also being updated the same way. This patch fixes
>> the capacity of a group to reflect the capacity of the heterogeneous
>> CPUs in it, this capacity being both the full capacity of the group:
>> capacity_orig and the capacity available for the fair tasks. So I feel
>> in the subject as well as the changelog it would suffice to say 'capacity'.
> 
> IIRC, we discussed that point on a former version. The patch changes
> only the behavior of capacity_orig but the behavior of capacity stays
> unchanged as all archs can already set the capacity whereas the
> capacity_orig was configurable only if the SD_SHARE_CPUCAPACITY was
> set in the sched_domain. This is no more the case with this patch
> which creates arch_scale_cpu_capacity for setting capacity_orig.

Yes, sorry I overlooked that.

Reviewed-by: Preeti U. Murthy 

Regards
Preeti U Murthy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-03 Thread Preeti U Murthy 
On 09/01/2014 01:35 PM, Vincent Guittot wrote:
 On 30 August 2014 19:07, Preeti U Murthy pre...@linux.vnet.ibm.com wrote:
 Hi Vincent,

 On 08/26/2014 04:36 PM, Vincent Guittot wrote:
 capacity_orig is only changed for system with a SMT sched_domain level in 
 order

 I think I had asked this before, but why only capacity_orig? The
 capacity of a group is also being updated the same way. This patch fixes
 the capacity of a group to reflect the capacity of the heterogeneous
 CPUs in it, this capacity being both the full capacity of the group:
 capacity_orig and the capacity available for the fair tasks. So I feel
 in the subject as well as the changelog it would suffice to say 'capacity'.
 
 IIRC, we discussed that point on a former version. The patch changes
 only the behavior of capacity_orig but the behavior of capacity stays
 unchanged as all archs can already set the capacity whereas the
 capacity_orig was configurable only if the SD_SHARE_CPUCAPACITY was
 set in the sched_domain. This is no more the case with this patch
 which creates arch_scale_cpu_capacity for setting capacity_orig.

Yes, sorry I overlooked that.

Reviewed-by: Preeti U. Murthy pre...@linux.vnet.ibm.com

Regards
Preeti U Murthy

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-01 Thread Vincent Guittot 
On 30 August 2014 19:07, Preeti U Murthy  wrote:
> Hi Vincent,
>
> On 08/26/2014 04:36 PM, Vincent Guittot wrote:
>> capacity_orig is only changed for system with a SMT sched_domain level in 
>> order
>
> I think I had asked this before, but why only capacity_orig? The
> capacity of a group is also being updated the same way. This patch fixes
> the capacity of a group to reflect the capacity of the heterogeneous
> CPUs in it, this capacity being both the full capacity of the group:
> capacity_orig and the capacity available for the fair tasks. So I feel
> in the subject as well as the changelog it would suffice to say 'capacity'.

IIRC, we discussed that point on a former version. The patch changes
only the behavior of capacity_orig but the behavior of capacity stays
unchanged as all archs can already set the capacity whereas the
capacity_orig was configurable only if the SD_SHARE_CPUCAPACITY was
set in the sched_domain. This is no more the case with this patch
which creates arch_scale_cpu_capacity for setting capacity_orig.

>
>> to reflect the lower capacity of CPUs. Heterogenous system also have to 
>> reflect an
>> original capacity that is different from the default value.
>>
>> Create a more generic function arch_scale_cpu_capacity that can be also used 
>> by
>> non SMT platform to set capacity_orig.
>>
>> The weak behavior of arch_scale_cpu_capacity is the previous SMT one in 
>> order to
>> keep backward compatibility in the use of capacity_orig.
>>
>> arch_scale_smt_capacity and default_scale_smt_capacity have been removed as
>> they were not use elsewhere than in arch_scale_cpu_capacity.
>>
>> Signed-off-by: Vincent Guittot 
>> ---
>>  kernel/sched/fair.c | 25 ++---
>>  1 file changed, 6 insertions(+), 19 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index b85e9f7..8176bda 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -5705,19 +5705,12 @@ unsigned long __weak arch_scale_freq_capacity(struct 
>> sched_domain *sd, int cpu)
>>   return default_scale_capacity(sd, cpu);
>>  }
>>
>> -static unsigned long default_scale_smt_capacity(struct sched_domain *sd, 
>> int cpu)
>> +unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
>> cpu)
>>  {
>> - unsigned long weight = sd->span_weight;
>> - unsigned long smt_gain = sd->smt_gain;
>> + if ((sd->flags & SD_SHARE_CPUCAPACITY) && (sd->span_weight > 1))
>> + return sd->smt_gain / sd->span_weight;
>>
>> - smt_gain /= weight;
>> -
>> - return smt_gain;
>> -}
>> -
>> -unsigned long __weak arch_scale_smt_capacity(struct sched_domain *sd, int 
>> cpu)
>> -{
>> - return default_scale_smt_capacity(sd, cpu);
>> + return SCHED_CAPACITY_SCALE;
>>  }
>>
>>  static unsigned long scale_rt_capacity(int cpu)
>> @@ -5756,18 +5749,12 @@ static unsigned long scale_rt_capacity(int cpu)
>>
>>  static void update_cpu_capacity(struct sched_domain *sd, int cpu)
>>  {
>> - unsigned long weight = sd->span_weight;
>>   unsigned long capacity = SCHED_CAPACITY_SCALE;
>>   struct sched_group *sdg = sd->groups;
>>
>> - if ((sd->flags & SD_SHARE_CPUCAPACITY) && weight > 1) {
>> - if (sched_feat(ARCH_CAPACITY))
>
> Aren't you missing this check above? I understand that it is not
> crucial, but that would also mean removing ARCH_CAPACITY sched_feat
> altogether, wouldn't it?

Peter has proposed to remove all those checks and to keep only the
default behavior because no arch uses arch_scale_smt_capacity.
Then, ARCH_CAPACITY is still used in update_cpu_capacity

Regards,
Vincent
>
> Regards
> Preeti U Murthy
>> - capacity *= arch_scale_smt_capacity(sd, cpu);
>> - else
>> - capacity *= default_scale_smt_capacity(sd, cpu);
>> + capacity *= arch_scale_cpu_capacity(sd, cpu);
>>
>> - capacity >>= SCHED_CAPACITY_SHIFT;
>> - }
>> + capacity >>= SCHED_CAPACITY_SHIFT;
>>
>>   sdg->sgc->capacity_orig = capacity;
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-09-01 Thread Vincent Guittot 
On 30 August 2014 19:07, Preeti U Murthy pre...@linux.vnet.ibm.com wrote:
 Hi Vincent,

 On 08/26/2014 04:36 PM, Vincent Guittot wrote:
 capacity_orig is only changed for system with a SMT sched_domain level in 
 order

 I think I had asked this before, but why only capacity_orig? The
 capacity of a group is also being updated the same way. This patch fixes
 the capacity of a group to reflect the capacity of the heterogeneous
 CPUs in it, this capacity being both the full capacity of the group:
 capacity_orig and the capacity available for the fair tasks. So I feel
 in the subject as well as the changelog it would suffice to say 'capacity'.

IIRC, we discussed that point on a former version. The patch changes
only the behavior of capacity_orig but the behavior of capacity stays
unchanged as all archs can already set the capacity whereas the
capacity_orig was configurable only if the SD_SHARE_CPUCAPACITY was
set in the sched_domain. This is no more the case with this patch
which creates arch_scale_cpu_capacity for setting capacity_orig.


 to reflect the lower capacity of CPUs. Heterogenous system also have to 
 reflect an
 original capacity that is different from the default value.

 Create a more generic function arch_scale_cpu_capacity that can be also used 
 by
 non SMT platform to set capacity_orig.

 The weak behavior of arch_scale_cpu_capacity is the previous SMT one in 
 order to
 keep backward compatibility in the use of capacity_orig.

 arch_scale_smt_capacity and default_scale_smt_capacity have been removed as
 they were not use elsewhere than in arch_scale_cpu_capacity.

 Signed-off-by: Vincent Guittot vincent.guit...@linaro.org
 ---
  kernel/sched/fair.c | 25 ++---
  1 file changed, 6 insertions(+), 19 deletions(-)

 diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
 index b85e9f7..8176bda 100644
 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -5705,19 +5705,12 @@ unsigned long __weak arch_scale_freq_capacity(struct 
 sched_domain *sd, int cpu)
   return default_scale_capacity(sd, cpu);
  }

 -static unsigned long default_scale_smt_capacity(struct sched_domain *sd, 
 int cpu)
 +unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
 cpu)
  {
 - unsigned long weight = sd-span_weight;
 - unsigned long smt_gain = sd-smt_gain;
 + if ((sd-flags  SD_SHARE_CPUCAPACITY)  (sd-span_weight  1))
 + return sd-smt_gain / sd-span_weight;

 - smt_gain /= weight;
 -
 - return smt_gain;
 -}
 -
 -unsigned long __weak arch_scale_smt_capacity(struct sched_domain *sd, int 
 cpu)
 -{
 - return default_scale_smt_capacity(sd, cpu);
 + return SCHED_CAPACITY_SCALE;
  }

  static unsigned long scale_rt_capacity(int cpu)
 @@ -5756,18 +5749,12 @@ static unsigned long scale_rt_capacity(int cpu)

  static void update_cpu_capacity(struct sched_domain *sd, int cpu)
  {
 - unsigned long weight = sd-span_weight;
   unsigned long capacity = SCHED_CAPACITY_SCALE;
   struct sched_group *sdg = sd-groups;

 - if ((sd-flags  SD_SHARE_CPUCAPACITY)  weight  1) {
 - if (sched_feat(ARCH_CAPACITY))

 Aren't you missing this check above? I understand that it is not
 crucial, but that would also mean removing ARCH_CAPACITY sched_feat
 altogether, wouldn't it?

Peter has proposed to remove all those checks and to keep only the
default behavior because no arch uses arch_scale_smt_capacity.
Then, ARCH_CAPACITY is still used in update_cpu_capacity

Regards,
Vincent

 Regards
 Preeti U Murthy
 - capacity *= arch_scale_smt_capacity(sd, cpu);
 - else
 - capacity *= default_scale_smt_capacity(sd, cpu);
 + capacity *= arch_scale_cpu_capacity(sd, cpu);

 - capacity = SCHED_CAPACITY_SHIFT;
 - }
 + capacity = SCHED_CAPACITY_SHIFT;

   sdg-sgc-capacity_orig = capacity;


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-08-30 Thread Preeti U Murthy 
Hi Vincent,

On 08/26/2014 04:36 PM, Vincent Guittot wrote:
> capacity_orig is only changed for system with a SMT sched_domain level in 
> order

I think I had asked this before, but why only capacity_orig? The
capacity of a group is also being updated the same way. This patch fixes
the capacity of a group to reflect the capacity of the heterogeneous
CPUs in it, this capacity being both the full capacity of the group:
capacity_orig and the capacity available for the fair tasks. So I feel
in the subject as well as the changelog it would suffice to say 'capacity'.

> to reflect the lower capacity of CPUs. Heterogenous system also have to 
> reflect an
> original capacity that is different from the default value.
> 
> Create a more generic function arch_scale_cpu_capacity that can be also used 
> by
> non SMT platform to set capacity_orig.
> 
> The weak behavior of arch_scale_cpu_capacity is the previous SMT one in order 
> to
> keep backward compatibility in the use of capacity_orig.
> 
> arch_scale_smt_capacity and default_scale_smt_capacity have been removed as
> they were not use elsewhere than in arch_scale_cpu_capacity.
> 
> Signed-off-by: Vincent Guittot 
> ---
>  kernel/sched/fair.c | 25 ++---
>  1 file changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index b85e9f7..8176bda 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5705,19 +5705,12 @@ unsigned long __weak arch_scale_freq_capacity(struct 
> sched_domain *sd, int cpu)
>   return default_scale_capacity(sd, cpu);
>  }
> 
> -static unsigned long default_scale_smt_capacity(struct sched_domain *sd, int 
> cpu)
> +unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
> cpu)
>  {
> - unsigned long weight = sd->span_weight;
> - unsigned long smt_gain = sd->smt_gain;
> + if ((sd->flags & SD_SHARE_CPUCAPACITY) && (sd->span_weight > 1))
> + return sd->smt_gain / sd->span_weight;
> 
> - smt_gain /= weight;
> -
> - return smt_gain;
> -}
> -
> -unsigned long __weak arch_scale_smt_capacity(struct sched_domain *sd, int 
> cpu)
> -{
> - return default_scale_smt_capacity(sd, cpu);
> + return SCHED_CAPACITY_SCALE;
>  }
> 
>  static unsigned long scale_rt_capacity(int cpu)
> @@ -5756,18 +5749,12 @@ static unsigned long scale_rt_capacity(int cpu)
> 
>  static void update_cpu_capacity(struct sched_domain *sd, int cpu)
>  {
> - unsigned long weight = sd->span_weight;
>   unsigned long capacity = SCHED_CAPACITY_SCALE;
>   struct sched_group *sdg = sd->groups;
> 
> - if ((sd->flags & SD_SHARE_CPUCAPACITY) && weight > 1) {
> - if (sched_feat(ARCH_CAPACITY))

Aren't you missing this check above? I understand that it is not
crucial, but that would also mean removing ARCH_CAPACITY sched_feat
altogether, wouldn't it?

Regards
Preeti U Murthy
> - capacity *= arch_scale_smt_capacity(sd, cpu);
> - else
> - capacity *= default_scale_smt_capacity(sd, cpu);
> + capacity *= arch_scale_cpu_capacity(sd, cpu);
> 
> - capacity >>= SCHED_CAPACITY_SHIFT;
> - }
> + capacity >>= SCHED_CAPACITY_SHIFT;
> 
>   sdg->sgc->capacity_orig = capacity;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-08-30 Thread Preeti U Murthy 
Hi Vincent,

On 08/26/2014 04:36 PM, Vincent Guittot wrote:
 capacity_orig is only changed for system with a SMT sched_domain level in 
 order

I think I had asked this before, but why only capacity_orig? The
capacity of a group is also being updated the same way. This patch fixes
the capacity of a group to reflect the capacity of the heterogeneous
CPUs in it, this capacity being both the full capacity of the group:
capacity_orig and the capacity available for the fair tasks. So I feel
in the subject as well as the changelog it would suffice to say 'capacity'.

 to reflect the lower capacity of CPUs. Heterogenous system also have to 
 reflect an
 original capacity that is different from the default value.
 
 Create a more generic function arch_scale_cpu_capacity that can be also used 
 by
 non SMT platform to set capacity_orig.
 
 The weak behavior of arch_scale_cpu_capacity is the previous SMT one in order 
 to
 keep backward compatibility in the use of capacity_orig.
 
 arch_scale_smt_capacity and default_scale_smt_capacity have been removed as
 they were not use elsewhere than in arch_scale_cpu_capacity.
 
 Signed-off-by: Vincent Guittot vincent.guit...@linaro.org
 ---
  kernel/sched/fair.c | 25 ++---
  1 file changed, 6 insertions(+), 19 deletions(-)
 
 diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
 index b85e9f7..8176bda 100644
 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -5705,19 +5705,12 @@ unsigned long __weak arch_scale_freq_capacity(struct 
 sched_domain *sd, int cpu)
   return default_scale_capacity(sd, cpu);
  }
 
 -static unsigned long default_scale_smt_capacity(struct sched_domain *sd, int 
 cpu)
 +unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int 
 cpu)
  {
 - unsigned long weight = sd-span_weight;
 - unsigned long smt_gain = sd-smt_gain;
 + if ((sd-flags  SD_SHARE_CPUCAPACITY)  (sd-span_weight  1))
 + return sd-smt_gain / sd-span_weight;
 
 - smt_gain /= weight;
 -
 - return smt_gain;
 -}
 -
 -unsigned long __weak arch_scale_smt_capacity(struct sched_domain *sd, int 
 cpu)
 -{
 - return default_scale_smt_capacity(sd, cpu);
 + return SCHED_CAPACITY_SCALE;
  }
 
  static unsigned long scale_rt_capacity(int cpu)
 @@ -5756,18 +5749,12 @@ static unsigned long scale_rt_capacity(int cpu)
 
  static void update_cpu_capacity(struct sched_domain *sd, int cpu)
  {
 - unsigned long weight = sd-span_weight;
   unsigned long capacity = SCHED_CAPACITY_SCALE;
   struct sched_group *sdg = sd-groups;
 
 - if ((sd-flags  SD_SHARE_CPUCAPACITY)  weight  1) {
 - if (sched_feat(ARCH_CAPACITY))

Aren't you missing this check above? I understand that it is not
crucial, but that would also mean removing ARCH_CAPACITY sched_feat
altogether, wouldn't it?

Regards
Preeti U Murthy
 - capacity *= arch_scale_smt_capacity(sd, cpu);
 - else
 - capacity *= default_scale_smt_capacity(sd, cpu);
 + capacity *= arch_scale_cpu_capacity(sd, cpu);
 
 - capacity = SCHED_CAPACITY_SHIFT;
 - }
 + capacity = SCHED_CAPACITY_SHIFT;
 
   sdg-sgc-capacity_orig = capacity;
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-08-27 Thread Kamalesh Babulal 
* Vincent Guittot  [2014-08-26 13:06:47]:

> capacity_orig is only changed for system with a SMT sched_domain level in 
> order
> to reflect the lower capacity of CPUs. Heterogenous system also have to 
> reflect an
> original capacity that is different from the default value.
> 
> Create a more generic function arch_scale_cpu_capacity that can be also used 
> by
> non SMT platform to set capacity_orig.
> 
> The weak behavior of arch_scale_cpu_capacity is the previous SMT one in order 
> to
> keep backward compatibility in the use of capacity_orig.
> 
> arch_scale_smt_capacity and default_scale_smt_capacity have been removed as
> they were not use elsewhere than in arch_scale_cpu_capacity.
> 
> Signed-off-by: Vincent Guittot 

Reviewed-by: Kamalesh Babulal 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-08-27 Thread Kamalesh Babulal 
* Vincent Guittot vincent.guit...@linaro.org [2014-08-26 13:06:47]:

 capacity_orig is only changed for system with a SMT sched_domain level in 
 order
 to reflect the lower capacity of CPUs. Heterogenous system also have to 
 reflect an
 original capacity that is different from the default value.
 
 Create a more generic function arch_scale_cpu_capacity that can be also used 
 by
 non SMT platform to set capacity_orig.
 
 The weak behavior of arch_scale_cpu_capacity is the previous SMT one in order 
 to
 keep backward compatibility in the use of capacity_orig.
 
 arch_scale_smt_capacity and default_scale_smt_capacity have been removed as
 they were not use elsewhere than in arch_scale_cpu_capacity.
 
 Signed-off-by: Vincent Guittot vincent.guit...@linaro.org

Reviewed-by: Kamalesh Babulal kamal...@linux.vnet.ibm.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-08-26 Thread Vincent Guittot 
capacity_orig is only changed for system with a SMT sched_domain level in order
to reflect the lower capacity of CPUs. Heterogenous system also have to reflect 
an
original capacity that is different from the default value.

Create a more generic function arch_scale_cpu_capacity that can be also used by
non SMT platform to set capacity_orig.

The weak behavior of arch_scale_cpu_capacity is the previous SMT one in order to
keep backward compatibility in the use of capacity_orig.

arch_scale_smt_capacity and default_scale_smt_capacity have been removed as
they were not use elsewhere than in arch_scale_cpu_capacity.

Signed-off-by: Vincent Guittot 
---
 kernel/sched/fair.c | 25 ++---
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b85e9f7..8176bda 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5705,19 +5705,12 @@ unsigned long __weak arch_scale_freq_capacity(struct 
sched_domain *sd, int cpu)
return default_scale_capacity(sd, cpu);
 }
 
-static unsigned long default_scale_smt_capacity(struct sched_domain *sd, int 
cpu)
+unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
 {
-   unsigned long weight = sd->span_weight;
-   unsigned long smt_gain = sd->smt_gain;
+   if ((sd->flags & SD_SHARE_CPUCAPACITY) && (sd->span_weight > 1))
+   return sd->smt_gain / sd->span_weight;
 
-   smt_gain /= weight;
-
-   return smt_gain;
-}
-
-unsigned long __weak arch_scale_smt_capacity(struct sched_domain *sd, int cpu)
-{
-   return default_scale_smt_capacity(sd, cpu);
+   return SCHED_CAPACITY_SCALE;
 }
 
 static unsigned long scale_rt_capacity(int cpu)
@@ -5756,18 +5749,12 @@ static unsigned long scale_rt_capacity(int cpu)
 
 static void update_cpu_capacity(struct sched_domain *sd, int cpu)
 {
-   unsigned long weight = sd->span_weight;
unsigned long capacity = SCHED_CAPACITY_SCALE;
struct sched_group *sdg = sd->groups;
 
-   if ((sd->flags & SD_SHARE_CPUCAPACITY) && weight > 1) {
-   if (sched_feat(ARCH_CAPACITY))
-   capacity *= arch_scale_smt_capacity(sd, cpu);
-   else
-   capacity *= default_scale_smt_capacity(sd, cpu);
+   capacity *= arch_scale_cpu_capacity(sd, cpu);
 
-   capacity >>= SCHED_CAPACITY_SHIFT;
-   }
+   capacity >>= SCHED_CAPACITY_SHIFT;
 
sdg->sgc->capacity_orig = capacity;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v5 04/12] sched: Allow all archs to set the capacity_orig

2014-08-26 Thread Vincent Guittot 
capacity_orig is only changed for system with a SMT sched_domain level in order
to reflect the lower capacity of CPUs. Heterogenous system also have to reflect 
an
original capacity that is different from the default value.

Create a more generic function arch_scale_cpu_capacity that can be also used by
non SMT platform to set capacity_orig.

The weak behavior of arch_scale_cpu_capacity is the previous SMT one in order to
keep backward compatibility in the use of capacity_orig.

arch_scale_smt_capacity and default_scale_smt_capacity have been removed as
they were not use elsewhere than in arch_scale_cpu_capacity.

Signed-off-by: Vincent Guittot vincent.guit...@linaro.org
---
 kernel/sched/fair.c | 25 ++---
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b85e9f7..8176bda 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5705,19 +5705,12 @@ unsigned long __weak arch_scale_freq_capacity(struct 
sched_domain *sd, int cpu)
return default_scale_capacity(sd, cpu);
 }
 
-static unsigned long default_scale_smt_capacity(struct sched_domain *sd, int 
cpu)
+unsigned long __weak arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
 {
-   unsigned long weight = sd-span_weight;
-   unsigned long smt_gain = sd-smt_gain;
+   if ((sd-flags  SD_SHARE_CPUCAPACITY)  (sd-span_weight  1))
+   return sd-smt_gain / sd-span_weight;
 
-   smt_gain /= weight;
-
-   return smt_gain;
-}
-
-unsigned long __weak arch_scale_smt_capacity(struct sched_domain *sd, int cpu)
-{
-   return default_scale_smt_capacity(sd, cpu);
+   return SCHED_CAPACITY_SCALE;
 }
 
 static unsigned long scale_rt_capacity(int cpu)
@@ -5756,18 +5749,12 @@ static unsigned long scale_rt_capacity(int cpu)
 
 static void update_cpu_capacity(struct sched_domain *sd, int cpu)
 {
-   unsigned long weight = sd-span_weight;
unsigned long capacity = SCHED_CAPACITY_SCALE;
struct sched_group *sdg = sd-groups;
 
-   if ((sd-flags  SD_SHARE_CPUCAPACITY)  weight  1) {
-   if (sched_feat(ARCH_CAPACITY))
-   capacity *= arch_scale_smt_capacity(sd, cpu);
-   else
-   capacity *= default_scale_smt_capacity(sd, cpu);
+   capacity *= arch_scale_cpu_capacity(sd, cpu);
 
-   capacity = SCHED_CAPACITY_SHIFT;
-   }
+   capacity = SCHED_CAPACITY_SHIFT;
 
sdg-sgc-capacity_orig = capacity;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/