| Message ID | 1442568465-71559-1-git-send-email-dingel@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18/09/2015 11:27, Dominik Dingel wrote: > Commit 2ee507c47293 ("sched: Add function single_task_running to let a task > check if it is the only task running on a cpu") referenced the current > runqueue with the smp_processor_id. When CONFIG_DEBUG_PREEMPT is enabled, > that is only allowed if preemption is disabled or the currrent task is > bound to the local cpu (e.g. kernel worker). > > With commit f78195129963 ("kvm: add halt_poll_ns module parameter") KVM > calls single_task_running. If CONFIG_DEBUG_PREEMPT is enabled that > generates a lot of kernel messages. > > To avoid adding preemption in that cases, as it would limit the usefulness, > we change single_task_running to access directly the cpu local runqueue. > > Cc: Tim Chen <tim.c.chen@linux.intel.com> > Suggested-by: Peter Zijlstra <peterz@infradead.org> > Cc: <stable@vger.kernel.org> # 4.2.x > Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com> > --- > kernel/sched/core.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 78b4bad10..5bfad0b 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2614,13 +2614,13 @@ unsigned long nr_running(void) > > /* > * Check if only the current task is running on the cpu. > + * > + * Caution result is subject to time-of-check-to-time-of-use race, > + * every caller is responsible to set up additional fences if necessary. Let's expand it a bit more: * Caution: this function does not check that the caller has disabled * preemption, thus the result might have a time-of-check-to-time-of-use * race. The caller is responsible to use this correctly, for example: * * - use it from a non-preemptable section * * - use it from a thread that is bound to a single CPU * * - use it in a loop where each iteration takes very little time * (e.g. a polling loop) */ I'll include it in my pull request. Paolo > */ > bool single_task_running(void) > { > - if (cpu_rq(smp_processor_id())->nr_running == 1) > - return true; > - else > - return false; > + return raw_rq()->nr_running == 1; > } > EXPORT_SYMBOL(single_task_running); > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 18 Sep 2015 13:26:53 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 18/09/2015 11:27, Dominik Dingel wrote: > > Commit 2ee507c47293 ("sched: Add function single_task_running to let a task > > check if it is the only task running on a cpu") referenced the current > > runqueue with the smp_processor_id. When CONFIG_DEBUG_PREEMPT is enabled, > > that is only allowed if preemption is disabled or the currrent task is > > bound to the local cpu (e.g. kernel worker). > > > > With commit f78195129963 ("kvm: add halt_poll_ns module parameter") KVM > > calls single_task_running. If CONFIG_DEBUG_PREEMPT is enabled that > > generates a lot of kernel messages. > > > > To avoid adding preemption in that cases, as it would limit the usefulness, > > we change single_task_running to access directly the cpu local runqueue. > > > > Cc: Tim Chen <tim.c.chen@linux.intel.com> > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > > Cc: <stable@vger.kernel.org> # 4.2.x > > Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com> > > --- > > kernel/sched/core.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index 78b4bad10..5bfad0b 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -2614,13 +2614,13 @@ unsigned long nr_running(void) > > > > /* > > * Check if only the current task is running on the cpu. > > + * > > + * Caution result is subject to time-of-check-to-time-of-use race, > > + * every caller is responsible to set up additional fences if necessary. > > Let's expand it a bit more: > > * Caution: this function does not check that the caller has disabled > * preemption, thus the result might have a time-of-check-to-time-of-use > * race. The caller is responsible to use this correctly, for example: > * > * - use it from a non-preemptable section > * > * - use it from a thread that is bound to a single CPU > * > * - use it in a loop where each iteration takes very little time > * (e.g. a polling loop) > */ > > I'll include it in my pull request. Sounds really good. Thank you! > Paolo > > > */ > > bool single_task_running(void) > > { > > - if (cpu_rq(smp_processor_id())->nr_running == 1) > > - return true; > > - else > > - return false; > > + return raw_rq()->nr_running == 1; > > } > > EXPORT_SYMBOL(single_task_running); > > > > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 18, 2015 at 01:26:53PM +0200, Paolo Bonzini wrote: > > +++ b/kernel/sched/core.c > > @@ -2614,13 +2614,13 @@ unsigned long nr_running(void) > > > > /* > > * Check if only the current task is running on the cpu. > > + * > > + * Caution result is subject to time-of-check-to-time-of-use race, > > + * every caller is responsible to set up additional fences if necessary. > > Let's expand it a bit more: > > * Caution: this function does not check that the caller has disabled > * preemption, thus the result might have a time-of-check-to-time-of-use > * race. The caller is responsible to use this correctly, for example: > * > * - use it from a non-preemptable section > * > * - use it from a thread that is bound to a single CPU > * > * - use it in a loop where each iteration takes very little time > * (e.g. a polling loop) > */ > > I'll include it in my pull request. In which case: Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 78b4bad10..5bfad0b 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2614,13 +2614,13 @@ unsigned long nr_running(void) /* * Check if only the current task is running on the cpu. + * + * Caution result is subject to time-of-check-to-time-of-use race, + * every caller is responsible to set up additional fences if necessary. */ bool single_task_running(void) { - if (cpu_rq(smp_processor_id())->nr_running == 1) - return true; - else - return false; + return raw_rq()->nr_running == 1; } EXPORT_SYMBOL(single_task_running);
Commit 2ee507c47293 ("sched: Add function single_task_running to let a task check if it is the only task running on a cpu") referenced the current runqueue with the smp_processor_id. When CONFIG_DEBUG_PREEMPT is enabled, that is only allowed if preemption is disabled or the currrent task is bound to the local cpu (e.g. kernel worker). With commit f78195129963 ("kvm: add halt_poll_ns module parameter") KVM calls single_task_running. If CONFIG_DEBUG_PREEMPT is enabled that generates a lot of kernel messages. To avoid adding preemption in that cases, as it would limit the usefulness, we change single_task_running to access directly the cpu local runqueue. Cc: Tim Chen <tim.c.chen@linux.intel.com> Suggested-by: Peter Zijlstra <peterz@infradead.org> Cc: <stable@vger.kernel.org> # 4.2.x Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com> --- kernel/sched/core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)