diff mbox

[RFCv6,09/10] sched: deadline: use deadline bandwidth in scale_rt_capacity

Message ID 1449641971-20827-10-git-send-email-smuckle@linaro.org
State New
Headers show

Commit Message

Steve Muckle Dec. 9, 2015, 6:19 a.m. UTC
From: Vincent Guittot <vincent.guittot@linaro.org>


Instead of monitoring the exec time of deadline tasks to evaluate the
CPU capacity consumed by deadline scheduler class, we can directly
calculate it thanks to the sum of utilization of deadline tasks on the
CPU.  We can remove deadline tasks from rt_avg metric and directly use
the average bandwidth of deadline scheduler in scale_rt_capacity.

Based in part on a similar patch from Luca Abeni <luca.abeni@unitn.it>.

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>

Signed-off-by: Steve Muckle <smuckle@linaro.org>

---
 kernel/sched/deadline.c | 33 +++++++++++++++++++++++++++++++--
 kernel/sched/fair.c     |  8 ++++++++
 kernel/sched/sched.h    |  2 ++
 3 files changed, 41 insertions(+), 2 deletions(-)

-- 
2.4.10

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Vincent Guittot Dec. 9, 2015, 8:50 a.m. UTC | #1
adding Lucas

On 9 December 2015 at 07:19, Steve Muckle <steve.muckle@linaro.org> wrote:
> From: Vincent Guittot <vincent.guittot@linaro.org>

>

> Instead of monitoring the exec time of deadline tasks to evaluate the

> CPU capacity consumed by deadline scheduler class, we can directly

> calculate it thanks to the sum of utilization of deadline tasks on the

> CPU.  We can remove deadline tasks from rt_avg metric and directly use

> the average bandwidth of deadline scheduler in scale_rt_capacity.

>

> Based in part on a similar patch from Luca Abeni <luca.abeni@unitn.it>.

>

> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>

> Signed-off-by: Steve Muckle <smuckle@linaro.org>

> ---

>  kernel/sched/deadline.c | 33 +++++++++++++++++++++++++++++++--

>  kernel/sched/fair.c     |  8 ++++++++

>  kernel/sched/sched.h    |  2 ++

>  3 files changed, 41 insertions(+), 2 deletions(-)

>

> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c

> index 8b0a15e..9d9eb50 100644

> --- a/kernel/sched/deadline.c

> +++ b/kernel/sched/deadline.c

> @@ -43,6 +43,24 @@ static inline int on_dl_rq(struct sched_dl_entity *dl_se)

>         return !RB_EMPTY_NODE(&dl_se->rb_node);

>  }

>

> +static void add_average_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)

> +{

> +       u64 se_bw = dl_se->dl_bw;

> +

> +       dl_rq->avg_bw += se_bw;

> +}

> +

> +static void clear_average_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)

> +{

> +       u64 se_bw = dl_se->dl_bw;

> +

> +       dl_rq->avg_bw -= se_bw;

> +       if (dl_rq->avg_bw < 0) {

> +               WARN_ON(1);

> +               dl_rq->avg_bw = 0;

> +       }

> +}

> +

>  static inline int is_leftmost(struct task_struct *p, struct dl_rq *dl_rq)

