[PATCH v4] sched/deadline: remove useless param from setup_new_dl_entity

* [PATCH v4] sched/deadline: remove useless param from setup_new_dl_entity
@ 2016-07-13 10:58 Juri Lelli
  2016-07-15 10:39 ` Xunlei Pang
  0 siblings, 1 reply; 9+ messages in thread
From: Juri Lelli @ 2016-07-13 10:58 UTC (permalink / raw)
  To: peterz; +Cc: rostedt, linux-kernel, mingo, luca.abeni, juri.lelli, xpang

setup_new_dl_entity() takes two parameters, but it only actually uses
one of them, under a different name, to setup a new dl_entity, after:

 2f9f3fdc928 "sched/deadline: Remove dl_new from struct sched_dl_entity"

as we currently do

 setup_new_dl_entity(&p->dl, &p->dl)

However, before Luca's change we were doing

 setup_new_dl_entity(dl_se, pi_se)

in update_dl_entity() for a dl_se->new entity: we were using pi_se's
parameters (the potential PI donor) for setting up a new entity.

Restore this behaviour (as we want to correctly initialize parameters of
a boosted task that enters DEADLINE) by removing the useless second
parameter of setup_new_dl_entity() and retrieving the top waiter
directly from inside that function.

While we are at it we also optimize things further calling setup_new_dl_
entity only for already queued tasks, since (as pointed out by Xunlei)
we already do the very same update at tasks wakeup time anyway.

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Luca Abeni <luca.abeni@unitn.it>
Cc: Xunlei Pang <xpang@redhat.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>

---
 Changes from v3:
   - call setup_new_dl_entity only if task is enqueued already

 Changes from v2:
   - optimize by calling rt_mutex_get_top_task only if the task is
     boosted (as suggested by Steve)

 Changes from v1:
   - Steve pointed out that we were actually using the second parameter
     to permorm initialization
   - Luca confirmed that behavior is slightly changed w.r.t. before his
     change
   - changelog updated and original behavior restored
---
 kernel/sched/deadline.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index fcb7f0217ff4..dc56f5be0112 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -346,11 +346,12 @@ static void check_preempt_curr_dl(struct rq *rq, struct task_struct *p,
  * one, and to (try to!) reconcile itself with its own scheduling
  * parameters.
  */
-static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
-				       struct sched_dl_entity *pi_se)
+static inline void setup_new_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);
+	struct task_struct *pi_task;
+	struct sched_dl_entity *pi_se = dl_se;
 
 	WARN_ON(dl_time_before(rq_clock(rq), dl_se->deadline));
 
@@ -363,6 +364,15 @@ static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se,
 		return;
 
 	/*
+	 * Use the scheduling parameters of the top pi-waiter task,
+	 * if we have one from which we can inherit a deadline.
+	 */
+	if (dl_se->dl_boosted &&
+	    (pi_task = rt_mutex_get_top_task(dl_task_of(dl_se))) &&
+	    dl_prio(pi_task->normal_prio))
+		pi_se = &pi_task->dl;
+
+	/*
 	 * We use the regular wall clock time to set deadlines in the
 	 * future; in fact, we must consider execution overheads (time
 	 * spent on hardirq context, etc.).
@@ -1720,19 +1730,26 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
  */
 static void switched_to_dl(struct rq *rq, struct task_struct *p)
 {
-	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
-		setup_new_dl_entity(&p->dl, &p->dl);
 
-	if (task_on_rq_queued(p) && rq->curr != p) {
+	if (task_on_rq_queued(p)) {
+		/*
+		 * If p is not queued we will update its parameters at next
+		 * wakeup.
+		 */
+		if (dl_time_before(p->dl.deadline, rq_clock(rq)))
+			setup_new_dl_entity(&p->dl);
+
+		if (rq->curr != p) {
 #ifdef CONFIG_SMP
-		if (tsk_nr_cpus_allowed(p) > 1 && rq->dl.overloaded)
-			queue_push_tasks(rq);
+			if (tsk_nr_cpus_allowed(p) > 1 && rq->dl.overloaded)
+				queue_push_tasks(rq);
 #else
-		if (dl_task(rq->curr))
-			check_preempt_curr_dl(rq, p, 0);
-		else
-			resched_curr(rq);
+			if (dl_task(rq->curr))
+				check_preempt_curr_dl(rq, p, 0);
+			else
+				resched_curr(rq);
 #endif
+		}
 	}
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread