All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Juri Lelli <juri.lelli@gmail.com>, Ingo Molnar <mingo@kernel.org>,
	Clark Williams <williams@redhat.com>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	John Kacur <jkacur@redhat.com>
Subject: Re: [PATCH] sched/deadline: Always calculate end of period on sched_yield()
Date: Tue, 23 Feb 2016 13:28:22 +0100	[thread overview]
Message-ID: <20160223122822.GP6357@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20160212181020.7dbaf326@gandalf.local.home>

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);
 	/*

  parent reply	other threads:[~2016-02-23 12:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-12 23:10 [PATCH] sched/deadline: Always calculate end of period on sched_yield() Steven Rostedt
2016-02-15 10:18 ` Juri Lelli
2016-02-15 12:37   ` Daniel Bristot de Oliveira
2016-02-15 16:22   ` Steven Rostedt
2016-02-23 12:28 ` Peter Zijlstra [this message]
2016-02-23 13:12   ` Steven Rostedt
2016-02-23 15:04   ` Steven Rostedt
2016-02-29 11:14   ` [tip:sched/core] " tip-bot for Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160223122822.GP6357@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bristot@redhat.com \
    --cc=jkacur@redhat.com \
    --cc=juri.lelli@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=williams@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.