[0/2] disabling halt polling for nested virtualization
mbox series

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

Message

Christian Borntraeger March 5, 2019, 10:30 a.m. UTC
Folks,

this is a very simple variant to disable halt polling when the KVM host
is already running virtualized. We could imagine more complex variants
(like tuning down the halt polling value) but this seems to do the trick
for some kvm deployment scenarios on s390x. (e.g. having multiple LPARS
with KVMs that are in itself already overcommitted). 
This still needs tuning and the right default value, but I want to start
the discussion.


Christian Borntraeger (2):
  KVM: polling: add architecture backend to disable polling
  KVM: s390: provide kvm_arch_no_poll function

 arch/s390/include/asm/kvm_host.h |  6 ++++++
 arch/s390/kvm/Kconfig            |  1 +
 include/linux/kvm_host.h         | 10 ++++++++++
 virt/kvm/Kconfig                 |  3 +++
 virt/kvm/kvm_main.c              |  2 +-
 5 files changed, 21 insertions(+), 1 deletion(-)

Comments

David Hildenbrand March 5, 2019, 10:41 a.m. UTC | #1
On 05.03.19 11:30, Christian Borntraeger wrote:
> Folks,
> 
> this is a very simple variant to disable halt polling when the KVM host
> is already running virtualized. We could imagine more complex variants
> (like tuning down the halt polling value) but this seems to do the trick
> for some kvm deployment scenarios on s390x. (e.g. having multiple LPARS
> with KVMs that are in itself already overcommitted). 
> This still needs tuning and the right default value, but I want to start
> the discussion.
> 
> 
> Christian Borntraeger (2):
>   KVM: polling: add architecture backend to disable polling
>   KVM: s390: provide kvm_arch_no_poll function
> 
>  arch/s390/include/asm/kvm_host.h |  6 ++++++
>  arch/s390/kvm/Kconfig            |  1 +
>  include/linux/kvm_host.h         | 10 ++++++++++
>  virt/kvm/Kconfig                 |  3 +++
>  virt/kvm/kvm_main.c              |  2 +-
>  5 files changed, 21 insertions(+), 1 deletion(-)
> 


Makes perfect sense to me.
Cornelia Huck March 5, 2019, 10:56 a.m. UTC | #2
On Tue,  5 Mar 2019 05:30:00 -0500
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> Folks,
> 
> this is a very simple variant to disable halt polling when the KVM host
> is already running virtualized. We could imagine more complex variants
> (like tuning down the halt polling value) but this seems to do the trick
> for some kvm deployment scenarios on s390x. (e.g. having multiple LPARS
> with KVMs that are in itself already overcommitted). 

Yes, it makes sense to disable halt polling in that scenario, but I
wonder whether we can make "disable halt polling for high steal times"
more architecture-independent (can we obtain steal time quickly in some
kind of architecture-independent way?)

Btw, power seems to have its own halt polling code; not sure if it
makes sense there as well.

> This still needs tuning and the right default value, but I want to start
> the discussion.

Maybe optionally log a stat?

> 
> 
> Christian Borntraeger (2):
>   KVM: polling: add architecture backend to disable polling
>   KVM: s390: provide kvm_arch_no_poll function
> 
>  arch/s390/include/asm/kvm_host.h |  6 ++++++
>  arch/s390/kvm/Kconfig            |  1 +
>  include/linux/kvm_host.h         | 10 ++++++++++
>  virt/kvm/Kconfig                 |  3 +++
>  virt/kvm/kvm_main.c              |  2 +-
>  5 files changed, 21 insertions(+), 1 deletion(-)
>
Christian Borntraeger March 5, 2019, 12:35 p.m. UTC | #3
On 05.03.2019 11:56, Cornelia Huck wrote:
> On Tue,  5 Mar 2019 05:30:00 -0500
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> Folks,
>>
>> this is a very simple variant to disable halt polling when the KVM host
>> is already running virtualized. We could imagine more complex variants
>> (like tuning down the halt polling value) but this seems to do the trick
>> for some kvm deployment scenarios on s390x. (e.g. having multiple LPARS
>> with KVMs that are in itself already overcommitted). 
> 
> Yes, it makes sense to disable halt polling in that scenario, but I
> wonder whether we can make "disable halt polling for high steal times"
> more architecture-independent (can we obtain steal time quickly in some
> kind of architecture-independent way?)

Not sure. I think an per-architecture callback should be simple enough
Having it generic is likely going to make it more complicated than necessary.

> 
> Btw, power seems to have its own halt polling code; not sure if it
> makes sense there as well.
> 
>> This still needs tuning and the right default value, but I want to start
>> the discussion.
> 
> Maybe optionally log a stat?

I will have a look.
Cornelia Huck March 5, 2019, 12:43 p.m. UTC | #4
On Tue, 5 Mar 2019 13:35:27 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 05.03.2019 11:56, Cornelia Huck wrote:
> > On Tue,  5 Mar 2019 05:30:00 -0500
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >   
> >> Folks,
> >>
> >> this is a very simple variant to disable halt polling when the KVM host
> >> is already running virtualized. We could imagine more complex variants
> >> (like tuning down the halt polling value) but this seems to do the trick
> >> for some kvm deployment scenarios on s390x. (e.g. having multiple LPARS
> >> with KVMs that are in itself already overcommitted).   
> > 
> > Yes, it makes sense to disable halt polling in that scenario, but I
> > wonder whether we can make "disable halt polling for high steal times"
> > more architecture-independent (can we obtain steal time quickly in some
> > kind of architecture-independent way?)  
> 
> Not sure. I think an per-architecture callback should be simple enough
> Having it generic is likely going to make it more complicated than necessary.

Fair enough.

> 
> > 
> > Btw, power seems to have its own halt polling code; not sure if it
> > makes sense there as well.
> >   
> >> This still needs tuning and the right default value, but I want to start
> >> the discussion.  
> > 
> > Maybe optionally log a stat?  
> 
> I will have a look.
>
Paolo Bonzini March 15, 2019, 5:50 p.m. UTC | #5
On 05/03/19 11:30, Christian Borntraeger wrote:
> Folks,
> 
> this is a very simple variant to disable halt polling when the KVM host
> is already running virtualized. We could imagine more complex variants
> (like tuning down the halt polling value) but this seems to do the trick
> for some kvm deployment scenarios on s390x. (e.g. having multiple LPARS
> with KVMs that are in itself already overcommitted). 
> This still needs tuning and the right default value, but I want to start
> the discussion.
> 
> 
> Christian Borntraeger (2):
>   KVM: polling: add architecture backend to disable polling
>   KVM: s390: provide kvm_arch_no_poll function
> 
>  arch/s390/include/asm/kvm_host.h |  6 ++++++
>  arch/s390/kvm/Kconfig            |  1 +
>  include/linux/kvm_host.h         | 10 ++++++++++
>  virt/kvm/Kconfig                 |  3 +++
>  virt/kvm/kvm_main.c              |  2 +-
>  5 files changed, 21 insertions(+), 1 deletion(-)
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Can you resubmit with a fix for the issue pointed out by the test robot?

Thanks,

Paolo
Christian Borntraeger March 18, 2019, 8:39 a.m. UTC | #6
On 15.03.19 18:50, Paolo Bonzini wrote:
> On 05/03/19 11:30, Christian Borntraeger wrote:
>> Folks,
>>
>> this is a very simple variant to disable halt polling when the KVM host
>> is already running virtualized. We could imagine more complex variants
>> (like tuning down the halt polling value) but this seems to do the trick
>> for some kvm deployment scenarios on s390x. (e.g. having multiple LPARS
>> with KVMs that are in itself already overcommitted). 
>> This still needs tuning and the right default value, but I want to start
>> the discussion.
>>
>>
>> Christian Borntraeger (2):
>>   KVM: polling: add architecture backend to disable polling
>>   KVM: s390: provide kvm_arch_no_poll function
>>
>>  arch/s390/include/asm/kvm_host.h |  6 ++++++
>>  arch/s390/kvm/Kconfig            |  1 +
>>  include/linux/kvm_host.h         | 10 ++++++++++
>>  virt/kvm/Kconfig                 |  3 +++
>>  virt/kvm/kvm_main.c              |  2 +-
>>  5 files changed, 21 insertions(+), 1 deletion(-)
>>
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Can you resubmit with a fix for the issue pointed out by the test robot?

Yes, this was based on a patch that was still missing in linux-next. 
Its now there.


I will also add some instrumentation (kvm stat counter I think) in the next
version.