>  {

>         struct sched_dl_entity *dl_se = &p->dl;

> @@ -494,6 +512,9 @@ static void update_dl_entity(struct sched_dl_entity *dl_se,

>         struct dl_rq *dl_rq = dl_rq_of_se(dl_se);

>         struct rq *rq = rq_of_dl_rq(dl_rq);

>

> +       if (dl_se->dl_new)

> +               add_average_bw(dl_se, dl_rq);

> +

>         /*

>          * The arrival of a new instance needs special treatment, i.e.,

>          * the actual scheduling parameters have to be "renewed".

> @@ -741,8 +762,6 @@ static void update_curr_dl(struct rq *rq)

>         curr->se.exec_start = rq_clock_task(rq);

>         cpuacct_charge(curr, delta_exec);

>

> -       sched_rt_avg_update(rq, delta_exec);

> -

>         dl_se->runtime -= dl_se->dl_yielded ? 0 : delta_exec;

>         if (dl_runtime_exceeded(dl_se)) {

>                 dl_se->dl_throttled = 1;

> @@ -1241,6 +1260,8 @@ static void task_fork_dl(struct task_struct *p)

>  static void task_dead_dl(struct task_struct *p)

>  {

>         struct dl_bw *dl_b = dl_bw_of(task_cpu(p));

> +       struct dl_rq *dl_rq = dl_rq_of_se(&p->dl);

> +       struct rq *rq = rq_of_dl_rq(dl_rq);

>

>         /*

>          * Since we are TASK_DEAD we won't slip out of the domain!

> @@ -1249,6 +1270,8 @@ static void task_dead_dl(struct task_struct *p)

>         /* XXX we should retain the bw until 0-lag */

>         dl_b->total_bw -= p->dl.dl_bw;

>         raw_spin_unlock_irq(&dl_b->lock);

> +

> +       clear_average_bw(&p->dl, &rq->dl);

>  }

>

>  static void set_curr_task_dl(struct rq *rq)

> @@ -1556,7 +1579,9 @@ retry:

>         }

>

>         deactivate_task(rq, next_task, 0);

> +       clear_average_bw(&next_task->dl, &rq->dl);

>         set_task_cpu(next_task, later_rq->cpu);

> +       add_average_bw(&next_task->dl, &later_rq->dl);

>         activate_task(later_rq, next_task, 0);

>         ret = 1;

>

> @@ -1644,7 +1669,9 @@ static void pull_dl_task(struct rq *this_rq)

>                         resched = true;

>

>                         deactivate_task(src_rq, p, 0);

> +                       clear_average_bw(&p->dl, &src_rq->dl);

>                         set_task_cpu(p, this_cpu);

> +                       add_average_bw(&p->dl, &this_rq->dl);

>                         activate_task(this_rq, p, 0);

>                         dmin = p->dl.deadline;

>

> @@ -1750,6 +1777,8 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)

>         if (!start_dl_timer(p))

>                 __dl_clear_params(p);

>

> +       clear_average_bw(&p->dl, &rq->dl);

> +

>         /*

>          * Since this might be the only -deadline task on the rq,

>          * this is the right place to try to pull some other one

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

> index 4c49f76..ce05f61 100644

> --- a/kernel/sched/fair.c

> +++ b/kernel/sched/fair.c

> @@ -6203,6 +6203,14 @@ static unsigned long scale_rt_capacity(int cpu)

>

>         used = div_u64(avg, total);

>

> +       /*

> +        * deadline bandwidth is defined at system level so we must

> +        * weight this bandwidth with the max capacity of the system.

> +        * As a reminder, avg_bw is 20bits width and

> +        * scale_cpu_capacity is 10 bits width

> +        */

> +       used += div_u64(rq->dl.avg_bw, arch_scale_cpu_capacity(NULL, cpu));

> +

>         if (likely(used < SCHED_CAPACITY_SCALE))

>                 return SCHED_CAPACITY_SCALE - used;

>

> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h

> index 08858d1..e44c6be 100644

> --- a/kernel/sched/sched.h

> +++ b/kernel/sched/sched.h

> @@ -519,6 +519,8 @@ struct dl_rq {

>  #else

>         struct dl_bw dl_bw;

>  #endif

> +       /* This is the "average utilization" for this runqueue */

> +       s64 avg_bw;

>  };

>

>  #ifdef CONFIG_SMP

> --

> 2.4.10

>

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Juri Lelli Dec. 14, 2015, 4:07 p.m. UTC | #2
On 14/12/15 16:56, Vincent Guittot wrote:
> On 14 December 2015 at 16:17, Peter Zijlstra <peterz@infradead.org> wrote:

