[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 1/5] virtio-balloon: Remove unnecessary MADV_WILLN
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-ppc] [PATCH 1/5] virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate |
Date: |
Thu, 28 Feb 2019 08:36:58 -0500 |
On Thu, Feb 14, 2019 at 03:39:12PM +1100, 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.
>
> This patch simplify's the balloon operation by dropping the madvise()
> on deflate. This might have an impact on performance - it will move a
> delay at deflate time until that memory is actually touched, which
> might be more latency sensitive. However:
>
> * 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 needed (or even be faulted out again if
> the host is under memory pressure).
>
> * Usually, the timescale over which you'll be adjusting the balloon
> is long enough that a few extra faults after deflation aren't
> going to make a difference.
>
> Signed-off-by: David Gibson <address@hidden>
> Reviewed-by: David Hildenbrand <address@hidden>
> Reviewed-by: Michael S. Tsirkin <address@hidden>
I'm having second thoughts about this. It might affect performance but
probably won't but we have no idea. Might cause latency jitter after
deflate where it previously didn't happen. This kind of patch should
really be accompanied by benchmarking results, not philosophy.
> ---
> 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 a12677d4d5..43af521884 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);
> }
> }
>
> --
> 2.20.1
[Qemu-ppc] [PATCH 4/5] virtio-balloon: Use ram_block_discard_range() instead of raw madvise(), David Gibson, 2019/02/13
[Qemu-ppc] [PATCH 5/5] virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size, David Gibson, 2019/02/13
Re: [Qemu-ppc] [PATCH 0/5] Improve balloon handling of pagesizes other than 4kiB, Michael S. Tsirkin, 2019/02/28