Re: [PATCH 2/2] sched/deadline: fix start high-res preemption tick for a non-running task

From: Wanpeng Li
Date: Mon Nov 17 2014 - 06:02:46 EST


Hi Juri,
On 11/17/14, 5:50 PM, Juri Lelli wrote:
Hi,

On 13/11/14 10:30, Wanpeng Li wrote:
Hi Juri,
On 11/13/14, 5:37 PM, Juri Lelli wrote:
Hi,

not sure I understand what the problem is here.

On 11/11/14 01:52, Wanpeng Li wrote:
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.

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?
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?

Ok, I will do it and send out a new version tomorrow. ;-) Btw, could you review this patch? https://lkml.org/lkml/2014/11/13/51 The bug is introduced by deadline.

Regards,
Wanpeng Li

Thanks,

- Juri

Regards,
Wanpeng Li

Thanks,

- Juri

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

Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxxxxxx>
---
kernel/sched/deadline.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 56674f6..2a6a5bb 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


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