sched/deadline: Zero out positive runtime after throttling constrained tasks

PATCH Superseded Xunlei Pang 6 participants 8 messages May 10, 2017 – Jun 8, 2017 Scheduler

Share & tools

Xunlei Pang

author

2017-05-10 13:02 UTC #0 lore raw

PATCH 1/1 sched/deadline: Zero out positive runtime after throttling constrained tasks

When a contrained task is throttled by dl_check_constrained_dl(), it may carry the remaining positive runtime, as a result when dl_task_timer() fires and calls replenish_dl_entity(), it will not be replenished correctly due to the positive dl_se->runtime.

This patch assigns its runtime to 0 if positive after throttling.

Fixes df8eac8cafce ("sched/deadline: Throttle a constrained deadline task activated after the deadline)

Cc Daniel Bristot de Oliveira <[email protected]>

Signed-off-by Xunlei Pang <[email protected]>

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

kernel/sched/deadline.c +2 -0

index a2ce590..d3d291e 100644
@@ -723,6 +723,8 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se)
 		if (unlikely(dl_se->dl_boosted || !start_dl_timer(p)))
 			return;
 		dl_se->dl_throttled = 1;
+		if (dl_se->runtime > 0)
+			dl_se->runtime = 0;
 	}
 }

Signature · 1 line

1.8.3.1

Steven Rostedt

reviewer

2017-05-10 13:36 UTC #1 lore raw

Quoted 19 lines · On Wed, 10 May 2017 21:03:37 +0800 Xunlei Pang <[email protected]> wrote:

When a contrained task is throttled by dl_check_constrained_dl(), it may carry the remaining positive runtime, as a result when dl_task_timer() fires and calls replenish_dl_entity(), it will not be replenished correctly due to the positive dl_se->runtime.
This patch assigns its runtime to 0 if positive after throttling.
Fixes df8eac8cafce ("sched/deadline: Throttle a constrained deadline task activated after the deadline)
Cc Daniel Bristot de Oliveira <[email protected]>
Signed-off-by Xunlei Pang <[email protected]>
---
 kernel/sched/deadline.c | 2 ++
 1 file changed, 2 insertions(+)
kernel/sched/deadline.c +0 -0
index a2ce590..d3d291e 100644

kernel/sched/deadline.c static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se) · line 727

Quoted 5 lines

kernel/sched/deadline.c +2 -0

@@ -723,6 +723,8 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se)
 		if (unlikely(dl_se->dl_boosted || !start_dl_timer(p)))
 			return;
 		dl_se->dl_throttled = 1;
+		if (dl_se->runtime > 0)
+			dl_se->runtime = 0;

This makes sense to me, but should we have any accounting for runtime that was missed due to wakeups and such?

-- Steve

Quoted 3 lines

kernel/sched/deadline.c +0 -0
 	}
 }
 

Xunlei Pang

2017-05-11 01:37 UTC #2 lore raw

Quoted 17 lines · Steven Rostedt

Quoted 14 lines · On Wed, 10 May 2017 21:03:37 +0800 Xunlei Pang <[email protected]> wrote:
When a contrained task is throttled by dl_check_constrained_dl(), it may carry the remaining positive runtime, as a result when dl_task_timer() fires and calls replenish_dl_entity(), it will not be replenished correctly due to the positive dl_se->runtime.
This patch assigns its runtime to 0 if positive after throttling.
Fixes df8eac8cafce ("sched/deadline: Throttle a constrained deadline task activated after the deadline)
Cc Daniel Bristot de Oliveira <[email protected]>
Signed-off-by Xunlei Pang <[email protected]>
---
 kernel/sched/deadline.c | 2 ++
 1 file changed, 2 insertions(+)

Quoted 12 lines

kernel/sched/deadline.c static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se) · line 727
Quoted 10 lines
kernel/sched/deadline.c +2 -0
index a2ce590..d3d291e 100644
@@ -723,6 +723,8 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se)
 		if (unlikely(dl_se->dl_boosted || !start_dl_timer(p)))
 			return;
 		dl_se->dl_throttled = 1;
