| Message ID | 1467977331-31116-1-git-send-email-juri.lelli@arm.com |
|---|---|
| State | New |
| Headers | show |
On 11/07/16 16:16, Xunlei Pang wrote: > On 2016/07/11 at 16:01, luca abeni wrote: > > Hello, > > > > On Mon, 11 Jul 2016 13:03:56 +0800 > > Xunlei Pang <xpang@redhat.com> wrote: > > > >> On 2016/07/08 at 19:28, Juri Lelli wrote: > > [...] > >>> @@ -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.). > >>> @@ -1721,7 +1731,7 @@ 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); > >>> + setup_new_dl_entity(&p->dl); > >> I'm curious why we even call setup_new_dl_entity() for non-queued > >> cases? It seems more reasonable to do it when it really gets queued. > >> We can see that enqueue_task_dl()->update_dl_entity() also has the > >> same update logic as switched_to_dl(). > > I wondered the same when removing the dl_new field from > > sched_dl_entity... But then I realised that enqueue_dl_entity() does > > not always invoke update_dl_entity() or replenish_dl_entity()... For > > example, when a task switches from SCHED_OTHER (or RT) to -deadline due > > to sched_setattr() (or similar) these functions are not invoked. > > Yeah, but for wake-up cases it does, as ENQUEUE_WAKEUP is set. > What I meant is, can we only update for queued tasks in switched_to_dl()? > Looks sensible to do. I think we can use the already present task_on_rq_queued() check in there. I'll put together a v4 ASAP (if Luca doesn't have already something). Thanks, - Juri
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index fcb7f0217ff4..7aeb21963d3c 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.). @@ -1721,7 +1731,7 @@ 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); + setup_new_dl_entity(&p->dl); if (task_on_rq_queued(p) && rq->curr != p) { #ifdef CONFIG_SMP
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. 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> Signed-off-by: Juri Lelli <juri.lelli@arm.com> --- 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 | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) -- 2.7.4