[PATCH] sched: write better comments for weight calculations

[PATCH] sched: write better comments for weight calculations

[Joel Fernandes]

This patch rewrites comments related task priorities and CPU usage
along with an example to show how it works.

Cc: Juri Lelli <Juri.Lelli@arm.com>
Cc: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Joel Fernandes <joelaf@google.com>
---
 kernel/sched/core.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c56fb57f2991..2175bf663f3d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8823,16 +8823,27 @@ void dump_cpu_task(int cpu)
 }
 
 /*
- * Nice levels are multiplicative, with a gentle 10% change for every
- * nice level changed. I.e. when a CPU-bound task goes from nice 0 to
- * nice 1, it will get ~10% less CPU time than another CPU-bound task
- * that remained on nice 0.
+ * Nice levels are multiplicative, with a gentle 10% relative change
+ * for every nice level changed. I.e. if there were 2 CPU-bound tasks
+ * of equal nice value and one of them goes from a nice level of 0 to 1
+ * then the task at nice level 1 will get ~5% less CPU time than before
+ * the change and the task that remained at nice level 0 will get ~5%
+ * more CPU time.
  *
  * The "10% effect" is relative and cumulative: from _any_ nice level,
- * if you go up 1 level, it's -10% CPU usage, if you go down 1 level
- * it's +10% CPU usage. (to achieve that we use a multiplier of 1.25.
- * If a task goes up by ~10% and another task goes down by ~10% then
- * the relative distance between them is ~25%.)
+ * if you go up 1 level, it's -10% relative CPU usage, if you go down
+ * by 1 level it's +10% CPU usage. To achieve that, we use a multiplier
+ * of 1.25. If a task goes up by ~5% and another task goes down by ~5%
+ * then the relative distance between their weights is ~25% as shown
+ * in the following example:
+ *
+ * Consider 2 tasks T1 and T2 which are scheduled within a sched_period
+ * of 10ms. Say T1 has a nice value 0 and T2 has a nice value 1,
+ * then their corresponding weights are 1024 for T1 and 820 for T2.
+ *
+ * The relative delta between their weights is ~25% (1.25 * 820 ~= 1024)
+ * T1's CPU slice = (1024 / (820 + 1024)) * 10 ~= 5.5ms  (55% usage)
+ * T2's CPU slice = (820  / (820 + 1024)) * 10 ~= 4.5ms  (45% usage)
  */
 const int sched_prio_to_weight[40] = {
  /* -20 */     88761,     71755,     56483,     46273,     36291,
-- 
2.12.0.246.ga2ecc84866-goog

Re: [PATCH] sched: write better comments for weight calculations

[Patrick Bellasi]

Few comments inline, otherwise LGTM.

Cheers Patrick

On 10-Mar 12:47, Joel Fernandes wrote:
> This patch rewrites comments related task priorities and CPU usage
> along with an example to show how it works.
> 
> Cc: Juri Lelli <Juri.Lelli@arm.com>
> Cc: Patrick Bellasi <patrick.bellasi@arm.com>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Joel Fernandes <joelaf@google.com>
> ---
>  kernel/sched/core.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index c56fb57f2991..2175bf663f3d 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8823,16 +8823,27 @@ void dump_cpu_task(int cpu)
>  }
>  
>  /*
> - * Nice levels are multiplicative, with a gentle 10% change for every
> - * nice level changed. I.e. when a CPU-bound task goes from nice 0 to
> - * nice 1, it will get ~10% less CPU time than another CPU-bound task
> - * that remained on nice 0.
> + * Nice levels are multiplicative, with a gentle 10% relative change
> + * for every nice level changed. I.e. if there were 2 CPU-bound tasks
> + * of equal nice value and one of them goes from a nice level of 0 to 1
> + * then the task at nice level 1 will get ~5% less CPU time than before
> + * the change and the task that remained at nice level 0 will get ~5%
> + * more CPU time.
>   *
>   * The "10% effect" is relative and cumulative: from _any_ nice level,
> - * if you go up 1 level, it's -10% CPU usage, if you go down 1 level
> - * it's +10% CPU usage. (to achieve that we use a multiplier of 1.25.
> - * If a task goes up by ~10% and another task goes down by ~10% then
> - * the relative distance between them is ~25%.)
> + * if you go up 1 level, it's -10% relative CPU usage, if you go down
> + * by 1 level it's +10% CPU usage.
                          ^
                       relative
> + * To achieve that, we use a multiplier of 1.25.


The following sentence:

> + * If a task goes up by ~5% and another task goes down by ~5%
> + * then the relative distance between their weights is ~25% as shown
> + * in the following example:

is still confusing to me, mainly because we are mixing the "shares
percentage" with the CPU usage percentage.

What about this:

      If two tasks have a 25% relative distance between their weights
      then they will get a 10% difference in CPU usage as shown in the
      following example.

> + *
> + * Consider 2 tasks T1 and T2 which are scheduled within a sched_period
> + * of 10ms. Say T1 has a nice value 0 and T2 has a nice value 1,
> + * then their corresponding weights are 1024 for T1 and 820 for T2.
> + *
> + * The relative delta between their weights is ~25% (1.25 * 820 ~= 1024)
> + * T1's CPU slice = (1024 / (820 + 1024)) * 10 ~= 5.5ms  (55% usage)
                                                 ^
                                                 ms
> + * T2's CPU slice = (820  / (820 + 1024)) * 10 ~= 4.5ms  (45% usage)
                                                 ^
                                                 ms
>   */
>  const int sched_prio_to_weight[40] = {
>   /* -20 */     88761,     71755,     56483,     46273,     36291,
> -- 
> 2.12.0.246.ga2ecc84866-goog
> 

-- 
#include <best/regards.h>

Patrick Bellasi

Re: [PATCH] sched: write better comments for weight calculations

[Joel Fernandes]

On Wed, Mar 15, 2017 at 5:04 AM, Patrick Bellasi
<patrick.bellasi@arm.com> wrote:
> Few comments inline, otherwise LGTM.

Ok, I'll take that as an Acked-by with the following comment addressed
if that's Ok with you.

>
> On 10-Mar 12:47, Joel Fernandes wrote:
>> This patch rewrites comments related task priorities and CPU usage
>> along with an example to show how it works.
>>
>> Cc: Juri Lelli <Juri.Lelli@arm.com>
>> Cc: Patrick Bellasi <patrick.bellasi@arm.com>
>> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Signed-off-by: Joel Fernandes <joelaf@google.com>
>> ---
>>  kernel/sched/core.c | 27 +++++++++++++++++++--------
>>  1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index c56fb57f2991..2175bf663f3d 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -8823,16 +8823,27 @@ void dump_cpu_task(int cpu)
>>  }
>>
>>  /*
>> - * Nice levels are multiplicative, with a gentle 10% change for every
>> - * nice level changed. I.e. when a CPU-bound task goes from nice 0 to
>> - * nice 1, it will get ~10% less CPU time than another CPU-bound task
>> - * that remained on nice 0.
>> + * Nice levels are multiplicative, with a gentle 10% relative change
>> + * for every nice level changed. I.e. if there were 2 CPU-bound tasks
>> + * of equal nice value and one of them goes from a nice level of 0 to 1
>> + * then the task at nice level 1 will get ~5% less CPU time than before
>> + * the change and the task that remained at nice level 0 will get ~5%
>> + * more CPU time.
>>   *
>>   * The "10% effect" is relative and cumulative: from _any_ nice level,
>> - * if you go up 1 level, it's -10% CPU usage, if you go down 1 level
>> - * it's +10% CPU usage. (to achieve that we use a multiplier of 1.25.
>> - * If a task goes up by ~10% and another task goes down by ~10% then
>> - * the relative distance between them is ~25%.)
>> + * if you go up 1 level, it's -10% relative CPU usage, if you go down
>> + * by 1 level it's +10% CPU usage.
>                           ^
>                        relative
>> + * To achieve that, we use a multiplier of 1.25.
>
>
> The following sentence:
>
>> + * If a task goes up by ~5% and another task goes down by ~5%
>> + * then the relative distance between their weights is ~25% as shown
>> + * in the following example:
>
> is still confusing to me, mainly because we are mixing the "shares
> percentage" with the CPU usage percentage.
>
> What about this:
>
>       If two tasks have a 25% relative distance between their weights
>       then they will get a 10% difference in CPU usage as shown in the
>       following example.

I agree your statement is clearer and I will use it in the repost.

J.

Re: [PATCH] sched: write better comments for weight calculations

[Patrick Bellasi]

On 15-Mar 05:35, Joel Fernandes wrote:
> On Wed, Mar 15, 2017 at 5:04 AM, Patrick Bellasi
> <patrick.bellasi@arm.com> wrote:
> > Few comments inline, otherwise LGTM.
> 
> Ok, I'll take that as an Acked-by with the following comment addressed
> if that's Ok with you.

Well, I cannot really ACK anything... you should defenitively ask
someone else in CC for such a tag ;-)

FWIW, if you like, you can add instead a Reviewed-by tag.

Cheers Patrick

> >
> > On 10-Mar 12:47, Joel Fernandes wrote:
> >> This patch rewrites comments related task priorities and CPU usage
> >> along with an example to show how it works.
> >>
> >> Cc: Juri Lelli <Juri.Lelli@arm.com>
> >> Cc: Patrick Bellasi <patrick.bellasi@arm.com>
> >> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> >> Cc: Peter Zijlstra <peterz@infradead.org>
> >> Cc: Ingo Molnar <mingo@redhat.com>
> >> Signed-off-by: Joel Fernandes <joelaf@google.com>
> >> ---
> >>  kernel/sched/core.c | 27 +++++++++++++++++++--------
> >>  1 file changed, 19 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> >> index c56fb57f2991..2175bf663f3d 100644
> >> --- a/kernel/sched/core.c
> >> +++ b/kernel/sched/core.c
> >> @@ -8823,16 +8823,27 @@ void dump_cpu_task(int cpu)
> >>  }
> >>
> >>  /*
> >> - * Nice levels are multiplicative, with a gentle 10% change for every
> >> - * nice level changed. I.e. when a CPU-bound task goes from nice 0 to
> >> - * nice 1, it will get ~10% less CPU time than another CPU-bound task
> >> - * that remained on nice 0.
> >> + * Nice levels are multiplicative, with a gentle 10% relative change
> >> + * for every nice level changed. I.e. if there were 2 CPU-bound tasks
> >> + * of equal nice value and one of them goes from a nice level of 0 to 1
> >> + * then the task at nice level 1 will get ~5% less CPU time than before
> >> + * the change and the task that remained at nice level 0 will get ~5%
> >> + * more CPU time.
> >>   *
> >>   * The "10% effect" is relative and cumulative: from _any_ nice level,
> >> - * if you go up 1 level, it's -10% CPU usage, if you go down 1 level
> >> - * it's +10% CPU usage. (to achieve that we use a multiplier of 1.25.
> >> - * If a task goes up by ~10% and another task goes down by ~10% then
> >> - * the relative distance between them is ~25%.)
> >> + * if you go up 1 level, it's -10% relative CPU usage, if you go down
> >> + * by 1 level it's +10% CPU usage.
> >                           ^
> >                        relative
> >> + * To achieve that, we use a multiplier of 1.25.
> >
> >
> > The following sentence:
> >
> >> + * If a task goes up by ~5% and another task goes down by ~5%
> >> + * then the relative distance between their weights is ~25% as shown
> >> + * in the following example:
> >
> > is still confusing to me, mainly because we are mixing the "shares
> > percentage" with the CPU usage percentage.
> >
> > What about this:
> >
> >       If two tasks have a 25% relative distance between their weights
> >       then they will get a 10% difference in CPU usage as shown in the
> >       following example.
> 
> I agree your statement is clearer and I will use it in the repost.
> 
> J.

-- 
#include <best/regards.h>

Patrick Bellasi

Re: [PATCH] sched: write better comments for weight calculations

[Joel Fernandes]

On Wed, Mar 15, 2017 at 7:43 AM, Patrick Bellasi
<patrick.bellasi@arm.com> wrote:
> On 15-Mar 05:35, Joel Fernandes wrote:
>> On Wed, Mar 15, 2017 at 5:04 AM, Patrick Bellasi
>> <patrick.bellasi@arm.com> wrote:
>> > Few comments inline, otherwise LGTM.
>>
>> Ok, I'll take that as an Acked-by with the following comment addressed
>> if that's Ok with you.
>
> Well, I cannot really ACK anything... you should defenitively ask
> someone else in CC for such a tag ;-)
>
> FWIW, if you like, you can add instead a Reviewed-by tag.

Oh, ok. That works :-) Thanks!

Re: [PATCH] sched: write better comments for weight calculations

[Peter Zijlstra]

On Fri, Mar 10, 2017 at 12:47:43PM -0800, Joel Fernandes wrote:
> This patch rewrites comments related task priorities and CPU usage
> along with an example to show how it works.

To what purpose? Bigger word count?

Re: [PATCH] sched: write better comments for weight calculations

[Joel Fernandes]

On Wed, Mar 22, 2017 at 7:18 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 10, 2017 at 12:47:43PM -0800, Joel Fernandes wrote:
>> This patch rewrites comments related task priorities and CPU usage
>> along with an example to show how it works.
>
> To what purpose? Bigger word count?

The intention is to improve the comments to make it more
understandable (the weight calculations, factor of 1.25 etc).

On reading through the comments the first time, I felt they could be
improved. Is your concern more about the addition of an example
increasing the word-count? Perhaps you'd rather this be added to
Documentation/ instead?

Regards,
Joel

Re: [PATCH] sched: write better comments for weight calculations

[Peter Zijlstra]

On Wed, Mar 22, 2017 at 09:25:02AM -0700, Joel Fernandes wrote:
> On Wed, Mar 22, 2017 at 7:18 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Fri, Mar 10, 2017 at 12:47:43PM -0800, Joel Fernandes wrote:
> >> This patch rewrites comments related task priorities and CPU usage
> >> along with an example to show how it works.
> >
> > To what purpose? Bigger word count?
> 
> The intention is to improve the comments to make it more
> understandable (the weight calculations, factor of 1.25 etc).
> 
> On reading through the comments the first time, I felt they could be
> improved. Is your concern more about the addition of an example
> increasing the word-count? Perhaps you'd rather this be added to
> Documentation/ instead?

It might just be verbiage; I sometimes have trouble condensing text.
That is; some people need repetition, I get stuck trying to figure out
if its saying the same or not.

In any case; if you want to clarify where the 1.25 comes from, maybe do
an abstract example, instead of an explicit one?

 -10% = .9, +10% = 1.1 -> 1.1/.9 = 1.(2) ~ 1.25

Or, starting with the weight thing:

.45 = wa / (wa+wb) -> .45 (wa+wb) = wa ->
                      .45wa + .45wb = wa ->
		      .45wb = .55wa ->
		      wb/wa = .55/.45 = 1.(2) ~ 1.25

That's actually simpler to follow no?

Now IIRC the whole thing is backwards anyway, we started with 1.25 and
got the ~10% from there.

Re: [PATCH] sched: write better comments for weight calculations

[Joel Fernandes]

On Wed, Mar 22, 2017 at 10:55 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Mar 22, 2017 at 09:25:02AM -0700, Joel Fernandes wrote:
>> On Wed, Mar 22, 2017 at 7:18 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Fri, Mar 10, 2017 at 12:47:43PM -0800, Joel Fernandes wrote:
>> >> This patch rewrites comments related task priorities and CPU usage
>> >> along with an example to show how it works.
>> >
>> > To what purpose? Bigger word count?
>>
>> The intention is to improve the comments to make it more
>> understandable (the weight calculations, factor of 1.25 etc).
>>
>> On reading through the comments the first time, I felt they could be
>> improved. Is your concern more about the addition of an example
>> increasing the word-count? Perhaps you'd rather this be added to
>> Documentation/ instead?
>
> It might just be verbiage; I sometimes have trouble condensing text.
> That is; some people need repetition, I get stuck trying to figure out
> if its saying the same or not.
>
> In any case; if you want to clarify where the 1.25 comes from, maybe do
> an abstract example, instead of an explicit one?
>
>  -10% = .9, +10% = 1.1 -> 1.1/.9 = 1.(2) ~ 1.25
>
> Or, starting with the weight thing:
>
> .45 = wa / (wa+wb) -> .45 (wa+wb) = wa ->
>                       .45wa + .45wb = wa ->
>                       .45wb = .55wa ->
>                       wb/wa = .55/.45 = 1.(2) ~ 1.25
>
> That's actually simpler to follow no?
>
> Now IIRC the whole thing is backwards anyway, we started with 1.25 and
> got the ~10% from there.

Yes we could condense it further and explain it without using an
explicit example. I will work on a better patch along these lines.

Thanks,
Joel