Re: [Qemu-devel] [RFC 1/5] virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate

[David Hildenbrand]

On 12/10/2018 05:24, David Gibson wrote:
> When the balloon is inflated, we discard memory place in it using madvise()
> with MADV_DONTNEED.  And when we deflate it we use MADV_WILLNEED, which
> sounds like it makes sense but is actually unnecessary.
> 
> The misleadingly named MADV_DONTNEED just discards the memory in question,
> it doesn't set any persistent state on it in-kernel; all that's necessary
> to bring the memory back is to touch it.  MADV_WILLNEED in contrast
> specifically says that the memory will be used soon and faults it in.
> 
> Memory that's being given back to the guest by deflating the balloon
> *might* be used soon, but it equally could just sit around in the guest's
> pools until it actually needs it.  And, over the general timescale that
> memory ballooning operates, it seems unlikely that one extra fault for the
> guest will be a vast performance issue.
> 
> So, simplify the balloon's operation by dropping the madvise() on deflate.
> 
> Signed-off-by: David Gibson <address@hidden>
> ---
>  hw/virtio/virtio-balloon.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 1728e4f83a..6ec4bcf4e1 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -35,9 +35,8 @@
>  
>  static void balloon_page(void *addr, int deflate)
>  {
> -    if (!qemu_balloon_is_inhibited()) {
> -        qemu_madvise(addr, BALLOON_PAGE_SIZE,
> -                deflate ? QEMU_MADV_WILLNEED : QEMU_MADV_DONTNEED);
> +    if (!qemu_balloon_is_inhibited() && !deflate) {
> +        qemu_madvise(addr, BALLOON_PAGE_SIZE, QEMU_MADV_DONTNEED);
>      }
>  }
>  
> 

Care to rename the function and drop the deflate parameter?
(call it only when actually inflating)

apart from that

Reviewed-by: David Hildenbrand <address@hidden>

-- 

Thanks,

David / dhildenb