On Fri, 17 Jun 2016 17:28:37 +0100
Juri Lelli <juri.le...@arm.com> wrote:
[...]
> True, but we were practically already using the same parameter, under a
> different name though, after
>
> 2f9f3fdc928 "sched/deadline: Remove dl_new from struct sched_dl_entity"
>
> as we currently do:
>
> setup_new_dl_entity(&p->dl, &p->dl)
>
> > This patch reverts part of the change done in
> > commit 2d3d891d334 "sched/deadline: Add SCHED_DEADLINE inheritance
> > logic"
> >
>
> 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. So, I guess the question
> is actually why we wanted to use pi_se's parameters (the potential PI
> donor) for setting up a new entity?
That's a good question :)
> Maybe we broke the situation where a
> task is currently boosted by a DEADLINE waiter and we swich the holder
> to DEADLINE?
I remember I tested this setup (using linaro's version of rt-app), and
it seemed to work correctly...
Re-reading the code now, I actually wonder why my patch did not break
inheritance in this situation...
Luca
>
> > It would be nice to have the reason in the change log.
> >
>
> Thanks a lot for pointing out what might be more than inaccuracy in the
> changelog.
>
> Best,
>
> - Juri
>
> >
> > > Remove the second, useless, parameter.
> > >
> > > Cc: Ingo Molnar <mi...@redhat.com>
> > > Cc: Peter Zijlstra <pet...@infradead.org>
> > > Cc: Steven Rostedt <rost...@goodmis.org>
> > > Cc: Luca Abeni <luca.ab...@unitn.it>
> > > Signed-off-by: Juri Lelli <juri.le...@arm.com>
> > > ---
> > > kernel/sched/deadline.c | 9 ++++-----
> > > 1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> > > index fcb7f0217ff4..5229788a4765 100644
> > > --- a/kernel/sched/deadline.c
> > > +++ b/kernel/sched/deadline.c
> > > @@ -346,8 +346,7 @@ 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);
> > > @@ -367,8 +366,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;
> > > }
> > >
> > > /*
> > > @@ -1721,7 +1720,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
> >
