| Message ID | 20230306224127.1689967-16-vipinsh@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NUMA aware page table allocation | expand |
On Mon, Mar 06, 2023 at 02:41:24PM -0800, Vipin Sharma wrote: > Add NUMA node id variable in struct kvm_mmu_memory_cache{}. This > variable denotes preferable NUMA node from which memory will be > allocated under this memory cache. > > Set this variable to NUMA_NO_NODE if there is no preferred node. > > MIPS doesn't do any sort of initializatino of struct > kvm_mmu_memory_cache{}. Keep things similar in MIPS by setting gfp_zero > to 0 as INIT_KVM_MMU_MEMORY_CACHE() will initialize it to __GFP_ZERO. > > "node" cannot be left as 0, as 0 is a valid NUMA node value. > > Signed-off-by: Vipin Sharma <vipinsh@google.com> > --- > arch/mips/kvm/mips.c | 3 +++ > include/linux/kvm_types.h | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c > index 36c8991b5d39..5ec5ce919918 100644 > --- a/arch/mips/kvm/mips.c > +++ b/arch/mips/kvm/mips.c > @@ -294,6 +294,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > HRTIMER_MODE_REL); > vcpu->arch.comparecount_timer.function = kvm_mips_comparecount_wakeup; > > + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_cache); > + vcpu->arch.mmu_page_cache.gfp_zero = 0; Oh MIPS is here. Why isn't MIPS covered in the previous commits? > + > /* > * Allocate space for host mode exception handlers that handle > * guest mode exits > diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h > index 5da7953532ce..b2a405c8e629 100644 > --- a/include/linux/kvm_types.h > +++ b/include/linux/kvm_types.h > @@ -97,10 +97,13 @@ struct kvm_mmu_memory_cache { > struct kmem_cache *kmem_cache; > int capacity; > void **objects; > + /* Preferred NUMA node of memory allocation. */ > + int node; > }; > > #define KVM_MMU_MEMORY_CACHE_INIT() { \ > .gfp_zero = __GFP_ZERO, \ > + .node = NUMA_NO_NODE, \ > } > > #define KVM_MMU_MEMORY_CACHE(_name) \ > -- > 2.40.0.rc0.216.gc4246ad0f0-goog >
On Thu, Mar 23, 2023 at 3:30 PM David Matlack <dmatlack@google.com> wrote: > > On Mon, Mar 06, 2023 at 02:41:24PM -0800, Vipin Sharma wrote: > > Add NUMA node id variable in struct kvm_mmu_memory_cache{}. This > > variable denotes preferable NUMA node from which memory will be > > allocated under this memory cache. > > > > Set this variable to NUMA_NO_NODE if there is no preferred node. > > > > MIPS doesn't do any sort of initializatino of struct > > kvm_mmu_memory_cache{}. Keep things similar in MIPS by setting gfp_zero > > to 0 as INIT_KVM_MMU_MEMORY_CACHE() will initialize it to __GFP_ZERO. > > > > "node" cannot be left as 0, as 0 is a valid NUMA node value. > > > > Signed-off-by: Vipin Sharma <vipinsh@google.com> > > --- > > arch/mips/kvm/mips.c | 3 +++ > > include/linux/kvm_types.h | 3 +++ > > 2 files changed, 6 insertions(+) > > > > diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c > > index 36c8991b5d39..5ec5ce919918 100644 > > --- a/arch/mips/kvm/mips.c > > +++ b/arch/mips/kvm/mips.c > > @@ -294,6 +294,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > > HRTIMER_MODE_REL); > > vcpu->arch.comparecount_timer.function = kvm_mips_comparecount_wakeup; > > > > + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_cache); > > + vcpu->arch.mmu_page_cache.gfp_zero = 0; > > Oh MIPS is here. Why isn't MIPS covered in the previous commits? > Because this is the patch where MIPS get impacted. MIPS doesn't initialize gfp_zero, so there was no need to change the code in MIPS. However, with the addition of "node" in kvm_mmu_memory_cache{} in this patch, we need initialization in MIPS to (1) Set node to NUMA_NO_NODE as 0 is now a valid value, and (2) INIT_KVM_MMU_MEMORY_CACHE() will set gfp_zero to __GFP_ZERO which is different than existing code in MIPS to keep it 0. I asked MIPS maintainers in the previous version to see if GFP_ZERO can be added but didn't get any response. https://lore.kernel.org/lkml/CAHVum0c+17Z-RbGAFdU-xmRejDjDQ+MKOfN4XaObh2SwgWAjLg@mail.gmail.com/ > > + > > /* > > * Allocate space for host mode exception handlers that handle > > * guest mode exits > > diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h > > index 5da7953532ce..b2a405c8e629 100644 > > --- a/include/linux/kvm_types.h > > +++ b/include/linux/kvm_types.h > > @@ -97,10 +97,13 @@ struct kvm_mmu_memory_cache { > > struct kmem_cache *kmem_cache; > > int capacity; > > void **objects; > > + /* Preferred NUMA node of memory allocation. */ > > + int node; > > }; > > > > #define KVM_MMU_MEMORY_CACHE_INIT() { \ > > .gfp_zero = __GFP_ZERO, \ > > + .node = NUMA_NO_NODE, \ > > } > > > > #define KVM_MMU_MEMORY_CACHE(_name) \ > > -- > > 2.40.0.rc0.216.gc4246ad0f0-goog > >
On Tue, Mar 28, 2023 at 10:51 AM Vipin Sharma <vipinsh@google.com> wrote: > > On Thu, Mar 23, 2023 at 3:30 PM David Matlack <dmatlack@google.com> wrote: > > > > On Mon, Mar 06, 2023 at 02:41:24PM -0800, Vipin Sharma wrote: > > > Add NUMA node id variable in struct kvm_mmu_memory_cache{}. This > > > variable denotes preferable NUMA node from which memory will be > > > allocated under this memory cache. > > > > > > Set this variable to NUMA_NO_NODE if there is no preferred node. > > > > > > MIPS doesn't do any sort of initializatino of struct > > > kvm_mmu_memory_cache{}. Keep things similar in MIPS by setting gfp_zero > > > to 0 as INIT_KVM_MMU_MEMORY_CACHE() will initialize it to __GFP_ZERO. > > > > > > "node" cannot be left as 0, as 0 is a valid NUMA node value. > > > > > > Signed-off-by: Vipin Sharma <vipinsh@google.com> > > > --- > > > arch/mips/kvm/mips.c | 3 +++ > > > include/linux/kvm_types.h | 3 +++ > > > 2 files changed, 6 insertions(+) > > > > > > diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c > > > index 36c8991b5d39..5ec5ce919918 100644 > > > --- a/arch/mips/kvm/mips.c > > > +++ b/arch/mips/kvm/mips.c > > > @@ -294,6 +294,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > > > HRTIMER_MODE_REL); > > > vcpu->arch.comparecount_timer.function = kvm_mips_comparecount_wakeup; > > > > > > + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_cache); > > > + vcpu->arch.mmu_page_cache.gfp_zero = 0; > > > > Oh MIPS is here. Why isn't MIPS covered in the previous commits? > > Because this is the patch where MIPS get impacted. MIPS doesn't > initialize gfp_zero, so there was no need to change the code in MIPS. > However, with the addition of "node" in kvm_mmu_memory_cache{} in this > patch, we need initialization in MIPS to (1) Set node to NUMA_NO_NODE > as 0 is now a valid value, and (2) INIT_KVM_MMU_MEMORY_CACHE() will > set gfp_zero to __GFP_ZERO which is different than existing code in > MIPS to keep it 0. > > I asked MIPS maintainers in the previous version to see if GFP_ZERO > can be added but didn't get any response. > https://lore.kernel.org/lkml/CAHVum0c+17Z-RbGAFdU-xmRejDjDQ+MKOfN4XaObh2SwgWAjLg@mail.gmail.com/ I see. IMO it's more logical to convert the MIPS cache to INIT_KVM_MMU_MEMORY_CACHE() in patch 13, along with all the other users of struct kvm_mmu_memory_cache. Then in patch 14, add the line to set gfp_zero to 0 for MIPS to preserve the existing behavior. That produces a very simple chain of changes: Patch 13: Convert all users of struct kvm_mmu_memory_cache to INIT() Patch 14: Invert the default value of kvm_mmu_memory_cache.gfp_zero Patch 15: Add node to kvm_mmu_memory_cache > > > > + > > > /* > > > * Allocate space for host mode exception handlers that handle > > > * guest mode exits > > > diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h > > > index 5da7953532ce..b2a405c8e629 100644 > > > --- a/include/linux/kvm_types.h > > > +++ b/include/linux/kvm_types.h > > > @@ -97,10 +97,13 @@ struct kvm_mmu_memory_cache { > > > struct kmem_cache *kmem_cache; > > > int capacity; > > > void **objects; > > > + /* Preferred NUMA node of memory allocation. */ > > > + int node; > > > }; > > > > > > #define KVM_MMU_MEMORY_CACHE_INIT() { \ > > > .gfp_zero = __GFP_ZERO, \ > > > + .node = NUMA_NO_NODE, \ > > > } > > > > > > #define KVM_MMU_MEMORY_CACHE(_name) \ > > > -- > > > 2.40.0.rc0.216.gc4246ad0f0-goog > > >
On Tue, Mar 28, 2023 at 4:25 PM David Matlack <dmatlack@google.com> wrote: > > On Tue, Mar 28, 2023 at 10:51 AM Vipin Sharma <vipinsh@google.com> wrote: > > > > On Thu, Mar 23, 2023 at 3:30 PM David Matlack <dmatlack@google.com> wrote: > > > > > > On Mon, Mar 06, 2023 at 02:41:24PM -0800, Vipin Sharma wrote: > > > > + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_cache); > > > > + vcpu->arch.mmu_page_cache.gfp_zero = 0; > > > > > > Oh MIPS is here. Why isn't MIPS covered in the previous commits? > > > > Because this is the patch where MIPS get impacted. MIPS doesn't > > initialize gfp_zero, so there was no need to change the code in MIPS. > > However, with the addition of "node" in kvm_mmu_memory_cache{} in this > > patch, we need initialization in MIPS to (1) Set node to NUMA_NO_NODE > > as 0 is now a valid value, and (2) INIT_KVM_MMU_MEMORY_CACHE() will > > set gfp_zero to __GFP_ZERO which is different than existing code in > > MIPS to keep it 0. > > > > I asked MIPS maintainers in the previous version to see if GFP_ZERO > > can be added but didn't get any response. > > https://lore.kernel.org/lkml/CAHVum0c+17Z-RbGAFdU-xmRejDjDQ+MKOfN4XaObh2SwgWAjLg@mail.gmail.com/ > > I see. IMO it's more logical to convert the MIPS cache to > INIT_KVM_MMU_MEMORY_CACHE() in patch 13, along with all the other > users of struct kvm_mmu_memory_cache. Then in patch 14, add the line > to set gfp_zero to 0 for MIPS to preserve the existing behavior. That > produces a very simple chain of changes: > > Patch 13: Convert all users of struct kvm_mmu_memory_cache to INIT() > Patch 14: Invert the default value of kvm_mmu_memory_cache.gfp_zero > Patch 15: Add node to kvm_mmu_memory_cache > > Yeah, this looks better. I will do this.
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c index 36c8991b5d39..5ec5ce919918 100644 --- a/arch/mips/kvm/mips.c +++ b/arch/mips/kvm/mips.c @@ -294,6 +294,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) HRTIMER_MODE_REL); vcpu->arch.comparecount_timer.function = kvm_mips_comparecount_wakeup; + INIT_KVM_MMU_MEMORY_CACHE(&vcpu->arch.mmu_page_cache); + vcpu->arch.mmu_page_cache.gfp_zero = 0; + /* * Allocate space for host mode exception handlers that handle * guest mode exits diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h index 5da7953532ce..b2a405c8e629 100644 --- a/include/linux/kvm_types.h +++ b/include/linux/kvm_types.h @@ -97,10 +97,13 @@ struct kvm_mmu_memory_cache { struct kmem_cache *kmem_cache; int capacity; void **objects; + /* Preferred NUMA node of memory allocation. */ + int node; }; #define KVM_MMU_MEMORY_CACHE_INIT() { \ .gfp_zero = __GFP_ZERO, \ + .node = NUMA_NO_NODE, \ } #define KVM_MMU_MEMORY_CACHE(_name) \
Add NUMA node id variable in struct kvm_mmu_memory_cache{}. This variable denotes preferable NUMA node from which memory will be allocated under this memory cache. Set this variable to NUMA_NO_NODE if there is no preferred node. MIPS doesn't do any sort of initializatino of struct kvm_mmu_memory_cache{}. Keep things similar in MIPS by setting gfp_zero to 0 as INIT_KVM_MMU_MEMORY_CACHE() will initialize it to __GFP_ZERO. "node" cannot be left as 0, as 0 is a valid NUMA node value. Signed-off-by: Vipin Sharma <vipinsh@google.com> --- arch/mips/kvm/mips.c | 3 +++ include/linux/kvm_types.h | 3 +++ 2 files changed, 6 insertions(+)