[PATCH] kvm tools: Make the whole guest memory mergeable

[PATCH] kvm tools: Make the whole guest memory mergeable

[zanghongyong]

From: Hongyong Zang <zanghongyong@huawei.com>

If a guest's ram_size exceeds KVM_32BIT_GAP_START, the corresponding kvm tool's
virtual address size should be (ram_size + KVM_32BIT_GAP_SIZE), rather than ram_size.

Signed-off-by: Hongyong Zang <zanghongyong@huawei.com>
---
 tools/kvm/x86/kvm.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
index bc52ef3..6107a2c 100644
--- a/tools/kvm/x86/kvm.c
+++ b/tools/kvm/x86/kvm.c
@@ -175,7 +175,9 @@ void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, const char *hugetlbfs_
 	if (kvm->ram_start == MAP_FAILED)
 		die("out of memory");
 
-	madvise(kvm->ram_start, kvm->ram_size, MADV_MERGEABLE);
+	madvise(kvm->ram_start, 
+		(ram_size < KVM_32BIT_GAP_START) ? ram_size : (ram_size + KVM_32BIT_GAP_SIZE), 
+		MADV_MERGEABLE);
 
 	ret = ioctl(kvm->vm_fd, KVM_CREATE_IRQCHIP);
 	if (ret < 0)
-- 
1.7.1

Re: [PATCH] kvm tools: Make the whole guest memory mergeable

[Sasha Levin]

On Fri, 2011-12-16 at 09:01 +0800, zanghongyong@huawei.com wrote:
> If a guest's ram_size exceeds KVM_32BIT_GAP_START, the corresponding kvm tool's
> virtual address size should be (ram_size + KVM_32BIT_GAP_SIZE), rather than ram_size. 

You're right.

There are more places than just the madvise() code which make the same
error you've spotted (for example, the memslot allocation code), so
instead of trying to fix all of them I'd suggest to just update ram_size
in kvm__arch_init() before allocating everything - that should fix all
of them at once.

-- 

Sasha.

Re: [PATCH] kvm tools: Make the whole guest memory mergeable

[Zang Hongyong]

于 2011/12/16,星期五 13:50, Sasha Levin 写道:
> On Fri, 2011-12-16 at 09:01 +0800, zanghongyong@huawei.com wrote:
>> If a guest's ram_size exceeds KVM_32BIT_GAP_START, the corresponding kvm tool's
>> virtual address size should be (ram_size + KVM_32BIT_GAP_SIZE), rather than ram_size.
> You're right.
>
> There are more places than just the madvise() code which make the same
> error you've spotted (for example, the memslot allocation code), so
> instead of trying to fix all of them I'd suggest to just update ram_size
> in kvm__arch_init() before allocating everything - that should fix all
> of them at once.
>
Yes. There are other scenarios with the same error.
However ram_size sometimes means real guest ram size, and sometimes 
means virtual address
size of kvm tool's user space. Shall we define a new variable?

Re: [PATCH] kvm tools: Make the whole guest memory mergeable

[Sasha Levin]

On Fri, 2011-12-16 at 15:02 +0800, Zang Hongyong wrote:
> 于 2011/12/16,星期五 13:50, Sasha Levin 写道:
> > On Fri, 2011-12-16 at 09:01 +0800, zanghongyong@huawei.com wrote:
> >> If a guest's ram_size exceeds KVM_32BIT_GAP_START, the corresponding kvm tool's
> >> virtual address size should be (ram_size + KVM_32BIT_GAP_SIZE), rather than ram_size.
> > You're right.
> >
> > There are more places than just the madvise() code which make the same
> > error you've spotted (for example, the memslot allocation code), so
> > instead of trying to fix all of them I'd suggest to just update ram_size
> > in kvm__arch_init() before allocating everything - that should fix all
> > of them at once.
> >
> Yes. There are other scenarios with the same error.
> However ram_size sometimes means real guest ram size, and sometimes 
> means virtual address
> size of kvm tool's user space. Shall we define a new variable?

Let's keep it simple. If the user requests more than RAM than
KVM_32BIT_GAP_START just increase it by KVM_32BIT_GAP_SIZE, this way
mapped size == guest size always (we can madvise(MADV_DONTNEED) the gap
in the mmapped ram).

Since a user which requests more than KVM_32BIT_GAP_START will have to
be on 64bit host anyway, there shouldn't be any issue with that.

-- 

Sasha.

Re: [PATCH] kvm tools: Make the whole guest memory mergeable

[Zang Hongyong]

