| Message ID | 20190211190252.198101-4-bgardon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/4] kvm: Add memcg accounting to KVM allocations | expand |
On 11/02/19 20:02, Ben Gardon wrote: > There are many KVM kernel memory allocations which are tied to the life of > the VM process and should be charged to the VM process's cgroup. If the > allocations aren't tied to the process, the OOM killer will not know > that killing the process will free the associated kernel memory. > Add __GFP_ACCOUNT flags to many of the allocations which are not yet being > charged to the VM process's cgroup. > > Tested: > Ran all kvm-unit-tests on a 64 bit Haswell machine, the patch > introduced no new failures. > Ran a kernel memory accounting test which creates a VM to touch > memory and then checks that the kernel memory allocated for the > process is within certain bounds. > With this patch we account for much more of the vmalloc and slab memory > allocated for the VM. > > Signed-off-by: Ben Gardon <bgardon@google.com> > Reviewed-by: Shakeel Butt <shakeelb@google.com> > --- > arch/x86/kvm/vmx/nested.c | 9 +++++++-- > arch/x86/kvm/vmx/vmx.c | 27 ++++++++++++++++++--------- > arch/x86/kvm/vmx/vmx.h | 5 +++-- > 3 files changed, 28 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 3170e291215d0..88d20904b16e0 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -4140,11 +4140,12 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu) > if (r < 0) > goto out_vmcs02; > > - vmx->nested.cached_vmcs12 = kmalloc(VMCS12_SIZE, GFP_KERNEL); > + vmx->nested.cached_vmcs12 = kmalloc(VMCS12_SIZE, GFP_KERNEL_ACCOUNT); > if (!vmx->nested.cached_vmcs12) > goto out_cached_vmcs12; > > - vmx->nested.cached_shadow_vmcs12 = kmalloc(VMCS12_SIZE, GFP_KERNEL); > + vmx->nested.cached_shadow_vmcs12 = kmalloc(VMCS12_SIZE, > + GFP_KERNEL_ACCOUNT); > if (!vmx->nested.cached_shadow_vmcs12) > goto out_cached_shadow_vmcs12; > > @@ -5686,6 +5687,10 @@ __init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *)) > enable_shadow_vmcs = 0; > if (enable_shadow_vmcs) { > for (i = 0; i < VMX_BITMAP_NR; i++) { > + /* > + * The vmx_bitmap is not tied to a VM and so should > + * not be charged to a memcg. > + */ > vmx_bitmap[i] = (unsigned long *) > __get_free_page(GFP_KERNEL); > if (!vmx_bitmap[i]) { > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 4d39f731bc332..9efa4ceb45c84 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -245,6 +245,10 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) > > if (l1tf != VMENTER_L1D_FLUSH_NEVER && !vmx_l1d_flush_pages && > !boot_cpu_has(X86_FEATURE_FLUSH_L1D)) { > + /* > + * This allocation for vmx_l1d_flush_pages is not tied to a VM > + * lifetime and so should not be charged to a memcg. > + */ > page = alloc_pages(GFP_KERNEL, L1D_CACHE_ORDER); > if (!page) > return -ENOMEM; > @@ -2389,13 +2393,13 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf, > return 0; > } > > -struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu) > +struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu, gfp_t flags) > { > int node = cpu_to_node(cpu); > struct page *pages; > struct vmcs *vmcs; > > - pages = __alloc_pages_node(node, GFP_KERNEL, vmcs_config.order); > + pages = __alloc_pages_node(node, flags, vmcs_config.order); > if (!pages) > return NULL; > vmcs = page_address(pages); > @@ -2442,7 +2446,8 @@ int alloc_loaded_vmcs(struct loaded_vmcs *loaded_vmcs) > loaded_vmcs_init(loaded_vmcs); > > if (cpu_has_vmx_msr_bitmap()) { > - loaded_vmcs->msr_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL); > + loaded_vmcs->msr_bitmap = (unsigned long *) > + __get_free_page(GFP_KERNEL_ACCOUNT); > if (!loaded_vmcs->msr_bitmap) > goto out_vmcs; > memset(loaded_vmcs->msr_bitmap, 0xff, PAGE_SIZE); > @@ -2483,7 +2488,7 @@ static __init int alloc_kvm_area(void) > for_each_possible_cpu(cpu) { > struct vmcs *vmcs; > > - vmcs = alloc_vmcs_cpu(false, cpu); > + vmcs = alloc_vmcs_cpu(false, cpu, GFP_KERNEL); > if (!vmcs) { > free_kvm_area(); > return -ENOMEM; > @@ -6652,7 +6657,9 @@ STACK_FRAME_NON_STANDARD(vmx_vcpu_run); > > static struct kvm *vmx_vm_alloc(void) > { > - struct kvm_vmx *kvm_vmx = vzalloc(sizeof(struct kvm_vmx)); > + struct kvm_vmx *kvm_vmx = __vmalloc(sizeof(struct kvm_vmx), > + GFP_KERNEL_ACCOUNT | __GFP_ZERO, > + PAGE_KERNEL); > return &kvm_vmx->kvm; > } > > @@ -6680,14 +6687,16 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu) > static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id) > { > int err; > - struct vcpu_vmx *vmx = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); > + struct vcpu_vmx *vmx; > unsigned long *msr_bitmap; > int cpu; > > + vmx = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL_ACCOUNT); > if (!vmx) > return ERR_PTR(-ENOMEM); > > - vmx->vcpu.arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache, GFP_KERNEL); > + vmx->vcpu.arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache, > + GFP_KERNEL_ACCOUNT); > if (!vmx->vcpu.arch.guest_fpu) { > printk(KERN_ERR "kvm: failed to allocate vcpu's fpu\n"); > err = -ENOMEM; > @@ -6709,12 +6718,12 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id) > * for the guest, etc. > */ > if (enable_pml) { > - vmx->pml_pg = alloc_page(GFP_KERNEL | __GFP_ZERO); > + vmx->pml_pg = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO); > if (!vmx->pml_pg) > goto uninit_vcpu; > } > > - vmx->guest_msrs = kmalloc(PAGE_SIZE, GFP_KERNEL); > + vmx->guest_msrs = kmalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT); > BUILD_BUG_ON(ARRAY_SIZE(vmx_msr_index) * sizeof(vmx->guest_msrs[0]) > > PAGE_SIZE); > > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h > index 99328954c2fc1..23aa757cecf19 100644 > --- a/arch/x86/kvm/vmx/vmx.h > +++ b/arch/x86/kvm/vmx/vmx.h > @@ -478,7 +478,7 @@ static inline struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu) > return &(to_vmx(vcpu)->pi_desc); > } > > -struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu); > +struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu, gfp_t flags); > void free_vmcs(struct vmcs *vmcs); > int alloc_loaded_vmcs(struct loaded_vmcs *loaded_vmcs); > void free_loaded_vmcs(struct loaded_vmcs *loaded_vmcs); > @@ -487,7 +487,8 @@ void loaded_vmcs_clear(struct loaded_vmcs *loaded_vmcs); > > static inline struct vmcs *alloc_vmcs(bool shadow) > { > - return alloc_vmcs_cpu(shadow, raw_smp_processor_id()); > + return alloc_vmcs_cpu(shadow, raw_smp_processor_id(), > + GFP_KERNEL_ACCOUNT); > } > > u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa); > Queued all, thanks. Paolo
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 3170e291215d0..88d20904b16e0 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -4140,11 +4140,12 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu) if (r < 0) goto out_vmcs02; - vmx->nested.cached_vmcs12 = kmalloc(VMCS12_SIZE, GFP_KERNEL); + vmx->nested.cached_vmcs12 = kmalloc(VMCS12_SIZE, GFP_KERNEL_ACCOUNT); if (!vmx->nested.cached_vmcs12) goto out_cached_vmcs12; - vmx->nested.cached_shadow_vmcs12 = kmalloc(VMCS12_SIZE, GFP_KERNEL); + vmx->nested.cached_shadow_vmcs12 = kmalloc(VMCS12_SIZE, + GFP_KERNEL_ACCOUNT); if (!vmx->nested.cached_shadow_vmcs12) goto out_cached_shadow_vmcs12; @@ -5686,6 +5687,10 @@ __init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *)) enable_shadow_vmcs = 0; if (enable_shadow_vmcs) { for (i = 0; i < VMX_BITMAP_NR; i++) { + /* + * The vmx_bitmap is not tied to a VM and so should + * not be charged to a memcg. + */ vmx_bitmap[i] = (unsigned long *) __get_free_page(GFP_KERNEL); if (!vmx_bitmap[i]) { diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 4d39f731bc332..9efa4ceb45c84 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -245,6 +245,10 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) if (l1tf != VMENTER_L1D_FLUSH_NEVER && !vmx_l1d_flush_pages && !boot_cpu_has(X86_FEATURE_FLUSH_L1D)) { + /* + * This allocation for vmx_l1d_flush_pages is not tied to a VM + * lifetime and so should not be charged to a memcg. + */ page = alloc_pages(GFP_KERNEL, L1D_CACHE_ORDER); if (!page) return -ENOMEM; @@ -2389,13 +2393,13 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf, return 0; } -struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu) +struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu, gfp_t flags) { int node = cpu_to_node(cpu); struct page *pages; struct vmcs *vmcs; - pages = __alloc_pages_node(node, GFP_KERNEL, vmcs_config.order); + pages = __alloc_pages_node(node, flags, vmcs_config.order); if (!pages) return NULL; vmcs = page_address(pages); @@ -2442,7 +2446,8 @@ int alloc_loaded_vmcs(struct loaded_vmcs *loaded_vmcs) loaded_vmcs_init(loaded_vmcs); if (cpu_has_vmx_msr_bitmap()) { - loaded_vmcs->msr_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL); + loaded_vmcs->msr_bitmap = (unsigned long *) + __get_free_page(GFP_KERNEL_ACCOUNT); if (!loaded_vmcs->msr_bitmap) goto out_vmcs; memset(loaded_vmcs->msr_bitmap, 0xff, PAGE_SIZE); @@ -2483,7 +2488,7 @@ static __init int alloc_kvm_area(void) for_each_possible_cpu(cpu) { struct vmcs *vmcs; - vmcs = alloc_vmcs_cpu(false, cpu); + vmcs = alloc_vmcs_cpu(false, cpu, GFP_KERNEL); if (!vmcs) { free_kvm_area(); return -ENOMEM; @@ -6652,7 +6657,9 @@ STACK_FRAME_NON_STANDARD(vmx_vcpu_run); static struct kvm *vmx_vm_alloc(void) { - struct kvm_vmx *kvm_vmx = vzalloc(sizeof(struct kvm_vmx)); + struct kvm_vmx *kvm_vmx = __vmalloc(sizeof(struct kvm_vmx), + GFP_KERNEL_ACCOUNT | __GFP_ZERO, + PAGE_KERNEL); return &kvm_vmx->kvm; } @@ -6680,14 +6687,16 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu) static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id) { int err; - struct vcpu_vmx *vmx = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); + struct vcpu_vmx *vmx; unsigned long *msr_bitmap; int cpu; + vmx = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL_ACCOUNT); if (!vmx) return ERR_PTR(-ENOMEM); - vmx->vcpu.arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache, GFP_KERNEL); + vmx->vcpu.arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache, + GFP_KERNEL_ACCOUNT); if (!vmx->vcpu.arch.guest_fpu) { printk(KERN_ERR "kvm: failed to allocate vcpu's fpu\n"); err = -ENOMEM; @@ -6709,12 +6718,12 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id) * for the guest, etc. */ if (enable_pml) { - vmx->pml_pg = alloc_page(GFP_KERNEL | __GFP_ZERO); + vmx->pml_pg = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO); if (!vmx->pml_pg) goto uninit_vcpu; } - vmx->guest_msrs = kmalloc(PAGE_SIZE, GFP_KERNEL); + vmx->guest_msrs = kmalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT); BUILD_BUG_ON(ARRAY_SIZE(vmx_msr_index) * sizeof(vmx->guest_msrs[0]) > PAGE_SIZE); diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h index 99328954c2fc1..23aa757cecf19 100644 --- a/arch/x86/kvm/vmx/vmx.h +++ b/arch/x86/kvm/vmx/vmx.h @@ -478,7 +478,7 @@ static inline struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu) return &(to_vmx(vcpu)->pi_desc); } -struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu); +struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu, gfp_t flags); void free_vmcs(struct vmcs *vmcs); int alloc_loaded_vmcs(struct loaded_vmcs *loaded_vmcs); void free_loaded_vmcs(struct loaded_vmcs *loaded_vmcs); @@ -487,7 +487,8 @@ void loaded_vmcs_clear(struct loaded_vmcs *loaded_vmcs); static inline struct vmcs *alloc_vmcs(bool shadow) { - return alloc_vmcs_cpu(shadow, raw_smp_processor_id()); + return alloc_vmcs_cpu(shadow, raw_smp_processor_id(), + GFP_KERNEL_ACCOUNT); } u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa);