[PATCH] kernel/sched/rt: Add a rescheduling point

[PATCH] kernel/sched/rt: Add a rescheduling point

[Sebastian Andrzej Siewior]

Since the change in commit fd7a4bed1835 ("sched, rt: Convert
switched_{from, to}_rt() / prio_changed_rt() to balance callbacks") we
don't reschedule a task under certain circumstances:

Lets say taskA, SCHED_OTHER, is running on CPU0 (and it may run only on
CPU0) and holds a PI lock. This task is removed from the CPU because it
used up its time slice and another SCHED_OTHER task is running. TaskB on
CPU1 runs at RT priority and asks for the lock owned by taskA. This
results in a priority boost for taskA. TaskB goes to sleep until the
lock has been made available. TaskA is already runable (but not active)
so it receives no wake up.
The reality now is that taskA gets on the CPU once the scheduler decides
to remove the current task despite the fact that a high priority task is
enqueued and waiting. This may take a long time.
The desired behaviour is that CPU0 immediately reschedules after the
priority boost which made taskA the task with the lowest priority.

Fixes: fd7a4bed1835 ("sched, rt: Convert switched_{from, to}_rt() /
		     prio_changed_rt() to balance callbacks")
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/sched/rt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 88254be118b0..cdba8d58dbc5 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -2198,10 +2198,10 @@ static void switched_to_rt(struct rq *rq, struct task_struct *p)
 #ifdef CONFIG_SMP
 		if (tsk_nr_cpus_allowed(p) > 1 && rq->rt.overloaded)
 			queue_push_tasks(rq);
-#else
+		else
+#endif /* CONFIG_SMP */
 		if (p->prio < rq->curr->prio)
 			resched_curr(rq);
-#endif /* CONFIG_SMP */
 	}
 }
 
-- 
2.11.0

Re: [PATCH] kernel/sched/rt: Add a rescheduling point

[Sebastian Andrzej Siewior]

