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

From: Peter Xu <peterx@redhat.com>
To: Shivam Kumar <shivam.kumar1@nutanix.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Marc Zyngier <maz@kernel.org>,
	pbonzini@redhat.com, james.morse@arm.com,
	borntraeger@linux.ibm.com, david@redhat.com, kvm@vger.kernel.org,
	Shaju Abraham <shaju.abraham@nutanix.com>,
	Manish Mishra <manish.mishra@nutanix.com>,
	Anurag Madnawat <anurag.madnawat@nutanix.com>
Subject: Re: [PATCH v4 1/4] KVM: Implement dirty quota-based throttling of vcpus
Date: Thu, 14 Jul 2022 14:04:13 -0400	[thread overview]
Message-ID: <YtBanRozLuP9qoWs@xz-m1.local> (raw)
In-Reply-To: <2e5198b3-54ea-010e-c418-f98054befe1b@nutanix.com>

Hi, Shivam,

On Tue, Jul 05, 2022 at 12:51:01PM +0530, Shivam Kumar wrote:
> Hi, here's a summary of what needs to be changed and what should be kept as
> it is (purely my opinion based on the discussions we have had so far):
> 
> i) Moving the dirty quota check to mark_page_dirty_in_slot. Use kvm requests
> in dirty quota check. I hope that the ceiling-based approach, with proper
> documentation and an ioctl exposed for resetting 'dirty_quota' and
> 'pages_dirtied', is good enough. Please post your suggestions if you think
> otherwise.

An ioctl just for this could be an overkill to me.

Currently you exposes only "quota" to kvm_run, then when vmexit you have
exit fields contain both "quota" and "count".  I always think it's a bit
redundant.

What I'm thinking is:

  (1) Expose both "quota" and "count" in kvm_run, then:

      "quota" should only be written by userspace and read by kernel.
      "count" should only be written by kernel and read by the userspace. [*]

      [*] One special case is when the userspace found that there's risk of
      quota & count overflow, then the userspace:

        - Kick the vcpu out (so the kernel won't write to "count" anymore)
        - Update both "quota" and "count" to safe values
        - Resume the KVM_RUN

  (2) When quota reached, we don't need to copy quota/count in vmexit
      fields, since the userspace can read the realtime values in kvm_run.

Would this work?

> ii) The change in VMX's handle_invalid_guest_state() remains as it is.
> iii) For now, we are lazily updating dirty quota, i.e. we are updating it
> only when the vcpu exits to userspace with the exit reason
> KVM_EXIT_DIRTY_QUOTA_EXHAUSTED.

At least with above design, IMHO we can update kvm_run.quota in real time
without kicking vcpu threads (the quota only increases except the reset
phase for overflow).  Or is there any other reason you need to kick vcpu
out when updating quota?

Thanks,

-- 
Peter Xu