[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/4] balloon: Allow multiple inhibit users
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/4] balloon: Allow multiple inhibit users |
Date: |
Tue, 7 Aug 2018 14:08:26 -0600 |
On Tue, 7 Aug 2018 22:44:11 +0300
"Michael S. Tsirkin" <address@hidden> wrote:
> On Tue, Aug 07, 2018 at 01:31:22PM -0600, Alex Williamson wrote:
> > A simple true/false internal state does not allow multiple users. Fix
> > this within the existing interface by converting to a counter, so long
> > as the counter is elevated, ballooning is inhibited.
> >
> > Reviewed-by: David Hildenbrand <address@hidden>
> > Reviewed-by: Peter Xu <address@hidden>
> > Reviewed-by: Cornelia Huck <address@hidden>
> > Signed-off-by: Alex Williamson <address@hidden>
> > ---
> > balloon.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/balloon.c b/balloon.c
> > index 6bf0a9681377..931987983858 100644
> > --- a/balloon.c
> > +++ b/balloon.c
> > @@ -26,6 +26,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "qemu-common.h"
> > +#include "qemu/atomic.h"
> > #include "exec/cpu-common.h"
> > #include "sysemu/kvm.h"
> > #include "sysemu/balloon.h"
> > @@ -37,16 +38,22 @@
> > static QEMUBalloonEvent *balloon_event_fn;
> > static QEMUBalloonStatus *balloon_stat_fn;
> > static void *balloon_opaque;
> > -static bool balloon_inhibited;
> > +static int balloon_inhibit_count;
> >
> > bool qemu_balloon_is_inhibited(void)
> > {
> > - return balloon_inhibited;
> > + return atomic_read(&balloon_inhibit_count) > 0;
> > }
> >
> > void qemu_balloon_inhibit(bool state)
> > {
> > - balloon_inhibited = state;
> > + if (state) {
> > + atomic_inc(&balloon_inhibit_count);
> > + } else {
> > + atomic_dec(&balloon_inhibit_count);
> > + }
> > +
> > + assert(atomic_read(&balloon_inhibit_count) >= 0);
> > }
> >
> > static bool have_balloon(Error **errp)
>
> This blocks QEMU_MADV_WONTNEED but it also blocks QEMU_MADV_WILLNEED.
> Is this necessarily a good idea?
This is existing balloon inhibitor behavior, but do you have some
reason to suspect WILLNEED is necessary? It's my impression that
WILLNEED is a purely optional prefetch directive that's entirely
unnecessary if the page wasn't previously zapped with WONTNEED. If the
page was zapped, it will fault in on demand, potentially with higher
latency, but functionally correct. With vfio usage of the inhibitor,
we expect pinning to fault in any previously ballooned pages, so
calling WILLNEED on pages where the inhibit count is elevated due to an
assigned device seems unnecessary. Thanks,
Alex
- [Qemu-devel] [PATCH v3 0/4] Balloon inhibit enhancements, vfio restriction, Alex Williamson, 2018/08/07
- [Qemu-devel] [PATCH v3 2/4] kvm: Use inhibit to prevent ballooning without synchronous mmu, Alex Williamson, 2018/08/07
- [Qemu-devel] [PATCH v3 3/4] vfio: Inhibit ballooning based on group attachment to a container, Alex Williamson, 2018/08/07
- [Qemu-devel] [PATCH v3 4/4] vfio/ccw/pci: Allow devices to opt-in for ballooning, Alex Williamson, 2018/08/07
- Re: [Qemu-devel] [PATCH v3 0/4] Balloon inhibit enhancements, vfio restriction, Michael S. Tsirkin, 2018/08/07