Messages in this thread Patch in this message | | | Subject | Re: disabling halt polling broken? (was Re: [PATCH 00/14] KVM: Halt-polling fixes, cleanups and a new stat) | From | "wangyanan (Y)" <> | Date | Tue, 15 Nov 2022 11:28:56 +0800 |
| |
Hi Sean, Paolo,
I recently also notice the behavior change of param halt_poll_ns. Now it loses the ability to: 1) dynamically disable halt polling for all the running VMs by `echo 0 > /sys` 2) dynamically adjust the halt polling interval for all the running VMs by `echo * > /sys`
While in our cases, we usually use above two abilities, and KVM_CAP_HALT_POLL is not used yet.
On 2021/9/28 1:33, Sean Christopherson wrote: > On Mon, Sep 27, 2021, Paolo Bonzini wrote: >> On Mon, Sep 27, 2021 at 5:17 PM Christian Borntraeger >> <borntraeger@de.ibm.com> wrote: >>>> So I think there are two possibilities that makes sense: >>>> >>>> * track what is using KVM_CAP_HALT_POLL, and make writes to halt_poll_ns follow that >>> what about using halt_poll_ns for those VMs that did not uses KVM_CAP_HALT_POLL and the private number for those that did. >> Yes, that's what I meant. David pointed out that doesn't allow you to >> disable halt polling altogether, but for that you can always ask each >> VM's userspace one by one, or just not use KVM_CAP_HALT_POLL. (Also, I >> don't know about Google's usecase, but mine was actually more about >> using KVM_CAP_HALT_POLL to *disable* halt polling on some VMs!). > I kinda like the idea if special-casing halt_poll_ns=0, e.g. for testing or > in-the-field mitigation if halt-polling is broken. It'd be trivial to support, e.g. Do we have any plan to repost the diff as a fix? I would be very nice that this issue can be solved.
Besides, I think we may need some Doc for users to describe how halt_poll_ns works with KVM_CAP_HALT_POLL, like "Documentation/virt/guest-halt-polling.rst". > @@ -3304,19 +3304,23 @@ void kvm_vcpu_halt(struct kvm_vcpu *vcpu) > update_halt_poll_stats(vcpu, start, poll_end, !waited); > > if (halt_poll_allowed) { > + max_halt_poll_ns = vcpu->kvm->max_halt_poll_ns; > + if (!max_halt_poll_ns || !halt_poll_ns) <------ squish the max if halt_poll_ns==0 > + max_halt_poll_ns = halt_poll_ns; > + Does this mean that KVM_CAP_HALT_POLL will not be able to disable halt polling for a VM individually when halt_poll_ns !=0? > if (!vcpu_valid_wakeup(vcpu)) { > shrink_halt_poll_ns(vcpu); > - } else if (vcpu->kvm->max_halt_poll_ns) { > + } else if (max_halt_poll_ns) { > if (halt_ns <= vcpu->halt_poll_ns) > ; > /* we had a long block, shrink polling */ > else if (vcpu->halt_poll_ns && > - halt_ns > vcpu->kvm->max_halt_poll_ns) > + halt_ns > max_halt_poll_ns) > shrink_halt_poll_ns(vcpu); > /* we had a short halt and our poll time is too small */ > - else if (vcpu->halt_poll_ns < vcpu->kvm->max_halt_poll_ns && > - halt_ns < vcpu->kvm->max_halt_poll_ns) > - grow_halt_poll_ns(vcpu); > + else if (vcpu->halt_poll_ns < max_halt_poll_ns && > + halt_ns < max_halt_poll_ns) > + grow_halt_poll_ns(vcpu, max_halt_poll_ns); > } else { > vcpu->halt_poll_ns = 0; > } > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm > . Thanks, Yanan
| |