Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759228AbbFBN6z (ORCPT ); Tue, 2 Jun 2015 09:58:55 -0400 Received: from forward18p.cmail.yandex.net ([77.88.31.38]:40347 "EHLO forward18p.cmail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759159AbbFBN6k (ORCPT ); Tue, 2 Jun 2015 09:58:40 -0400 From: Kirill Tkhai To: Peter Zijlstra , "umgwanakikbuti@gmail.com" , "mingo@elte.hu" Cc: "ktkhai@parallels.com" , "rostedt@goodmis.org" , "juri.lelli@gmail.com" , "pang.xunlei@linaro.org" , "oleg@redhat.com" , "linux-kernel@vger.kernel.org" In-Reply-To: <20150601140839.852669281@infradead.org> References: <20150601135818.506080835@infradead.org> <20150601140839.852669281@infradead.org> Subject: Re: [RFC][PATCH 3/7] sched: Allow balance callbacks for check_class_changed() MIME-Version: 1.0 Message-Id: <449191433253514@web6g.yandex.ru> X-Mailer: Yamail [ http://yandex.ru ] 5.0 Date: Tue, 02 Jun 2015 16:58:34 +0300 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=koi8-r Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2813 Lines: 67 01.06.2015, 17:13, "Peter Zijlstra" : > 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 > Signed-off-by: Peter Zijlstra (Intel) > --- > ?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); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/