Re: [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig
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
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
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
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
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
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
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
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
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
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
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
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
* 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
* 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/