> > On Tue, Dec 08, 2015 at 10:19:30PM -0800, Steve Muckle wrote:

> >> From: Vincent Guittot <vincent.guittot@linaro.org>

> >

> >> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c

> >> index 8b0a15e..9d9eb50 100644

> >> --- a/kernel/sched/deadline.c

> >> +++ b/kernel/sched/deadline.c

> >> @@ -43,6 +43,24 @@ static inline int on_dl_rq(struct sched_dl_entity *dl_se)

> >>       return !RB_EMPTY_NODE(&dl_se->rb_node);

> >>  }

> >>

> >> +static void add_average_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)

> >> +{

> >> +     u64 se_bw = dl_se->dl_bw;

> >> +

> >> +     dl_rq->avg_bw += se_bw;

> >> +}

> >> +

> >> +static void clear_average_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)

> >> +{

> >> +     u64 se_bw = dl_se->dl_bw;

> >> +

> >> +     dl_rq->avg_bw -= se_bw;

> >> +     if (dl_rq->avg_bw < 0) {

> >> +             WARN_ON(1);

> >> +             dl_rq->avg_bw = 0;

> >> +     }

> >> +}

> >

> >

> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

> >> index 4c49f76..ce05f61 100644

> >> --- a/kernel/sched/fair.c

> >> +++ b/kernel/sched/fair.c

> >> @@ -6203,6 +6203,14 @@ static unsigned long scale_rt_capacity(int cpu)

> >>

> >>       used = div_u64(avg, total);

> >>

> >> +     /*

> >> +      * deadline bandwidth is defined at system level so we must

> >> +      * weight this bandwidth with the max capacity of the system.

> >> +      * As a reminder, avg_bw is 20bits width and

> >> +      * scale_cpu_capacity is 10 bits width

> >> +      */

> >> +     used += div_u64(rq->dl.avg_bw, arch_scale_cpu_capacity(NULL, cpu));

> >> +

> >>       if (likely(used < SCHED_CAPACITY_SCALE))

> >>               return SCHED_CAPACITY_SCALE - used;

> >>

> >> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h

> >> index 08858d1..e44c6be 100644

> >> --- a/kernel/sched/sched.h

> >> +++ b/kernel/sched/sched.h

> >> @@ -519,6 +519,8 @@ struct dl_rq {

> >>  #else

> >>       struct dl_bw dl_bw;

> >>  #endif

> >> +     /* This is the "average utilization" for this runqueue */

> >> +     s64 avg_bw;

> >>  };

> >

> > So I don't think this is right. AFAICT this projects the WCET as the

> > amount of time actually used by DL. This will, under many circumstances,

> > vastly overestimate the amount of time actually spend on it. Therefore

> > unduly pessimisme the fair capacity of this CPU.

> 

> I agree that if the WCET is far from reality, we will underestimate

> available capacity for CFS. Have you got some use case in mind which

> overestimates the WCET ?


I guess simply the fact that one task can be admitted to the system, but
then in practice sleep, waiting from some event to happen.

> If we can't rely on this parameters to evaluate the amount of capacity

> used by deadline scheduler on a core, this will imply that we can't

> also use it for requesting capacity to cpufreq and we should fallback

> on a monitoring mechanism which reacts to a change instead of

> anticipating it.

> 


There is at least one way in the middle: use utilization of active
servers (as I think Luca was already mentioning). This solution should
remove some of the pessimism, but still be safe for our needs. I should
be able to play with this alternative in the (hopefully) near future.

Thanks,

- Juri
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vincent Guittot Dec. 15, 2015, 4:43 a.m. UTC | #3
On 14 December 2015 at 17:51, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Dec 14, 2015 at 04:56:17PM +0100, Vincent Guittot wrote:

>> I agree that if the WCET is far from reality, we will underestimate

>> available capacity for CFS. Have you got some use case in mind which

>> overestimates the WCET ?

>

> Pretty much any 'correct' WCET is pessimistic. There's heaps of smart