+		if (dl_se->runtime > 0)
+			dl_se->runtime = 0;
This makes sense to me, but should we have any accounting for runtime that was missed due to wakeups and such?

It sounds a good idea, will try to add that to "/proc/<pid>/sched".

Looks like we should also catch and handle the deadline miss in dl_runtime_exceeded(), I guess we can add the accounting together.

Regards,
Xunlei

Xunlei Pang

2017-05-12 03:34 UTC #3 lore raw

Quoted 30 lines · Xunlei Pang

Quoted 17 lines · Steven Rostedt
Quoted 14 lines · On Wed, 10 May 2017 21:03:37 +0800 Xunlei Pang <[email protected]> wrote:
When a contrained task is throttled by dl_check_constrained_dl(), it may carry the remaining positive runtime, as a result when dl_task_timer() fires and calls replenish_dl_entity(), it will not be replenished correctly due to the positive dl_se->runtime.
This patch assigns its runtime to 0 if positive after throttling.
Fixes df8eac8cafce ("sched/deadline: Throttle a constrained deadline task activated after the deadline)
Cc Daniel Bristot de Oliveira <[email protected]>
Signed-off-by Xunlei Pang <[email protected]>
---
 kernel/sched/deadline.c | 2 ++
 1 file changed, 2 insertions(+)
Quoted 12 lines
kernel/sched/deadline.c static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se) · line 727
Quoted 10 lines
kernel/sched/deadline.c +2 -0
index a2ce590..d3d291e 100644
@@ -723,6 +723,8 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se)
 		if (unlikely(dl_se->dl_boosted || !start_dl_timer(p)))
 			return;
 		dl_se->dl_throttled = 1;
+		if (dl_se->runtime > 0)
+			dl_se->runtime = 0;
This makes sense to me, but should we have any accounting for runtime that was missed due to wakeups and such?

Quoted 4 lines

It sounds a good idea, will try to add that to "/proc/<pid>/sched".
Looks like we should also catch and handle the deadline miss in dl_runtime_exceeded(), I guess we can add the accounting together.

Hi all,

Thanks for all your valuable review!

FYI: I just sent v2 added two more patches.

Regards,
Xunlei

Juri Lelli

2017-05-11 14:20 UTC #4 lore raw

In reply to Xunlei Pang

Hi,

Quoted 19 lines · Xunlei Pang

When a contrained task is throttled by dl_check_constrained_dl(), it may carry the remaining positive runtime, as a result when dl_task_timer() fires and calls replenish_dl_entity(), it will not be replenished correctly due to the positive dl_se->runtime.
This patch assigns its runtime to 0 if positive after throttling.
Fixes df8eac8cafce ("sched/deadline: Throttle a constrained deadline task activated after the deadline)
Cc Daniel Bristot de Oliveira <[email protected]>
Signed-off-by Xunlei Pang <[email protected]>
---
 kernel/sched/deadline.c | 2 ++
 1 file changed, 2 insertions(+)
kernel/sched/deadline.c +0 -0
index a2ce590..d3d291e 100644

kernel/sched/deadline.c static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se) · line 728

Quoted 6 lines

kernel/sched/deadline.c +2 -0

@@ -723,6 +723,8 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se)
 		if (unlikely(dl_se->dl_boosted || !start_dl_timer(p)))
 			return;
 		dl_se->dl_throttled = 1;
+		if (dl_se->runtime > 0)
+			dl_se->runtime = 0;
 	}

Looks good to me. Although, we could alternatively add a flag and use that in replenish_dl_entity() to reset runtime (as we do for dl_yielded).

Flags need refactoring, though.

Thanks,

- Juri

Daniel Bristot de Oliveira

Acked-by on v1 1/1

2017-05-11 14:36 UTC #5 lore raw

In reply to Xunlei Pang

Quoted 19 lines · Xunlei Pang

