lkml.org 
[lkml]   [2016]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] sched/deadline: remove useless param from setup_new_dl_entity
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

\
 
 \ /
  Last update: 2016-07-12 11:21    [W:2.753 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site