> people working on improving WCET bounds, but they're still out there.

> This is mostly because of the .00001% tail cases that 'never' happen but

> would make your tokamak burn a hole just when you're outside.

>

>> If we can't rely on this parameters to evaluate the amount of capacity

>> used by deadline scheduler on a core, this will imply that we can't

>> also use it for requesting capacity to cpufreq and we should fallback

>> on a monitoring mechanism which reacts to a change instead of

>> anticipating it.

>

> No, since the WCET can and _will_ happen, its the best you can do with

> cpufreq. If you were to set it lower you could not be able to execute

> correctly in your 'never' tail cases.


In the context of frequency scaling, This mean that we will never
reach low frequency


>

> There 'might' be smart pants ways around this, where you run part of the

> execution at lower speed and switch to a higher speed to 'catch' up if

> you exceed some boundary, such that, on average, you run at the same

> speed the WCET mandates, but I'm not sure that's worth it. Juri/Luca

> might know.

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vincent Guittot Dec. 15, 2015, 4:59 a.m. UTC | #4
On 14 December 2015 at 22:12, Luca Abeni <luca.abeni@unitn.it> wrote:
> On Mon, 14 Dec 2015 16:56:17 +0100

> Vincent Guittot <vincent.guittot@linaro.org> wrote:

> [...]

>> >> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h

>> >> index 08858d1..e44c6be 100644

>> >> --- a/kernel/sched/sched.h

>> >> +++ b/kernel/sched/sched.h

>> >> @@ -519,6 +519,8 @@ struct dl_rq {

>> >>  #else

>> >>       struct dl_bw dl_bw;

>> >>  #endif

>> >> +     /* This is the "average utilization" for this runqueue */

>> >> +     s64 avg_bw;

>> >>  };

>> >

>> > So I don't think this is right. AFAICT this projects the WCET as the

>> > amount of time actually used by DL. This will, under many

>> > circumstances, vastly overestimate the amount of time actually

>> > spend on it. Therefore unduly pessimisme the fair capacity of this

>> > CPU.

>>

>> I agree that if the WCET is far from reality, we will underestimate

>> available capacity for CFS. Have you got some use case in mind which

>> overestimates the WCET ?

>> If we can't rely on this parameters to evaluate the amount of capacity

>> used by deadline scheduler on a core, this will imply that we can't

>> also use it for requesting capacity to cpufreq and we should fallback

>> on a monitoring mechanism which reacts to a change instead of

>> anticipating it.

> I think a more "theoretically sound" approach would be to track the

> _active_ utilisation (informally speaking, the sum of the utilisations

> of the tasks that are actually active on a core - the exact definition

> of "active" is the trick here).


The point is that we probably need 2 definitions of "active" tasks.
The 1st one would be used to scale the frequency. From a power saving
point of view, it have to reflect the minimum frequency needed at the
current time to handle all works without missing deadline. This one
should be updated quite often with the wake up and the sleep of tasks
as well as the throttling.
The 2nd definition is used to compute the remaining capacity for the
CFS scheduler. This one doesn't need to be updated at each wake/sleep
of a deadline task but should reflect the capacity used by deadline in
a larger time scale. The latter will be used by the CFS scheduler  at
the periodic load balance pace

> As done, for example, here:

> https://github.com/lucabe72/linux-reclaiming/tree/track-utilisation-v2

> (in particular, see

> https://github.com/lucabe72/linux-reclaiming/commit/49fc786a1c453148625f064fa38ea538470df55b

> )

> I understand this approach might look too complex... But I think it is

> much less pessimistic while still being "safe".

> If there is something that I can do to make that code more acceptable,

> let me know.

>

>

>                         Luca

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vincent Guittot Dec. 15, 2015, 12:43 p.m. UTC | #5
On 15 December 2015 at 09:50, Luca Abeni <luca.abeni@unitn.it> wrote:
> On 12/15/2015 05:59 AM, Vincent Guittot wrote:

