Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1340584 > unrolled thread
| Started by | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| First post | 2016-02-23 13:30 +0100 |
| Last post | 2016-02-29 12:20 +0100 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH] sched/deadline: Always calculate end of period on sched_yield() Peter Zijlstra <peterz@infradead.org> - 2016-02-23 13:30 +0100
Re: [PATCH] sched/deadline: Always calculate end of period on sched_yield() Steven Rostedt <rostedt@goodmis.org> - 2016-02-23 14:20 +0100
Re: [PATCH] sched/deadline: Always calculate end of period on sched_yield() Steven Rostedt <rostedt@goodmis.org> - 2016-02-23 16:10 +0100
[tip:sched/core] sched/deadline: Always calculate end of period on sched_yield() tip-bot for Peter Zijlstra <tipbot@zytor.com> - 2016-02-29 12:20 +0100
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-02-23 13:30 +0100 |
| Subject | Re: [PATCH] sched/deadline: Always calculate end of period on sched_yield() |
| Message-ID | <r5kgi-5mq-17@gated-at.bofh.it> |
On Fri, Feb 12, 2016 at 06:10:20PM -0500, Steven Rostedt wrote:
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index cd64c979d0e1..1dd180cda574 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -735,7 +735,7 @@ static void update_curr_dl(struct rq *rq)
> * approach need further study.
> */
> delta_exec = rq_clock_task(rq) - curr->se.exec_start;
> - if (unlikely((s64)delta_exec <= 0))
> + if (unlikely((s64)delta_exec <= 0 && !dl_se->dl_yielded))
> return;
>
> schedstat_set(curr->se.statistics.exec_max,
Would something like this make sense instead?
It also retains the ->runtime while yielded, and would actually 'fix' a
case where, when we call yield, we would have had a negative runtime
after update_curr_dl().
The current code will 'gift' us extra runtime in that case.
---
kernel/sched/deadline.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 57b939c81bce..c2bca80d3388 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -399,6 +399,9 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se,
dl_se->runtime = pi_se->dl_runtime;
}
+ if (dl_se->dl_yielded && dl_se->runtime > 0)
+ dl_se->runtime = 0;
+
/*
* We keep moving the deadline away until we get some
* available runtime for the entity. This ensures correct
@@ -735,8 +738,11 @@ static void update_curr_dl(struct rq *rq)
* approach need further study.
*/
delta_exec = rq_clock_task(rq) - curr->se.exec_start;
- if (unlikely((s64)delta_exec <= 0))
+ if (unlikely((s64)delta_exec <= 0)) {
+ if (unlikely(dl_se->dl_yielded))
+ goto throttle;
return;
+ }
schedstat_set(curr->se.statistics.exec_max,
max(curr->se.statistics.exec_max, delta_exec));
@@ -749,8 +755,10 @@ static void update_curr_dl(struct rq *rq)
sched_rt_avg_update(rq, delta_exec);
- dl_se->runtime -= dl_se->dl_yielded ? 0 : delta_exec;
- if (dl_runtime_exceeded(dl_se)) {
+ dl_se->runtime -= delta_exec;
+
+throttle:
+ if (dl_runtime_exceeded(dl_se) || dl_se->dl_yielded) {
dl_se->dl_throttled = 1;
__dequeue_task_dl(rq, curr, 0);
if (unlikely(dl_se->dl_boosted || !start_dl_timer(curr)))
@@ -1002,10 +1010,8 @@ static void yield_task_dl(struct rq *rq)
* it and the bandwidth timer will wake it up and will give it
* new scheduling parameters (thanks to dl_yielded=1).
*/
- if (p->dl.runtime > 0) {
- rq->curr->dl.dl_yielded = 1;
- p->dl.runtime = 0;
- }
+ rq->curr->dl.dl_yielded = 1;
+
update_rq_clock(rq);
update_curr_dl(rq);
/*
[toc] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2016-02-23 14:20 +0100 |
| Message-ID | <r5l2G-5VT-11@gated-at.bofh.it> |
| In reply to | #1340584 |
On Tue, 23 Feb 2016 13:28:22 +0100 Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Feb 12, 2016 at 06:10:20PM -0500, Steven Rostedt wrote: > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > > index cd64c979d0e1..1dd180cda574 100644 > > --- a/kernel/sched/deadline.c > > +++ b/kernel/sched/deadline.c > > @@ -735,7 +735,7 @@ static void update_curr_dl(struct rq *rq) > > * approach need further study. > > */ > > delta_exec = rq_clock_task(rq) - curr->se.exec_start; > > - if (unlikely((s64)delta_exec <= 0)) > > + if (unlikely((s64)delta_exec <= 0 && !dl_se->dl_yielded)) > > return; > > > > schedstat_set(curr->se.statistics.exec_max, > > > Would something like this make sense instead? > I'll test it and see if it works. -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2016-02-23 16:10 +0100 |
| Message-ID | <r5mL7-7at-11@gated-at.bofh.it> |
| In reply to | #1340584 |
On Tue, 23 Feb 2016 13:28:22 +0100 Peter Zijlstra <peterz@infradead.org> wrote: > Would something like this make sense instead? It works perfectly. Reported-by: Steven Rostedt <rostedt@goodmis.org> Tested-by: Steven Rostedt <rostedt@goodmis.org> Thanks! -- Steve > > It also retains the ->runtime while yielded, and would actually 'fix' a > case where, when we call yield, we would have had a negative runtime > after update_curr_dl(). > > The current code will 'gift' us extra runtime in that case. > > ---
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Peter Zijlstra <tipbot@zytor.com> |
|---|---|
| Date | 2016-02-29 12:20 +0100 |
| Subject | [tip:sched/core] sched/deadline: Always calculate end of period on sched_yield() |
| Message-ID | <r7u1S-2ia-51@gated-at.bofh.it> |
| In reply to | #1340584 |
Commit-ID: 48be3a67da7413d62e5efbcf2c73a9dddf61fb96
Gitweb: http://git.kernel.org/tip/48be3a67da7413d62e5efbcf2c73a9dddf61fb96
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Tue, 23 Feb 2016 13:28:22 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 29 Feb 2016 09:41:51 +0100
sched/deadline: Always calculate end of period on sched_yield()
Steven noticed that occasionally a sched_yield() call would not result
in a wait for the next period edge as expected.
It turns out that when we call update_curr_dl() and end up with
delta_exec <= 0, we will bail early and fail to throttle.
Further inspection of the yield code revealed that yield_task_dl()
clearing dl.runtime is wrong too, it will not account the last bit of
runtime which could result in dl.runtime < 0, which in turn means that
replenish would gift us with too much runtime.
Fix both issues by not relying on the dl.runtime value for yield.
Reported-by: Steven Rostedt <rostedt@goodmis.org>
Tested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Clark Williams <williams@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: Juri Lelli <juri.lelli@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20160223122822.GP6357@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/deadline.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 57b939c..04a569c 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -399,6 +399,9 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se,
dl_se->runtime = pi_se->dl_runtime;
}
+ if (dl_se->dl_yielded && dl_se->runtime > 0)
+ dl_se->runtime = 0;
+
/*
* We keep moving the deadline away until we get some
* available runtime for the entity. This ensures correct
@@ -735,8 +738,11 @@ static void update_curr_dl(struct rq *rq)
* approach need further study.
*/
delta_exec = rq_clock_task(rq) - curr->se.exec_start;
- if (unlikely((s64)delta_exec <= 0))
+ if (unlikely((s64)delta_exec <= 0)) {
+ if (unlikely(dl_se->dl_yielded))
+ goto throttle;
return;
+ }
schedstat_set(curr->se.statistics.exec_max,
max(curr->se.statistics.exec_max, delta_exec));
@@ -749,8 +755,10 @@ static void update_curr_dl(struct rq *rq)
sched_rt_avg_update(rq, delta_exec);
- dl_se->runtime -= dl_se->dl_yielded ? 0 : delta_exec;
- if (dl_runtime_exceeded(dl_se)) {
+ dl_se->runtime -= delta_exec;
+
+throttle:
+ if (dl_runtime_exceeded(dl_se) || dl_se->dl_yielded) {
dl_se->dl_throttled = 1;
__dequeue_task_dl(rq, curr, 0);
if (unlikely(dl_se->dl_boosted || !start_dl_timer(curr)))
@@ -994,18 +1002,14 @@ static void dequeue_task_dl(struct rq *rq, struct task_struct *p, int flags)
*/
static void yield_task_dl(struct rq *rq)
{
- struct task_struct *p = rq->curr;
-
/*
* We make the task go to sleep until its current deadline by
* forcing its runtime to zero. This way, update_curr_dl() stops
* it and the bandwidth timer will wake it up and will give it
* new scheduling parameters (thanks to dl_yielded=1).
*/
- if (p->dl.runtime > 0) {
- rq->curr->dl.dl_yielded = 1;
- p->dl.runtime = 0;
- }
+ rq->curr->dl.dl_yielded = 1;
+
update_rq_clock(rq);
update_curr_dl(rq);
/*
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web