When a contrained task is throttled by dl_check_constrained_dl(), it may carry the remaining positive runtime, as a result when dl_task_timer() fires and calls replenish_dl_entity(), it will not be replenished correctly due to the positive dl_se->runtime.
This patch assigns its runtime to 0 if positive after throttling.
Fixes df8eac8cafce ("sched/deadline: Throttle a constrained deadline task activated after the deadline)
Cc Daniel Bristot de Oliveira <[email protected]>
Signed-off-by Xunlei Pang <[email protected]>
---
 kernel/sched/deadline.c | 2 ++
 1 file changed, 2 insertions(+)
kernel/sched/deadline.c +0 -0
index a2ce590..d3d291e 100644

kernel/sched/deadline.c static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se) · line 729

Quoted 7 lines

kernel/sched/deadline.c +2 -0

@@ -723,6 +723,8 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se)
 		if (unlikely(dl_se->dl_boosted || !start_dl_timer(p)))
 			return;
 		dl_se->dl_throttled = 1;
+		if (dl_se->runtime > 0)
+			dl_se->runtime = 0;
 	}
 }

Acked-by Daniel Bristot de Oliveira <[email protected]>

-- Daniel

Xunlei Pang

2017-05-24 08:50 UTC #6 lore raw

Hi Peter,

Quoted 15 lines · Daniel Bristot de Oliveira

Quoted 14 lines · Xunlei Pang
When a contrained task is throttled by dl_check_constrained_dl(), it may carry the remaining positive runtime, as a result when dl_task_timer() fires and calls replenish_dl_entity(), it will not be replenished correctly due to the positive dl_se->runtime.
This patch assigns its runtime to 0 if positive after throttling.
Fixes df8eac8cafce ("sched/deadline: Throttle a constrained deadline task activated after the deadline)
Cc Daniel Bristot de Oliveira <[email protected]>
Signed-off-by Xunlei Pang <[email protected]>
---
 kernel/sched/deadline.c | 2 ++
 1 file changed, 2 insertions(+)

Quoted 13 lines

kernel/sched/deadline.c static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se) · line 729
Quoted 12 lines
kernel/sched/deadline.c +2 -0
index a2ce590..d3d291e 100644
@@ -723,6 +723,8 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se)
 		if (unlikely(dl_se->dl_boosted || !start_dl_timer(p)))
 			return;
 		dl_se->dl_throttled = 1;
+		if (dl_se->runtime > 0)
+			dl_se->runtime = 0;
 	}
 }
Acked-by Daniel Bristot de Oliveira <[email protected]>

Could you please have this one? Thanks!

Regards,
Xunlei

tip-bot for Xunlei Pang

merge bot

2017-06-08 09:32 UTC #7 lore raw

In reply to Xunlei Pang

sched/deadline: Zero out positive runtime after throttling constrained tasks

Commit-ID:  ae83b56a56f8d9643dedbee86b457fa1c5d42f59
Gitweb:     http://git.kernel.org/tip/ae83b56a56f8d9643dedbee86b457fa1c5d42f59
Author:     Xunlei Pang <[email protected]>
AuthorDate: Wed, 10 May 2017 21:03:37 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 8 Jun 2017 10:31:58 +0200

sched/deadline: Zero out positive runtime after throttling constrained tasks

This patch assigns its runtime to 0 if positive after throttling.

Signed-off-by Xunlei Pang <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Acked-by Daniel Bristot de Oliveira <[email protected]>

Cc Juri Lelli <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Luca Abeni <[email protected]>

Cc Mike Galbraith <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Thomas Gleixner <[email protected]>

Fixes df8eac8cafce ("sched/deadline: Throttle a constrained deadline task activated after the deadline)

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

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

kernel/sched/deadline.c +2 -0

index e3b25df..54302cf 100644
@@ -941,6 +941,8 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se)
 		if (unlikely(dl_se->dl_boosted || !start_dl_timer(p)))
 			return;
 		dl_se->dl_throttled = 1;
+		if (dl_se->runtime > 0)
+			dl_se->runtime = 0;
 	}
 }