2014-11-18 02:34:03

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2] sched/deadline: fix start high-res preemption tick for a non-leftmost task

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.
So the curr maybe not the left most dl task in the rb tree any more
after this immediately replenished since reschedule is needed.

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. If replenished immediately(fail to start dl timer),
hrtick is start for task w/ queued == 1 && p->dl.runtime > 0 && p is
not the left most task.

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-leftmost dl task.

Signed-off-by: Wanpeng Li <[email protected]>
---
v1 -> v2:
* add a more explanatory changelog
* add a comment just above the check

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

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index e5db8c6..981479a 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1090,7 +1090,18 @@ 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)
+ /*
+ * Queued tick means that hrtick is fired, when the budget is
+ * consumed the task has to be throttled. However, task will
+ * be replenished immediately if fail to start a dl timer, in
+ * addition, the task maybe not the left most any more. Don't
+ * start high-res preemption tick for the task who is not the
+ * left most. The high-res preemption tick will be started for
+ * this task just when it can be picked in pick_next_task_dl()
+ * again.
+ */
+ if (hrtick_enabled(rq) && queued && p->dl.runtime > 0 &&
+ is_leftmost(p, &rq->dl))
start_hrtick_dl(rq, p);
}

--
1.9.1


2014-11-18 17:30:40

by Juri Lelli

[permalink] [raw]
Subject: Re: [PATCH v2] sched/deadline: fix start high-res preemption tick for a non-leftmost task

Hi,

On 18/11/14 02:32, 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.
>
> Dl task will be replenished immediately if fail to start a dl timer.
> So the curr maybe not the left most dl task in the rb tree any more
> after this immediately replenished since reschedule is needed.
>
> 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. If replenished immediately(fail to start dl timer),
> hrtick is start for task w/ queued == 1 && p->dl.runtime > 0 && p is
> not the left most task.
>
> 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-leftmost dl task.
>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> v1 -> v2:
> * add a more explanatory changelog
> * add a comment just above the check
>
> kernel/sched/deadline.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index e5db8c6..981479a 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1090,7 +1090,18 @@ 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)
> + /*
> + * Queued tick means that hrtick is fired, when the budget is
> + * consumed the task has to be throttled. However, task will
> + * be replenished immediately if fail to start a dl timer, in
> + * addition, the task maybe not the left most any more. Don't
> + * start high-res preemption tick for the task who is not the
> + * left most. The high-res preemption tick will be started for
> + * this task just when it can be picked in pick_next_task_dl()
> + * again.
> + */
> + if (hrtick_enabled(rq) && queued && p->dl.runtime > 0 &&
> + is_leftmost(p, &rq->dl))
> start_hrtick_dl(rq, p);
> }
>
>

Acked-by: Juri Lelli <[email protected]>

Thanks,

- Juri