于 2011/12/16,星期五 15:23, Sasha Levin 写道:
> On Fri, 2011-12-16 at 15:02 +0800, Zang Hongyong wrote:
>> 于 2011/12/16,星期五 13:50, Sasha Levin 写道:
>>> On Fri, 2011-12-16 at 09:01 +0800, zanghongyong@huawei.com wrote:
>>>> If a guest's ram_size exceeds KVM_32BIT_GAP_START, the corresponding kvm tool's
>>>> virtual address size should be (ram_size + KVM_32BIT_GAP_SIZE), rather than ram_size.
>>> You're right.
>>>
>>> There are more places than just the madvise() code which make the same
>>> error you've spotted (for example, the memslot allocation code), so
>>> instead of trying to fix all of them I'd suggest to just update ram_size
>>> in kvm__arch_init() before allocating everything - that should fix all
>>> of them at once.
>>>
>> Yes. There are other scenarios with the same error.
>> However ram_size sometimes means real guest ram size, and sometimes 
>> means virtual address
>> size of kvm tool's user space. Shall we define a new variable?
> Let's keep it simple. If the user requests more than RAM than
> KVM_32BIT_GAP_START just increase it by KVM_32BIT_GAP_SIZE, this way
> mapped size == guest size always (we can madvise(MADV_DONTNEED) the gap
> in the mmapped ram).
>
> Since a user which requests more than KVM_32BIT_GAP_START will have to
> be on 64bit host anyway, there shouldn't be any issue with that.
>
Do you mean increase *kvm->ram_size* by KVM_32BIT_GAP_SIZE?
but sometimes kvm->ram_size stands for guest physical ram size (for
example in kvm__init_ram() code).

Re: [PATCH] kvm tools: Make the whole guest memory mergeable

[Sasha Levin]

On Fri, 2011-12-16 at 16:36 +0800, Zang Hongyong wrote:
> 于 2011/12/16,星期五 15:23, Sasha Levin 写道:
> > On Fri, 2011-12-16 at 15:02 +0800, Zang Hongyong wrote:
> >> 于 2011/12/16,星期五 13:50, Sasha Levin 写道:
> >>> On Fri, 2011-12-16 at 09:01 +0800, zanghongyong@huawei.com wrote:
> >>>> If a guest's ram_size exceeds KVM_32BIT_GAP_START, the corresponding kvm tool's
> >>>> virtual address size should be (ram_size + KVM_32BIT_GAP_SIZE), rather than ram_size.
> >>> You're right.
> >>>
> >>> There are more places than just the madvise() code which make the same
> >>> error you've spotted (for example, the memslot allocation code), so
> >>> instead of trying to fix all of them I'd suggest to just update ram_size
> >>> in kvm__arch_init() before allocating everything - that should fix all
> >>> of them at once.
> >>>
> >> Yes. There are other scenarios with the same error.
> >> However ram_size sometimes means real guest ram size, and sometimes 
> >> means virtual address
> >> size of kvm tool's user space. Shall we define a new variable?
> > Let's keep it simple. If the user requests more than RAM than
> > KVM_32BIT_GAP_START just increase it by KVM_32BIT_GAP_SIZE, this way
> > mapped size == guest size always (we can madvise(MADV_DONTNEED) the gap
> > in the mmapped ram).
> >
> > Since a user which requests more than KVM_32BIT_GAP_START will have to
> > be on 64bit host anyway, there shouldn't be any issue with that.
> >
> Do you mean increase *kvm->ram_size* by KVM_32BIT_GAP_SIZE?
> but sometimes kvm->ram_size stands for guest physical ram size (for
> example in kvm__init_ram() code).

Yup, kvm->ram_size.

If the user requested more than KVM_32BIT_GAP_START, we pretty much have
to create the gap, so instead of playing around with different
interpretations of ram_size, lets add the gap size - this will let us
have just one ram_size.

mmap()ing extra space for the gap is free, and that was the plan in the
first place (we just got the math wrong :) ).

Do you see an issue with increasing kvm->ram_size?

-- 

Sasha.

Re: [PATCH] kvm tools: Make the whole guest memory mergeable

[Zang Hongyong]

