Re: [Qemu-ppc] [PATCH 3/3] virtio-balloon: Restore MADV_WILLNEED hint on balloon deflate

[David Hildenbrand]

On 06.03.19 04:06, David Gibson wrote:
> Prior to f6deb6d9 "virtio-balloon: Remove unnecessary MADV_WILLNEED on
> deflate", the balloon device issued an madvise() MADV_WILLNEED on
> pages removed from the balloon.  That would hint to the host kernel
> that the pages were likely to be needed by the guest in the near
> future.
> 
> It's unclear if this is actually valuable or not, and so f6deb6d9
> removed this, essentially ignoring balloon deflate requests.  However,
> concerns have been raised that this might cause a performance
> regression by causing extra latency for the guest in certain
> configurations.
> 
> So, until we can get actual benchmark data to see if that's the case,
> this restores the old behaviour, issuing a MADV_WILLNEED when a page is
> removed from the balloon.
> 
> Signed-off-by: David Gibson <address@hidden>
> ---
>  hw/virtio/virtio-balloon.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 7412bf8c85..ac36988605 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -118,6 +118,8 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
>      RAMBlock *rb;
>      size_t rb_page_size;
>      ram_addr_t ram_offset, host_page_base;
> +    void *host_addr;
> +    int ret;
>  
>      /* XXX is there a better way to get to the RAMBlock than via a
>       * host address? */
> @@ -146,6 +148,17 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
>              balloon->pbp = NULL;
>          }
>      }
> +
> +    host_addr = (void *)((uintptr_t)addr & ~(rb_page_size - 1));
> +
> +    /* When a page is deflated, we hint the whole host page it lives
> +     * on, since we can't do anything smaller */
> +    ret = qemu_madvise(host_addr, rb_page_size, QEMU_MADV_WILLNEED);
> +    if (ret != 0) {
> +        warn_report("Couldn't MADV_WILLNEED on balloon deflate: %s",
> +                    strerror(errno));
> +        /* Otherwise ignore, failing to page hint shouldn't be fatal */
> +    }

As far as I understand the man page, only MADV_DONTNEED will complain
about locked, huge or VM_PFNMAP pages. So I guess MADV_WILLNEED will
work for these cases (in the worst case be a NOP).

-- 

Thanks,

David / dhildenb