[1/2] KVM: polling: add architecture backend to disable polling
diff mbox series

Message ID 20190305103002.5801-2-borntraeger@de.ibm.com
State New
Headers show
Series
  • disabling halt polling for nested virtualization
Related show

Commit Message

Christian Borntraeger March 5, 2019, 10:30 a.m. UTC
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(-)

Comments

Cornelia Huck March 5, 2019, 11:03 a.m. UTC | #1
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?
Christian Borntraeger March 5, 2019, 12:36 p.m. UTC | #2
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?
Cornelia Huck March 5, 2019, 12:42 p.m. UTC | #3
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.

Patch
diff mbox series

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;