On Fri, 20 Mar 2015 08:59:03 -0300
Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote:
> On Fri, Mar 20, 2015 at 09:51:26AM +0000, Igor Mammedov wrote:
> > KVM guest can fail to startup with following trace on host:
> >
> > qemu-system-x86: page allocation failure: order:4, mode:0x40d0
> > Call Trace:
> > dump_stack+0x47/0x67
> > warn_alloc_failed+0xee/0x150
> > __alloc_pages_direct_compact+0x14a/0x150
> > __alloc_pages_nodemask+0x776/0xb80
> > alloc_kmem_pages+0x3a/0x110
> > kmalloc_order+0x13/0x50
> > kmemdup+0x1b/0x40
> > __kvm_set_memory_region+0x24a/0x9f0 [kvm]
> > kvm_set_ioapic+0x130/0x130 [kvm]
> > kvm_set_memory_region+0x21/0x40 [kvm]
> > kvm_vm_ioctl+0x43f/0x750 [kvm]
> >
> > Failure happens when attempting to allocate pages for
> > 'struct kvm_memslots', however it doesn't have to be
> > present in physically contiguous (kmalloc-ed) address
> > space, change allocation to kvm_kvzalloc() so that
> > it will be vmalloc-ed when its size is more then a page.
> >
> > Signed-off-by: Igor Mammedov <imammedo@xxxxxxxxxx>
>
> Igor, two things:
>
> 1) kvm_create_vm should also use vmalloc
>
> r = -ENOMEM;
> kvm->memslots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL);
> if (!kvm->memslots)
> goto out_err_no_srcu;
>
> 2) there are additional places where its necessary to use
> proper freeing function, i believe:
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ce7888a..651ff2d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -522,7 +522,7 @@ out_err_no_srcu:
> out_err_no_disable:
> for (i = 0; i < KVM_NR_BUSES; i++)
> kfree(kvm->buses[i]);
> - kfree(kvm->memslots);
> + kvfree(kvm->memslots);
> kvm_arch_free_vm(kvm);
> return ERR_PTR(r);
> }
> @@ -570,7 +570,7 @@ static void kvm_free_physmem(struct kvm *kvm)
> kvm_for_each_memslot(memslot, slots)
> kvm_free_physmem_slot(kvm, memslot, NULL);
>
> - kfree(kvm->memslots);
> + kvfree(kvm->memslots);
> }
>
> static void kvm_destroy_devices(struct kvm *kvm)
> @@ -909,7 +922,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
> kvm_arch_commit_memory_region(kvm, mem, &old, change);
>
> kvm_free_physmem_slot(kvm, &old, &new);
> - kfree(old_memslots);
> + kvfree(old_memslots);
>
> /*
> * IOMMU mapping: New slots need to be mapped. Old slots need to be
Thanks for review,
I'll respin patch after fixing it up.
>
> > ---
> > TODO:
> > - work on follow up patches to allocate space for
> > actual amount of memory_slots instead of possible maximum.
> > ---
> > virt/kvm/kvm_main.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index a2214d9..7ed1f5c 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -871,10 +871,10 @@ int __kvm_set_memory_region(struct kvm *kvm,
> > goto out_free;
> > }
> >
> > - slots = kmemdup(kvm->memslots, sizeof(struct kvm_memslots),
> > - GFP_KERNEL);
> > + slots = kvm_kvzalloc(sizeof(struct kvm_memslots));
> > if (!slots)
> > goto out_free;
> > + memcpy(slots, kvm->memslots, sizeof(struct kvm_memslots));
> >
> > if ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) {
> > slot = id_to_memslot(slots, mem->slot);
> > @@ -936,7 +936,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
> > return 0;
> >
> > out_slots:
> > - kfree(slots);
> > + kvfree(slots);
> > out_free:
> > kvm_free_physmem_slot(kvm, &new, &old);
> > out:
> > --
> > 1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