> [...]

>>>>>

>>>>> So I don't think this is right. AFAICT this projects the WCET as the

>>>>> amount of time actually used by DL. This will, under many

>>>>> circumstances, vastly overestimate the amount of time actually

>>>>> spend on it. Therefore unduly pessimisme the fair capacity of this

>>>>> CPU.

>>>>

>>>>

>>>> I agree that if the WCET is far from reality, we will underestimate

>>>> available capacity for CFS. Have you got some use case in mind which

>>>> overestimates the WCET ?

>>>> If we can't rely on this parameters to evaluate the amount of capacity

>>>> used by deadline scheduler on a core, this will imply that we can't

>>>> also use it for requesting capacity to cpufreq and we should fallback

>>>> on a monitoring mechanism which reacts to a change instead of

>>>> anticipating it.

>>>

>>> I think a more "theoretically sound" approach would be to track the

>>> _active_ utilisation (informally speaking, the sum of the utilisations

>>> of the tasks that are actually active on a core - the exact definition

>>> of "active" is the trick here).

>>

>>

>> The point is that we probably need 2 definitions of "active" tasks.

>

> Ok; thanks for clarifying. I do not know much about the remaining capacity

> used by CFS; however, from what you write I guess CFS really need an

> "average"

> utilisation (while frequency scaling needs the active utilisation).


yes. this patch is only about the "average" utilization

> So, I suspect you really need to track 2 different things.

> From a quick look at the code that is currently in mainline, it seems to

> me that it does a reasonable thing for tracking the remaining capacity

> used by CFS...

>

>> The 1st one would be used to scale the frequency. From a power saving

>> point of view, it have to reflect the minimum frequency needed at the

>> current time to handle all works without missing deadline.

>

> Right. And it can be computed as shown in the GRUB-PA paper I mentioned

> in a previous mail (that is, by tracking the active utilisation, as done

> by my patches).


I fully trust you on that part.
>

>> This one

>> should be updated quite often with the wake up and the sleep of tasks

>> as well as the throttling.

>

> Strictly speaking, the active utilisation must be updated when a task

