mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	fes@google.com, aarcange@redhat.com, riel@redhat.com,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	mikew@google.com, yinghan@google.com,
	virtualization@lists.linux-foundation.org, yvugenfi@redhat.com,
	vrozenfe@redhat.com
Subject: Re: [PATCH] virtio-balloon spec: provide a version of the "silent deflate" feature that works
Date: Fri, 7 Sep 2012 17:25:45 +0300	[thread overview]
Message-ID: <20120907142545.GC17397@redhat.com> (raw)
In-Reply-To: <5049FEDD.40303@redhat.com>

On Fri, Sep 07, 2012 at 04:04:13PM +0200, Paolo Bonzini wrote:
> Il 07/09/2012 14:44, Michael S. Tsirkin ha scritto:
> >> Well, according to your reading of the spec.
> >>
> >> In the spec I'm reading "Host must be told before pages from the balloon
> >> are used".  Doesn't say anything about the guest.
> > 
> > No? How is host told then? By divine force?
> 
> For a simple madvise-based implementation such as the one in QEMU, the
> host doesn't need to be told about anything (except periodic updating of
> the "actual" field, or the statsq).
> 
> The guest doesn't need at all to use the deflateq in fact.
> 
> >> Now, indeed a very free interpretation is "Guest will tell host before
> >> pages from the balloon are used".  It turns out that it's exactly what
> >> guests have been doing, hence that's exactly what I'm proposing now:
> >> rename the feature to VIRTIO_BALLOON_F_WILL_TELL_HOST.
> > 
> > Rename is fine.
> 
> Cool.
> 
> >> Yes, that part we agree on I think.  We disagree that (after the rename)
> >> QEMU should start always proposing VIRTIO_BALLOON_F_WILL_TELL_HOST.
> > 
> > Not always. It must be off if compatibility with old qemu is disabled.
> 
> Yes, of course.
> 
> >> _Plus_ adding the new feature bit, which is needed to actually tell the
> > 
> > This part I do not get.  What is silent deflate, why is it useful
> > and what it has to do with what we are discussing here?
> 
> Silent deflate is deflating just by using the page, without using the
> deflateq at all.  So it can be done even from GFP_ATOMIC context.

BTW I don't see a need to avoid deflateq.
Without MUST_TELL_HOST you can just deflate and then
bounce telling the host out to a thread.

> But knowing that the guest will _not_ deflate silently also benefits the
> host. This is the cause of unpinning ballooned pages and pinning them
> again upon deflation. This allows cooperative memory overcommit even if
> the guests' memory is pinned, similar to what Xen did before it
> supported overcommit.  This is the second feature bit.

This is the MUST_TELL_HOST.

> There are three cases:
> 
> * guest will never do silent deflation: current Linux driver.  Host may
> do munlock/mlock dance.  Negotiates VIRTIO_BALLOON_F_WILL_TELL_HOST
> only, features = VIRTIO_BALLOON_F_WILL_TELL_HOST.
> 
> * guest will always do silent deflation: current Windows driver.

Not exactly. It is not silent. It tells host
just after use.

> Negotiates nothing, or if it cares it can negotiate
> VIRTIO_BALLOON_F_SILENT_DEFLATE.  Host mustn't do munlock/mlock dance.
>
> * guest may do silent deflation if available: combo of Linux driver +
> Frank's driver.  Negotiates both features, looks at
> VIRTIO_BALLOON_F_SILENT_DEFLATE host feature to decide how to behave:
> 
>   ** If PCI passthrough, the host will not negotiate
>      VIRTIO_BALLOON_F_SILENT_DEFLATE.  The driver will behave as the
>      current Linux driver, the host can do the munlock/mlock dance.

So this case is just existing interface. OK.

>   ** If no PCI passthrough, the host will negotiate
>      VIRTIO_BALLOON_F_SILENT_DEFLATE, and the driver can balloon more
>      aggressively and not use the deflateq.
> 
> Paolo

This last trickery confuses me.  It just does not make sense to set both
SILENT and TELL: either you are silent or you tell.

What is the benefit of avoiding host notification?
It seems cleaner for the host to know the state.



-- 
MST

  reply	other threads:[~2012-09-07 14:24 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-06  7:46 Paolo Bonzini
2012-09-06  8:47 ` Michael S. Tsirkin
2012-09-06  9:24   ` Paolo Bonzini
2012-09-06  9:44     ` Michael S. Tsirkin
2012-09-06  9:57       ` Paolo Bonzini
2012-09-06 10:53         ` Michael S. Tsirkin
2012-09-06 12:13           ` Paolo Bonzini
2012-09-06 12:51             ` Michael S. Tsirkin
2012-09-06 13:12               ` Paolo Bonzini
2012-09-06 23:45             ` Rusty Russell
2012-09-07  5:42               ` Michael S. Tsirkin
2012-09-07  6:39                 ` Rusty Russell
2012-09-07  9:27                   ` Paolo Bonzini
2012-09-07 10:53                     ` Michael S. Tsirkin
2012-09-07 11:20                       ` Paolo Bonzini
2012-09-07 12:17                         ` Michael S. Tsirkin
2012-09-07 12:22                           ` Paolo Bonzini
2012-09-07 12:44                             ` Michael S. Tsirkin
2012-09-07 14:04                               ` Paolo Bonzini
2012-09-07 14:25                                 ` Michael S. Tsirkin [this message]
2012-09-07 14:44                                   ` Paolo Bonzini
2012-09-08 22:22                                     ` Michael S. Tsirkin
2012-09-10  5:50                                       ` Paolo Bonzini
2012-09-10  6:03                                         ` Michael S. Tsirkin
2012-09-10  6:38                                           ` Paolo Bonzini
2012-09-10  6:47                                             ` Michael S. Tsirkin
2012-09-10  6:55                                               ` Paolo Bonzini
2012-09-07 10:43                   ` Michael S. Tsirkin
2012-09-08  5:06                     ` Rusty Russell
2012-09-08 10:32                       ` Paolo Bonzini
2012-09-08 22:37                       ` Michael S. Tsirkin
2012-09-10  1:43                         ` Rusty Russell
2012-09-10  5:51                           ` Paolo Bonzini
2012-09-10  5:51                           ` Michael S. Tsirkin
2012-09-12  6:24                             ` Rusty Russell
2012-09-06 23:41 ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120907142545.GC17397@redhat.com \
    --to=mst@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=fes@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikew@google.com \
    --cc=pbonzini@redhat.com \
    --cc=riel@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=vrozenfe@redhat.com \
    --cc=yinghan@google.com \
    --cc=yvugenfi@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome