Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1309386 > unrolled thread

[RFC 4/8] Improve the tracking of active utilisation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC 4/8] Improve the tracking of active utilisation Luca Abeni <luca.abeni@unitn.it> - 2016-01-14 16:30 +0100
    Re: [RFC 4/8] Improve the tracking of active utilisation Peter Zijlstra <peterz@infradead.org> - 2016-01-14 18:20 +0100
      Re: [RFC 4/8] Improve the tracking of active utilisation Luca Abeni <luca.abeni@unitn.it> - 2016-01-15 07:50 +0100
    Re: [RFC 4/8] Improve the tracking of active utilisation Peter Zijlstra <peterz@infradead.org> - 2016-01-14 20:50 +0100
      Re: [RFC 4/8] Improve the tracking of active utilisation Luca Abeni <luca.abeni@unitn.it> - 2016-01-15 10:30 +0100
      Re: [RFC 4/8] Improve the tracking of active utilisation Luca Abeni <luca.abeni@unitn.it> - 2016-01-19 13:30 +0100
        Re: [RFC 4/8] Improve the tracking of active utilisation Peter Zijlstra <peterz@infradead.org> - 2016-01-19 14:50 +0100
          Re: [RFC 4/8] Improve the tracking of active utilisation Luca Abeni <luca.abeni@unitn.it> - 2016-01-27 14:40 +0100
            Re: [RFC 4/8] Improve the tracking of active utilisation Peter Zijlstra <peterz@infradead.org> - 2016-01-27 15:40 +0100
              Re: [RFC 4/8] Improve the tracking of active utilisation Luca Abeni <luca.abeni@unitn.it> - 2016-01-27 15:50 +0100
    Re: [RFC 4/8] Improve the tracking of active utilisation Peter Zijlstra <peterz@infradead.org> - 2016-01-14 20:50 +0100
      Re: [RFC 4/8] Improve the tracking of active utilisation Luca Abeni <luca.abeni@unitn.it> - 2016-01-15 09:20 +0100
        Re: [RFC 4/8] Improve the tracking of active utilisation Peter Zijlstra <peterz@infradead.org> - 2016-01-15 09:40 +0100

#1309386 — [RFC 4/8] Improve the tracking of active utilisation

This patch implements a more theoretically sound algorithm for
thracking the active utilisation: instead of decreasing it when a
task blocks, use a timer (the "inactive timer", named after the
"Inactive" task state of the GRUB algorithm) to decrease the
active utilisaation at the so called "0-lag time".
---
 include/linux/sched.h   |   1 +
 kernel/sched/core.c     |   1 +
 kernel/sched/deadline.c | 152 ++++++++++++++++++++++++++++++++++++++++++------
 kernel/sched/sched.h    |   1 +
 4 files changed, 137 insertions(+), 18 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 61aa9bb..50f212f 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1346,6 +1346,7 @@ struct sched_dl_entity {
 	 * own bandwidth to be enforced, thus we need one timer per task.
 	 */
 	struct hrtimer dl_timer;
+	struct hrtimer inactive_timer;
 };
 
 union rcu_special {
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 44253ad..7ca17e4c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2215,6 +2215,7 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p)
 
 	RB_CLEAR_NODE(&p->dl.rb_node);
 	init_dl_task_timer(&p->dl);
+	init_inactive_task_timer(&p->dl);
 	__dl_clear_params(p);
 
 	INIT_LIST_HEAD(&p->rt.run_list);
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index d8e9962..0efa596 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -49,6 +49,7 @@ static void add_running_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
 {
 	u64 se_bw = dl_se->dl_bw;
 
+	lockdep_assert_held(&(rq_of_dl_rq(dl_rq))->lock);
 	dl_rq->running_bw += se_bw;
 	trace_sched_stat_running_bw_add(dl_task_of(dl_se), se_bw, dl_rq->running_bw);
 }
@@ -57,6 +58,7 @@ static void clear_running_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
 {
 	u64 se_bw = dl_se->dl_bw;
 
+	lockdep_assert_held(&(rq_of_dl_rq(dl_rq))->lock);
 	dl_rq->running_bw -= se_bw;
 	trace_sched_stat_running_bw_clear(dl_task_of(dl_se), se_bw, dl_rq->running_bw);
 	if (dl_rq->running_bw < 0) {
@@ -65,6 +67,62 @@ static void clear_running_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
 	}
 }
 
+static void task_go_inactive(struct task_struct *p)
+{
+	struct sched_dl_entity *dl_se = &p->dl;
+	struct hrtimer *timer = &dl_se->inactive_timer;
+	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
+	struct rq *rq = rq_of_dl_rq(dl_rq);
+	ktime_t now, act;
+	s64 delta;
+	u64 zerolag_time;
+
+	WARN_ON(dl_se->dl_runtime == 0);
+
+	/* If the inactive timer is already armed, return immediately */
+	if (hrtimer_active(&dl_se->inactive_timer))
+		return;
+
+
+	/*
+	 * We want the timer to fire at the "0 lag time", but considering
+	 * that it is actually coming from rq->clock and not from
+	 * hrtimer's time base reading.
+	 */
+        zerolag_time = dl_se->deadline - div64_long((dl_se->runtime * dl_se->dl_period), dl_se->dl_runtime);
+
+	act = ns_to_ktime(zerolag_time);
+	now = hrtimer_cb_get_time(timer);
+	delta = ktime_to_ns(now) - rq_clock(rq);
+	act = ktime_add_ns(act, delta);
+
+	/*
+	 * If the "0-lag time" already passed, decrease the active
+	 * utilization now, instead of starting a timer
+	 */
+	if (ktime_us_delta(act, now) < 0) {
+		clear_running_bw(dl_se, dl_rq);
+		if (!dl_task(p)) {
+			__dl_clear_params(p);
+		}
+		return;
+	}
+
+	if (!hrtimer_is_queued(timer)) {
+		hrtimer_start(timer, act, HRTIMER_MODE_ABS);
+	}
+
+	if (hrtimer_active(timer) == 0) {
+		printk("Problem activating inactive_timer!\n");
+		clear_running_bw(dl_se, dl_rq);
+		if (!dl_task(p)) {
+			__dl_clear_params(p);
+		}
+	} else {
+		get_task_struct(p);
+	}
+}
+
 static inline int is_leftmost(struct task_struct *p, struct dl_rq *dl_rq)
 {
 	struct sched_dl_entity *dl_se = &p->dl;
@@ -522,7 +580,6 @@ 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);
 
-	add_running_bw(dl_se, dl_rq);
 
 	/*
 	 * The arrival of a new instance needs special treatment, i.e.,
@@ -530,9 +587,20 @@ static void update_dl_entity(struct sched_dl_entity *dl_se,
 	 */
 	if (dl_se->dl_new) {
 		setup_new_dl_entity(dl_se, pi_se);
+		add_running_bw(dl_se, dl_rq);
 		return;
 	}
 
+	/* If the "inactive timer" is still active, stop it adn leave
+	 * the active utilisation unchanged.
+	 * If it is running, increase the active utilisation
+	 */
+	if (hrtimer_active(&dl_se->inactive_timer)) {
+		hrtimer_try_to_cancel(&dl_se->inactive_timer);
+	} else {
+	        add_running_bw(dl_se, dl_rq);
+	}
+
 	if (dl_time_before(dl_se->deadline, rq_clock(rq)) ||
 	    dl_entity_overflow(dl_se, pi_se, rq_clock(rq))) {
 		dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
@@ -619,12 +687,7 @@ static enum hrtimer_restart dl_task_timer(struct hrtimer *timer)
 
 	rq = task_rq_lock(p, &flags);
 
-	/*
-	 * The task might have changed its scheduling policy to something
-	 * different than SCHED_DEADLINE (through switched_fromd_dl()).
-	 */
 	if (!dl_task(p)) {
-		__dl_clear_params(p);
 		goto unlock;
 	}
 
@@ -811,6 +874,49 @@ static void update_curr_dl(struct rq *rq)
 	}
 }
 
+static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)
+{
+	struct sched_dl_entity *dl_se = container_of(timer,
+						     struct sched_dl_entity,
+						     inactive_timer);
+	struct task_struct *p = dl_task_of(dl_se);
+	unsigned long flags;
+	struct rq *rq;
+
+	rq = task_rq_lock(p, &flags);
+
+	if (dl_se->dl_new) {
+		printk("Problem! New task was inactive?\n");
+		goto unlock;
+	}
+	if (!dl_task(p)) {
+		__dl_clear_params(p);
+
+		goto unlock;
+	}
+	if (p->state == TASK_RUNNING) {
+		goto unlock;
+	}
+
+	sched_clock_tick();
+	update_rq_clock(rq);
+
+	clear_running_bw(dl_se, &rq->dl);
+unlock:
+	task_rq_unlock(rq, p, &flags);
+	put_task_struct(p);
+
+	return HRTIMER_NORESTART;
+}
+
+void init_inactive_task_timer(struct sched_dl_entity *dl_se)
+{
+	struct hrtimer *timer = &dl_se->inactive_timer;
+
+	hrtimer_init(timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	timer->function = inactive_task_timer;
+}
+
 #ifdef CONFIG_SMP
 
 static void inc_dl_deadline(struct dl_rq *dl_rq, u64 deadline)
@@ -987,7 +1093,10 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)
 	 * run yet) will take care of this.
 	 */
 	if (p->dl.dl_throttled && !(flags & ENQUEUE_REPLENISH)) {
-		add_running_bw(&p->dl, &rq->dl);
+		if (hrtimer_try_to_cancel(&p->dl.inactive_timer) < 0) {
+			printk("Waking up a depleted task, but cannot cancel inactive timer!\n");
+			add_running_bw(&p->dl, &rq->dl);
+		}
 		return;
 	}
 
@@ -1009,7 +1118,7 @@ static void dequeue_task_dl(struct rq *rq, struct task_struct *p, int flags)
 	update_curr_dl(rq);
 	__dequeue_task_dl(rq, p, flags);
 	if (flags & DEQUEUE_SLEEP)
-		clear_running_bw(&p->dl, &rq->dl);
+		task_go_inactive(p);
 }
 
 /*
@@ -1087,6 +1196,19 @@ select_task_rq_dl(struct task_struct *p, int cpu, int sd_flag, int flags)
 	}
 	rcu_read_unlock();
 
+	if (rq != cpu_rq(cpu)) {
+		if (hrtimer_active(&p->dl.inactive_timer)) {
+			raw_spin_lock(&rq->lock);
+			clear_running_bw(&p->dl, &rq->dl);
+			raw_spin_unlock(&rq->lock);
+			rq = cpu_rq(cpu);
+			raw_spin_lock(&rq->lock);
+			add_running_bw(&p->dl, &rq->dl);
+			raw_spin_unlock(&rq->lock);
+		}
+	}
+
+
 out:
 	return cpu;
 }
@@ -1248,8 +1370,6 @@ 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!
@@ -1258,10 +1378,6 @@ 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);
-
-	if (task_on_rq_queued(p)) {
-		clear_running_bw(&p->dl, &rq->dl);
-	}
 }
 
 static void set_curr_task_dl(struct rq *rq)
@@ -1742,12 +1858,12 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
 	 * SCHED_DEADLINE until the deadline passes, the timer will reset the
 	 * task.
 	 */
-	if (!start_dl_timer(p))
+	if (task_on_rq_queued(p))
+		task_go_inactive(p);
+	if (!hrtimer_active(&p->dl.inactive_timer))
 		__dl_clear_params(p);
-
-	if (task_on_rq_queued(p)) {
+	else if (!hrtimer_callback_running(&p->dl.inactive_timer))
 		clear_running_bw(&p->dl, &rq->dl);
-	}
 
 	/*
 	 * Since this might be the only -deadline task on the rq,
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 826ca6a..9d0fdb1 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1278,6 +1278,7 @@ extern void init_rt_bandwidth(struct rt_bandwidth *rt_b, u64 period, u64 runtime
 extern struct dl_bandwidth def_dl_bandwidth;
 extern void init_dl_bandwidth(struct dl_bandwidth *dl_b, u64 period, u64 runtime);
 extern void init_dl_task_timer(struct sched_dl_entity *dl_se);
+extern void init_inactive_task_timer(struct sched_dl_entity *dl_se);
 
 unsigned long to_ratio(u64 period, u64 runtime);
 
-- 
1.9.1

[toc] | [next] | [standalone]

#1309495

On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:
> @@ -65,6 +67,62 @@ static void clear_running_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
>  	}
>  }
>  
> +static void task_go_inactive(struct task_struct *p)
> +{
> +	struct sched_dl_entity *dl_se = &p->dl;
> +	struct hrtimer *timer = &dl_se->inactive_timer;
> +	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
> +	struct rq *rq = rq_of_dl_rq(dl_rq);
> +	ktime_t now, act;
> +	s64 delta;
> +	u64 zerolag_time;
> +
> +	WARN_ON(dl_se->dl_runtime == 0);
> +
> +	/* If the inactive timer is already armed, return immediately */
> +	if (hrtimer_active(&dl_se->inactive_timer))
> +		return;
> +
> +
> +	/*
> +	 * We want the timer to fire at the "0 lag time", but considering
> +	 * that it is actually coming from rq->clock and not from
> +	 * hrtimer's time base reading.
> +	 */
> +        zerolag_time = dl_se->deadline - div64_long((dl_se->runtime * dl_se->dl_period), dl_se->dl_runtime);

whitespace damage

> @@ -530,9 +587,20 @@ static void update_dl_entity(struct sched_dl_entity *dl_se,
>  	 */
>  	if (dl_se->dl_new) {
>  		setup_new_dl_entity(dl_se, pi_se);
> +		add_running_bw(dl_se, dl_rq);
>  		return;
>  	}
>  
> +	/* If the "inactive timer" is still active, stop it adn leave
> +	 * the active utilisation unchanged.
> +	 * If it is running, increase the active utilisation
> +	 */
> +	if (hrtimer_active(&dl_se->inactive_timer)) {
> +		hrtimer_try_to_cancel(&dl_se->inactive_timer);

what if cancel fails?

> +	} else {
> +	        add_running_bw(dl_se, dl_rq);
> +	}
> +
>  	if (dl_time_before(dl_se->deadline, rq_clock(rq)) ||
>  	    dl_entity_overflow(dl_se, pi_se, rq_clock(rq))) {
>  		dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;

> @@ -1248,8 +1370,6 @@ 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!
> @@ -1258,10 +1378,6 @@ 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);
> -
> -	if (task_on_rq_queued(p)) {
> -		clear_running_bw(&p->dl, &rq->dl);
> -	}

what happens if the timer is still active here? then we get the timer
storage freed while enqueued?

> @@ -1742,12 +1858,12 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
>  	 * SCHED_DEADLINE until the deadline passes, the timer will reset the
>  	 * task.
>  	 */
> -	if (!start_dl_timer(p))
> +	if (task_on_rq_queued(p))
> +		task_go_inactive(p);
> +	if (!hrtimer_active(&p->dl.inactive_timer))
>  		__dl_clear_params(p);
> -
> -	if (task_on_rq_queued(p)) {
> +	else if (!hrtimer_callback_running(&p->dl.inactive_timer))
>  		clear_running_bw(&p->dl, &rq->dl);
> -	}
>  
>  	/*
>  	 * Since this might be the only -deadline task on the rq,

idem, what if the task dies while !dl but with timer pending?

[toc] | [prev] | [next] | [standalone]

#1309897

Hi Peter,


On Thu, 14 Jan 2016 18:16:19 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

[...]
> > +	/* If the "inactive timer" is still active, stop it adn
> > leave
> > +	 * the active utilisation unchanged.
> > +	 * If it is running, increase the active utilisation
> > +	 */
> > +	if (hrtimer_active(&dl_se->inactive_timer)) {
> > +		hrtimer_try_to_cancel(&dl_se->inactive_timer);
> 
> what if cancel fails?
Eh, this is a tricky point :)
In this case, the "if (p->state == TASK_RUNNING) {" in
inactive_task_timer() should detect what happened, and avoid decreasing
the active utilization. So, we should be safe... At least, this was my
plan, maybe I missed something.


> > @@ -1248,8 +1370,6 @@ 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!
> > @@ -1258,10 +1378,6 @@ 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);
> > -
> > -	if (task_on_rq_queued(p)) {
> > -		clear_running_bw(&p->dl, &rq->dl);
> > -	}
> 
> what happens if the timer is still active here? then we get the timer
> storage freed while enqueued?
I think here (and in the successive comment) we are safe because of the
get_task_struct() you mention in another email, right? Or am I missing
something else?


			Thanks,
				Luca

[toc] | [prev] | [next] | [standalone]

#1309609

On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:
> This patch implements a more theoretically sound algorithm for
> thracking the active utilisation: instead of decreasing it when a
> task blocks, use a timer (the "inactive timer", named after the
> "Inactive" task state of the GRUB algorithm) to decrease the
> active utilisaation at the so called "0-lag time".

See also the large-ish comment in __setparam_dl().

If we go do proper 0-lag, as GRUB requires, then we might as well use it
for that.

But we need to sort the issue of the task exiting with an armed timer.
The solution suggested there is keeping a task reference with the timer.

[toc] | [prev] | [next] | [standalone]

#1309982

On Thu, 14 Jan 2016 20:43:23 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:
> > This patch implements a more theoretically sound algorithm for
> > thracking the active utilisation: instead of decreasing it when a
> > task blocks, use a timer (the "inactive timer", named after the
> > "Inactive" task state of the GRUB algorithm) to decrease the
> > active utilisaation at the so called "0-lag time".
> 
> See also the large-ish comment in __setparam_dl().
> 
> If we go do proper 0-lag, as GRUB requires, then we might as well use
> it for that.
Yes, I initially tried to do this, but I found some issues (I do not
remember, but I think they were related to tasks moving from
SCHED_DEADLINE to SCHED_OTHER, and then migrating to some other
runqueue while SCHED_OTHER but before the 0-lag time)

I'll search my notes for this issue in the next days and check
again (maybe when I wrote this code I was just misunderstanding
something)


			Luca

> 
> But we need to sort the issue of the task exiting with an armed timer.
> The solution suggested there is keeping a task reference with the
> timer.

[toc] | [prev] | [next] | [standalone]

#1312178

Hi Peter,

On 01/14/2016 08:43 PM, Peter Zijlstra wrote:
> On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:
>> This patch implements a more theoretically sound algorithm for
>> thracking the active utilisation: instead of decreasing it when a
>> task blocks, use a timer (the "inactive timer", named after the
>> "Inactive" task state of the GRUB algorithm) to decrease the
>> active utilisaation at the so called "0-lag time".
>
> See also the large-ish comment in __setparam_dl().
>
> If we go do proper 0-lag, as GRUB requires, then we might as well use it
> for that.
Just to check if I understand correctly:
I would need to remove "dl_b->total_bw -= p->dl.dl_bw;" from task_dead_dl(),
and __dl_clear() from "else if (!dl_policy(policy) && task_has_dl_policy(p))"
in dl_overflow(). Then, arm the inactive_timer in these cases, and add the
__dl_clear() in the "if (!dl_task(p))" in inactive_task_timer()... Right?

If this understanding is correct (modulo some details that I'll figure out
during testing), I'll try this.

In theory, the inactive_timer would be the right place to also decrease
the active utilisation when a task switches from SCHED_DEADLINE to something
else... But this is problematic if the task migrates after switching from
SCHED_DEADLINE and before the timer fires.



			Thanks,
				Luca

[toc] | [prev] | [next] | [standalone]

#1312249

