[RESEND,2/2] sched/rt: Add check_preempt_equal_prio() logic in pick_next_task_rt()

Message ID 1423012341-30265-2-git-send-email-xlpang@126.com
State New
Headers show

Commit Message

Xunlei Pang Feb. 4, 2015, 1:12 a.m.
From: Xunlei Pang <pang.xunlei@linaro.org>

check_preempt_curr() doesn't call sched_class::check_preempt_curr
when the class of current is a higher level. So if there is a DL
task running when doing this for RT, check_preempt_equal_prio()
will definitely miss, which may result in some response latency
for this RT task if it is pinned and there're some same-priority
migratable rt tasks already queued.

We should do the similar thing in select_task_rq_rt() when first
picking rt tasks after running out of DL tasks.

This patch tackles the issue by peeking the next rt task(RT1), and
if find RT1 migratable, just requeue it to the tail of the rq using
requeue_task_rt(rq, p, 0). In this way:
- If there do have another rt task(RT2) with the same priority as
  RT1, RT2 will finally be picked as the running task. While RT1
  will be pushed onto another cpu via RT1's post_schedule(), as
  RT1 is migratable. The difference from check_preempt_equal_prio()
  here is that we just don't care whether RT2 is migratable.

- Otherwise, if there's no rt task with the same priority as RT1,
  RT1 will still be picked as the running task after the requeuing.

Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
---
 kernel/sched/rt.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

pang.xunlei Feb. 4, 2015, 1:05 p.m. | #1
Hi Steve,

On 4 February 2015 at 11:17, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed,  4 Feb 2015 09:12:21 +0800
> Xunlei Pang <xlpang@126.com> wrote:
>
>> From: Xunlei Pang <pang.xunlei@linaro.org>
>>
>> check_preempt_curr() doesn't call sched_class::check_preempt_curr
>> when the class of current is a higher level. So if there is a DL
>> task running when doing this for RT, check_preempt_equal_prio()
>> will definitely miss, which may result in some response latency
>> for this RT task if it is pinned and there're some same-priority
>> migratable rt tasks already queued.
>>
>> We should do the similar thing in select_task_rq_rt() when first
>> picking rt tasks after running out of DL tasks.
>>
>> This patch tackles the issue by peeking the next rt task(RT1), and
>> if find RT1 migratable, just requeue it to the tail of the rq using
>> requeue_task_rt(rq, p, 0). In this way:
>> - If there do have another rt task(RT2) with the same priority as
>>   RT1, RT2 will finally be picked as the running task. While RT1
>>   will be pushed onto another cpu via RT1's post_schedule(), as
>>   RT1 is migratable. The difference from check_preempt_equal_prio()
>>   here is that we just don't care whether RT2 is migratable.
>>
>> - Otherwise, if there's no rt task with the same priority as RT1,
>>   RT1 will still be picked as the running task after the requeuing.
>>
>> Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
>> ---
>>  kernel/sched/rt.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
>> index 4dacb6e..b2385ee 100644
>> --- a/kernel/sched/rt.c
>> +++ b/kernel/sched/rt.c
>> @@ -1477,6 +1477,21 @@ pick_next_task_rt(struct rq *rq, struct task_struct *prev)
>>
>>       put_prev_task(rq, prev);
>>
>> +#ifdef CONFIG_SMP
>> +             /*
>> +              * If there's a running deadline task, check_preempt_curr()
>> +              * doesn't invoke check_preempt_curr_rt() for rt tasks, so
>> +              * we can do it here.
>> +              */
>
> Why the strange indentation?
>

Thanks for catching this, I'll fix it.

>> +             if (prev->sched_class == &dl_sched_class &&
>> +                 rq->rt.rt_nr_total > 1) {
>> +                     p = _pick_next_task_rt(rq, 1); /* peek only */
>
> I hate the "peek only". Just split the function into two, where you
> have something like check_next_task(rq) which does your "peek only"
> and the __pick_next_task_rt() calls check_next_task() first and then
> runs the rest of the code.
>

This sounds good, I'll make a new peek_next_task_rt() as the base one.

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

Patch

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 4dacb6e..b2385ee 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1477,6 +1477,21 @@  pick_next_task_rt(struct rq *rq, struct task_struct *prev)
 
 	put_prev_task(rq, prev);
 
+#ifdef CONFIG_SMP
+		/*
+		 * If there's a running deadline task, check_preempt_curr()
+		 * doesn't invoke check_preempt_curr_rt() for rt tasks, so
+		 * we can do it here.
+		 */
+		if (prev->sched_class == &dl_sched_class &&
+		    rq->rt.rt_nr_total > 1) {
+			p = _pick_next_task_rt(rq, 1); /* peek only */
+			if (p->nr_cpus_allowed != 1 &&
+			    cpupri_find(&rq->rd->cpupri, p, NULL))
+				requeue_task_rt(rq, p, 0);
+		}
+#endif
+
 	p = _pick_next_task_rt(rq, 0);
 
 	/* The running task is never eligible for pushing */