All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] sched: Kick bandwidth timer immediately on start up
@ 2016-02-16 23:37 Steven Rostedt
  2016-02-18 14:15 ` Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Steven Rostedt @ 2016-02-16 23:37 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Juri Lelli, Ingo Molnar, Clark Williams,
	Daniel Bristot de Oliveira, John Kacur, Thomas Gleixner

I've been debugging why deadline tasks can cause the RT scheduler to
throttle, even when the deadline tasks are only taking up 50% of the
CPU and RT tasks are not even using 1% of the CPU. Here's what I found.

In order to keep a CPU from being hogged by RT tasks, the deadline
scheduler adds its run time (delta_exec) to the rt_time of the RT
bandwidth. That way, if the two use more than 95% of the CPU within one
second (default settings), the RT tasks are throttled to allow non RT
tasks to run.

Although the deadline tasks add their run time to the RT bandwidth, it
lets the RT tasks do the accounting. This is where the problem lies. If
a deadline task runs for a bit, and no RT tasks are running, then it
will continually add to the RT rt_time that is used to calculate how
much CPU the RT tasks use. But no RT period is in play, and this
accumulation of the runtime never gets reset.

When an RT task finally gets to run, and the watchdog goes off, it can
see that the RT task has used more than it should of, because the
deadline task added all this runtime to its rt_time. Then the RT task
that just woke up gets throttled for no good reason.

I also noticed that when an RT task is queued, it starts the timer to
account for overload and such. But that timer goes off one period
later, which may be too late and the extra rt_time will trigger a
throttle.

This is a quick work around to the problem. When a new RT task is
queued, the bandwidth timer is set to go off immediately. Then the
timer can clear out the extra time added to the rt_time while there was
no RT task running. This stops my tests from triggering the throttle,
and it will still throttle if an RT task runs too much, even while a
deadline task is running.

A better solution may be to subtract the bandwidth that the deadline
task uses from the rt_runtime, and add it back when its finished. Then
there wont be a need for runtime tracking of the time used by deadline
tasks.

I may play with that solution tomorrow.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/sched/rt.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Index: linux-trace.git/kernel/sched/rt.c
===================================================================
--- linux-trace.git.orig/kernel/sched/rt.c	2016-02-16 16:31:12.041035819 -0500
+++ linux-trace.git/kernel/sched/rt.c	2016-02-16 16:31:19.997905282 -0500
@@ -58,7 +58,15 @@ static void start_rt_bandwidth(struct rt
 	raw_spin_lock(&rt_b->rt_runtime_lock);
 	if (!rt_b->rt_period_active) {
 		rt_b->rt_period_active = 1;
-		hrtimer_forward_now(&rt_b->rt_period_timer, rt_b->rt_period);
+		/*
+		 * SCHED_DEADLINE updates the bandwidth, as a run away
+		 * RT task with a DL task could hog a CPU. But DL does
+		 * not reset the period. If a deadline task was running
+		 * without an RT task running, it can cause RT tasks to
+		 * throttle when they start up. Kick the timer right away
+		 * to update the period.
+		 */
+		hrtimer_forward_now(&rt_b->rt_period_timer, ns_to_ktime(0));
 		hrtimer_start_expires(&rt_b->rt_period_timer, HRTIMER_MODE_ABS_PINNED);
 	}
 	raw_spin_unlock(&rt_b->rt_runtime_lock);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC][PATCH] sched: Kick bandwidth timer immediately on start up
  2016-02-16 23:37 [RFC][PATCH] sched: Kick bandwidth timer immediately on start up Steven Rostedt
@ 2016-02-18 14:15 ` Steven Rostedt
  2016-02-19  8:24   ` Juri Lelli
  2016-02-23 14:29 ` Peter Zijlstra
  2016-02-29 11:17 ` [tip:sched/core] sched/rt: Kick RT " tip-bot for Steven Rostedt
  2 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2016-02-18 14:15 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Juri Lelli, Ingo Molnar, Clark Williams,
	Daniel Bristot de Oliveira, John Kacur, Thomas Gleixner

On Tue, 16 Feb 2016 18:37:46 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> A better solution may be to subtract the bandwidth that the deadline
> task uses from the rt_runtime, and add it back when its finished. Then
> there wont be a need for runtime tracking of the time used by deadline
> tasks.
> 
> I may play with that solution tomorrow.
> 

OK, so I played with this solution. It's much more complex than I was
hoping it to be. The main issue is there's no one to one relationship
with the deadline bandwidth and the rt bandwidth. Each runqueue has its
own rt ratio, but each root domain that has its own deadline ratio.
Thus, when you add to the dl ratio, it makes adding that to the rt
bandwidths that more complex. It's doable, but it will make getting the
dl_bw right with new root domains harder than it already is, and that
is currently not working.

My recommendation is to hold off on a better solution till we can find a
way to merge rt bandwidth with the Constant Bandwidth Server (CBS).

Thus, please accept this current patch as it appears to fix the problem
without any other side effects that I can find.

-- Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC][PATCH] sched: Kick bandwidth timer immediately on start up
  2016-02-18 14:15 ` Steven Rostedt
