linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sched/deadline: Unthrottle PI boosted threads while enqueuing
@ 2020-09-16  7:06 Daniel Bristot de Oliveira
  2020-09-18  6:00 ` Juri Lelli
  2020-10-05  7:43 ` [tip: sched/core] " tip-bot2 for Daniel Bristot de Oliveira
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel Bristot de Oliveira @ 2020-09-16  7:06 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli
  Cc: Daniel Bristot de Oliveira, Mark Simmons, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	linux-kernel

stress-ng has a test (stress-ng --cyclic) that creates a set of threads
under SCHED_DEADLINE with the following parameters:

    dl_runtime   =  10000 (10 us)
    dl_deadline  = 100000 (100 us)
    dl_period    = 100000 (100 us)

These parameters are very aggressive. When using a system without HRTICK
set, these threads can easily execute longer than the dl_runtime because
the throttling happens with 1/HZ resolution.

During the main part of the test, the system works just fine because
the workload does not try to run over the 10 us. The problem happens at
the end of the test, on the exit() path. During exit(), the threads need
to do some cleanups that require real-time mutex locks, mainly those
related to memory management, resulting in this scenario:

Note: locks are rt_mutexes...
 ------------------------------------------------------------------------
    TASK A:		TASK B:				TASK C:
    activation
							activation
			activation

    lock(a): OK!	lock(b): OK!
    			<overrun runtime>
    			lock(a)
    			-> block (task A owns it)
			  -> self notice/set throttled
 +--<			  -> arm replenished timer
 |    			switch-out
 |    							lock(b)
 |    							-> <C prio > B prio>
 |    							-> boost TASK B
 |  unlock(a)						switch-out
 |  -> handle lock a to B
 |    -> wakeup(B)
 |      -> B is throttled:
 |        -> do not enqueue
 |     switch-out
 |
 |
 +---------------------> replenishment timer
			-> TASK B is boosted:
			  -> do not enqueue
 ------------------------------------------------------------------------

BOOM: TASK B is runnable but !enqueued, holding TASK C: the system
crashes with hung task C.

This problem is avoided by removing the throttle state from the boosted
thread while boosting it (by TASK A in the example above), allowing it to
be queued and run boosted.

The next replenishment will take care of the runtime overrun, pushing
the deadline further away. See the "while (dl_se->runtime <= 0)" on
replenish_dl_entity() for more information.

Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Reported-by: Mark Simmons <msimmons@redhat.com>
Reviewed-by: Juri Lelli <juri.lelli@redhat.com>
Tested-by: Mark Simmons <msimmons@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ben Segall <bsegall@google.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: linux-kernel@vger.kernel.org

