Re: [Qemu-devel] [5874] Add virtio-balloon support

Re: [Qemu-devel] [5874] Add virtio-balloon support

[Hollis Blanchard]

On Thu, 2008-12-04 at 20:33 +0000, Anthony Liguori wrote:
> 
> +static void balloon_page(void *addr, int deflate)
> +{
> +#if defined(__linux__)
> +    if (!kvm_enabled() || kvm_has_sync_mmu())
> +        madvise(addr, TARGET_PAGE_SIZE,
> +                deflate ? MADV_WILLNEED : MADV_DONTNEED);
> +#endif
> +}

Hmm, I just noticed this... we need to use VIRTIO_BALLOON_PFN_SHIFT like
Rusty did on the kernel side.

However, in general I'm not sure how this is supposed to work. Isn't it
true that madvise() is a no-op if 0 < length < getpagesize()? If so, how
should the guest know the chunk size needed on the host?

What happens when a guest tries to balloon 4K pages when it's backed on
the host by hugetlbfs? We can't even use getpagesize() there.

Maybe the virtio balloon interface needs to advertise a unit size from
the host, and use that size instead of alloc_page() in the guest?

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [Qemu-devel] [5874] Add virtio-balloon support

[Anthony Liguori]

Hollis Blanchard wrote:
> On Thu, 2008-12-04 at 20:33 +0000, Anthony Liguori wrote:
>   
>> +static void balloon_page(void *addr, int deflate)
>> +{
>> +#if defined(__linux__)
>> +    if (!kvm_enabled() || kvm_has_sync_mmu())
>> +        madvise(addr, TARGET_PAGE_SIZE,
>> +                deflate ? MADV_WILLNEED : MADV_DONTNEED);
>> +#endif
>> +}
>>     
>
> Hmm, I just noticed this... we need to use VIRTIO_BALLOON_PFN_SHIFT like
> Rusty did on the kernel side.
>
> However, in general I'm not sure how this is supposed to work. Isn't it
> true that madvise() is a no-op if 0 < length < getpagesize()? If so, how
> should the guest know the chunk size needed on the host?
>   

We need to pass multiple of TARGET_PAGE_SIZE to madvise() but we can 
certainly adjust that depending on VIRTIO_BALLOON_PFN_SHIFT.  But 
basically, if the two aren't equal, we shouldn't even try madvise().

> What happens when a guest tries to balloon 4K pages when it's backed on
> the host by hugetlbfs? We can't even use getpagesize() there.
>   

Nothing.  For ballooning to work in this circumstance, the guest would 
have to balloon 2MB pages which isn't something that's reasonable for it 
to do.

> Maybe the virtio balloon interface needs to advertise a unit size from
> the host, and use that size instead of alloc_page() in the guest?
>   

That's a possibility.  May make sense to give it the ability to balloon 
memory up to unit size because I don't think it'll be able to meet the 
reservation for large pages only.

Regards,

Anthony Liguori

Re: [5874] Add virtio-balloon support

[Paul Brook]

On Thursday 04 December 2008, Anthony Liguori wrote:
> Hollis Blanchard wrote:
> > On Thu, 2008-12-04 at 20:33 +0000, Anthony Liguori wrote:
> >> +static void balloon_page(void *addr, int deflate)
> >> +{
> >> +#if defined(__linux__)
> >> +    if (!kvm_enabled() || kvm_has_sync_mmu())
> >> +        madvise(addr, TARGET_PAGE_SIZE,
> >> +                deflate ? MADV_WILLNEED : MADV_DONTNEED);
> >> +#endif
> >> +}
> >
> > Hmm, I just noticed this... we need to use VIRTIO_BALLOON_PFN_SHIFT like
> > Rusty did on the kernel side.
> >
> > However, in general I'm not sure how this is supposed to work. Isn't it
> > true that madvise() is a no-op if 0 < length < getpagesize()? If so, how
> > should the guest know the chunk size needed on the host?
>
> We need to pass multiple of TARGET_PAGE_SIZE to madvise()

You mean HOST_PAGE_SIZE?

My linux manpage says that the address must be page aligned.

Paul

Re: [Qemu-devel] [5874] Add virtio-balloon support

[Anthony Liguori]

Paul Brook wrote:
> On Thursday 04 December 2008, Anthony Liguori wrote:
>   
>> Hollis Blanchard wrote:
>>     
>>> On Thu, 2008-12-04 at 20:33 +0000, Anthony Liguori wrote:
>>>       
>>>> +static void balloon_page(void *addr, int deflate)
>>>> +{
>>>> +#if defined(__linux__)
>>>> +    if (!kvm_enabled() || kvm_has_sync_mmu())
>>>> +        madvise(addr, TARGET_PAGE_SIZE,
>>>> +                deflate ? MADV_WILLNEED : MADV_DONTNEED);
>>>> +#endif
>>>> +}
>>>>         
>>> Hmm, I just noticed this... we need to use VIRTIO_BALLOON_PFN_SHIFT like
>>> Rusty did on the kernel side.
>>>
>>> However, in general I'm not sure how this is supposed to work. Isn't it
>>> true that madvise() is a no-op if 0 < length < getpagesize()? If so, how
>>> should the guest know the chunk size needed on the host?
>>>       
>> We need to pass multiple of TARGET_PAGE_SIZE to madvise()
>>     
>
> You mean HOST_PAGE_SIZE?
>   

I change my previous answer :-)

I think we should just pass addr and the appropriate size of the memory 
the guest is ballooning.  Whether the host can handle it is up to it.  
Worst case scenario, madvise() returns an error and no harm is done.

Regards,

Anthony Liguori

> My linux manpage says that the address must be page aligned.
>
> Paul
>