sched/deadline: introduce start_hrtick_dl for !CONFIG_SCHED_HRTICK

Queued ticks are scheduled to match the budget, which means the budget is overall consumed and the dl task should be throttled.

Dl task will
be replenished immediately if fail to start a dl timer.

However, the curr maybe not the left most dl task in the rb tree any more after this immediately replenished and reschedule is needed. Start high-res preemption tick for this upcoming rescheduled dl task is not correct.

This patch fix it by not starting high-res preemption tick for a non-running dl task.

Signed-off-by Wanpeng Li <[email protected]>

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

kernel/sched/deadline.c +2 -1

index 56674f6..2a6a5bb 100644
@@ -1090,7 +1090,8 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
 {
 	update_curr_dl(rq);
 
-	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
+	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0 &&
+	    is_leftmost(p, &rq->dl))
 		start_hrtick_dl(rq, p);
 }
 

Hi,

not sure I understand what the problem is here.

Quoted 2 lines · Wanpeng Li

Queued ticks are scheduled to match the budget, which means the budget is overall consumed and the dl task should be throttled.

... enforce the budget? It means that when the budget is consumed the task has to be throttled...

Quoted 9 lines

Dl task will
be replenished immediately if fail to start a dl timer.

However, the curr maybe not the left most dl task in the rb tree any more after this immediately replenished and reschedule is needed. Start high-res preemption tick for this upcoming rescheduled dl task is not correct.

So, the task that is going to preempt curr is picked by pick_next_task_dl(), that correctly starts the hrtick for this new task.

Maybe you can add more information about what you are seeing? A callpath maybe?

Thanks,

- Juri

Quoted 14 lines

This patch fix it by not starting high-res preemption tick for a non-running dl task.

Signed-off-by Wanpeng Li <[email protected]>

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

kernel/sched/deadline.c +0 -0

index 56674f6..2a6a5bb 100644

kernel/sched/deadline.c static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued) · line 1097

Quoted 9 lines

kernel/sched/deadline.c +2 -1

@@ -1090,7 +1090,8 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
  {
  	update_curr_dl(rq);
 
-	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
+	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0 &&
+	    is_leftmost(p, &rq->dl))
  		start_hrtick_dl(rq, p);
  }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hi Juri,

Quoted 9 lines · Juri Lelli

Hi,

not sure I understand what the problem is here.

Quoted 2 lines · Wanpeng Li

Queued ticks are scheduled to match the budget, which means the budget is overall consumed and the dl task should be throttled.

... enforce the budget? It means that when the budget is consumed the task has to be throttled...

Agreed.

Quoted 12 lines

Quoted 9 lines

Dl task will
be replenished immediately if fail to start a dl timer.

However, the curr maybe not the left most dl task in the rb tree any more after this immediately replenished and reschedule is needed. Start high-res preemption tick for this upcoming rescheduled dl task is not correct.

So, the task that is going to preempt curr is picked by pick_next_task_dl(), that correctly starts the hrtick for this new task.

Quoted 2 lines

Maybe you can add more information about what you are seeing? A callpath maybe?

The parameter of task_tick_dl() queued == 1 means that hrtick is fired. hrtick() => task_tick_dl( , ,1), so p->dl.runtime should be <= 0 if queued == 1. What I see is queued == 1 && p->dl.runtime > 0 && p is not the left most task and hrtick is start for this task.

Regards,
Wanpeng Li

Quoted 33 lines

Thanks,

- Juri

Quoted 14 lines

This patch fix it by not starting high-res preemption tick for a non-running dl task.

Signed-off-by Wanpeng Li <[email protected]>

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

kernel/sched/deadline.c +0 -0

index 56674f6..2a6a5bb 100644

kernel/sched/deadline.c static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued) · line 1097

Quoted 9 lines

kernel/sched/deadline.c +2 -1

@@ -1090,7 +1090,8 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
  {
  	update_curr_dl(rq);
 
-	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
+	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0 &&
+	    is_leftmost(p, &rq->dl))
  		start_hrtick_dl(rq, p);
  }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hi,

Quoted 24 lines · Wanpeng Li

Hi Juri,

Quoted 9 lines · Juri Lelli

Hi,

not sure I understand what the problem is here.

Quoted 2 lines · Wanpeng Li

Queued ticks are scheduled to match the budget, which means the budget is overall consumed and the dl task should be throttled.

... enforce the budget? It means that when the budget is consumed the task has to be throttled...

Agreed.

Quoted 11 lines

Quoted 8 lines

Dl task will
be replenished immediately if fail to start a dl timer.

However, the curr maybe not the left most dl task in the rb tree any more after this immediately replenished and reschedule is needed. Start high-res preemption tick for this upcoming rescheduled dl task is not correct.

So, the task that is going to preempt curr is picked by pick_next_task_dl(), that correctly starts the hrtick for this new task.

Quoted 7 lines

Quoted 2 lines

Maybe you can add more information about what you are seeing? A callpath maybe?

The parameter of task_tick_dl() queued == 1 means that hrtick is fired. hrtick() => task_tick_dl( , ,1), so p->dl.runtime should be <= 0 if queued == 1. What I see is queued == 1 && p->dl.runtime > 0 && p is not the left most task and hrtick is start for this task.

Oh, right, now it makes sense. Good catch! :)

Could you please post a second version with a more explanatory changelog and maybe a comment just above the check?

Thanks,

- Juri

Quoted 36 lines

Regards,
Wanpeng Li

Quoted 32 lines

Thanks,

- Juri

Quoted 14 lines

This patch fix it by not starting high-res preemption tick for a non-running dl task.

Signed-off-by Wanpeng Li <[email protected]>

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

kernel/sched/deadline.c +0 -0

index 56674f6..2a6a5bb 100644

kernel/sched/deadline.c static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued) · line 1097

Quoted 9 lines

kernel/sched/deadline.c +2 -1

@@ -1090,7 +1090,8 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
   {
   	update_curr_dl(rq);
 
-	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
+	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0 &&
+	    is_leftmost(p, &rq->dl))
   		start_hrtick_dl(rq, p);
   }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Introduce start_hrtick_dl for !CONFIG_SCHED_HRTICK to align with fair class.

Signed-off-by Wanpeng Li <[email protected]>

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

kernel/sched/deadline.c +4 -2

index 04c2cbb..56674f6 100644
@@ -1013,6 +1013,10 @@ static void start_hrtick_dl(struct rq *rq, struct task_struct *p)
 {
 	hrtick_start(rq, p->dl.runtime);
 }
+#else /* !CONFIG_SCHED_HRTICK */
+static void start_hrtick_dl(struct rq *rq, struct task_struct *p)
+{
+}
 #endif
 
 static struct sched_dl_entity *pick_next_dl_entity(struct rq *rq,

@@ -1066,10 +1070,8 @@ struct task_struct *pick_next_task_dl(struct rq *rq, struct task_struct *prev)
 	/* Running task will never be pushed. */
        dequeue_pushable_dl_task(rq, p);
 
-#ifdef CONFIG_SCHED_HRTICK
 	if (hrtick_enabled(rq))
 		start_hrtick_dl(rq, p);
-#endif
 
 	set_post_schedule(rq);