Re: Question about prio_changed_dl()

From: Juri Lelli
Date: Thu Feb 25 2016 - 09:16:07 EST


On 25/02/16 15:01, Peter Zijlstra wrote:
> On Fri, Feb 19, 2016 at 01:43:45PM +0100, luca abeni wrote:
> > Hi,
> >
> > when playing with the __dl_{add,sub}_ac() stuff recently posted by
> > Juri, I found something that looks strange in prio_changed_dl():
> >
> > static void prio_changed_dl(struct rq *rq, struct task_struct *p,
> > int oldprio)
> > {
> > if (task_on_rq_queued(p) || rq->curr == p) {
> > [...]
> > } else
> > switched_to_dl(rq, p);
> > }
> > but switched_to_dl() does:
> > static void switched_to_dl(struct rq *rq, struct task_struct *p)
> > {
> > if (task_on_rq_queued(p) && rq->curr != p) {
> > [...]
> > }
> > }
> >
> > so, prio_changed_dl() invokes switched_to_dl() if task_on_rq_queued()
> > is false, but in this case switched_to_dl() does nothing... Am I
> > missing something, or the
> > } else
> > switched_to_dl(rq, p);
> > is useless?
>
> Agreed, see below.
>
> > (BTW, it seems to me that switched_to_dl() is never invoked, for some
> > reason...)
>
> Hmm, it should be invoked if you do sched_setattr() to get
> SCHED_DEADLINE.
>
> ---
> Subject: sched/deadline: Remove superfluous call to switched_to_dl()
>
> if (A || B) {
>
> } else if (A && !B) {
>
> }
>
> If A we'll take the first branch, if !A we will not satisfy the second.
> Therefore the second branch will never be taken.
>
> Cc: Juri Lelli <juri.lelli@xxxxxxx>
> Reported-by: luca abeni <luca.abeni@xxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>

Indeed!

Acked-by: Juri Lelli <juri.lelli@xxxxxxx>

Thanks,

- Juri