* [PATCH] sched: Handle priority boosted tasks proper in setscheduler()
@ 2015-05-05 16:08 Thomas Gleixner
2015-05-05 16:29 ` Steven Rostedt
2015-05-08 13:18 ` [tip:sched/core] " tip-bot for Thomas Gleixner
0 siblings, 2 replies; 9+ messages in thread
From: Thomas Gleixner @ 2015-05-05 16:08 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Mike Galbraith, Ronny Meeus, LKML, Peter Zijlstra
Ronny reported that the following scenario is not handled correctly:
T1 (prio = 10)
lock(rtmutex);
T2 (prio = 20)
lock(rtmutex)
boost T1
T1 (prio = 20)
sys_set_scheduler(prio = 30)
T1 prio = 30
....
sys_set_scheduler(prio = 10)
T1 prio = 30
The last step is wrong as T1 should now be back at prio 20.
commit c365c292d0590 "sched: Consider pi boosting in setscheduler()"
only handles the case where a boosted tasks tries to lower its
priority.
Fix it by taking the new effective priority into account for the
decision whether a change of the priority is required.
Reported-by: Ronny Meeus <ronny.meeus@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
kernel/locking/rtmutex.c | 10 ++++++----
kernel/sched/core.c | 11 +++++------
2 files changed, 11 insertions(+), 10 deletions(-)
Index: tip/kernel/locking/rtmutex.c
===================================================================
--- tip.orig/kernel/locking/rtmutex.c
+++ tip/kernel/locking/rtmutex.c
@@ -265,15 +265,17 @@ struct task_struct *rt_mutex_get_top_tas
}
/*
- * Called by sched_setscheduler() to check whether the priority change
- * is overruled by a possible priority boosting.
+ * Called by sched_setscheduler() to get the priority which will be
+ * effective after the change.
*/
int rt_mutex_check_prio(struct task_struct *task, int newprio)
{
if (!task_has_pi_waiters(task))
- return 0;
+ return newprio;
- return task_top_pi_waiter(task)->task->prio <= newprio;
+ if (task_top_pi_waiter(task)->task->prio <= newprio)
+ return task_top_pi_waiter(task)->task->prio;
+ return newprio;
}
/*
Index: tip/kernel/sched/core.c
===================================================================
--- tip.orig/kernel/sched/core.c
+++ tip/kernel/sched/core.c
@@ -3414,7 +3414,7 @@ static int __sched_setscheduler(struct t
int newprio = dl_policy(attr->sched_policy) ? MAX_DL_PRIO - 1 :
MAX_RT_PRIO - 1 - attr->sched_priority;
int retval, oldprio, oldpolicy = -1, queued, running;
- int policy = attr->sched_policy;
+ int new_effective_prio, policy = attr->sched_policy;
unsigned long flags;
const struct sched_class *prev_class;
struct rq *rq;
@@ -3596,15 +3596,14 @@ change:
oldprio = p->prio;
/*
- * Special case for priority boosted tasks.
- *
- * If the new priority is lower or equal (user space view)
- * than the current (boosted) priority, we just store the new
+ * Take priority boosted tasks into account. If the new
+ * effective priority is unchanged, we just store the new
* normal parameters and do not touch the scheduler class and
* the runqueue. This will be done when the task deboost
* itself.
*/
- if (rt_mutex_check_prio(p, newprio)) {
+ new_effective_prio = rt_mutex_check_prio(p, newprio);
+ if (new_effective_prio == oldprio) {
__setscheduler_params(p, attr);
task_rq_unlock(rq, p, &flags);
return 0;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sched: Handle priority boosted tasks proper in setscheduler()
2015-05-05 16:08 [PATCH] sched: Handle priority boosted tasks proper in setscheduler() Thomas Gleixner
@ 2015-05-05 16:29 ` Steven Rostedt
2015-05-05 16:31 ` Thomas Gleixner
2015-05-08 13:18 ` [tip:sched/core] " tip-bot for Thomas Gleixner
1 sibling, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2015-05-05 16:29 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Mike Galbraith, Ronny Meeus, LKML, Peter Zijlstra
On Tue, 5 May 2015 18:08:01 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:
> Reported-by: Ronny Meeus <ronny.meeus@gmail.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> kernel/locking/rtmutex.c | 10 ++++++----
> kernel/sched/core.c | 11 +++++------
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> Index: tip/kernel/locking/rtmutex.c
> ===================================================================
> --- tip.orig/kernel/locking/rtmutex.c
> +++ tip/kernel/locking/rtmutex.c
> @@ -265,15 +265,17 @@ struct task_struct *rt_mutex_get_top_tas
> }
>
> /*
> - * Called by sched_setscheduler() to check whether the priority change
> - * is overruled by a possible priority boosting.
> + * Called by sched_setscheduler() to get the priority which will be
> + * effective after the change.
> */
> int rt_mutex_check_prio(struct task_struct *task, int newprio)
> {
> if (!task_has_pi_waiters(task))
> - return 0;
> + return newprio;
>
> - return task_top_pi_waiter(task)->task->prio <= newprio;
> + if (task_top_pi_waiter(task)->task->prio <= newprio)
> + return task_top_pi_waiter(task)->task->prio;
> + return newprio;
> }
>
> /*
> Index: tip/kernel/sched/core.c
> ===================================================================
> --- tip.orig/kernel/sched/core.c
> +++ tip/kernel/sched/core.c
> @@ -3414,7 +3414,7 @@ static int __sched_setscheduler(struct t
> int newprio = dl_policy(attr->sched_policy) ? MAX_DL_PRIO - 1 :
> MAX_RT_PRIO - 1 - attr->sched_priority;
> int retval, oldprio, oldpolicy = -1, queued, running;
> - int policy = attr->sched_policy;
> + int new_effective_prio, policy = attr->sched_policy;
> unsigned long flags;
> const struct sched_class *prev_class;
> struct rq *rq;
> @@ -3596,15 +3596,14 @@ change:
> oldprio = p->prio;
>
> /*
> - * Special case for priority boosted tasks.
> - *
> - * If the new priority is lower or equal (user space view)
> - * than the current (boosted) priority, we just store the new
> + * Take priority boosted tasks into account. If the new
> + * effective priority is unchanged, we just store the new
> * normal parameters and do not touch the scheduler class and
> * the runqueue. This will be done when the task deboost
> * itself.
> */
> - if (rt_mutex_check_prio(p, newprio)) {
> + new_effective_prio = rt_mutex_check_prio(p, newprio);
> + if (new_effective_prio == oldprio) {
When I first heard of this problem, I started writing code to fix this
and came up with pretty much the exact same answer.
I got pulled onto other things so I never finished it, but one thing
that worried me about this fix is this:
T1 - FIFO policy (prio = 10)
lock(rtmutex);
T2 (prio = 20)
lock(rtmutex)
boost T1 (prio = 20)
TI (prio = 20)
sys_sched_setscheduler(prio = 30)
TI (prio = 30)
T1 (prio = 30)
sys_sched_setscheduler(SCHED_OTHER)
new_effective_prio = 20, oldprio = 30
Before the code stopped at the rt_mutex_check_prio(), but now it
continues. Will having the policy change cause problems here?
-- Steve
> __setscheduler_params(p, attr);
> task_rq_unlock(rq, p, &flags);
> return 0;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sched: Handle priority boosted tasks proper in setscheduler()
2015-05-05 16:29 ` Steven Rostedt
@ 2015-05-05 16:31 ` Thomas Gleixner
2015-05-05 16:42 ` Steven Rostedt
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2015-05-05 16:31 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Mike Galbraith, Ronny Meeus, LKML, Peter Zijlstra
On Tue, 5 May 2015, Steven Rostedt wrote:
> I got pulled onto other things so I never finished it, but one thing
> that worried me about this fix is this:
>
> T1 - FIFO policy (prio = 10)
> lock(rtmutex);
>
> T2 (prio = 20)
> lock(rtmutex)
> boost T1 (prio = 20)
>
> TI (prio = 20)
> sys_sched_setscheduler(prio = 30)
> TI (prio = 30)
>
> T1 (prio = 30)
> sys_sched_setscheduler(SCHED_OTHER)
> new_effective_prio = 20, oldprio = 30
>
> Before the code stopped at the rt_mutex_check_prio(), but now it
> continues. Will having the policy change cause problems here?
No, because it stays effective in the FIFO domain.
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sched: Handle priority boosted tasks proper in setscheduler()
2015-05-05 16:31 ` Thomas Gleixner
@ 2015-05-05 16:42 ` Steven Rostedt
2015-05-05 16:50 ` Thomas Gleixner
0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2015-05-05 16:42 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Mike Galbraith, Ronny Meeus, LKML, Peter Zijlstra
On Tue, 5 May 2015 18:31:20 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 5 May 2015, Steven Rostedt wrote:
> > I got pulled onto other things so I never finished it, but one thing
> > that worried me about this fix is this:
> >
> > T1 - FIFO policy (prio = 10)
> > lock(rtmutex);
> >
> > T2 (prio = 20)
> > lock(rtmutex)
> > boost T1 (prio = 20)
> >
> > TI (prio = 20)
> > sys_sched_setscheduler(prio = 30)
> > TI (prio = 30)
> >
> > T1 (prio = 30)
> > sys_sched_setscheduler(SCHED_OTHER)
> > new_effective_prio = 20, oldprio = 30
> >
> > Before the code stopped at the rt_mutex_check_prio(), but now it
> > continues. Will having the policy change cause problems here?
>
> No, because it stays effective in the FIFO domain.
>
Ah, the policy passed in isn't used, so we are safe. But, but I also
found that we still call __setscheduler(), which does:
p->prio = normal_prio();
Isn't that going to null out the boosting?
-- Steve
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sched: Handle priority boosted tasks proper in setscheduler()
2015-05-05 16:42 ` Steven Rostedt
@ 2015-05-05 16:50 ` Thomas Gleixner
2015-05-05 17:01 ` Steven Rostedt
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2015-05-05 16:50 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Mike Galbraith, Ronny Meeus, LKML, Peter Zijlstra
On Tue, 5 May 2015, Steven Rostedt wrote:
> On Tue, 5 May 2015 18:31:20 +0200 (CEST)
> Thomas Gleixner <tglx@linutronix.de> wrote:
>
> > On Tue, 5 May 2015, Steven Rostedt wrote:
> > > I got pulled onto other things so I never finished it, but one thing
> > > that worried me about this fix is this:
> > >
> > > T1 - FIFO policy (prio = 10)
> > > lock(rtmutex);
> > >
> > > T2 (prio = 20)
> > > lock(rtmutex)
> > > boost T1 (prio = 20)
> > >
> > > TI (prio = 20)
> > > sys_sched_setscheduler(prio = 30)
> > > TI (prio = 30)
> > >
> > > T1 (prio = 30)
> > > sys_sched_setscheduler(SCHED_OTHER)
> > > new_effective_prio = 20, oldprio = 30
> > >
> > > Before the code stopped at the rt_mutex_check_prio(), but now it
> > > continues. Will having the policy change cause problems here?
> >
> > No, because it stays effective in the FIFO domain.
> >
>
> Ah, the policy passed in isn't used, so we are safe. But, but I also
> found that we still call __setscheduler(), which does:
>
> p->prio = normal_prio();
>
> Isn't that going to null out the boosting?
Crap. Yes, I missed that. So __setscheduler() assumes that there is no
boosting going on. So we need:
p->prio = effective_prio(p);
there instead.
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sched: Handle priority boosted tasks proper in setscheduler()
2015-05-05 16:50 ` Thomas Gleixner
@ 2015-05-05 17:01 ` Steven Rostedt
2015-05-05 17:49 ` [PATCH V2] " Thomas Gleixner
0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2015-05-05 17:01 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Mike Galbraith, Ronny Meeus, LKML, Peter Zijlstra
On Tue, 5 May 2015 18:50:17 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:
> Crap. Yes, I missed that. So __setscheduler() assumes that there is no
> boosting going on. So we need:
>
> p->prio = effective_prio(p);
>
> there instead.
Of course then we need to do something about normalize_task() (for
sysrq), which depends on it being normal_prio().
-- Steve
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V2] sched: Handle priority boosted tasks proper in setscheduler()
2015-05-05 17:01 ` Steven Rostedt
@ 2015-05-05 17:49 ` Thomas Gleixner
2015-05-05 20:20 ` Steven Rostedt
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2015-05-05 17:49 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Mike Galbraith, Ronny Meeus, LKML, Peter Zijlstra
Ronny reported that the following scenario is not handled correctly:
T1 (prio = 10)
lock(rtmutex);
T2 (prio = 20)
lock(rtmutex)
boost T1
T1 (prio = 20)
sys_set_scheduler(prio = 30)
T1 prio = 30
....
sys_set_scheduler(prio = 10)
T1 prio = 30
The last step is wrong as T1 should now be back at prio 20.
commit c365c292d0590 "sched: Consider pi boosting in setscheduler()"
only handles the case where a boosted tasks tries to lower its
priority.
Fix it by taking the new effective priority into account for the
decision whether a change of the priority is required.
Reported-by: Ronny Meeus <ronny.meeus@gmail.com>
Fixes: commit c365c292d0590 "sched: Consider pi boosting in setscheduler()"
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Mike Galbraith <umgwanakikbuti@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/alpine.DEB.2.11.1505051806060.4225@nanos
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
V2: Prevent __set_scheduler() from stomping over p->prio (pointed out
by Steven) and fix the !RT_MUTEX case
include/linux/sched/rt.h | 7 ++++---
kernel/locking/rtmutex.c | 12 +++++++-----
kernel/sched/core.c | 26 ++++++++++++++------------
3 files changed, 25 insertions(+), 20 deletions(-)
Index: linux/include/linux/sched/rt.h
===================================================================
--- linux.orig/include/linux/sched/rt.h
+++ linux/include/linux/sched/rt.h
@@ -18,7 +18,7 @@ static inline int rt_task(struct task_st
#ifdef CONFIG_RT_MUTEXES
extern int rt_mutex_getprio(struct task_struct *p);
extern void rt_mutex_setprio(struct task_struct *p, int prio);
-extern int rt_mutex_check_prio(struct task_struct *task, int newprio);
+extern int rt_mutex_get_effective_prio(struct task_struct *task, int newprio);
extern struct task_struct *rt_mutex_get_top_task(struct task_struct *task);
extern void rt_mutex_adjust_pi(struct task_struct *p);
static inline bool tsk_is_pi_blocked(struct task_struct *tsk)
@@ -31,9 +31,10 @@ static inline int rt_mutex_getprio(struc
return p->normal_prio;
}
-static inline int rt_mutex_check_prio(struct task_struct *task, int newprio)
+static inline int rt_mutex_get_effective_prio(struct task_struct *task,
+ int newprio)
{
- return 0;
+ return newprio;
}
static inline struct task_struct *rt_mutex_get_top_task(struct task_struct *task)
Index: linux/kernel/locking/rtmutex.c
===================================================================
--- linux.orig/kernel/locking/rtmutex.c
+++ linux/kernel/locking/rtmutex.c
@@ -265,15 +265,17 @@ struct task_struct *rt_mutex_get_top_tas
}
/*
- * Called by sched_setscheduler() to check whether the priority change
- * is overruled by a possible priority boosting.
+ * Called by sched_setscheduler() to get the priority which will be
+ * effective after the change.
*/
-int rt_mutex_check_prio(struct task_struct *task, int newprio)
+int rt_mutex_get_effective_prio(struct task_struct *task, int newprio)
{
if (!task_has_pi_waiters(task))
- return 0;
+ return newprio;
- return task_top_pi_waiter(task)->task->prio <= newprio;
+ if (task_top_pi_waiter(task)->task->prio <= newprio)
+ return task_top_pi_waiter(task)->task->prio;
+ return newprio;
}
/*
Index: linux/kernel/sched/core.c
===================================================================
--- linux.orig/kernel/sched/core.c
+++ linux/kernel/sched/core.c
@@ -3300,15 +3300,18 @@ static void __setscheduler_params(struct
/* Actually do priority change: must hold pi & rq lock. */
static void __setscheduler(struct rq *rq, struct task_struct *p,
- const struct sched_attr *attr)
+ const struct sched_attr *attr, bool keep_boost)
{
__setscheduler_params(p, attr);
/*
- * If we get here, there was no pi waiters boosting the
- * task. It is safe to use the normal prio.
+ * Keep a potential priority boosting if called from
+ * sched_setscheduler().
*/
- p->prio = normal_prio(p);
+ if (keep_boost)
+ p->prio = rt_mutex_get_effective_prio(p, normal_prio(p));
+ else
+ p->prio = normal_prio(p);
if (dl_prio(p->prio))
p->sched_class = &dl_sched_class;
@@ -3408,7 +3411,7 @@ static int __sched_setscheduler(struct t
int newprio = dl_policy(attr->sched_policy) ? MAX_DL_PRIO - 1 :
MAX_RT_PRIO - 1 - attr->sched_priority;
int retval, oldprio, oldpolicy = -1, queued, running;
- int policy = attr->sched_policy;
+ int new_effective_prio, policy = attr->sched_policy;
unsigned long flags;
const struct sched_class *prev_class;
struct rq *rq;
@@ -3590,15 +3593,14 @@ change:
oldprio = p->prio;
/*
- * Special case for priority boosted tasks.
- *
- * If the new priority is lower or equal (user space view)
- * than the current (boosted) priority, we just store the new
+ * Take priority boosted tasks into account. If the new
+ * effective priority is unchanged, we just store the new
* normal parameters and do not touch the scheduler class and
* the runqueue. This will be done when the task deboost
* itself.
*/
- if (rt_mutex_check_prio(p, newprio)) {
+ new_effective_prio = rt_mutex_get_effective_prio(p, newprio);
+ if (new_effective_prio == oldprio) {
__setscheduler_params(p, attr);
task_rq_unlock(rq, p, &flags);
return 0;
@@ -3612,7 +3614,7 @@ change:
put_prev_task(rq, p);
prev_class = p->sched_class;
- __setscheduler(rq, p, attr);
+ __setscheduler(rq, p, attr, true);
if (running)
p->sched_class->set_curr_task(rq);
@@ -7346,7 +7348,7 @@ static void normalize_task(struct rq *rq
queued = task_on_rq_queued(p);
if (queued)
dequeue_task(rq, p, 0);
- __setscheduler(rq, p, &attr);
+ __setscheduler(rq, p, &attr, false);
if (queued) {
enqueue_task(rq, p, 0);
resched_curr(rq);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2] sched: Handle priority boosted tasks proper in setscheduler()
2015-05-05 17:49 ` [PATCH V2] " Thomas Gleixner
@ 2015-05-05 20:20 ` Steven Rostedt
0 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2015-05-05 20:20 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Mike Galbraith, Ronny Meeus, LKML, Peter Zijlstra
On Tue, 5 May 2015 19:49:49 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:
> Ronny reported that the following scenario is not handled correctly:
>
> T1 (prio = 10)
> lock(rtmutex);
>
> T2 (prio = 20)
> lock(rtmutex)
> boost T1
>
> T1 (prio = 20)
> sys_set_scheduler(prio = 30)
> T1 prio = 30
> ....
> sys_set_scheduler(prio = 10)
> T1 prio = 30
>
> The last step is wrong as T1 should now be back at prio 20.
>
> commit c365c292d0590 "sched: Consider pi boosting in setscheduler()"
> only handles the case where a boosted tasks tries to lower its
> priority.
>
> Fix it by taking the new effective priority into account for the
> decision whether a change of the priority is required.
>
> Reported-by: Ronny Meeus <ronny.meeus@gmail.com>
> Fixes: commit c365c292d0590 "sched: Consider pi boosting in setscheduler()"
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mike Galbraith <umgwanakikbuti@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Link: http://lkml.kernel.org/r/alpine.DEB.2.11.1505051806060.4225@nanos
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Tested-by: Steven Rostedt <rostedt@goodmis.org>
Note, this should be marked stable. I can get a nasty kernel spat with
playing around with RT priorities without this patch.
[ 42.226836] ------------[ cut here ]------------
[ 42.227495] WARNING: CPU: 0 PID: 2271 at /home/rostedt/work/git/linux-trace.git/kernel/sched/rt.c:1114 dequeue_rt_stack+0x220/0x22b()
[ 42.227495] Modules linked in: [..]
[ 42.227495] CPU: 0 PID: 2271 Comm: check_pi_deboos Not tainted 4.1.0-rc1-test+ #415
[ 42.227495] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS SDBLI944.86P 05/08/2007
[ 42.227495] 0000000000000009 ffff880078a5fd28 ffffffff815fcc4c 0000000080000002
[ 42.227495] 0000000000000000 ffff880078a5fd68 ffffffff810496ea 0000000000000000
[ 42.227495] ffffffff81075d5a ffff88007669e208 ffff88007d416460 0000000000000054
[ 42.227495] Call Trace:
[ 42.227495] [<ffffffff815fcc4c>] dump_stack+0x4f/0x7b
[ 42.227495] [<ffffffff810496ea>] warn_slowpath_common+0xa1/0xbb
[ 42.227495] [<ffffffff81075d5a>] ? dequeue_rt_stack+0x220/0x22b
[ 42.227495] [<ffffffff810497a7>] warn_slowpath_null+0x1a/0x1c
[ 42.227495] [<ffffffff81075d5a>] dequeue_rt_stack+0x220/0x22b
[ 42.227495] [<ffffffff810762dc>] dequeue_rt_entity+0x1f/0x58
[ 42.227495] [<ffffffff810767a1>] dequeue_task_rt+0x24/0x34
[ 42.227495] [<ffffffff81068f03>] dequeue_task+0x69/0x70
[ 42.227495] [<ffffffff8106dfd7>] sched_move_task+0x4e/0xdb
[ 42.227495] [<ffffffff8106e07b>] cpu_cgroup_exit+0x17/0x19
[ 42.227495] [<ffffffff810b8065>] cgroup_exit+0x9f/0xbe
[ 42.227495] [<ffffffff8104abc7>] do_exit+0x429/0x92f
[ 42.227495] [<ffffffff8104bdb1>] SyS_exit+0x17/0x17
[ 42.227495] [<ffffffff81604c97>] system_call_fastpath+0x12/0x6a
[ 42.227495] ---[ end trace 14460ffaa77bf181 ]---
[ 42.412352] ------------[ cut here ]------------
-- Steve
^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip:sched/core] sched: Handle priority boosted tasks proper in setscheduler()
2015-05-05 16:08 [PATCH] sched: Handle priority boosted tasks proper in setscheduler() Thomas Gleixner
2015-05-05 16:29 ` Steven Rostedt
@ 2015-05-08 13:18 ` tip-bot for Thomas Gleixner
1 sibling, 0 replies; 9+ messages in thread
From: tip-bot for Thomas Gleixner @ 2015-05-08 13:18 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, bp, rostedt, ronny.meeus, stable, tglx, hpa, mingo,
umgwanakikbuti, peterz
Commit-ID: 0782e63bc6fe7e2d3408d250df11d388b7799c6b
Gitweb: http://git.kernel.org/tip/0782e63bc6fe7e2d3408d250df11d388b7799c6b
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 5 May 2015 19:49:49 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 8 May 2015 11:53:55 +0200
sched: Handle priority boosted tasks proper in setscheduler()
Ronny reported that the following scenario is not handled correctly:
T1 (prio = 10)
lock(rtmutex);
T2 (prio = 20)
lock(rtmutex)
boost T1
T1 (prio = 20)
sys_set_scheduler(prio = 30)
T1 prio = 30
....
sys_set_scheduler(prio = 10)
T1 prio = 30
The last step is wrong as T1 should now be back at prio 20.
Commit c365c292d059 ("sched: Consider pi boosting in setscheduler()")
only handles the case where a boosted tasks tries to lower its
priority.
Fix it by taking the new effective priority into account for the
decision whether a change of the priority is required.
Reported-by: Ronny Meeus <ronny.meeus@gmail.com>
Tested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Cc: <stable@vger.kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Mike Galbraith <umgwanakikbuti@gmail.com>
Fixes: c365c292d059 ("sched: Consider pi boosting in setscheduler()")
Link: http://lkml.kernel.org/r/alpine.DEB.2.11.1505051806060.4225@nanos
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/sched/rt.h | 7 ++++---
kernel/locking/rtmutex.c | 12 +++++++-----
kernel/sched/core.c | 26 ++++++++++++++------------
3 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h
index 6341f5b..a30b172 100644
--- a/include/linux/sched/rt.h
+++ b/include/linux/sched/rt.h
@@ -18,7 +18,7 @@ static inline int rt_task(struct task_struct *p)
#ifdef CONFIG_RT_MUTEXES
extern int rt_mutex_getprio(struct task_struct *p);
extern void rt_mutex_setprio(struct task_struct *p, int prio);
-extern int rt_mutex_check_prio(struct task_struct *task, int newprio);
+extern int rt_mutex_get_effective_prio(struct task_struct *task, int newprio);
extern struct task_struct *rt_mutex_get_top_task(struct task_struct *task);
extern void rt_mutex_adjust_pi(struct task_struct *p);
static inline bool tsk_is_pi_blocked(struct task_struct *tsk)
@@ -31,9 +31,10 @@ static inline int rt_mutex_getprio(struct task_struct *p)
return p->normal_prio;
}
-static inline int rt_mutex_check_prio(struct task_struct *task, int newprio)
+static inline int rt_mutex_get_effective_prio(struct task_struct *task,
+ int newprio)
{
- return 0;
+ return newprio;
}
static inline struct task_struct *rt_mutex_get_top_task(struct task_struct *task)
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index b732793..b025295 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -265,15 +265,17 @@ struct task_struct *rt_mutex_get_top_task(struct task_struct *task)
}
/*
- * Called by sched_setscheduler() to check whether the priority change
- * is overruled by a possible priority boosting.
+ * Called by sched_setscheduler() to get the priority which will be
+ * effective after the change.
*/
-int rt_mutex_check_prio(struct task_struct *task, int newprio)
+int rt_mutex_get_effective_prio(struct task_struct *task, int newprio)
{
if (!task_has_pi_waiters(task))
- return 0;
+ return newprio;
- return task_top_pi_waiter(task)->task->prio <= newprio;
+ if (task_top_pi_waiter(task)->task->prio <= newprio)
+ return task_top_pi_waiter(task)->task->prio;
+ return newprio;
}
/*
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fe22f75..34db9bf 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3300,15 +3300,18 @@ static void __setscheduler_params(struct task_struct *p,
/* Actually do priority change: must hold pi & rq lock. */
static void __setscheduler(struct rq *rq, struct task_struct *p,
- const struct sched_attr *attr)
+ const struct sched_attr *attr, bool keep_boost)
{
__setscheduler_params(p, attr);
/*
- * If we get here, there was no pi waiters boosting the
- * task. It is safe to use the normal prio.
+ * Keep a potential priority boosting if called from
+ * sched_setscheduler().
*/
- p->prio = normal_prio(p);
+ if (keep_boost)
+ p->prio = rt_mutex_get_effective_prio(p, normal_prio(p));
+ else
+ p->prio = normal_prio(p);
if (dl_prio(p->prio))
p->sched_class = &dl_sched_class;
@@ -3408,7 +3411,7 @@ static int __sched_setscheduler(struct task_struct *p,
int newprio = dl_policy(attr->sched_policy) ? MAX_DL_PRIO - 1 :
MAX_RT_PRIO - 1 - attr->sched_priority;
int retval, oldprio, oldpolicy = -1, queued, running;
- int policy = attr->sched_policy;
+ int new_effective_prio, policy = attr->sched_policy;
unsigned long flags;
const struct sched_class *prev_class;
struct rq *rq;
@@ -3590,15 +3593,14 @@ change:
oldprio = p->prio;
/*
- * Special case for priority boosted tasks.
- *
- * If the new priority is lower or equal (user space view)
- * than the current (boosted) priority, we just store the new
+ * Take priority boosted tasks into account. If the new
+ * effective priority is unchanged, we just store the new
* normal parameters and do not touch the scheduler class and
* the runqueue. This will be done when the task deboost
* itself.
*/
- if (rt_mutex_check_prio(p, newprio)) {
+ new_effective_prio = rt_mutex_get_effective_prio(p, newprio);
+ if (new_effective_prio == oldprio) {
__setscheduler_params(p, attr);
task_rq_unlock(rq, p, &flags);
return 0;
@@ -3612,7 +3614,7 @@ change:
put_prev_task(rq, p);
prev_class = p->sched_class;
- __setscheduler(rq, p, attr);
+ __setscheduler(rq, p, attr, true);
if (running)
p->sched_class->set_curr_task(rq);
@@ -7346,7 +7348,7 @@ static void normalize_task(struct rq *rq, struct task_struct *p)
queued = task_on_rq_queued(p);
if (queued)
dequeue_task(rq, p, 0);
- __setscheduler(rq, p, &attr);
+ __setscheduler(rq, p, &attr, false);
if (queued) {
enqueue_task(rq, p, 0);
resched_curr(rq);
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-05-08 13:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-05 16:08 [PATCH] sched: Handle priority boosted tasks proper in setscheduler() Thomas Gleixner
2015-05-05 16:29 ` Steven Rostedt
2015-05-05 16:31 ` Thomas Gleixner
2015-05-05 16:42 ` Steven Rostedt
2015-05-05 16:50 ` Thomas Gleixner
2015-05-05 17:01 ` Steven Rostedt
2015-05-05 17:49 ` [PATCH V2] " Thomas Gleixner
2015-05-05 20:20 ` Steven Rostedt
2015-05-08 13:18 ` [tip:sched/core] " tip-bot for Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).