@ 2016-02-19  8:24   ` Juri Lelli
  0 siblings, 0 replies; 5+ messages in thread
From: Juri Lelli @ 2016-02-19  8:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Peter Zijlstra, Juri Lelli, Ingo Molnar, Clark Williams,
	Daniel Bristot de Oliveira, John Kacur, Thomas Gleixner

Hi Steve,

On 18/02/16 09:15, Steven Rostedt wrote:
> On Tue, 16 Feb 2016 18:37:46 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > 
> > A better solution may be to subtract the bandwidth that the deadline
> > task uses from the rt_runtime, and add it back when its finished. Then
> > there wont be a need for runtime tracking of the time used by deadline
> > tasks.
> > 
> > I may play with that solution tomorrow.
> > 
> 
> OK, so I played with this solution. It's much more complex than I was
> hoping it to be. The main issue is there's no one to one relationship
> with the deadline bandwidth and the rt bandwidth. Each runqueue has its
> own rt ratio, but each root domain that has its own deadline ratio.
> Thus, when you add to the dl ratio, it makes adding that to the rt
> bandwidths that more complex. It's doable, but it will make getting the
> dl_bw right with new root domains harder than it already is, and that
> is currently not working.
> 
> My recommendation is to hold off on a better solution till we can find a
> way to merge rt bandwidth with the Constant Bandwidth Server (CBS).
> 

I couldn't yet test your change, but it makes sense to me. IIUC, the
work around is only used if an RT task gets enqueued and the rt_period
is not active yet. So, it should fix the over accumulation of rt_time
due to DL and still work for RT, since if a second RT task gets enqueued
when the period is already active it will be eventually throttled when
no more runtime is available.

And yes, we should be able to come up with a better fix once DL will
have groups support. However, since that we'll require some time, I also
think we could work with what you propose for the time being.

Best,

- Juri

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC][PATCH] sched: Kick bandwidth timer immediately on start up
  2016-02-16 23:37 [RFC][PATCH] sched: Kick bandwidth timer immediately on start up Steven Rostedt
  2016-02-18 14:15 ` Steven Rostedt
@ 2016-02-23 14:29 ` Peter Zijlstra
  2016-02-29 11:17 ` [tip:sched/core] sched/rt: Kick RT " tip-bot for Steven Rostedt
  2 siblings, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2016-02-23 14:29 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Juri Lelli, Ingo Molnar, Clark Williams,
	Daniel Bristot de Oliveira, John Kacur, Thomas Gleixner

On Tue, Feb 16, 2016 at 06:37:46PM -0500, Steven Rostedt wrote:
> Index: linux-trace.git/kernel/sched/rt.c
> ===================================================================
> --- linux-trace.git.orig/kernel/sched/rt.c	2016-02-16 16:31:12.041035819 -0500
> +++ linux-trace.git/kernel/sched/rt.c	2016-02-16 16:31:19.997905282 -0500
> @@ -58,7 +58,15 @@ static void start_rt_bandwidth(struct rt
>  	raw_spin_lock(&rt_b->rt_runtime_lock);
>  	if (!rt_b->rt_period_active) {
>  		rt_b->rt_period_active = 1;
> -		hrtimer_forward_now(&rt_b->rt_period_timer, rt_b->rt_period);
> +		/*
> +		 * SCHED_DEADLINE updates the bandwidth, as a run away
> +		 * RT task with a DL task could hog a CPU. But DL does
> +		 * not reset the period. If a deadline task was running
> +		 * without an RT task running, it can cause RT tasks to
> +		 * throttle when they start up. Kick the timer right away
> +		 * to update the period.
> +		 */
> +		hrtimer_forward_now(&rt_b->rt_period_timer, ns_to_ktime(0));

That's a bit icky, but yeah, probably the best we can do without making
a giant mess of things..

>  		hrtimer_start_expires(&rt_b->rt_period_timer, HRTIMER_MODE_ABS_PINNED);
>  	}
>  	raw_spin_unlock(&rt_b->rt_runtime_lock);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip:sched/core] sched/rt: Kick RT bandwidth timer immediately on start up
  2016-02-16 23:37 [RFC][PATCH] sched: Kick bandwidth timer immediately on start up Steven Rostedt
  2016-02-18 14:15 ` Steven Rostedt
  2016-02-23 14:29 ` Peter Zijlstra
