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 <da...@gibson.dropbear.id.au>
> ---
> 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 <da...@redhat.com>
--
Thanks,
David / dhildenb