On 2017-01-24 15:40:06 [+0100], To Peter Zijlstra wrote:
> Since the change in commit fd7a4bed1835 ("sched, rt: Convert
> switched_{from, to}_rt() / prio_changed_rt() to balance callbacks") we
> don't reschedule a task under certain circumstances:

|switched_to_dl()
|{
|…
|         if (rq->curr != p) {
| #ifdef CONFIG_SMP
|                 if (tsk_nr_cpus_allowed(p) > 1 && rq->dl.overloaded)
|                         queue_push_tasks(rq);
| #else
|                 if (dl_task(rq->curr))
|                         check_preempt_curr_dl(rq, p, 0);
|                 else
|                         resched_curr(rq);
| #endif
|         }
| }

This looks like it asks for a similar change.

Sebastian

Re: [PATCH] kernel/sched/rt: Add a rescheduling point

[Peter Zijlstra]

On Tue, Jan 24, 2017 at 03:43:54PM +0100, Sebastian Andrzej Siewior wrote:
> On 2017-01-24 15:40:06 [+0100], To Peter Zijlstra wrote:
> > Since the change in commit fd7a4bed1835 ("sched, rt: Convert
> > switched_{from, to}_rt() / prio_changed_rt() to balance callbacks") we
> > don't reschedule a task under certain circumstances:
> 
> |switched_to_dl()
> |{
> |…
> |         if (rq->curr != p) {
> | #ifdef CONFIG_SMP
> |                 if (tsk_nr_cpus_allowed(p) > 1 && rq->dl.overloaded)
> |                         queue_push_tasks(rq);
> | #else
> |                 if (dl_task(rq->curr))
> |                         check_preempt_curr_dl(rq, p, 0);
> |                 else
> |                         resched_curr(rq);
> | #endif
> |         }
> | }
> 
> This looks like it asks for a similar change.

Indeed.

Re: [PATCH] kernel/sched/rt: Add a rescheduling point

[Peter Zijlstra]

On Tue, Jan 24, 2017 at 03:40:06PM +0100, Sebastian Andrzej Siewior wrote:
> Since the change in commit fd7a4bed1835 ("sched, rt: Convert
> switched_{from, to}_rt() / prio_changed_rt() to balance callbacks") we
> don't reschedule a task under certain circumstances:
> 
> Lets say taskA, SCHED_OTHER, is running on CPU0 (and it may run only on
> CPU0) and holds a PI lock. This task is removed from the CPU because it
> used up its time slice and another SCHED_OTHER task is running. TaskB on
> CPU1 runs at RT priority and asks for the lock owned by taskA. This
> results in a priority boost for taskA. TaskB goes to sleep until the
> lock has been made available. TaskA is already runable (but not active)
> so it receives no wake up.
> The reality now is that taskA gets on the CPU once the scheduler decides
> to remove the current task despite the fact that a high priority task is
> enqueued and waiting. This may take a long time.
> The desired behaviour is that CPU0 immediately reschedules after the
> priority boost which made taskA the task with the lowest priority.
> 
> Fixes: fd7a4bed1835 ("sched, rt: Convert switched_{from, to}_rt() /
> 		     prio_changed_rt() to balance callbacks")
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  kernel/sched/rt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 88254be118b0..cdba8d58dbc5 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -2198,10 +2198,10 @@ static void switched_to_rt(struct rq *rq, struct task_struct *p)
>  #ifdef CONFIG_SMP
>  		if (tsk_nr_cpus_allowed(p) > 1 && rq->rt.overloaded)
>  			queue_push_tasks(rq);
> -#else
> +		else

I killed that "else" as well, because the queue_push_tasks() can fail to
actually push the task, in which case we'd still miss the preemption.

> +#endif /* CONFIG_SMP */
>  		if (p->prio < rq->curr->prio)
>  			resched_curr(rq);
> -#endif /* CONFIG_SMP */
>  	}
>  }
>  
> -- 
> 2.11.0
> 

[tip:sched/core] sched/rt: Add a missing rescheduling point

[tip-bot for Sebastian Andrzej Siewior]

Commit-ID:  619bd4a71874a8fd78eb6ccf9f272c5e98bcc7b7
Gitweb:     http://git.kernel.org/tip/619bd4a71874a8fd78eb6ccf9f272c5e98bcc7b7
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Tue, 24 Jan 2017 15:40:06 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 30 Jan 2017 11:46:37 +0100

sched/rt: Add a missing rescheduling point

Since the change in commit:

  fd7a4bed1835 ("sched, rt: Convert switched_{from, to}_rt() / prio_changed_rt() to balance callbacks")

... we don't reschedule a task under certain circumstances:

Lets say task-A, SCHED_OTHER, is running on CPU0 (and it may run only on
CPU0) and holds a PI lock. This task is removed from the CPU because it
used up its time slice and another SCHED_OTHER task is running. Task-B on
CPU1 runs at RT priority and asks for the lock owned by task-A. This
results in a priority boost for task-A. Task-B goes to sleep until the
lock has been made available. Task-A is already runnable (but not active),
so it receives no wake up.

The reality now is that task-A gets on the CPU once the scheduler decides
to remove the current task despite the fact that a high priority task is
enqueued and waiting. This may take a long time.

The desired behaviour is that CPU0 immediately reschedules after the
priority boost which made task-A the task with the lowest priority.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: fd7a4bed1835 ("sched, rt: Convert switched_{from, to}_rt() prio_changed_rt() to balance callbacks")
Link: http://lkml.kernel.org/r/20170124144006.29821-1-bigeasy@linutronix.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/deadline.c | 3 +--
 kernel/sched/rt.c       | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 491ff66..27737f3 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1729,12 +1729,11 @@ static void switched_to_dl(struct rq *rq, struct task_struct *p)
 #ifdef CONFIG_SMP
 		if (tsk_nr_cpus_allowed(p) > 1 && rq->dl.overloaded)
 			queue_push_tasks(rq);
-#else
+#endif
 		if (dl_task(rq->curr))
 			check_preempt_curr_dl(rq, p, 0);
 		else
 			resched_curr(rq);
-#endif
 	}
 }
 
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 88254be..704f2b8 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -2198,10 +2198,9 @@ static void switched_to_rt(struct rq *rq, struct task_struct *p)
 #ifdef CONFIG_SMP
 		if (tsk_nr_cpus_allowed(p) > 1 && rq->rt.overloaded)
 			queue_push_tasks(rq);
-#else
+#endif /* CONFIG_SMP */
 		if (p->prio < rq->curr->prio)
 			resched_curr(rq);
-#endif /* CONFIG_SMP */
 	}
 }