Re: [PATCH 2/2] sched/rt: Optimizate task_woken_rt()
On Fri, 1 May 2015 10:02:47 +0800 pang.xun...@zte.com.cn wrote: > > > > > > - Remove "!test_tsk_need_resched(rq->curr)" condition, because > > > the flag might be set right before the waking up, but we still > > > need to push equal or lower priority tasks, it should be removed. > > > Without this condition, we actually get the right logic. > > > > But doesn't that happen when we schedule? > > It does, but will have some latency. What latency? The need_resched flag is set, that means as soon as this CPU is in a preemptable situation, it will schedule. The most common case would be return from interrupt, as interrupts or softirqs are usually what trigger the wakeups. But if we do it here, the need resched flag could be set because another higher priority task is about to preempt the current one that is higher than what just woke up. So we move it now to another CPU, and then on return of the interrupt we schedule. Then the current running task gets preempted by the higher priority task and it too can migrate to the CPU we just pushed the other one to, and it still doesn't run, but yet it got moved for no reason at all. I feel better if the need resched flag is set, we wait till a schedule happens to see where everything is about to be moved. > > Still "rq->curr->prio <= p->prio" will be enough for us to ensure the > proper > push_rt_tasks() without this condition. I have no idea what the above means. > > Beside that, for "rq->curr->nr_cpus_allowed < 2", I noticed it was > introduced > by commit b3bc211cfe7d5fe94b, but with "!test_tsk_need_resched(rq->curr)", > it actaully can't be satisfied. What can't be satisfied? > > So, I think this condition should be removed. I'm still not convinced. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] sched/rt: Optimizate task_woken_rt()
On Fri, 1 May 2015 00:33:18 +0800 Xunlei Pang wrote: > From: Xunlei Pang > > - Remove "has_pushable_tasks(rq)" condition, because for queued p, > "!task_running(rq, p)" and "p->nr_cpus_allowed > 1" implies true > "has_pushable_tasks(rq)". This makes sense. > > - Remove "!test_tsk_need_resched(rq->curr)" condition, because > the flag might be set right before the waking up, but we still > need to push equal or lower priority tasks, it should be removed. > Without this condition, we actually get the right logic. But doesn't that happen when we schedule? -- Steve > > Signed-off-by: Xunlei Pang > --- > kernel/sched/rt.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > index a9d33a3..9d735da 100644 > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -2233,8 +2233,6 @@ out: > static void task_woken_rt(struct rq *rq, struct task_struct *p) > { > if (!task_running(rq, p) && > - !test_tsk_need_resched(rq->curr) && > - has_pushable_tasks(rq) && > p->nr_cpus_allowed > 1 && > (dl_task(rq->curr) || rt_task(rq->curr)) && > (rq->curr->nr_cpus_allowed < 2 || -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] sched/rt: Optimizate task_woken_rt()
On Fri, 1 May 2015 00:33:18 +0800 Xunlei Pang xlp...@126.com wrote: From: Xunlei Pang pang.xun...@linaro.org - Remove has_pushable_tasks(rq) condition, because for queued p, !task_running(rq, p) and p-nr_cpus_allowed 1 implies true has_pushable_tasks(rq). This makes sense. - Remove !test_tsk_need_resched(rq-curr) condition, because the flag might be set right before the waking up, but we still need to push equal or lower priority tasks, it should be removed. Without this condition, we actually get the right logic. But doesn't that happen when we schedule? -- Steve Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- kernel/sched/rt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index a9d33a3..9d735da 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -2233,8 +2233,6 @@ out: static void task_woken_rt(struct rq *rq, struct task_struct *p) { if (!task_running(rq, p) - !test_tsk_need_resched(rq-curr) - has_pushable_tasks(rq) p-nr_cpus_allowed 1 (dl_task(rq-curr) || rt_task(rq-curr)) (rq-curr-nr_cpus_allowed 2 || -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] sched/rt: Optimizate task_woken_rt()
On Fri, 1 May 2015 10:02:47 +0800 pang.xun...@zte.com.cn wrote: - Remove !test_tsk_need_resched(rq-curr) condition, because the flag might be set right before the waking up, but we still need to push equal or lower priority tasks, it should be removed. Without this condition, we actually get the right logic. But doesn't that happen when we schedule? It does, but will have some latency. What latency? The need_resched flag is set, that means as soon as this CPU is in a preemptable situation, it will schedule. The most common case would be return from interrupt, as interrupts or softirqs are usually what trigger the wakeups. But if we do it here, the need resched flag could be set because another higher priority task is about to preempt the current one that is higher than what just woke up. So we move it now to another CPU, and then on return of the interrupt we schedule. Then the current running task gets preempted by the higher priority task and it too can migrate to the CPU we just pushed the other one to, and it still doesn't run, but yet it got moved for no reason at all. I feel better if the need resched flag is set, we wait till a schedule happens to see where everything is about to be moved. Still rq-curr-prio = p-prio will be enough for us to ensure the proper push_rt_tasks() without this condition. I have no idea what the above means. Beside that, for rq-curr-nr_cpus_allowed 2, I noticed it was introduced by commit b3bc211cfe7d5fe94b, but with !test_tsk_need_resched(rq-curr), it actaully can't be satisfied. What can't be satisfied? So, I think this condition should be removed. I'm still not convinced. -- Steve -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/