diff mbox series

[gmem,2/6] KVM: guest_memfd: Only call kvm_arch_gmem_prepare hook if necessary

Message ID 20240329212444.395559-3-michael.roth@amd.com (mailing list archive)
State New, archived
Headers show
Series gmem fix-ups and interfaces for populating gmem pages | expand

Commit Message

Michael Roth March 29, 2024, 9:24 p.m. UTC
It has been reported that the internal workings of
kvm_gmem_prepare_folio() incurs noticeable overhead for large guests
even for platforms where kvm_arch_gmem_prepare() is a no-op.

Provide a new kvm_arch_gmem_prepare_needed() hook so that architectures
that set CONFIG_HAVE_KVM_GMEM_PREPARE can still opt-out of issuing the
kvm_arch_gmem_prepare() callback if the particular KVM instance doesn't
require any sort of special preparation of its gmem pages prior to use.

Link: https://lore.kernel.org/lkml/20240228202906.GB10568@ls.amr.corp.intel.com/
Suggested-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 include/linux/kvm_host.h |  1 +
 virt/kvm/guest_memfd.c   | 10 ++++++++++
 2 files changed, 11 insertions(+)

Comments

Binbin Wu April 1, 2024, 5:06 a.m. UTC | #1
On 3/30/2024 5:24 AM, Michael Roth wrote:
> It has been reported that the internal workings of
> kvm_gmem_prepare_folio() incurs noticeable overhead for large guests
> even for platforms where kvm_arch_gmem_prepare() is a no-op.
>
> Provide a new kvm_arch_gmem_prepare_needed() hook so that architectures
> that set CONFIG_HAVE_KVM_GMEM_PREPARE can still opt-out of issuing the
> kvm_arch_gmem_prepare() callback

Just wondering which part has big impact on performance,
the issue of kvm_arch_gmem_prepare() callback or the preparation code for
the kvm_arch_gmem_prepare()?


> if the particular KVM instance doesn't
> require any sort of special preparation of its gmem pages prior to use.
>
> Link: https://lore.kernel.org/lkml/20240228202906.GB10568@ls.amr.corp.intel.com/
> Suggested-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>   include/linux/kvm_host.h |  1 +
>   virt/kvm/guest_memfd.c   | 10 ++++++++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 2f5074eff958..5b8308b5e4af 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2466,6 +2466,7 @@ static inline int kvm_gmem_undo_get_pfn(struct kvm *kvm,
>   
>   #ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
>   int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_order);
> +bool kvm_arch_gmem_prepare_needed(struct kvm *kvm);
>   #endif
>   
>   #ifdef CONFIG_HAVE_KVM_GMEM_INVALIDATE
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 74e19170af8a..4ce0056d1149 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -13,6 +13,13 @@ struct kvm_gmem {
>   	struct list_head entry;
>   };
>   
> +#ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
> +bool __weak kvm_arch_gmem_prepare_needed(struct kvm *kvm)
> +{
> +	return false;
> +}
> +#endif
> +
>   static int kvm_gmem_prepare_folio(struct inode *inode, pgoff_t index, struct folio *folio)
>   {
>   #ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
> @@ -27,6 +34,9 @@ static int kvm_gmem_prepare_folio(struct inode *inode, pgoff_t index, struct fol
>   		gfn_t gfn;
>   		int rc;
>   
> +		if (!kvm_arch_gmem_prepare_needed(kvm))
> +			continue;

Can multiple gmems (if any) bound to the same inode's address space 
belong to different kvm instances?
If not, just return here?

> +
>   		slot = xa_load(&gmem->bindings, index);
>   		if (!slot)
>   			continue;
Isaku Yamahata April 2, 2024, 9:50 p.m. UTC | #2
On Mon, Apr 01, 2024 at 01:06:07PM +0800,
Binbin Wu <binbin.wu@linux.intel.com> wrote:

> 
> 
> On 3/30/2024 5:24 AM, Michael Roth wrote:
> > It has been reported that the internal workings of
> > kvm_gmem_prepare_folio() incurs noticeable overhead for large guests
> > even for platforms where kvm_arch_gmem_prepare() is a no-op.
> > 
> > Provide a new kvm_arch_gmem_prepare_needed() hook so that architectures
> > that set CONFIG_HAVE_KVM_GMEM_PREPARE can still opt-out of issuing the
> > kvm_arch_gmem_prepare() callback
> 
> Just wondering which part has big impact on performance,
> the issue of kvm_arch_gmem_prepare() callback or the preparation code for
> the kvm_arch_gmem_prepare()?

I'm fine without this patch for now baecause this is optimization.
diff mbox series

Patch

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2f5074eff958..5b8308b5e4af 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2466,6 +2466,7 @@  static inline int kvm_gmem_undo_get_pfn(struct kvm *kvm,
 
 #ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
 int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_order);
+bool kvm_arch_gmem_prepare_needed(struct kvm *kvm);
 #endif
 
 #ifdef CONFIG_HAVE_KVM_GMEM_INVALIDATE
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 74e19170af8a..4ce0056d1149 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -13,6 +13,13 @@  struct kvm_gmem {
 	struct list_head entry;
 };
 
+#ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
+bool __weak kvm_arch_gmem_prepare_needed(struct kvm *kvm)
+{
+	return false;
+}
+#endif
+
 static int kvm_gmem_prepare_folio(struct inode *inode, pgoff_t index, struct folio *folio)
 {
 #ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
@@ -27,6 +34,9 @@  static int kvm_gmem_prepare_folio(struct inode *inode, pgoff_t index, struct fol
 		gfn_t gfn;
 		int rc;
 
+		if (!kvm_arch_gmem_prepare_needed(kvm))
+			continue;
+
 		slot = xa_load(&gmem->bindings, index);
 		if (!slot)
 			continue;