@ 2016-02-29 11:17 ` tip-bot for Steven Rostedt
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Steven Rostedt @ 2016-02-29 11:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, efault, hpa, linux-kernel, juri.lelli, bristot, mingo,
	williams, torvalds, jkacur, rostedt, peterz

Commit-ID:  c3a990dc9fab590fb88608410f1cc2bc866bdf30
Gitweb:     http://git.kernel.org/tip/c3a990dc9fab590fb88608410f1cc2bc866bdf30
Author:     Steven Rostedt <rostedt@goodmis.org>
AuthorDate: Tue, 16 Feb 2016 18:37:46 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 29 Feb 2016 09:53:07 +0100

sched/rt: Kick RT bandwidth timer immediately on start up

I've been debugging why deadline tasks can cause the RT scheduler to
throttle, even when the deadline tasks are only taking up 50% of the
CPU and RT tasks are not even using 1% of the CPU. Here's what I found.

In order to keep a CPU from being hogged by RT tasks, the deadline
scheduler adds its run time (delta_exec) to the rt_time of the RT
bandwidth. That way, if the two use more than 95% of the CPU within one
second (default settings), the RT tasks are throttled to allow non RT
tasks to run.

Although the deadline tasks add their run time to the RT bandwidth, it
lets the RT tasks do the accounting. This is where the problem lies. If
a deadline task runs for a bit, and no RT tasks are running, then it
will continually add to the RT rt_time that is used to calculate how
much CPU the RT tasks use. But no RT period is in play, and this
accumulation of the runtime never gets reset.

When an RT task finally gets to run, and the watchdog goes off, it can
see that the RT task has used more than it should of, because the
deadline task added all this runtime to its rt_time. Then the RT task
that just woke up gets throttled for no good reason.

I also noticed that when an RT task is queued, it starts the timer to
account for overload and such. But that timer goes off one period
later, which may be too late and the extra rt_time will trigger a
throttle.

This is a quick work around to the problem. When a new RT task is
queued, the bandwidth timer is set to go off immediately. Then the
timer can clear out the extra time added to the rt_time while there was
no RT task running. This stops my tests from triggering the throttle,
and it will still throttle if an RT task runs too much, even while a
deadline task is running.

A better solution may be to subtract the bandwidth that the deadline
task uses from the rt_runtime, and add it back when its finished. Then
there wont be a need for runtime tracking of the time used by deadline
tasks.

I may play with that solution tomorrow.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <juri.lelli@gmail.com>
Cc: <williams@redhat.com>
Cc: Clark Williams
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: Juri Lelli
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20160216183746.349ec98b@gandalf.local.home
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/rt.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 406a9c2..a774b4d 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -58,7 +58,15 @@ static void start_rt_bandwidth(struct rt_bandwidth *rt_b)
 	raw_spin_lock(&rt_b->rt_runtime_lock);
 	if (!rt_b->rt_period_active) {
 		rt_b->rt_period_active = 1;
-		hrtimer_forward_now(&rt_b->rt_period_timer, rt_b->rt_period);
+		/*
+		 * SCHED_DEADLINE updates the bandwidth, as a run away
+		 * RT task with a DL task could hog a CPU. But DL does
+		 * not reset the period. If a deadline task was running
+		 * without an RT task running, it can cause RT tasks to
+		 * throttle when they start up. Kick the timer right away
+		 * to update the period.
+		 */
+		hrtimer_forward_now(&rt_b->rt_period_timer, ns_to_ktime(0));
 		hrtimer_start_expires(&rt_b->rt_period_timer, HRTIMER_MODE_ABS_PINNED);
 	}
 	raw_spin_unlock(&rt_b->rt_runtime_lock);

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-02-29 11:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-16 23:37 [RFC][PATCH] sched: Kick bandwidth timer immediately on start up Steven Rostedt
2016-02-18 14:15 ` Steven Rostedt
2016-02-19  8:24   ` Juri Lelli
2016-02-23 14:29 ` Peter Zijlstra
2016-02-29 11:17 ` [tip:sched/core] sched/rt: Kick RT " tip-bot for Steven Rostedt

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.