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(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 04c2cbb..56674f6 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -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);
@@ -1088,10 +1090,8 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
{
update_curr_dl(rq);
-#ifdef CONFIG_SCHED_HRTICK
if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
start_hrtick_dl(rq, p);
-#endif
}
static void task_fork_dl(struct task_struct *p)
--
1.9.1
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(-)
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);
}
--
1.9.1
Hi,
On 11/11/14 01:52, Wanpeng Li wrote:
> 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(-)
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 04c2cbb..56674f6 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -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);
>
> @@ -1088,10 +1090,8 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
> {
> update_curr_dl(rq);
>
> -#ifdef CONFIG_SCHED_HRTICK
> if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
> start_hrtick_dl(rq, p);
> -#endif
> }
>
> static void task_fork_dl(struct task_struct *p)
>
Looks good!
Acked-by: Juri Lelli <[email protected]>
Thanks,
- Juri
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...
> 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
> 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(-)
>
> 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);
> }
>
>
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.
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 <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Commit-ID: 36ce98818a4df66c8134c31fd6e768b4119c7a90
Gitweb: http://git.kernel.org/tip/36ce98818a4df66c8134c31fd6e768b4119c7a90
Author: Wanpeng Li <[email protected]>
AuthorDate: Tue, 11 Nov 2014 09:52:26 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Sun, 16 Nov 2014 10:59:03 +0100
sched/deadline: Introduce start_hrtick_dl() for !CONFIG_SCHED_HRTICK
Introduce start_hrtick_dl for !CONFIG_SCHED_HRTICK to align with
the fair class.
Signed-off-by: Wanpeng Li <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Kirill Tkhai <[email protected]>
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/sched/deadline.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 9594c12..e5db8c6 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -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);
@@ -1088,10 +1090,8 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
{
update_curr_dl(rq);
-#ifdef CONFIG_SCHED_HRTICK
if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
start_hrtick_dl(rq, p);
-#endif
}
static void task_fork_dl(struct task_struct *p)
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?
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 <[email protected]>
>>> ---
>>> 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 [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,
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 <[email protected]>
>>>> ---
>>>> 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 [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>