> wakes up and when a task sleeps/terminates (but when a task

> sleeps/terminates

> you cannot decrease the active utilisation immediately: you have to wait

> some time because the task might already have used part of its "future

> utilisation").

> The active utilisation must not be updated when a task is throttled: a

> task is throttled when its current runtime is 0, so it already used all

> of its utilisation for the current period (think about two tasks with

> runtime=50ms and period 100ms: they consume 100% of the time on a CPU,

> and when the first task consumed all of its runtime, you cannot decrease

> the active utilisation).


 I haven't read the paper you pointed in the previous email but it's
on my todo list. Does the GRUB-PA take into account the frequency
transition when selecting the best frequency ?

>

>> The 2nd definition is used to compute the remaining capacity for the

>> CFS scheduler. This one doesn't need to be updated at each wake/sleep

>> of a deadline task but should reflect the capacity used by deadline in

>> a larger time scale. The latter will be used by the CFS scheduler  at

>> the periodic load balance pace

>

> Ok, so as I wrote above this really looks like an average utilisation.

> My impression (but I do not know the CFS code too much) is that the mainline

> kernel is currently doing the right thing to compute it, so maybe there is

> no

> need to change the current code in this regard.

> If the current code is not acceptable for some reason, an alternative would

> be to measure the active utilisation for frequency scaling, and then apply a

> low-pass filter to it for CFS.

>

>

>                                 Luca

>

>

>>

>>> As done, for example, here:

>>> https://github.com/lucabe72/linux-reclaiming/tree/track-utilisation-v2

>>> (in particular, see

>>>

>>> https://github.com/lucabe72/linux-reclaiming/commit/49fc786a1c453148625f064fa38ea538470df55b

>>> )

>>> I understand this approach might look too complex... But I think it is

>>> much less pessimistic while still being "safe".

>>> If there is something that I can do to make that code more acceptable,

>>> let me know.

>>>

>>>

>>>                          Luca

>

>

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vincent Guittot Dec. 15, 2015, 12:46 p.m. UTC | #6
On 15 December 2015 at 13:20, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Dec 15, 2015 at 09:50:14AM +0100, Luca Abeni wrote:

>> On 12/15/2015 05:59 AM, Vincent Guittot wrote:

>

>> >The 2nd definition is used to compute the remaining capacity for the

>> >CFS scheduler. This one doesn't need to be updated at each wake/sleep

>> >of a deadline task but should reflect the capacity used by deadline in

>> >a larger time scale. The latter will be used by the CFS scheduler  at

>> >the periodic load balance pace

>

>> Ok, so as I wrote above this really looks like an average utilisation.

>> My impression (but I do not know the CFS code too much) is that the mainline

>> kernel is currently doing the right thing to compute it, so maybe there is no

>> need to change the current code in this regard.

>> If the current code is not acceptable for some reason, an alternative would

>> be to measure the active utilisation for frequency scaling, and then apply a

>> low-pass filter to it for CFS.

>

> So CFS really only needs a 'vague' average idea on how much time it will

> not get. Its best effort etc., so being a little wrong isn't a problem.

>

> The current code suffices, but I think the reason its been changed in

> this series is that they want/need separate tracking for fifo/rr and

> deadline in the next patch, and taking out deadline like proposed was

> the easiest way of achieving that.


yes. you're right. The goal was to minimize the overhead for tracking
separately  fifo/rr and deadline.


>

>

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vincent Guittot Dec. 15, 2015, 12:56 p.m. UTC | #7
On 15 December 2015 at 13:41, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Dec 15, 2015 at 05:43:44AM +0100, Vincent Guittot wrote:

>> On 14 December 2015 at 17:51, Peter Zijlstra <peterz@infradead.org> wrote:

>

>> > No, since the WCET can and _will_ happen, its the best you can do with

>> > cpufreq. If you were to set it lower you could not be able to execute

>> > correctly in your 'never' tail cases.

>>

>> In the context of frequency scaling, This mean that we will never

>> reach low frequency

>

> Only if you've stuffed your machine full of deadline tasks, if you take

> Luca's example of the I/B frame decoder thingy, then even the WCET for

> the I frames should not be very much (albeit significantly more than B

> frames).


But in this case, the impact of deadline scheduler on the remaining
capacity for CFS should not be that much as well. This will not
prevent a CFS task but only will only make it a bit smaller than the
other

>

> So while the WCET is pessimistic compared to the avg case, most CPUs can

> do video decoding without much effort at all, so even the WCET for the

> I-frames might allow us to drop to the lowest cpufreq.

>

> Now, if you were to decode 10 streams at the same time, different story

> of course ;-)

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vincent Guittot Dec. 15, 2015, 12:58 p.m. UTC | #8
On 15 December 2015 at 09:50, Luca Abeni <luca.abeni@unitn.it> wrote:
> On 12/15/2015 05:59 AM, Vincent Guittot wrote:

> [...]

>>>>>

>>>>> So I don't think this is right. AFAICT this projects the WCET as the

>>>>> amount of time actually used by DL. This will, under many

>>>>> circumstances, vastly overestimate the amount of time actually

>>>>> spend on it. Therefore unduly pessimisme the fair capacity of this

>>>>> CPU.

>>>>

>>>>


[snip]

>> The 2nd definition is used to compute the remaining capacity for the

>> CFS scheduler. This one doesn't need to be updated at each wake/sleep

>> of a deadline task but should reflect the capacity used by deadline in

>> a larger time scale. The latter will be used by the CFS scheduler  at

>> the periodic load balance pace

>

> Ok, so as I wrote above this really looks like an average utilisation.

> My impression (but I do not know the CFS code too much) is that the mainline

> kernel is currently doing the right thing to compute it, so maybe there is

