lkml.org 
[lkml]   [2015]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC][PATCH 3/7] sched: Allow balance callbacks for check_class_changed()
    Date
    01.06.2015, 17:13, "Peter Zijlstra" <peterz@infradead.org>:
    > In order to remove dropping rq->lock from the
    > switched_{to,from}()/prio_changed() sched_class methods, run the
    > balance callbacks after it.
    >
    > We need to remove dropping rq->lock because its buggy,
    > suppose using sched_setattr()/sched_setscheduler() to change a running
    > task from FIFO to OTHER.
    >
    > By the time we get to switched_from_rt() the task is already enqueued
    > on the cfs runqueues. If switched_from_rt() does pull_rt_task() and
    > drops rq->lock, load-balancing can come in and move our task @p to
    > another rq.
    >
    > The subsequent switched_to_fair() still assumes @p is on @rq and bad
    > things will happen.
    >
    > By using balance callbacks we delay the load-balancing operations
    > {rt,dl}x{push,pull} until we've done all the important work and the
    > task is fully set up.
    >
    > Furthermore, the balance callbacks do not know about @p, therefore
    > they cannot get confused like this.
    >
    > Reported-by: Mike Galbraith <umgwanakikbuti@gmail.com>
    > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    > ---
    >  kernel/sched/core.c |   25 ++++++++++++++++++++++---
    >  1 file changed, 22 insertions(+), 3 deletions(-)
    >
    > --- a/kernel/sched/core.c
    > +++ b/kernel/sched/core.c
    > @@ -1001,7 +1001,11 @@ inline int task_curr(const struct task_s
    >  }
    >
    >  /*
    > - * Can drop rq->lock because from sched_class::switched_from() methods drop it.
    > + * switched_from, switched_to and prio_changed must _NOT_ drop rq->lock,
    > + * use the balance_callback list if you want balancing.
    > + *
    > + * this means any call to check_class_changed() must be followed by a call to
    > + * balance_callback().
    >   */
    >  static inline void check_class_changed(struct rq *rq, struct task_struct *p,
    >                                         const struct sched_class *prev_class,
    > @@ -1010,7 +1014,7 @@ static inline void check_class_changed(s
    >          if (prev_class != p->sched_class) {
    >                  if (prev_class->switched_from)
    >                          prev_class->switched_from(rq, p);
    > - /* Possble rq->lock 'hole'.  */
    > +

    But switched_from_dl()->cancel_dl_timer() still unlocks rq->lock.

    It seems we should drop it (cancel_dl_timer) and move hrtimer_cancel()
    from switched_from_dl() to finish_task_switch(). It will be executed
    for all classes and completely take the functionality we implement
    cancel_dl_timer() for.

    >                  p->sched_class->switched_to(rq, p);
    >          } else if (oldprio != p->prio || dl_task(p))
    >                  p->sched_class->prio_changed(rq, p, oldprio);


    \
     
     \ /
      Last update: 2015-06-02 16:21    [W:3.285 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site