于 2011/12/16,星期五 16:45, Sasha Levin 写道:
> On Fri, 2011-12-16 at 16:36 +0800, Zang Hongyong wrote:
>> 于 2011/12/16,星期五 15:23, Sasha Levin 写道:
>>> On Fri, 2011-12-16 at 15:02 +0800, Zang Hongyong wrote:
>>>> 于 2011/12/16,星期五 13:50, Sasha Levin 写道:
>>>>> On Fri, 2011-12-16 at 09:01 +0800, zanghongyong@huawei.com wrote:
>>>>>> If a guest's ram_size exceeds KVM_32BIT_GAP_START, the corresponding kvm tool's
>>>>>> virtual address size should be (ram_size + KVM_32BIT_GAP_SIZE), rather than ram_size.
>>>>> You're right.
>>>>>
>>>>> There are more places than just the madvise() code which make the same
>>>>> error you've spotted (for example, the memslot allocation code), so
>>>>> instead of trying to fix all of them I'd suggest to just update ram_size
>>>>> in kvm__arch_init() before allocating everything - that should fix all
>>>>> of them at once.
>>>>>
>>>> Yes. There are other scenarios with the same error.
>>>> However ram_size sometimes means real guest ram size, and sometimes 
>>>> means virtual address
>>>> size of kvm tool's user space. Shall we define a new variable?
>>> Let's keep it simple. If the user requests more than RAM than
>>> KVM_32BIT_GAP_START just increase it by KVM_32BIT_GAP_SIZE, this way
>>> mapped size == guest size always (we can madvise(MADV_DONTNEED) the gap
>>> in the mmapped ram).
>>>
>>> Since a user which requests more than KVM_32BIT_GAP_START will have to
>>> be on 64bit host anyway, there shouldn't be any issue with that.
>>>
>> Do you mean increase *kvm->ram_size* by KVM_32BIT_GAP_SIZE?
>> but sometimes kvm->ram_size stands for guest physical ram size (for
>> example in kvm__init_ram() code).
> Yup, kvm->ram_size.
>
> If the user requested more than KVM_32BIT_GAP_START, we pretty much have
> to create the gap, so instead of playing around with different
> interpretations of ram_size, lets add the gap size - this will let us
> have just one ram_size.
>
> mmap()ing extra space for the gap is free, and that was the plan in the
> first place (we just got the math wrong :) ).
>
> Do you see an issue with increasing kvm->ram_size?
>
Yes, it will cause some problems after simply increase the kvm->ram_size.
For examples:
In kvm__init_ram() code we use kvm->ram_size to calculate the size of
the second
RAM range from 4GB to the end of RAM (phys_size = kvm->ram_size -
phys_size;),
so after increase the kvm->ram_size, it will goes wrong.
This problem also happens in e820_setup() code and load_bzimage() code.

Re: [PATCH] kvm tools: Make the whole guest memory mergeable

[Sasha Levin]

On Fri, 2011-12-16 at 17:33 +0800, Zang Hongyong wrote:
> > Do you see an issue with increasing kvm->ram_size?
> >
> Yes, it will cause some problems after simply increase the kvm->ram_size.
> For examples:
> In kvm__init_ram() code we use kvm->ram_size to calculate the size of
> the second
> RAM range from 4GB to the end of RAM (phys_size = kvm->ram_size -
> phys_size;),
> so after increase the kvm->ram_size, it will goes wrong.
> This problem also happens in e820_setup() code and load_bzimage() code. 

Yup, but fixing it is much easier than having two different sizes of the same thing.

For example, the fix for the problem in kvm__init_ram() (and e820_setup()) would be:

@@ -112,7 +112,7 @@ void kvm__init_ram(struct kvm *kvm)
                /* Second RAM range from 4GB to the end of RAM: */
 
                phys_start = 0x100000000ULL;
-               phys_size  = kvm->ram_size - phys_size;
+               phys_size  = kvm->ram_size - phys_start;
                host_mem   = kvm->ram_start + phys_start;
 
                kvm__register_mem(kvm, phys_start, phys_size, host_mem);

I basically want one memory map with one size which includes *everything*, even if that memory map includes a gap in the middle I still want the total size to include that gap.

btw, what problem do you see in load_bzimage()?

-- 

Sasha.

Re: [PATCH] kvm tools: Make the whole guest memory mergeable

[Zang Hongyong]

于 2011/12/16,星期五 17:46, Sasha Levin 写道:
> On Fri, 2011-12-16 at 17:33 +0800, Zang Hongyong wrote:
>>> Do you see an issue with increasing kvm->ram_size?
>>>
>> Yes, it will cause some problems after simply increase the kvm->ram_size.
>> For examples:
>> In kvm__init_ram() code we use kvm->ram_size to calculate the size of
>> the second
>> RAM range from 4GB to the end of RAM (phys_size = kvm->ram_size -
>> phys_size;),
>> so after increase the kvm->ram_size, it will goes wrong.
>> This problem also happens in e820_setup() code and load_bzimage() code.
> Yup, but fixing it is much easier than having two different sizes of the same thing.
>
> For example, the fix for the problem in kvm__init_ram() (and e820_setup()) would be:
>
> @@ -112,7 +112,7 @@ void kvm__init_ram(struct kvm *kvm)
>                  /* Second RAM range from 4GB to the end of RAM: */
>
>                  phys_start = 0x100000000ULL;
> -               phys_size  = kvm->ram_size - phys_size;
> +               phys_size  = kvm->ram_size - phys_start;
>                  host_mem   = kvm->ram_start + phys_start;
>
>                  kvm__register_mem(kvm, phys_start, phys_size, host_mem);
>
> I basically want one memory map with one size which includes *everything*, even if that memory map includes a gap in the middle I still want the total size to include that gap.
>
> btw, what problem do you see in load_bzimage()?
>
I've got what you mean.
And there's nothing wrong in load_bzimage(). It's my misunderstanding.