From: David Matlack <dmatlack@google.com>
To: Vipin Sharma <vipinsh@google.com>
Cc: seanjc@google.com, pbonzini@redhat.com, bgardon@google.com,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [Patch v3 6/9] KVM: Provide NUMA node support to kvm_mmu_memory_cache{}
Date: Thu, 29 Dec 2022 15:08:50 -0800 [thread overview]
Message-ID: <Y64eAvm4JglT1au4@google.com> (raw)
In-Reply-To: <20221222023457.1764-7-vipinsh@google.com>
On Wed, Dec 21, 2022 at 06:34:54PM -0800, Vipin Sharma wrote:
> Add 'node' variable in kvm_mmu_memory_cache{} to denote which NUMA node
> this cache should allocate memory from. Default initialize to
> NUMA_NO_NODE in all architectures.
>
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> ---
> arch/arm64/kvm/arm.c | 2 +-
> arch/arm64/kvm/mmu.c | 4 +++-
> arch/mips/kvm/mips.c | 2 ++
> arch/riscv/kvm/mmu.c | 2 +-
> arch/riscv/kvm/vcpu.c | 2 +-
> arch/x86/kvm/mmu/mmu.c | 22 ++++++++++++----------
> include/linux/kvm_host.h | 6 ++++++
> include/linux/kvm_types.h | 2 ++
> 8 files changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 9c5573bc4614..52a41f4532e2 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -340,7 +340,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> vcpu->arch.target = -1;
> bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);
>
> - vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
> + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_cache, NULL, NUMA_NO_NODE);
>
> /*
> * Default value for the FP state, will be overloaded at load
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 31d7fa4c7c14..bd07155e17fa 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -894,12 +894,14 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
> {
> phys_addr_t addr;
> int ret = 0;
> - struct kvm_mmu_memory_cache cache = { .gfp_zero = __GFP_ZERO };
> + struct kvm_mmu_memory_cache cache;
> struct kvm_pgtable *pgt = kvm->arch.mmu.pgt;
> enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_DEVICE |
> KVM_PGTABLE_PROT_R |
> (writable ? KVM_PGTABLE_PROT_W : 0);
>
> + INIT_KVM_MMU_MEMORY_CACHE(&cache, NULL, NUMA_NO_NODE);
This is not any better than setting cache.node = NUMA_NO_NODE directly.
Yes it's less lines of code, but it's harder to read (what does NULL
mean here?), and every user of kvm_mmu_memory_cache still has to know to
pass NUMA_NO_NODE.
When I originally gave this suggestion, I intended to suggest that
INIT_KVM_MMU_MEMORY_CACHE() provide just default initialization.
Non-default initialization for gfp_zero, gfp_custom, kmem_cache, and
node would remain as they are.
Yes this adds some more lines, but keeps things readable, and doesn't
every initialization site of kvm_mmu_memory_cache to know what to pass
for gfp_zero, node, and kmem_cache. It only needs to set the fields
*it* cares about.
Here's what I mean specifically, based on INIT_LIST_HEAD. I don't think
I got all the kvm_mmu_memory_cache users, but you get the point.
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 9c5573bc4614..0e138dcaf4d4 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -340,6 +340,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
vcpu->arch.target = -1;
bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);
+ INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_cache);
vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
/*
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 31d7fa4c7c14..f5fd78a4f084 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -894,12 +894,14 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
{
phys_addr_t addr;
int ret = 0;
- struct kvm_mmu_memory_cache cache = { .gfp_zero = __GFP_ZERO };
+ KVM_MMU_MEMORY_CACHE(cache);
struct kvm_pgtable *pgt = kvm->arch.mmu.pgt;
enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_DEVICE |
KVM_PGTABLE_PROT_R |
(writable ? KVM_PGTABLE_PROT_W : 0);
+ cache.gfp_zero = __GFP_ZERO;
+
if (is_protected_kvm_enabled())
return -EPERM;
diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index 34b57e0be2ef..7915a5a2d104 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -351,10 +351,11 @@ int kvm_riscv_gstage_ioremap(struct kvm *kvm, gpa_t gpa,
int ret = 0;
unsigned long pfn;
phys_addr_t addr, end;
- struct kvm_mmu_memory_cache pcache = {
- .gfp_custom = (in_atomic) ? GFP_ATOMIC | __GFP_ACCOUNT : 0,
- .gfp_zero = __GFP_ZERO,
- };
+ KVM_MMU_MEMORY_CACHE(pcache);
+
+ pcache.gfp_zero = __GFP_ZERO;
+ if (in_atomic)
+ pcache.gfp_custom = GFP_ATOMIC | __GFP_ACCOUNT;
end = (gpa + size + PAGE_SIZE - 1) & PAGE_MASK;
pfn = __phys_to_pfn(hpa);
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 7c08567097f0..3d73ab3ec9a4 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -161,6 +161,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
/* Mark this VCPU never ran */
vcpu->arch.ran_atleast_once = false;
+ INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_header_cache);
vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
bitmap_zero(vcpu->arch.isa, RISCV_ISA_EXT_MAX);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 254bc46234e0..d4cd8e64cc03 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5909,14 +5909,19 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
{
int ret;
+ INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_pte_list_desc_cache);
vcpu->arch.mmu_pte_list_desc_cache.kmem_cache = pte_list_desc_cache;
vcpu->arch.mmu_pte_list_desc_cache.gfp_zero = __GFP_ZERO;
+ INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_header_cache);
vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache;
vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO;
+ INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_shadow_page_cache);
vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO;
+ INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_shadowed_info_cache);
+
vcpu->arch.mmu = &vcpu->arch.root_mmu;
vcpu->arch.walk_mmu = &vcpu->arch.root_mmu;
@@ -6083,11 +6088,14 @@ int kvm_mmu_init_vm(struct kvm *kvm)
node->track_flush_slot = kvm_mmu_invalidate_zap_pages_in_memslot;
kvm_page_track_register_notifier(kvm, node);
+ INIT_KVM_MMU_MEMORY_CACHE(&kvm->arch.split_page_header_cache);
kvm->arch.split_page_header_cache.kmem_cache = mmu_page_header_cache;
kvm->arch.split_page_header_cache.gfp_zero = __GFP_ZERO;
+ INIT_KVM_MMU_MEMORY_CACHE(&kvm->arch.split_shadow_page_cache);
kvm->arch.split_shadow_page_cache.gfp_zero = __GFP_ZERO;
+ INIT_KVM_MMU_MEMORY_CACHE(&kvm->arch.split_desc_cache);
kvm->arch.split_desc_cache.kmem_cache = pte_list_desc_cache;
kvm->arch.split_desc_cache.gfp_zero = __GFP_ZERO;
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 76de36e56cdf..eb7ff9afa5c7 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -98,6 +98,17 @@ struct kvm_mmu_memory_cache {
int capacity;
void **objects;
};
+
+#define KVM_MMU_MEMORY_CACHE_INIT() (struct kvm_mmu_memory_cache) { \
+}
+
+#define KVM_MMU_MEMORY_CACHE(_name) \
+ struct kvm_mmu_memory_cache _name = KVM_MMU_MEMORY_CACHE_INIT()
+
+static inline void INIT_KVM_MMU_MEMORY_CACHE(struct kvm_mmu_memory_cache *cache)
+{
+ *cache = KVM_MMU_MEMORY_CACHE_INIT();
+}
#endif
#define HALT_POLL_HIST_COUNT 32
> +
> if (is_protected_kvm_enabled())
> return -EPERM;
>
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index a25e0b73ee70..b017c29a9340 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -304,6 +304,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> HRTIMER_MODE_REL);
> vcpu->arch.comparecount_timer.function = kvm_mips_comparecount_wakeup;
>
> + vcpu->arch.mmu_page_cache.node = NUMA_NO_NODE;
> +
> /*
> * Allocate space for host mode exception handlers that handle
> * guest mode exits
> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 34b57e0be2ef..119de4520cc6 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -353,9 +353,9 @@ int kvm_riscv_gstage_ioremap(struct kvm *kvm, gpa_t gpa,
> phys_addr_t addr, end;
> struct kvm_mmu_memory_cache pcache = {
> .gfp_custom = (in_atomic) ? GFP_ATOMIC | __GFP_ACCOUNT : 0,
> - .gfp_zero = __GFP_ZERO,
> };
>
> + INIT_KVM_MMU_MEMORY_CACHE(&pcache, NULL, NUMA_NO_NODE);
> end = (gpa + size + PAGE_SIZE - 1) & PAGE_MASK;
> pfn = __phys_to_pfn(hpa);
>
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 7c08567097f0..189b14feb365 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -161,7 +161,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>
> /* Mark this VCPU never ran */
> vcpu->arch.ran_atleast_once = false;
> - vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
> + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_cache, NULL, NUMA_NO_NODE);
> bitmap_zero(vcpu->arch.isa, RISCV_ISA_EXT_MAX);
>
> /* Setup ISA features available to VCPU */
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 6f6a10d7a871..23a3b82b2384 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5954,13 +5954,14 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
> {
> int ret;
>
> - vcpu->arch.mmu_pte_list_desc_cache.kmem_cache = pte_list_desc_cache;
> - vcpu->arch.mmu_pte_list_desc_cache.gfp_zero = __GFP_ZERO;
> + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_pte_list_desc_cache,
> + pte_list_desc_cache, NUMA_NO_NODE);
>
> - vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache;
> - vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO;
> + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_header_cache,
> + mmu_page_header_cache, NUMA_NO_NODE);
>
> - vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO;
> + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_shadow_page_cache,
> + NULL, NUMA_NO_NODE);
> spin_lock_init(&vcpu->arch.mmu_shadow_page_cache_lock);
>
> vcpu->arch.mmu = &vcpu->arch.root_mmu;
> @@ -6124,14 +6125,15 @@ int kvm_mmu_init_vm(struct kvm *kvm)
> node->track_flush_slot = kvm_mmu_invalidate_zap_pages_in_memslot;
> kvm_page_track_register_notifier(kvm, node);
>
> - kvm->arch.split_page_header_cache.kmem_cache = mmu_page_header_cache;
> - kvm->arch.split_page_header_cache.gfp_zero = __GFP_ZERO;
> + INIT_KVM_MMU_MEMORY_CACHE(&kvm->arch.split_page_header_cache,
> + mmu_page_header_cache, NUMA_NO_NODE);
>
> - kvm->arch.split_shadow_page_cache.gfp_zero = __GFP_ZERO;
> + INIT_KVM_MMU_MEMORY_CACHE(&kvm->arch.split_shadow_page_cache,
> + NULL, NUMA_NO_NODE);
> spin_lock_init(&kvm->arch.split_shadow_page_cache_lock);
>
> - kvm->arch.split_desc_cache.kmem_cache = pte_list_desc_cache;
> - kvm->arch.split_desc_cache.gfp_zero = __GFP_ZERO;
> + INIT_KVM_MMU_MEMORY_CACHE(&kvm->arch.split_desc_cache,
> + pte_list_desc_cache, NUMA_NO_NODE);
>
> return 0;
> }
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index a262e15ebd19..719687a37ef7 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2302,4 +2302,10 @@ static inline void kvm_account_pgtable_pages(void *virt, int nr)
> /* Max number of entries allowed for each kvm dirty ring */
> #define KVM_DIRTY_RING_MAX_ENTRIES 65536
>
> +#define INIT_KVM_MMU_MEMORY_CACHE(_cache, _kmem_cache, _node) ({ \
> + (_cache)->kmem_cache = _kmem_cache; \
> + (_cache)->gfp_zero = __GFP_ZERO; \
> + (_cache)->node = _node; \
> +})
> +
> #endif
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index 76de36e56cdf..9c70ce95e51f 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -97,6 +97,8 @@ struct kvm_mmu_memory_cache {
> struct kmem_cache *kmem_cache;
> int capacity;
> void **objects;
> + /* Node on which memory should be allocated by default */
> + int node;
> };
> #endif
>
> --
> 2.39.0.314.g84b9a713c41-goog
>
next prev parent reply other threads:[~2022-12-29 23:09 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-22 2:34 [Patch v3 0/9] NUMA aware page table's pages allocation Vipin Sharma
2022-12-22 2:34 ` [Patch v3 1/9] KVM: x86/mmu: Repurpose KVM MMU shrinker to purge shadow page caches Vipin Sharma
2022-12-27 18:37 ` Ben Gardon
2022-12-28 22:07 ` Vipin Sharma
2022-12-29 21:15 ` David Matlack
2023-01-03 17:38 ` Vipin Sharma
2022-12-29 21:54 ` David Matlack
2023-01-03 18:01 ` Vipin Sharma
2023-01-04 0:25 ` Vipin Sharma
2023-01-18 17:43 ` Sean Christopherson
2023-01-03 19:32 ` Mingwei Zhang
2023-01-04 1:00 ` Vipin Sharma
2023-01-04 6:29 ` Mingwei Zhang
2023-01-04 6:57 ` Mingwei Zhang
2023-01-18 17:36 ` Sean Christopherson
2023-01-16 4:14 ` kernel test robot
2022-12-22 2:34 ` [Patch v3 2/9] KVM: x86/mmu: Remove zapped_obsolete_pages from struct kvm_arch{} Vipin Sharma
2022-12-29 21:59 ` David Matlack
2022-12-22 2:34 ` [Patch v3 3/9] KVM: x86/mmu: Shrink split_shadow_page_cache via KVM MMU shrinker Vipin Sharma
2022-12-22 2:34 ` [Patch v3 4/9] KVM: Add module param to make page tables NUMA aware Vipin Sharma
2022-12-29 22:05 ` David Matlack
2022-12-22 2:34 ` [Patch v3 5/9] KVM: x86/mmu: Allocate TDP page table's page on correct NUMA node on split Vipin Sharma
2022-12-27 19:02 ` Ben Gardon
2022-12-28 22:07 ` Vipin Sharma
2022-12-29 22:30 ` David Matlack
2023-01-03 18:26 ` Vipin Sharma
2022-12-22 2:34 ` [Patch v3 6/9] KVM: Provide NUMA node support to kvm_mmu_memory_cache{} Vipin Sharma
2022-12-27 19:09 ` Ben Gardon
2022-12-28 22:07 ` Vipin Sharma
2022-12-29 18:22 ` Ben Gardon
2023-01-03 17:36 ` Vipin Sharma
2022-12-29 23:08 ` David Matlack [this message]
2022-12-29 23:11 ` David Matlack
2023-01-03 18:45 ` Vipin Sharma
2023-01-03 18:55 ` David Matlack
2022-12-22 2:34 ` [Patch v3 7/9] KVM: x86/mmu: Allocate page table's pages on NUMA node of the underlying pages Vipin Sharma
2022-12-27 19:34 ` Ben Gardon
2022-12-28 22:08 ` Vipin Sharma
2022-12-29 18:20 ` Ben Gardon
2022-12-22 2:34 ` [Patch v3 8/9] KVM: x86/mmu: Make split_shadow_page_cache NUMA aware Vipin Sharma
2022-12-27 19:42 ` Ben Gardon
2022-12-28 22:08 ` Vipin Sharma
2022-12-29 23:18 ` David Matlack
2023-01-03 18:49 ` Vipin Sharma
2022-12-22 2:34 ` [Patch v3 9/9] KVM: x86/mmu: Reduce default cache size in KVM from 40 to PT64_ROOT_MAX_LEVEL Vipin Sharma
2022-12-27 19:52 ` Ben Gardon
2022-12-28 22:08 ` Vipin Sharma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y64eAvm4JglT1au4@google.com \
--to=dmatlack@google.com \
--cc=bgardon@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=vipinsh@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.