On Tue, Jan 19, 2016 at 01:20:13PM +0100, Luca Abeni wrote:
> Hi Peter,
> 
> On 01/14/2016 08:43 PM, Peter Zijlstra wrote:
> >On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:
> >>This patch implements a more theoretically sound algorithm for
> >>thracking the active utilisation: instead of decreasing it when a
> >>task blocks, use a timer (the "inactive timer", named after the
> >>"Inactive" task state of the GRUB algorithm) to decrease the
> >>active utilisaation at the so called "0-lag time".
> >
> >See also the large-ish comment in __setparam_dl().
> >
> >If we go do proper 0-lag, as GRUB requires, then we might as well use it
> >for that.
> Just to check if I understand correctly:
> I would need to remove "dl_b->total_bw -= p->dl.dl_bw;" from task_dead_dl(),
> and __dl_clear() from "else if (!dl_policy(policy) && task_has_dl_policy(p))"
> in dl_overflow(). Then, arm the inactive_timer in these cases, and add the
> __dl_clear() in the "if (!dl_task(p))" in inactive_task_timer()... Right?

Correct.

> If this understanding is correct (modulo some details that I'll figure out
> during testing), I'll try this.

Yes, there's bound to be 'fun' details..

> In theory, the inactive_timer would be the right place to also decrease
> the active utilisation when a task switches from SCHED_DEADLINE to something
> else... But this is problematic if the task migrates after switching from
> SCHED_DEADLINE and before the timer fires.

urgh, yes.. details :-)

[toc] | [prev] | [next] | [standalone]

#1318949

Hi Peter,

On Tue, 19 Jan 2016 14:47:39 +0100
Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Jan 19, 2016 at 01:20:13PM +0100, Luca Abeni wrote:
> > Hi Peter,
> > 
> > On 01/14/2016 08:43 PM, Peter Zijlstra wrote:
> > >On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:
> > >>This patch implements a more theoretically sound algorithm for
> > >>thracking the active utilisation: instead of decreasing it when a
> > >>task blocks, use a timer (the "inactive timer", named after the
> > >>"Inactive" task state of the GRUB algorithm) to decrease the
> > >>active utilisaation at the so called "0-lag time".
> > >
> > >See also the large-ish comment in __setparam_dl().
> > >
> > >If we go do proper 0-lag, as GRUB requires, then we might as well
> > >use it for that.
> > Just to check if I understand correctly:
> > I would need to remove "dl_b->total_bw -= p->dl.dl_bw;" from
> > task_dead_dl(), and __dl_clear() from "else if (!dl_policy(policy)
> > && task_has_dl_policy(p))" in dl_overflow(). Then, arm the
> > inactive_timer in these cases, and add the __dl_clear() in the "if
> > (!dl_task(p))" in inactive_task_timer()... Right?
> 
> Correct.
> 
> > If this understanding is correct (modulo some details that I'll
> > figure out during testing), I'll try this.
> 
> Yes, there's bound to be 'fun' details..

Ok, so I implemented this idea, and I am currently testing it...
The first experiments seem to show that there are no problems, but I
just tried some simple workload (rt-app, or some other periodic taskset
scheduled by SCHED_DEADLINE). Do you have suggestions for more
"interesting" (and meaningful) tests/experiments?


			Thanks,
				Luca

[toc] | [prev] | [next] | [standalone]

#1319028

On Wed, Jan 27, 2016 at 02:36:51PM +0100, Luca Abeni wrote:
> Ok, so I implemented this idea, and I am currently testing it...
> The first experiments seem to show that there are no problems, but I
> just tried some simple workload (rt-app, or some other periodic taskset
> scheduled by SCHED_DEADLINE). Do you have suggestions for more
> "interesting" (and meaningful) tests/experiments?

rt-app is the workload generator, right?

I think the most interesting part here is the switched_from path, so
you'd want the workload to include a !rt task that gets PI boosted to
deadline every so often.

Also, does rt-app let tasks die? Or does it spawn N tasks and lets them
run jobs until the end? I think you want to put some effort in
task_dead_dl() as well.

After that, just make sure rt-app generates a _lot_ of tasks such that
the migration thing gets used.

Other than that, no, not really :-)

[toc] | [prev] | [next] | [standalone]

#1319034

Hi Peter,