> no

> need to change the current code in this regard.

> If the current code is not acceptable for some reason, an alternative would

> be to measure the active utilisation for frequency scaling, and then apply a

> low-pass filter to it for CFS.


In this case, it's probably easier to split what is already done into
a rt_avg metric  and a dl_avg metric

Vincent
>

>

>                                 Luca

>

>

>>

>>> As done, for example, here:

>>> https://github.com/lucabe72/linux-reclaiming/tree/track-utilisation-v2

>>> (in particular, see

>>>

>>> https://github.com/lucabe72/linux-reclaiming/commit/49fc786a1c453148625f064fa38ea538470df55b

>>> )

>>> I understand this approach might look too complex... But I think it is

>>> much less pessimistic while still being "safe".

>>> If there is something that I can do to make that code more acceptable,

>>> let me know.

>>>

>>>

>>>                          Luca

>

>

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Juri Lelli Dec. 16, 2015, 9:28 a.m. UTC | #9
Hi Luca,

On 15/12/15 22:24, Luca Abeni wrote:
> On Tue, 15 Dec 2015 14:42:29 +0100

> Peter Zijlstra <peterz@infradead.org> wrote:

> > On Tue, Dec 15, 2015 at 02:30:07PM +0100, Luca Abeni wrote:

> > 

> > > >So I remember something else from the BFQ code, which also had to

> > > >track entries for the 0-lag stuff, and I just had a quick peek at

> > > >that code again. And what they appear to do is keep inactive

> > > >entries with a lag deficit in a separate tree (the idle tree).

> > > >

> > > >And every time they update the vtime, they also push fwd the idle

> > > >tree and expire entries on that.

> > > I am not sure if I understand correctly the idea (I do not know the

> > > BFQ code; I'll have a look), but I think I tried something similar:

> > > - When a task blocks, instead of arming the inactive timer I can

> > > insert the task in an "active non contending" tree (to use GRUB

> > > terminology)

> > > - So, when some sched deadline function is invoked, I check the

> > > "0-lag time" of the first task in the "active non contending" tree,

> > > and if that time is passed I remove the task from the tree and

> > > adjust the active utilisation

> > > 

> > > The resulting code ended up being more complex (basically, I needed

> > > to handle the "active non contending" tree and to check it in

> > > task_tick_dl() and update_curr_dl()). But maybe I did it wrong...

> > > I'll try this approach again, after looking ad the BFQ code.

> > 

> > That sounds about right.

> > 

> > I've no idea if its more or less work. I just had vague memories on an

> > alternative approach to the timer.

> > 

> > Feel free to stick with the timer if that works better, just wanted to

> > mention there are indeed alternative solutions.

> Ok; I'll try to implement this alternative approach again, after

> looking at BFQ, to see if it turns out to be simpler or more complex

> than the timer-based approach.

> 

> If there is interest, I'll send an RFC with these patches after some

> testing.

> 


I think there's definitely interest, as next step will be to start using
the new API for freq selection from DL as well.

Thanks a lot for your time and efforts!

Best,

- Juri
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 8b0a15e..9d9eb50 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -43,6 +43,24 @@  static inline int on_dl_rq(struct sched_dl_entity *dl_se)
 	return !RB_EMPTY_NODE(&dl_se->rb_node);
 }
 
