| Message ID | 20190305103002.5801-2-borntraeger@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | disabling halt polling for nested virtualization | expand |
On Tue, 5 Mar 2019 05:30:01 -0500 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > There are cases where halt polling is unwanted. For example when running > KVM on an over committed LPAR we rather want to give back the CPU to > neighbour LPARs instead of polling. Let us provide a callback that > allows architectures to disable polling. > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > include/linux/kvm_host.h | 10 ++++++++++ > virt/kvm/Kconfig | 3 +++ > virt/kvm/kvm_main.c | 2 +- > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index c38cc5eb7e73..5d2bbcf0b1de 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -1283,6 +1283,16 @@ static inline bool vcpu_valid_wakeup(struct kvm_vcpu *vcpu) > } > #endif /* CONFIG_HAVE_KVM_INVALID_WAKEUPS */ > > +#ifdef CONFIG_HAVE_KVM_NO_POLL > +/* Callback that tells if we must not poll */ > +bool kvm_arch_no_poll(void); > +#else > +static inline bool kvm_arch_no_poll(void) > +{ > + return false; > +} > +#endif /* CONFIG_HAVE_KVM_NO_POLL */ > + > #ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL > long kvm_arch_vcpu_async_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg); > diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig > index ea434ddc8499..aad9284c043a 100644 > --- a/virt/kvm/Kconfig > +++ b/virt/kvm/Kconfig > @@ -57,3 +57,6 @@ config HAVE_KVM_VCPU_ASYNC_IOCTL > > config HAVE_KVM_VCPU_RUN_PID_CHANGE > bool > + > +config HAVE_KVM_NO_POLL > + bool > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 076bc38963bf..c76d83532e14 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2248,7 +2248,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) > u64 block_ns; > > start = cur = ktime_get(); > - if (vcpu->halt_poll_ns) { > + if (vcpu->halt_poll_ns && !kvm_arch_no_poll()) { > ktime_t stop = ktime_add_ns(ktime_get(), vcpu->halt_poll_ns); > > ++vcpu->stat.halt_attempted_poll; What this gives us is a way to check whether we want to disable halt polling right now (e.g. because we have a high steal time right now). Both the drawback and the point in favour is that we check this every time we enter kvm_vcpu_block(). Are there (known) scenarios where we might want to disable halt polling for some period of time? E.g., when we know via some event that it will be counterproductive for some time or until we get an event that tells us it might make sense again?
On 05.03.2019 12:03, Cornelia Huck wrote: > On Tue, 5 Mar 2019 05:30:01 -0500 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> There are cases where halt polling is unwanted. For example when running >> KVM on an over committed LPAR we rather want to give back the CPU to >> neighbour LPARs instead of polling. Let us provide a callback that >> allows architectures to disable polling. >> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> include/linux/kvm_host.h | 10 ++++++++++ >> virt/kvm/Kconfig | 3 +++ >> virt/kvm/kvm_main.c | 2 +- >> 3 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index c38cc5eb7e73..5d2bbcf0b1de 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -1283,6 +1283,16 @@ static inline bool vcpu_valid_wakeup(struct kvm_vcpu *vcpu) >> } >> #endif /* CONFIG_HAVE_KVM_INVALID_WAKEUPS */ >> >> +#ifdef CONFIG_HAVE_KVM_NO_POLL >> +/* Callback that tells if we must not poll */ >> +bool kvm_arch_no_poll(void); >> +#else >> +static inline bool kvm_arch_no_poll(void) >> +{ >> + return false; >> +} >> +#endif /* CONFIG_HAVE_KVM_NO_POLL */ >> + >> #ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL >> long kvm_arch_vcpu_async_ioctl(struct file *filp, >> unsigned int ioctl, unsigned long arg); >> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig >> index ea434ddc8499..aad9284c043a 100644 >> --- a/virt/kvm/Kconfig >> +++ b/virt/kvm/Kconfig >> @@ -57,3 +57,6 @@ config HAVE_KVM_VCPU_ASYNC_IOCTL >> >> config HAVE_KVM_VCPU_RUN_PID_CHANGE >> bool >> + >> +config HAVE_KVM_NO_POLL >> + bool >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index 076bc38963bf..c76d83532e14 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -2248,7 +2248,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) >> u64 block_ns; >> >> start = cur = ktime_get(); >> - if (vcpu->halt_poll_ns) { >> + if (vcpu->halt_poll_ns && !kvm_arch_no_poll()) { >> ktime_t stop = ktime_add_ns(ktime_get(), vcpu->halt_poll_ns); >> >> ++vcpu->stat.halt_attempted_poll; > > What this gives us is a way to check whether we want to disable halt > polling right now (e.g. because we have a high steal time right now). > Both the drawback and the point in favour is that we check this every > time we enter kvm_vcpu_block(). > > Are there (known) scenarios where we might want to disable halt polling > for some period of time? E.g., when we know via some event that it will > be counterproductive for some time or until we get an event that tells > us it might make sense again? For these cases we probably want to have a userspace tool that sets/unsets /sys/module/kvm/parameters/halt_poll_ns?
On Tue, 5 Mar 2019 13:36:30 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 05.03.2019 12:03, Cornelia Huck wrote: > > On Tue, 5 Mar 2019 05:30:01 -0500 > > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > > >> There are cases where halt polling is unwanted. For example when running > >> KVM on an over committed LPAR we rather want to give back the CPU to > >> neighbour LPARs instead of polling. Let us provide a callback that > >> allows architectures to disable polling. > >> > >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > >> --- > >> include/linux/kvm_host.h | 10 ++++++++++ > >> virt/kvm/Kconfig | 3 +++ > >> virt/kvm/kvm_main.c | 2 +- > >> 3 files changed, 14 insertions(+), 1 deletion(-) > >> > >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > >> index c38cc5eb7e73..5d2bbcf0b1de 100644 > >> --- a/include/linux/kvm_host.h > >> +++ b/include/linux/kvm_host.h > >> @@ -1283,6 +1283,16 @@ static inline bool vcpu_valid_wakeup(struct kvm_vcpu *vcpu) > >> } > >> #endif /* CONFIG_HAVE_KVM_INVALID_WAKEUPS */ > >> > >> +#ifdef CONFIG_HAVE_KVM_NO_POLL > >> +/* Callback that tells if we must not poll */ > >> +bool kvm_arch_no_poll(void); > >> +#else > >> +static inline bool kvm_arch_no_poll(void) > >> +{ > >> + return false; > >> +} > >> +#endif /* CONFIG_HAVE_KVM_NO_POLL */ > >> + > >> #ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL > >> long kvm_arch_vcpu_async_ioctl(struct file *filp, > >> unsigned int ioctl, unsigned long arg); > >> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig > >> index ea434ddc8499..aad9284c043a 100644 > >> --- a/virt/kvm/Kconfig > >> +++ b/virt/kvm/Kconfig > >> @@ -57,3 +57,6 @@ config HAVE_KVM_VCPU_ASYNC_IOCTL > >> > >> config HAVE_KVM_VCPU_RUN_PID_CHANGE > >> bool > >> + > >> +config HAVE_KVM_NO_POLL > >> + bool > >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > >> index 076bc38963bf..c76d83532e14 100644 > >> --- a/virt/kvm/kvm_main.c > >> +++ b/virt/kvm/kvm_main.c > >> @@ -2248,7 +2248,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) > >> u64 block_ns; > >> > >> start = cur = ktime_get(); > >> - if (vcpu->halt_poll_ns) { > >> + if (vcpu->halt_poll_ns && !kvm_arch_no_poll()) { > >> ktime_t stop = ktime_add_ns(ktime_get(), vcpu->halt_poll_ns); > >> > >> ++vcpu->stat.halt_attempted_poll; > > > > What this gives us is a way to check whether we want to disable halt > > polling right now (e.g. because we have a high steal time right now). > > Both the drawback and the point in favour is that we check this every > > time we enter kvm_vcpu_block(). > > > > Are there (known) scenarios where we might want to disable halt polling > > for some period of time? E.g., when we know via some event that it will > > be counterproductive for some time or until we get an event that tells > > us it might make sense again? > > For these cases we probably want to have a userspace tool that sets/unsets > /sys/module/kvm/parameters/halt_poll_ns? Yes, if userspace has/can get the relevant information, that makes sense.
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index c38cc5eb7e73..5d2bbcf0b1de 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1283,6 +1283,16 @@ static inline bool vcpu_valid_wakeup(struct kvm_vcpu *vcpu) } #endif /* CONFIG_HAVE_KVM_INVALID_WAKEUPS */ +#ifdef CONFIG_HAVE_KVM_NO_POLL +/* Callback that tells if we must not poll */ +bool kvm_arch_no_poll(void); +#else +static inline bool kvm_arch_no_poll(void) +{ + return false; +} +#endif /* CONFIG_HAVE_KVM_NO_POLL */ + #ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL long kvm_arch_vcpu_async_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg); diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig index ea434ddc8499..aad9284c043a 100644 --- a/virt/kvm/Kconfig +++ b/virt/kvm/Kconfig @@ -57,3 +57,6 @@ config HAVE_KVM_VCPU_ASYNC_IOCTL config HAVE_KVM_VCPU_RUN_PID_CHANGE bool + +config HAVE_KVM_NO_POLL + bool diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 076bc38963bf..c76d83532e14 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2248,7 +2248,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) u64 block_ns; start = cur = ktime_get(); - if (vcpu->halt_poll_ns) { + if (vcpu->halt_poll_ns && !kvm_arch_no_poll()) { ktime_t stop = ktime_add_ns(ktime_get(), vcpu->halt_poll_ns); ++vcpu->stat.halt_attempted_poll;
There are cases where halt polling is unwanted. For example when running KVM on an over committed LPAR we rather want to give back the CPU to neighbour LPARs instead of polling. Let us provide a callback that allows architectures to disable polling. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- include/linux/kvm_host.h | 10 ++++++++++ virt/kvm/Kconfig | 3 +++ virt/kvm/kvm_main.c | 2 +- 3 files changed, 14 insertions(+), 1 deletion(-)