linux-kernel - Re: [RFC][PATCH 3/7] sched: Allow balance callbacks for check_class

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <449191433253514@web6g.yandex.ru>
Date:	Tue, 02 Jun 2015 16:58:34 +0300
From:	Kirill Tkhai <tkhai@...dex.ru>
To:	Peter Zijlstra <peterz@...radead.org>,
	"umgwanakikbuti@...il.com" <umgwanakikbuti@...il.com>,
	"mingo@...e.hu" <mingo@...e.hu>
Cc:	"ktkhai@...allels.com" <ktkhai@...allels.com>,
	"rostedt@...dmis.org" <rostedt@...dmis.org>,
	"juri.lelli@...il.com" <juri.lelli@...il.com>,
	"pang.xunlei@...aro.org" <pang.xunlei@...aro.org>,
	"oleg@...hat.com" <oleg@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH 3/7] sched: Allow balance callbacks for check_class_changed()

01.06.2015, 17:13, "Peter Zijlstra" <peterz@...radead.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@...il.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/