+static void add_average_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
+{
+	u64 se_bw = dl_se->dl_bw;
+
+	dl_rq->avg_bw += se_bw;
+}
+
+static void clear_average_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
+{
+	u64 se_bw = dl_se->dl_bw;
+
+	dl_rq->avg_bw -= se_bw;
+	if (dl_rq->avg_bw < 0) {
+		WARN_ON(1);
+		dl_rq->avg_bw = 0;
+	}
+}
+
 static inline int is_leftmost(struct task_struct *p, struct dl_rq *dl_rq)
 {
 	struct sched_dl_entity *dl_se = &p->dl;
@@ -494,6 +512,9 @@  static void update_dl_entity(struct sched_dl_entity *dl_se,
 	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
 	struct rq *rq = rq_of_dl_rq(dl_rq);
 
+	if (dl_se->dl_new)
+		add_average_bw(dl_se, dl_rq);
+
 	/*
 	 * The arrival of a new instance needs special treatment, i.e.,
 	 * the actual scheduling parameters have to be "renewed".
@@ -741,8 +762,6 @@  static void update_curr_dl(struct rq *rq)
 	curr->se.exec_start = rq_clock_task(rq);
 	cpuacct_charge(curr, delta_exec);
 
-	sched_rt_avg_update(rq, delta_exec);
-
 	dl_se->runtime -= dl_se->dl_yielded ? 0 : delta_exec;
 	if (dl_runtime_exceeded(dl_se)) {
 		dl_se->dl_throttled = 1;
@@ -1241,6 +1260,8 @@  static void task_fork_dl(struct task_struct *p)
 static void task_dead_dl(struct task_struct *p)
 {
 	struct dl_bw *dl_b = dl_bw_of(task_cpu(p));
+	struct dl_rq *dl_rq = dl_rq_of_se(&p->dl);
+	struct rq *rq = rq_of_dl_rq(dl_rq);
 
 	/*
 	 * Since we are TASK_DEAD we won't slip out of the domain!
@@ -1249,6 +1270,8 @@  static void task_dead_dl(struct task_struct *p)
 	/* XXX we should retain the bw until 0-lag */
 	dl_b->total_bw -= p->dl.dl_bw;
 	raw_spin_unlock_irq(&dl_b->lock);
+
+	clear_average_bw(&p->dl, &rq->dl);
 }
 
 static void set_curr_task_dl(struct rq *rq)
@@ -1556,7 +1579,9 @@  retry:
 	}
 
 	deactivate_task(rq, next_task, 0);
+	clear_average_bw(&next_task->dl, &rq->dl);
 	set_task_cpu(next_task, later_rq->cpu);
+	add_average_bw(&next_task->dl, &later_rq->dl);
 	activate_task(later_rq, next_task, 0);
 	ret = 1;
 
@@ -1644,7 +1669,9 @@  static void pull_dl_task(struct rq *this_rq)
 			resched = true;
 
 			deactivate_task(src_rq, p, 0);
+			clear_average_bw(&p->dl, &src_rq->dl);
 			set_task_cpu(p, this_cpu);
+			add_average_bw(&p->dl, &this_rq->dl);
 			activate_task(this_rq, p, 0);
 			dmin = p->dl.deadline;
 
@@ -1750,6 +1777,8 @@  static void switched_from_dl(struct rq *rq, struct task_struct *p)
 	if (!start_dl_timer(p))
 		__dl_clear_params(p);
 
+	clear_average_bw(&p->dl, &rq->dl);
+
 	/*
 	 * Since this might be the only -deadline task on the rq,
 	 * this is the right place to try to pull some other one
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4c49f76..ce05f61 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6203,6 +6203,14 @@  static unsigned long scale_rt_capacity(int cpu)
 
 	used = div_u64(avg, total);
 
+	/*
+	 * deadline bandwidth is defined at system level so we must
+	 * weight this bandwidth with the max capacity of the system.
+	 * As a reminder, avg_bw is 20bits width and
+	 * scale_cpu_capacity is 10 bits width
+	 */
+	used += div_u64(rq->dl.avg_bw, arch_scale_cpu_capacity(NULL, cpu));
+
 	if (likely(used < SCHED_CAPACITY_SCALE))
 		return SCHED_CAPACITY_SCALE - used;
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 08858d1..e44c6be 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -519,6 +519,8 @@  struct dl_rq {
 #else
 	struct dl_bw dl_bw;
 #endif
+	/* This is the "average utilization" for this runqueue */
+	s64 avg_bw;
 };
 
 #ifdef CONFIG_SMP