Re: [PATCH v6 1/5] KVM: Implement dirty quota-based throttling of vcpus

[Sean Christopherson <seanjc@google.com>]

On Mon, Oct 10, 2022, Shivam Kumar wrote:
> 
> 
> On 08/10/22 12:50 am, Sean Christopherson wrote:
> > On Fri, Oct 07, 2022, Sean Christopherson wrote:
> > > On Thu, Sep 15, 2022, Shivam Kumar wrote:
> > > Let's keep kvm_vcpu_check_dirty_quota(), IMO that's still the least awful name.
> > > 
> > > [*] https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_Yo-2B82LjHSOdyxKzT-40google.com&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=4hVFP4-J13xyn-OcN0apTCh8iKZRosf5OJTQePXBMB8&m=0-XNirx6DRihxIvWzzJHJnErbZelq39geArwcitkIRgMl23nTXBs57QP543DuFnw&s=7zXRbLuhXLpsET-zMv7muSajxOFUoktaL97P3huVuhA&e=
> > 
> > Actually, I take that back.  The code snippet itself is also flawed.  If userspace
> > increases the quota (or disables it entirely) between KVM snapshotting the quota
> > and making the request, then there's no need for KVM to exit to userspace.
> > 
> > So I think this can be:
> > 
> > static void kvm_vcpu_is_dirty_quota_exchausted(struct kvm_vcpu *vcpu)
> > {
> > #ifdef CONFIG_HAVE_KVM_DIRTY_QUOTA
> > 	u64 dirty_quota = READ_ONCE(vcpu->run->dirty_quota);
> > 
> > 	return dirty_quota && (vcpu->stat.generic.pages_dirtied >= dirty_quota);
> > #else
> > 	return false;
> > #endif
> > }
> > 
> > and the usage becomes:
> > 
> > 		if (kvm_vcpu_is_dirty_quota_exhausted(vcpu))
> > 			kvm_make_request(KVM_REQ_DIRTY_QUOTA_EXIT, vcpu);
> > 
> > More thoughts in the x86 patch.
> 
> Snapshotting is not a requirement for now anyway. We have plans to lazily
> update the quota, i.e. only when it needs to do more dirtying. This helps us
> prevent overthrottling of the VM due to skewed cases where some vcpus are
> mostly reading and the others are mostly wirting.

I don't see how snapshotting can ever be a sane requirement.  Requiring KVM to
exit if KVM detects an exhausted quota even if userspace changes the quota is
nonsensical as the resulting behavior is 100% non-determinstic unless userspace
is spying on the number of dirty pages.  And if userspace is constly polling the
number of dirty pages, what's the point of the exit?  Requiring KVM to exit in
this case puts extra burden on KVM without any meaningful benefit.

In other words, we need consider about how KVM's behavior impacts KVM's uABI, not
just about what userspace "needs".