On Wed, 27 Jan 2016 15:39:46 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Jan 27, 2016 at 02:36:51PM +0100, Luca Abeni wrote:
> > Ok, so I implemented this idea, and I am currently testing it...
> > The first experiments seem to show that there are no problems, but I
> > just tried some simple workload (rt-app, or some other periodic
> > taskset scheduled by SCHED_DEADLINE). Do you have suggestions for
> > more "interesting" (and meaningful) tests/experiments?
> 
> rt-app is the workload generator, right?
> 
> I think the most interesting part here is the switched_from path, so
> you'd want the workload to include a !rt task that gets PI boosted to
> deadline every so often.
> 
> Also, does rt-app let tasks die? Or does it spawn N tasks and lets
> them run jobs until the end? I think you want to put some effort in
> task_dead_dl() as well.
> 
> After that, just make sure rt-app generates a _lot_ of tasks such that
> the migration thing gets used.

Thanks; I'll check with Juri how to do all of this with rt-app (or how
to modify rt-app to stress these functionalities).


			Thanks,
				Luca

[toc] | [prev] | [next] | [standalone]

#1309612

On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:

> +	if (!hrtimer_is_queued(timer)) {
> +		hrtimer_start(timer, act, HRTIMER_MODE_ABS);
> +	}
> +
> +	if (hrtimer_active(timer) == 0) {
> +		printk("Problem activating inactive_timer!\n");
> +		clear_running_bw(dl_se, dl_rq);
> +		if (!dl_task(p)) {
> +			__dl_clear_params(p);
> +		}
> +	} else {
> +		get_task_struct(p);

Ah, I missed that one. I would suggest putting that right _before_
hrtimer_start(), because hrtimer_start() guarantees the callback will
run.

> +	}

[toc] | [prev] | [next] | [standalone]

#1309943

On 01/14/2016 08:47 PM, Peter Zijlstra wrote:
> On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:
>
>> +	if (!hrtimer_is_queued(timer)) {
>> +		hrtimer_start(timer, act, HRTIMER_MODE_ABS);
>> +	}
>> +
>> +	if (hrtimer_active(timer) == 0) {
>> +		printk("Problem activating inactive_timer!\n");
>> +		clear_running_bw(dl_se, dl_rq);
>> +		if (!dl_task(p)) {
>> +			__dl_clear_params(p);
>> +		}
>> +	} else {
>> +		get_task_struct(p);
>
> Ah, I missed that one. I would suggest putting that right _before_
> hrtimer_start(), because hrtimer_start() guarantees the callback will
> run.
Ok. So, if I understand well, the "if (hrtimer_active(timer) == 0)" check
is useless (or should be somehow revised)... Right?



			Thanks,
				Luca

[toc] | [prev] | [next] | [standalone]

#1309953

On Fri, Jan 15, 2016 at 09:10:15AM +0100, Luca Abeni wrote:
> On 01/14/2016 08:47 PM, Peter Zijlstra wrote:
> >On Thu, Jan 14, 2016 at 04:24:49PM +0100, Luca Abeni wrote:
> >
> >>+	if (!hrtimer_is_queued(timer)) {
> >>+		hrtimer_start(timer, act, HRTIMER_MODE_ABS);
> >>+	}
> >>+
> >>+	if (hrtimer_active(timer) == 0) {
> >>+		printk("Problem activating inactive_timer!\n");
> >>+		clear_running_bw(dl_se, dl_rq);
> >>+		if (!dl_task(p)) {
> >>+			__dl_clear_params(p);
> >>+		}
> >>+	} else {
> >>+		get_task_struct(p);
> >
> >Ah, I missed that one. I would suggest putting that right _before_
> >hrtimer_start(), because hrtimer_start() guarantees the callback will
> >run.

> Ok. So, if I understand well, the "if (hrtimer_active(timer) == 0)" check
> is useless (or should be somehow revised)... Right?

Yes, ever since: c6eb3f70d448 ("hrtimer: Get rid of hrtimer softirq")
hrtimer_start() is guaranteed to work and result in a callback, even if
the time is in the past.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web