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

[Juri Lelli]

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@xxxxxxxxxx> 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