---
 kernel/sched/deadline.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 3862a28cd05d..50ba5fca0403 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1525,6 +1525,27 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)
 	 */
 	if (pi_task && dl_prio(pi_task->normal_prio) && p->dl.dl_boosted) {
 		pi_se = &pi_task->dl;
+		/*
+		 * Because of delays in the detection of the overrun of a
+		 * thread's runtime, it might be the case that a thread
+		 * goes to sleep in a rt mutex with negative runtime. As
+		 * a consequence, the thread will be throttled.
+		 *
+		 * While waiting for the mutex, this thread can also be
+		 * boosted via PI, resulting in a thread that is throttled
+		 * and boosted at the same time.
+		 *
+		 * In this case, the boost overrides the throttle.
+		 */
+		if (p->dl.dl_throttled) {
+			/*
+			 * The replenish timer needs to be canceled. No
+			 * problem if it fires concurrently: boosted threads
+			 * are ignored in dl_task_timer().
+			 */
+			hrtimer_try_to_cancel(&p->dl.dl_timer);
+			p->dl.dl_throttled = 0;
+		}
 	} else if (!dl_prio(p->normal_prio)) {
 		/*
 		 * Special case in which we have a !SCHED_DEADLINE task
-- 
2.26.2


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

* Re: [PATCH] sched/deadline: Unthrottle PI boosted threads while enqueuing
  2020-09-16  7:06 [PATCH] sched/deadline: Unthrottle PI boosted threads while enqueuing Daniel Bristot de Oliveira
@ 2020-09-18  6:00 ` Juri Lelli
  2020-10-02 15:57   ` Daniel Bristot de Oliveira
  2020-10-05  7:43 ` [tip: sched/core] " tip-bot2 for Daniel Bristot de Oliveira
  1 sibling, 1 reply; 5+ messages in thread
From: Juri Lelli @ 2020-09-18  6:00 UTC (permalink / raw)
  To: Daniel Bristot de Oliveira
  Cc: Ingo Molnar, Peter Zijlstra, Mark Simmons, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	linux-kernel

Hi Daniel,

On 16/09/20 09:06, Daniel Bristot de Oliveira wrote:
> stress-ng has a test (stress-ng --cyclic) that creates a set of threads
> under SCHED_DEADLINE with the following parameters:
> 
>     dl_runtime   =  10000 (10 us)
>     dl_deadline  = 100000 (100 us)
>     dl_period    = 100000 (100 us)
> 
> These parameters are very aggressive. When using a system without HRTICK
> set, these threads can easily execute longer than the dl_runtime because
> the throttling happens with 1/HZ resolution.
> 
> During the main part of the test, the system works just fine because
> the workload does not try to run over the 10 us. The problem happens at
> the end of the test, on the exit() path. During exit(), the threads need
> to do some cleanups that require real-time mutex locks, mainly those
> related to memory management, resulting in this scenario:
> 
> Note: locks are rt_mutexes...
>  ------------------------------------------------------------------------
>     TASK A:		TASK B:				TASK C:
>     activation
> 							activation
> 			activation
> 
>     lock(a): OK!	lock(b): OK!
>     			<overrun runtime>
>     			lock(a)
>     			-> block (task A owns it)
> 			  -> self notice/set throttled
>  +--<			  -> arm replenished timer
>  |    			switch-out
>  |    							lock(b)
>  |    							-> <C prio > B prio>
>  |    							-> boost TASK B
>  |  unlock(a)						switch-out
>  |  -> handle lock a to B
>  |    -> wakeup(B)
>  |      -> B is throttled:
>  |        -> do not enqueue
>  |     switch-out
>  |
>  |
>  +---------------------> replenishment timer
> 			-> TASK B is boosted:
> 			  -> do not enqueue
>  ------------------------------------------------------------------------
> 
> BOOM: TASK B is runnable but !enqueued, holding TASK C: the system
> crashes with hung task C.
> 
> This problem is avoided by removing the throttle state from the boosted
> thread while boosting it (by TASK A in the example above), allowing it to
> be queued and run boosted.
> 
> The next replenishment will take care of the runtime overrun, pushing
> the deadline further away. See the "while (dl_se->runtime <= 0)" on
> replenish_dl_entity() for more information.
> 
> Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
> Reported-by: Mark Simmons <msimmons@redhat.com>
> Reviewed-by: Juri Lelli <juri.lelli@redhat.com>
> Tested-by: Mark Simmons <msimmons@redhat.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ben Segall <bsegall@google.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Cc: linux-kernel@vger.kernel.org
> 
> ---

Thanks for this fix.

Acked-by: Juri Lelli <juri.lelli@redhat.com>

Best,
Juri


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

* Re: [PATCH] sched/deadline: Unthrottle PI boosted threads while enqueuing
  2020-09-18  6:00 ` Juri Lelli
@ 2020-10-02 15:57   ` Daniel Bristot de Oliveira
  2020-10-02 16:01     ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Bristot de Oliveira @ 2020-10-02 15:57 UTC (permalink / raw)
  To: Juri Lelli
  Cc: Ingo Molnar, Peter Zijlstra, Mark Simmons, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	linux-kernel

On 9/18/20 8:00 AM, Juri Lelli wrote:
> Hi Daniel,
> 
> On 16/09/20 09:06, Daniel Bristot de Oliveira wrote:
>> stress-ng has a test (stress-ng --cyclic) that creates a set of threads
>> under SCHED_DEADLINE with the following parameters:
>>
>>     dl_runtime   =  10000 (10 us)
>>     dl_deadline  = 100000 (100 us)
>>     dl_period    = 100000 (100 us)
>>
>> These parameters are very aggressive. When using a system without HRTICK
>> set, these threads can easily execute longer than the dl_runtime because
>> the throttling happens with 1/HZ resolution.
>>
>> During the main part of the test, the system works just fine because
>> the workload does not try to run over the 10 us. The problem happens at
>> the end of the test, on the exit() path. During exit(), the threads need
>> to do some cleanups that require real-time mutex locks, mainly those
>> related to memory management, resulting in this scenario:
>>
>> Note: locks are rt_mutexes...
>>  ------------------------------------------------------------------------
>>     TASK A:		TASK B:				TASK C:
>>     activation
>> 							activation
>> 			activation
>>
>>     lock(a): OK!	lock(b): OK!
>>     			<overrun runtime>
>>     			lock(a)
>>     			-> block (task A owns it)
>> 			  -> self notice/set throttled
>>  +--<			  -> arm replenished timer
>>  |    			switch-out
>>  |    							lock(b)
>>  |    							-> <C prio > B prio>
>>  |    							-> boost TASK B
>>  |  unlock(a)						switch-out
>>  |  -> handle lock a to B
>>  |    -> wakeup(B)
>>  |      -> B is throttled:
>>  |        -> do not enqueue
>>  |     switch-out
>>  |
>>  |
>>  +---------------------> replenishment timer
>> 			-> TASK B is boosted:
>> 			  -> do not enqueue
>>  ------------------------------------------------------------------------
>>
>> BOOM: TASK B is runnable but !enqueued, holding TASK C: the system
>> crashes with hung task C.
>>
>> This problem is avoided by removing the throttle state from the boosted
>> thread while boosting it (by TASK A in the example above), allowing it to
>> be queued and run boosted.
>>
>> The next replenishment will take care of the runtime overrun, pushing
>> the deadline further away. See the "while (dl_se->runtime <= 0)" on
>> replenish_dl_entity() for more information.
>>
>> Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
>> Reported-by: Mark Simmons <msimmons@redhat.com>
>> Reviewed-by: Juri Lelli <juri.lelli@redhat.com>
>> Tested-by: Mark Simmons <msimmons@redhat.com>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Juri Lelli <juri.lelli@redhat.com>
>> Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Ben Segall <bsegall@google.com>
>> Cc: Mel Gorman <mgorman@suse.de>
>> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
>> Cc: linux-kernel@vger.kernel.org
>>
>> ---
> 
> Thanks for this fix.
> 
> Acked-by: Juri Lelli <juri.lelli@redhat.com>

This is a gentle ping... [we are facing this bug in practice :-(].

-- Daniel

> Best,
> Juri
> 


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

* Re: [PATCH] sched/deadline: Unthrottle PI boosted threads while enqueuing
  2020-10-02 15:57   ` Daniel Bristot de Oliveira
@ 2020-10-02 16:01     ` Peter Zijlstra
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2020-10-02 16:01 UTC (permalink / raw)
  To: Daniel Bristot de Oliveira
  Cc: Juri Lelli, Ingo Molnar, Mark Simmons, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	linux-kernel

On Fri, Oct 02, 2020 at 05:57:52PM +0200, Daniel Bristot de Oliveira wrote:
> On 9/18/20 8:00 AM, Juri Lelli wrote:
> > Hi Daniel,
> > 
> > On 16/09/20 09:06, Daniel Bristot de Oliveira wrote:
> >> stress-ng has a test (stress-ng --cyclic) that creates a set of threads
> >> under SCHED_DEADLINE with the following parameters:
> >>
> >>     dl_runtime   =  10000 (10 us)
> >>     dl_deadline  = 100000 (100 us)
> >>     dl_period    = 100000 (100 us)
> >>
> >> These parameters are very aggressive. When using a system without HRTICK
> >> set, these threads can easily execute longer than the dl_runtime because
> >> the throttling happens with 1/HZ resolution.
> >>
> >> During the main part of the test, the system works just fine because
> >> the workload does not try to run over the 10 us. The problem happens at
> >> the end of the test, on the exit() path. During exit(), the threads need
> >> to do some cleanups that require real-time mutex locks, mainly those
> >> related to memory management, resulting in this scenario:
> >>
> >> Note: locks are rt_mutexes...
> >>  ------------------------------------------------------------------------
> >>     TASK A:		TASK B:				TASK C:
> >>     activation
> >> 							activation
> >> 			activation
> >>
> >>     lock(a): OK!	lock(b): OK!
> >>     			<overrun runtime>
> >>     			lock(a)
> >>     			-> block (task A owns it)
> >> 			  -> self notice/set throttled
> >>  +--<			  -> arm replenished timer
> >>  |    			switch-out
> >>  |    							lock(b)
> >>  |    							-> <C prio > B prio>
> >>  |    							-> boost TASK B
> >>  |  unlock(a)						switch-out
> >>  |  -> handle lock a to B
> >>  |    -> wakeup(B)
> >>  |      -> B is throttled:
> >>  |        -> do not enqueue
> >>  |     switch-out
> >>  |
> >>  |
> >>  +---------------------> replenishment timer
> >> 			-> TASK B is boosted:
> >> 			  -> do not enqueue
> >>  ------------------------------------------------------------------------
> >>
> >> BOOM: TASK B is runnable but !enqueued, holding TASK C: the system
> >> crashes with hung task C.
> >>
> >> This problem is avoided by removing the throttle state from the boosted
> >> thread while boosting it (by TASK A in the example above), allowing it to
> >> be queued and run boosted.
> >>
> >> The next replenishment will take care of the runtime overrun, pushing
> >> the deadline further away. See the "while (dl_se->runtime <= 0)" on
> >> replenish_dl_entity() for more information.
> >>
> >> Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
> >> Reported-by: Mark Simmons <msimmons@redhat.com>
> >> Reviewed-by: Juri Lelli <juri.lelli@redhat.com>
> >> Tested-by: Mark Simmons <msimmons@redhat.com>
> >> Cc: Ingo Molnar <mingo@redhat.com>
> >> Cc: Peter Zijlstra <peterz@infradead.org>
> >> Cc: Juri Lelli <juri.lelli@redhat.com>
> >> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> >> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> >> Cc: Steven Rostedt <rostedt@goodmis.org>
> >> Cc: Ben Segall <bsegall@google.com>
> >> Cc: Mel Gorman <mgorman@suse.de>
> >> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> >> Cc: linux-kernel@vger.kernel.org
> >>
> >> ---
> > 
> > Thanks for this fix.
> > 
> > Acked-by: Juri Lelli <juri.lelli@redhat.com>
> 
> This is a gentle ping... [we are facing this bug in practice :-(].

Sorry, queued now.

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

* [tip: sched/core] sched/deadline: Unthrottle PI boosted threads while enqueuing
  2020-09-16  7:06 [PATCH] sched/deadline: Unthrottle PI boosted threads while enqueuing Daniel Bristot de Oliveira
  2020-09-18  6:00 ` Juri Lelli
@ 2020-10-05  7:43 ` tip-bot2 for Daniel Bristot de Oliveira
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for Daniel Bristot de Oliveira @ 2020-10-05  7:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Mark Simmons, Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Juri Lelli, x86, LKML

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     feff2e65efd8d84cf831668e182b2ce73c604bbb
Gitweb:        https://git.kernel.org/tip/feff2e65efd8d84cf831668e182b2ce73c604bbb
Author:        Daniel Bristot de Oliveira <bristot@redhat.com>
AuthorDate:    Wed, 16 Sep 2020 09:06:39 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Sat, 03 Oct 2020 16:30:53 +02:00

sched/deadline: Unthrottle PI boosted threads while enqueuing

stress-ng has a test (stress-ng --cyclic) that creates a set of threads
under SCHED_DEADLINE with the following parameters:

    dl_runtime   =  10000 (10 us)
    dl_deadline  = 100000 (100 us)
    dl_period    = 100000 (100 us)

These parameters are very aggressive. When using a system without HRTICK
set, these threads can easily execute longer than the dl_runtime because
the throttling happens with 1/HZ resolution.

During the main part of the test, the system works just fine because
the workload does not try to run over the 10 us. The problem happens at
the end of the test, on the exit() path. During exit(), the threads need
to do some cleanups that require real-time mutex locks, mainly those
related to memory management, resulting in this scenario:

Note: locks are rt_mutexes...
 ------------------------------------------------------------------------
    TASK A:		TASK B:				TASK C:
    activation
							activation
			activation

    lock(a): OK!	lock(b): OK!
    			<overrun runtime>
    			lock(a)
    			-> block (task A owns it)
			  -> self notice/set throttled
 +--<			  -> arm replenished timer
 |    			switch-out
 |    							lock(b)
 |    							-> <C prio > B prio>
 |    							-> boost TASK B
 |  unlock(a)						switch-out
 |  -> handle lock a to B
 |    -> wakeup(B)
 |      -> B is throttled:
 |        -> do not enqueue
 |     switch-out
 |
 |
 +---------------------> replenishment timer
			-> TASK B is boosted:
			  -> do not enqueue
 ------------------------------------------------------------------------

BOOM: TASK B is runnable but !enqueued, holding TASK C: the system
crashes with hung task C.

This problem is avoided by removing the throttle state from the boosted
thread while boosting it (by TASK A in the example above), allowing it to
be queued and run boosted.

The next replenishment will take care of the runtime overrun, pushing
the deadline further away. See the "while (dl_se->runtime <= 0)" on
replenish_dl_entity() for more information.

Reported-by: Mark Simmons <msimmons@redhat.com>
Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Juri Lelli <juri.lelli@redhat.com>
Tested-by: Mark Simmons <msimmons@redhat.com>
Link: https://lkml.kernel.org/r/5076e003450835ec74e6fa5917d02c4fa41687e6.1600170294.git.bristot@redhat.com
---
 kernel/sched/deadline.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index c19c188..6d93f45 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1525,6 +1525,27 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)
 	 */
 	if (pi_task && dl_prio(pi_task->normal_prio) && p->dl.dl_boosted) {
 		pi_se = &pi_task->dl;
+		/*
+		 * Because of delays in the detection of the overrun of a
+		 * thread's runtime, it might be the case that a thread
+		 * goes to sleep in a rt mutex with negative runtime. As
+		 * a consequence, the thread will be throttled.
+		 *
+		 * While waiting for the mutex, this thread can also be
+		 * boosted via PI, resulting in a thread that is throttled
+		 * and boosted at the same time.
+		 *
+		 * In this case, the boost overrides the throttle.
+		 */
+		if (p->dl.dl_throttled) {
+			/*
+			 * The replenish timer needs to be canceled. No
+			 * problem if it fires concurrently: boosted threads
+			 * are ignored in dl_task_timer().
+			 */
+			hrtimer_try_to_cancel(&p->dl.dl_timer);
+			p->dl.dl_throttled = 0;
+		}
 	} else if (!dl_prio(p->normal_prio)) {
 		/*
 		 * Special case in which we have a !SCHED_DEADLINE task that is going

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

end of thread, other threads:[~2020-10-05  7:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-16  7:06 [PATCH] sched/deadline: Unthrottle PI boosted threads while enqueuing Daniel Bristot de Oliveira
2020-09-18  6:00 ` Juri Lelli
2020-10-02 15:57   ` Daniel Bristot de Oliveira
2020-10-02 16:01     ` Peter Zijlstra
2020-10-05  7:43 ` [tip: sched/core] " tip-bot2 for Daniel Bristot de Oliveira

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).