Re: [RFC PATCH v2 3/7] sched/idle: Add poll before enter real idle path
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 +, 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 Signed-off-by: Quan Xu Cc: Thomas Gleixner Cc: Ingo Molnar Cc: "H. Peter Anvin" Cc: x...@kernel.org Cc: Peter Zijlstra Cc: Borislav Petkov Cc: Kyle Huey Cc: Andy Lutomirski Cc: Len Brown 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
Re: [RFC PATCH v2 3/7] sched/idle: Add poll before enter real idle path
On Thu, Sep 14, 2017 at 04:41:39PM +0800, Quan Xu wrote: > > > On Tue, Aug 29, 2017 at 11:46:37AM +, 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 > > > > Signed-off-by: Quan Xu > > > 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. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [RFC PATCH v2 3/7] sched/idle: Add poll before enter real idle path
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 +, 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 Signed-off-by: Quan Xu 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?
Re: [RFC PATCH v2 3/7] sched/idle: Add poll before enter real idle path
on 2017/8/29 22:39, Borislav Petkov wrote: On Tue, Aug 29, 2017 at 11:46:37AM +, 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 Signed-off-by: Quan Xu Cc: Thomas Gleixner Cc: Ingo Molnar Cc: "H. Peter Anvin" Cc: x...@kernel.org Cc: Peter Zijlstra Cc: Borislav Petkov Cc: Kyle Huey Cc: Andy Lutomirski Cc: Len Brown 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
Re: [RFC PATCH v2 3/7] sched/idle: Add poll before enter real idle path
on 2017/8/29 20:45, Peter Zijlstra wrote: On Tue, Aug 29, 2017 at 11:46:37AM +, 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 Signed-off-by: Quan Xu 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?
Re: [RFC PATCH v2 3/7] sched/idle: Add poll before enter real idle path
On Tue, Aug 29, 2017 at 11:46:37AM +, 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 > Signed-off-by: Quan Xu > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: x...@kernel.org > Cc: Peter Zijlstra > Cc: Borislav Petkov > Cc: Kyle Huey > Cc: Andy Lutomirski > Cc: Len Brown > 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? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [RFC PATCH v2 3/7] sched/idle: Add poll before enter real idle path
On Tue, Aug 29, 2017 at 11:46:37AM +, 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 > Signed-off-by: Quan Xu 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?
[RFC PATCH v2 3/7] sched/idle: Add poll before enter real idle path
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 Signed-off-by: Quan Xu Cc: Thomas Gleixner Cc: Ingo Molnar Cc: "H. Peter Anvin" Cc: x...@kernel.org Cc: Peter Zijlstra Cc: Borislav Petkov Cc: Kyle Huey Cc: Andy Lutomirski Cc: Len Brown 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 + /* * 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(); -- 1.8.3.1