| Message ID | 1504007201-12904-4-git-send-email-yang.zhang.wz@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 29, 2017 at 11:46:37AM +0000, Yang Zhang wrote: > Add poll in do_idle. For UP VM, if there are running task, it will not > goes into idle path, so we only enable poll in SMP VM. > > Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> > Signed-off-by: Quan Xu <quan.xu0@gmail.com> Broken SoB chain. > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c > index 6c23e30..b374744 100644 > --- a/kernel/sched/idle.c > +++ b/kernel/sched/idle.c > @@ -74,6 +74,7 @@ static noinline int __cpuidle cpu_idle_poll(void) > } > > /* Weak implementations for optional arch specific functions */ > +void __weak arch_cpu_idle_poll(void) { } > void __weak arch_cpu_idle_prepare(void) { } > void __weak arch_cpu_idle_enter(void) { } And not a word on why we need a new arch hook. What's wrong with arch_cpu_idle_enter() for instance?
On Tue, Aug 29, 2017 at 11:46:37AM +0000, Yang Zhang wrote: > Add poll in do_idle. For UP VM, if there are running task, it will not > goes into idle path, so we only enable poll in SMP VM. > > Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> > Signed-off-by: Quan Xu <quan.xu0@gmail.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: x86@kernel.org > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Kyle Huey <me@kylehuey.com> > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Len Brown <len.brown@intel.com> > Cc: linux-kernel@vger.kernel.org > --- > arch/x86/kernel/process.c | 7 +++++++ > kernel/sched/idle.c | 2 ++ > 2 files changed, 9 insertions(+) > > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c > index 3ca1980..def4113 100644 > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -332,6 +332,13 @@ void arch_cpu_idle(void) > x86_idle(); > } > > +#if defined(CONFIG_SMP) && defined(CONFIG_PARAVIRT) > +void arch_cpu_idle_poll(void) > +{ > + paravirt_idle_poll(); > +} > +#endif So this will get called on any system which has CONFIG_PARAVIRT enabled *even* if they're not running any guests. Huh?
on 2017/8/29 20:45, Peter Zijlstra wrote: > On Tue, Aug 29, 2017 at 11:46:37AM +0000, Yang Zhang wrote: >> Add poll in do_idle. For UP VM, if there are running task, it will not >> goes into idle path, so we only enable poll in SMP VM. >> >> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> >> Signed-off-by: Quan Xu <quan.xu0@gmail.com> > Broken SoB chain. Peter, I can't follow 'Broken SoB chain'.. could you more about it? -Quan >> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c >> index 6c23e30..b374744 100644 >> --- a/kernel/sched/idle.c >> +++ b/kernel/sched/idle.c >> @@ -74,6 +74,7 @@ static noinline int __cpuidle cpu_idle_poll(void) >> } >> >> /* Weak implementations for optional arch specific functions */ >> +void __weak arch_cpu_idle_poll(void) { } >> void __weak arch_cpu_idle_prepare(void) { } >> void __weak arch_cpu_idle_enter(void) { } > And not a word on why we need a new arch hook. What's wrong with > arch_cpu_idle_enter() for instance?
on 2017/8/29 22:39, Borislav Petkov wrote: > On Tue, Aug 29, 2017 at 11:46:37AM +0000, Yang Zhang wrote: >> Add poll in do_idle. For UP VM, if there are running task, it will not >> goes into idle path, so we only enable poll in SMP VM. >> >> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> >> Signed-off-by: Quan Xu <quan.xu0@gmail.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: x86@kernel.org >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: Borislav Petkov <bp@alien8.de> >> Cc: Kyle Huey <me@kylehuey.com> >> Cc: Andy Lutomirski <luto@kernel.org> >> Cc: Len Brown <len.brown@intel.com> >> Cc: linux-kernel@vger.kernel.org >> --- >> arch/x86/kernel/process.c | 7 +++++++ >> kernel/sched/idle.c | 2 ++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c >> index 3ca1980..def4113 100644 >> --- a/arch/x86/kernel/process.c >> +++ b/arch/x86/kernel/process.c >> @@ -332,6 +332,13 @@ void arch_cpu_idle(void) >> x86_idle(); >> } >> >> +#if defined(CONFIG_SMP) && defined(CONFIG_PARAVIRT) >> +void arch_cpu_idle_poll(void) >> +{ >> + paravirt_idle_poll(); >> +} >> +#endif > So this will get called on any system which has CONFIG_PARAVIRT enabled > *even* if they're not running any guests. > > Huh? Borislav , yes, this will get called on any system which has CONFIG_PARAVIRT enabled. but if they are not running any guests, the callback is paravirt_nop() , IIUC which is as similar as the other paravirt_*, such as paravirt_pgd_free().. - Quan
on 2017/9/1 13:57, Quan Xu wrote: > on 2017/8/29 20:45, Peter Zijlstra wrote: > >> On Tue, Aug 29, 2017 at 11:46:37AM +0000, Yang Zhang wrote: >>> Add poll in do_idle. For UP VM, if there are running task, it will not >>> goes into idle path, so we only enable poll in SMP VM. >>> >>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> >>> Signed-off-by: Quan Xu <quan.xu0@gmail.com> >> Broken SoB chain. > Peter, I can't follow 'Broken SoB chain'.. could you explain more > about it? > Peter, Ping.. Quan > -Quan > >>> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c >>> index 6c23e30..b374744 100644 >>> --- a/kernel/sched/idle.c >>> +++ b/kernel/sched/idle.c >>> @@ -74,6 +74,7 @@ static noinline int __cpuidle cpu_idle_poll(void) >>> } >>> /* Weak implementations for optional arch specific functions */ >>> +void __weak arch_cpu_idle_poll(void) { } >>> void __weak arch_cpu_idle_prepare(void) { } >>> void __weak arch_cpu_idle_enter(void) { } >> And not a word on why we need a new arch hook. What's wrong with >> arch_cpu_idle_enter() for instance? >
On Thu, Sep 14, 2017 at 04:41:39PM +0800, Quan Xu wrote: > > > On Tue, Aug 29, 2017 at 11:46:37AM +0000, Yang Zhang wrote: > > > > Add poll in do_idle. For UP VM, if there are running task, it will not > > > > goes into idle path, so we only enable poll in SMP VM. > > > > > > > > Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> > > > > Signed-off-by: Quan Xu <quan.xu0@gmail.com> > > > Broken SoB chain. > > Peter, I can't follow 'Broken SoB chain'.. could you explain more > > about it? > > > Peter, Ping.. The SOB chain needs to show the path from the author to the upstream maintainer. Yours has Yang before you, which doesn't say what his role is. Read Documentation/process/submitting-patches.rst, section 11.
On 2017/9/1 14:49, Quan Xu wrote: > on 2017/8/29 22:39, Borislav Petkov wrote: >> On Tue, Aug 29, 2017 at 11:46:37AM +0000, Yang Zhang wrote: >>> Add poll in do_idle. For UP VM, if there are running task, it will not >>> goes into idle path, so we only enable poll in SMP VM. >>> >>> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> >>> Signed-off-by: Quan Xu <quan.xu0@gmail.com> >>> Cc: Thomas Gleixner <tglx@linutronix.de> >>> Cc: Ingo Molnar <mingo@redhat.com> >>> Cc: "H. Peter Anvin" <hpa@zytor.com> >>> Cc: x86@kernel.org >>> Cc: Peter Zijlstra <peterz@infradead.org> >>> Cc: Borislav Petkov <bp@alien8.de> >>> Cc: Kyle Huey <me@kylehuey.com> >>> Cc: Andy Lutomirski <luto@kernel.org> >>> Cc: Len Brown <len.brown@intel.com> >>> Cc: linux-kernel@vger.kernel.org >>> --- >>> arch/x86/kernel/process.c | 7 +++++++ >>> kernel/sched/idle.c | 2 ++ >>> 2 files changed, 9 insertions(+) >>> >>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c >>> index 3ca1980..def4113 100644 >>> --- a/arch/x86/kernel/process.c >>> +++ b/arch/x86/kernel/process.c >>> @@ -332,6 +332,13 @@ void arch_cpu_idle(void) >>> x86_idle(); >>> } >>> +#if defined(CONFIG_SMP) && defined(CONFIG_PARAVIRT) >>> +void arch_cpu_idle_poll(void) >>> +{ >>> + paravirt_idle_poll(); >>> +} >>> +#endif >> So this will get called on any system which has CONFIG_PARAVIRT enabled >> *even* if they're not running any guests. >> >> Huh? Borislav, think twice, it is better to run ander an IF condition, to make sure they are running any guests. Quan > Borislav , > yes, this will get called on any system which has CONFIG_PARAVIRT > enabled. > > but if they are not running any guests, the callback is > paravirt_nop() , > IIUC which is as similar as the other paravirt_*, such as > paravirt_pgd_free().. > > - Quan
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index 3ca1980..def4113 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -332,6 +332,13 @@ void arch_cpu_idle(void) x86_idle(); } +#if defined(CONFIG_SMP) && defined(CONFIG_PARAVIRT) +void arch_cpu_idle_poll(void) +{ + paravirt_idle_poll(); +} +#endif + /* * We use this if we don't have any better idle routine.. */ diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index 6c23e30..b374744 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -74,6 +74,7 @@ static noinline int __cpuidle cpu_idle_poll(void) } /* Weak implementations for optional arch specific functions */ +void __weak arch_cpu_idle_poll(void) { } void __weak arch_cpu_idle_prepare(void) { } void __weak arch_cpu_idle_enter(void) { } void __weak arch_cpu_idle_exit(void) { } @@ -219,6 +220,7 @@ static void do_idle(void) */ __current_set_polling(); + arch_cpu_idle_poll(); quiet_vmstat(); tick_nohz_idle_enter();