Re: [PATCH]sched/rt: Do not try to push tasks if pinned task switches to RT
On Wed, 12 Mar 2014 16:23:48 +0400 Kirill Tkhai wrote: > > would there not need to be a check for p->migrate_disable ? > > push_rt_task() is not checking and so a high prio RT task > > preemting a low prio RT task in a migrate_disable() section > > would actually push it off this cpu ? atleast I did not > > find why that would not happen. > > Hi, Nicholas! > > p is not rq->curr, so its p->migrate_disable state is already updated and > it can't be pushed (nr_cpus_allowed == 1 and it's not pushable). > > (If I understand right, that you worry about this). > Correct. If p has migrate_disabled set and scheduled out, then when it gets scheduled out, its cpu affinity gets set to only the current cpu and nr_cpu_allowed to 1. No need to check here if p->migrate_disabled is set. -- 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]sched/rt: Do not try to push tasks if pinned task switches to RT
On Wed, Mar 12, 2014 at 06:18:33AM -0400, Steven Rostedt wrote: > Peter, > > I'm going through my inbox (over a year old), and found this patch from > Kirill. It looks fine to me. You can apply it with my > Thanks! -- 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]sched/rt: Do not try to push tasks if pinned task switches to RT
12.03.2014, 14:39, "Nicholas Mc Guire" : > On Wed, 12 Mar 2014, Steven Rostedt wrote: > >> Peter, >> >> I'm going through my inbox (over a year old), and found this patch from >> Kirill. It looks fine to me. You can apply it with my >> >> Acked-by: Steven Rostedt >> >> -- Steve >> >> [PATCH]sched/rt: Do not try to push tasks if pinned task switches to RT >> >> Just switched pinned task is not able to be pushed. If the rq had had >> several RT tasks before they have already been considered as candidates >> to be pushed (or pulled). >> >> Signed-off-by: Kirill V Tkhai >> CC: Steven Rostedt >> CC: Ingo Molnar >> CC: Peter Zijlstra >> CC: linux-rt-users >> --- >> kernel/sched/rt.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c >> index 4e8f0f4..5aab032 100644 >> --- a/kernel/sched/rt.c >> +++ b/kernel/sched/rt.c >> @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct >> task_struct *p) >> */ >> if (p->on_rq && rq->curr != p) { >> #ifdef CONFIG_SMP >> - if (rq->rt.overloaded && push_rt_task(rq) && >> + if (p->nr_cpus_allowed > 1 && rq->rt.overloaded && >> /* Don't resched if we changed runqueues */ >> - rq != task_rq(p)) >> + push_rt_task(rq) && rq != task_rq(p)) >> check_resched = 0; >> #endif /* CONFIG_SMP */ >> if (check_resched && p->prio < rq->curr->prio) > > would there not need to be a check for p->migrate_disable ? > push_rt_task() is not checking and so a high prio RT task > preemting a low prio RT task in a migrate_disable() section > would actually push it off this cpu ? atleast I did not > find why that would not happen. Hi, Nicholas! p is not rq->curr, so its p->migrate_disable state is already updated and it can't be pushed (nr_cpus_allowed == 1 and it's not pushable). (If I understand right, that you worry about this). Kirill > thx! > hofrat -- 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]sched/rt: Do not try to push tasks if pinned task switches to RT
On Wed, 12 Mar 2014, Steven Rostedt wrote: > Peter, > > I'm going through my inbox (over a year old), and found this patch from > Kirill. It looks fine to me. You can apply it with my > > Acked-by: Steven Rostedt > > -- Steve > > [PATCH]sched/rt: Do not try to push tasks if pinned task switches to RT > > Just switched pinned task is not able to be pushed. If the rq had had > several RT tasks before they have already been considered as candidates > to be pushed (or pulled). > > Signed-off-by: Kirill V Tkhai > CC: Steven Rostedt > CC: Ingo Molnar > CC: Peter Zijlstra > CC: linux-rt-users > --- > kernel/sched/rt.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > index 4e8f0f4..5aab032 100644 > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct > task_struct *p) >*/ > if (p->on_rq && rq->curr != p) { > #ifdef CONFIG_SMP > - if (rq->rt.overloaded && push_rt_task(rq) && > + if (p->nr_cpus_allowed > 1 && rq->rt.overloaded && > /* Don't resched if we changed runqueues */ > - rq != task_rq(p)) > + push_rt_task(rq) && rq != task_rq(p)) > check_resched = 0; > #endif /* CONFIG_SMP */ > if (check_resched && p->prio < rq->curr->prio) would there not need to be a check for p->migrate_disable ? push_rt_task() is not checking and so a high prio RT task preemting a low prio RT task in a migrate_disable() section would actually push it off this cpu ? atleast I did not find why that would not happen. thx! hofrat -- 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]sched/rt: Do not try to push tasks if pinned task switches to RT
Peter, I'm going through my inbox (over a year old), and found this patch from Kirill. It looks fine to me. You can apply it with my Acked-by: Steven Rostedt -- Steve [PATCH]sched/rt: Do not try to push tasks if pinned task switches to RT Just switched pinned task is not able to be pushed. If the rq had had several RT tasks before they have already been considered as candidates to be pushed (or pulled). Signed-off-by: Kirill V Tkhai CC: Steven Rostedt CC: Ingo Molnar CC: Peter Zijlstra CC: linux-rt-users --- kernel/sched/rt.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 4e8f0f4..5aab032 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct task_struct *p) */ if (p->on_rq && rq->curr != p) { #ifdef CONFIG_SMP - if (rq->rt.overloaded && push_rt_task(rq) && + if (p->nr_cpus_allowed > 1 && rq->rt.overloaded && /* Don't resched if we changed runqueues */ - rq != task_rq(p)) + push_rt_task(rq) && rq != task_rq(p)) check_resched = 0; #endif /* CONFIG_SMP */ if (check_resched && p->prio < rq->curr->prio) -- 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]sched/rt: Do not try to push tasks if pinned task switches to RT
> On Tue, 2013-01-29 at 00:23 +0400, Kirill Tkhai wrote: > >> Just switched pinned task is not able to be pushed. If the rq had had >> several RT tasks before they have already been considered as candidates >> to be pushed (or pulled). > > Thanks, but I have one minor nit. > >> Signed-off-by: Kirill V Tkhai >> CC: Steven Rostedt >> CC: Ingo Molnar >> CC: Peter Zijlstra >> CC: linux-rt-users >> --- >> kernel/sched/rt.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c >> index 4e8f0f4..5f7d92b 100644 >> --- a/kernel/sched/rt.c >> +++ b/kernel/sched/rt.c >> @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct >> task_struct *p) >> */ >> if (p->on_rq && rq->curr != p) { >> #ifdef CONFIG_SMP >> - if (rq->rt.overloaded && push_rt_task(rq) && >> + if (rq->rt.overloaded && p->nr_cpus_allowed > 1 && > > I would swap the order here. That is, check p->nr_cpus_allowed before > rq->rt.overloaded. The order doesn't really matter, and even though > rq->rt.overloaded is probably more likely to be 0, it is used by other > CPUs. And as a micro-optimization, I rather read something that is used > by only one CPU than to read something that may be modified by many > CPUs. > > -- Steve > I corrected. Thanks. [PATCH]sched/rt: Do not try to push tasks if pinned task switches to RT Just switched pinned task is not able to be pushed. If the rq had had several RT tasks before they have already been considered as candidates to be pushed (or pulled). Signed-off-by: Kirill V Tkhai CC: Steven Rostedt CC: Ingo Molnar CC: Peter Zijlstra CC: linux-rt-users --- kernel/sched/rt.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 4e8f0f4..5aab032 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct task_struct *p) */ if (p->on_rq && rq->curr != p) { #ifdef CONFIG_SMP - if (rq->rt.overloaded && push_rt_task(rq) && + if (p->nr_cpus_allowed > 1 && rq->rt.overloaded && /* Don't resched if we changed runqueues */ - rq != task_rq(p)) + push_rt_task(rq) && rq != task_rq(p)) check_resched = 0; #endif /* CONFIG_SMP */ if (check_resched && p->prio < rq->curr->prio) -- 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]sched/rt: Do not try to push tasks if pinned task switches to RT
On Tue, 2013-01-29 at 00:23 +0400, Kirill Tkhai wrote: > Just switched pinned task is not able to be pushed. If the rq had had > several RT tasks before they have already been considered as candidates > to be pushed (or pulled). Thanks, but I have one minor nit. > > Signed-off-by: Kirill V Tkhai > CC: Steven Rostedt > CC: Ingo Molnar > CC: Peter Zijlstra > CC: linux-rt-users > --- > kernel/sched/rt.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > index 4e8f0f4..5f7d92b 100644 > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct > task_struct *p) >*/ > if (p->on_rq && rq->curr != p) { > #ifdef CONFIG_SMP > - if (rq->rt.overloaded && push_rt_task(rq) && > + if (rq->rt.overloaded && p->nr_cpus_allowed > 1 && I would swap the order here. That is, check p->nr_cpus_allowed before rq->rt.overloaded. The order doesn't really matter, and even though rq->rt.overloaded is probably more likely to be 0, it is used by other CPUs. And as a micro-optimization, I rather read something that is used by only one CPU than to read something that may be modified by many CPUs. -- Steve > /* Don't resched if we changed runqueues */ > - rq != task_rq(p)) > + push_rt_task(rq) && rq != task_rq(p)) > check_resched = 0; > #endif /* CONFIG_SMP */ > if (check_resched && p->prio < rq->curr->prio) -- 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: Re:[PATCH]sched/rt: Do not try to push tasks if pinned task switches to RT
On Tue, 2013-01-29 at 13:13 +0400, Kirill Tkhai wrote: > > > > The task of p->nr_cpus_allowed =1 would`t be added to pushable_tasks list > > (see the enqueue_task_rt())and this push_rt_task() need to push other tasks > > when rt.overloaded. > > If rq has already had 2 or more pushable tasks and we try to add a pinned > task then call of push_rt_task will just waste a time. > Correct, I just have some minor comments about the patch. -- 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]sched/rt: Do not try to push tasks if pinned task switches to RT
> From: Libo Chen > > On 2013-1-29 4:23, Kirill Tkhai wrote: > >> Just switched pinned task is not able to be pushed. If the rq had had >> several RT tasks before they have already been considered as candidates >> to be pushed (or pulled). >> >> Signed-off-by: Kirill V Tkhai >> CC: Steven Rostedt >> CC: Ingo Molnar >> CC: Peter Zijlstra >> CC: linux-rt-users >> --- >> kernel/sched/rt.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c >> index 4e8f0f4..5f7d92b 100644 >> --- a/kernel/sched/rt.c >> +++ b/kernel/sched/rt.c >> @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct >> task_struct *p) >> */ >> if (p->on_rq && rq->curr != p) { >> #ifdef CONFIG_SMP >> - if (rq->rt.overloaded && push_rt_task(rq) && >> + if (rq->rt.overloaded && p->nr_cpus_allowed > 1 && >> /* Don't resched if we changed runqueues */ >> - rq != task_rq(p)) >> + push_rt_task(rq) && rq != task_rq(p)) > > I think you worry about it was excess to call push_rt_task, since the task of > p->nr_cpus_allowed=1 can`t be pushed. is that right? Sure. > > The task of p->nr_cpus_allowed =1 would`t be added to pushable_tasks list > (see the enqueue_task_rt())and this push_rt_task() need to push other tasks > when rt.overloaded. If rq has already had 2 or more pushable tasks and we try to add a pinned task then call of push_rt_task will just waste a time. > > So I don`t agree this patch. > >> check_resched = 0; >> #endif /* CONFIG_SMP */ >> if (check_resched && p->prio < rq->curr->prio) >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- -- 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]sched/rt: Do not try to push tasks if pinned task switches to RT
From: Libo Chen On 2013-1-29 4:23, Kirill Tkhai wrote: > Just switched pinned task is not able to be pushed. If the rq had had > several RT tasks before they have already been considered as candidates > to be pushed (or pulled). > > Signed-off-by: Kirill V Tkhai > CC: Steven Rostedt > CC: Ingo Molnar > CC: Peter Zijlstra > CC: linux-rt-users > --- > kernel/sched/rt.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > index 4e8f0f4..5f7d92b 100644 > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct > task_struct *p) >*/ > if (p->on_rq && rq->curr != p) { > #ifdef CONFIG_SMP > - if (rq->rt.overloaded && push_rt_task(rq) && > + if (rq->rt.overloaded && p->nr_cpus_allowed > 1 && > /* Don't resched if we changed runqueues */ > - rq != task_rq(p)) > + push_rt_task(rq) && rq != task_rq(p)) I think you worry about it was excess to call push_rt_task, since the task of p->nr_cpus_allowed=1 can`t be pushed. is that right? The task of p->nr_cpus_allowed =1 would`t be added to pushable_tasks list (see the enqueue_task_rt())and this push_rt_task() need to push other tasks when rt.overloaded. So I don`t agree this patch. > check_resched = 0; > #endif /* CONFIG_SMP */ > if (check_resched && p->prio < rq->curr->prio) > -- > To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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/
[PATCH]sched/rt: Do not try to push tasks if pinned task switches to RT
Just switched pinned task is not able to be pushed. If the rq had had several RT tasks before they have already been considered as candidates to be pushed (or pulled). Signed-off-by: Kirill V Tkhai CC: Steven Rostedt CC: Ingo Molnar CC: Peter Zijlstra CC: linux-rt-users --- kernel/sched/rt.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 4e8f0f4..5f7d92b 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1925,9 +1925,9 @@ static void switched_to_rt(struct rq *rq, struct task_struct *p) */ if (p->on_rq && rq->curr != p) { #ifdef CONFIG_SMP - if (rq->rt.overloaded && push_rt_task(rq) && + if (rq->rt.overloaded && p->nr_cpus_allowed > 1 && /* Don't resched if we changed runqueues */ - rq != task_rq(p)) + push_rt_task(rq) && rq != task_rq(p)) check_resched = 0; #endif /* CONFIG_SMP */ if (check_resched && p->prio < rq->curr->prio) -- 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/