On 2016/08/05 at 18:09, Juri Lelli wrote: > 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. > > This change removes the useless second parameter of setup_new_dl_entity(). > 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. By doing > so, we don't need to worry about a potential PI donor anymore, as rt_ > mutex_setprio() takes care of that already for us. > > Cc: Ingo Molnar <[email protected]> > Cc: Peter Zijlstra <[email protected]> > Cc: Steven Rostedt <[email protected]> > Cc: Luca Abeni <[email protected]> > Cc: Xunlei Pang <[email protected]> > Signed-off-by: Juri Lelli <[email protected]>
Reviewed-by: Xunlei Pang <[email protected]> > > --- > Changes from v4: > - remove dead code checking for boosted status, as never triggered > after rt_mutex_setprio() > > 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 | 35 ++++++++++++++++++++++------------- > 1 file changed, 22 insertions(+), 13 deletions(-) > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index fcb7f0217ff4..9491dbe039e8 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -346,12 +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); > > + WARN_ON(dl_se->dl_boosted); > WARN_ON(dl_time_before(rq_clock(rq), dl_se->deadline)); > > /* > @@ -367,8 +367,8 @@ static inline void setup_new_dl_entity(struct > sched_dl_entity *dl_se, > * future; in fact, we must consider execution overheads (time > * spent on hardirq context, etc.). > */ > - dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline; > - dl_se->runtime = pi_se->dl_runtime; > + dl_se->deadline = rq_clock(rq) + dl_se->dl_deadline; > + dl_se->runtime = dl_se->dl_runtime; > } > > /* > @@ -1720,19 +1720,28 @@ 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 p is dl_boosted we already updated its params in > + * rt_mutex_setprio()->enqueue_task(..., ENQUEUE_REPLENISH), > + * p's deadline being now already after rq_clock(rq). > + */ > + 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 > + } > } > } >
