All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: fes@google.com, aarcange@redhat.com, riel@redhat.com,
	yvugenfi@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, mikew@google.com,
	yinghan@google.com, virtualization@lists.linux-foundation.org
Subject: Re: [PATCH] virtio-balloon spec: provide a version of the "silent deflate" feature that works
Date: Mon, 10 Sep 2012 09:03:59 +0300	[thread overview]
Message-ID: <20120910060359.GB16819@redhat.com> (raw)
In-Reply-To: <504D7F95.9070700@redhat.com>

On Mon, Sep 10, 2012 at 07:50:13AM +0200, Paolo Bonzini wrote:
> Il 09/09/2012 00:22, Michael S. Tsirkin ha scritto:
> >> Almost.  One is "the guest, if really needed, can tell the host of
> >> pages".  If not negotiated, and the host does not support it, the host
> >> must break the guest (e.g. fail to offer any virtqueues).
> > 
> > There is no way in spec to break the guest.
> > You can not fail to offer virtqueues.
> 
> You can always return 0 for the first queue.

I don't think guest drivers recover gracefully from this.
Do they?

> > Besides, there is no guarantee that virtqueue setup
> > happens after feature negotiation.
> 
> It is the only way that makes sense though (unless the guest would write
> 0 for its features).
>  Should we change that?

Not sure.  This was not always the case. Further
setup can fail with e.g ENOMEM and
driver could retry with a set of more conservative features.

I do think it would be nice to add a generic way for device to
notify guest about an internal failure.
This can only happen after DRIVER_OK status is written though,
and since existing drivers do not expect such failure, it might
be too late.

> >> The other is "the guest, though, would prefer not to do so".  It is
> >> different because the guest can proceed in a fallback mode even if the
> >> host doesn't offer it.
> > 
> > I think I get what your proposed SILENT means what I do not get
> > is the motivation. It looks like a premature optimization to me.
> 
> The motivation is to let the driver choose between two behaviors: the
> current one where ballooning is only done on request, and a more
> aggressive one.

Yes but why is being silent any good? Optimization?
Any data to show that it will help some workload?

...

> > OK so TELL says *when* to notify host, SILENT if set allows guest
> > to skip leak notifications? In this case TELL should just be ignored
> > when SILENT is set.
> 
> Yeah, that was my first idea.  However, there are existing drivers that
> ignore SILENT, so that would not be 100% exact.

Not sure I follow the logic.
They don't ack SILENT so that would be 100% exact.

-- 
MST

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: fes@google.com, aarcange@redhat.com, riel@redhat.com,
	kvm@vger.kernel.org, yvugenfi@redhat.com,
	linux-kernel@vger.kernel.org, mikew@google.com,
	yinghan@google.com, virtualization@lists.linux-foundation.org
Subject: Re: [PATCH] virtio-balloon spec: provide a version of the "silent deflate" feature that works
Date: Mon, 10 Sep 2012 09:03:59 +0300	[thread overview]
Message-ID: <20120910060359.GB16819@redhat.com> (raw)
In-Reply-To: <504D7F95.9070700@redhat.com>

On Mon, Sep 10, 2012 at 07:50:13AM +0200, Paolo Bonzini wrote:
> Il 09/09/2012 00:22, Michael S. Tsirkin ha scritto:
> >> Almost.  One is "the guest, if really needed, can tell the host of
> >> pages".  If not negotiated, and the host does not support it, the host
> >> must break the guest (e.g. fail to offer any virtqueues).
> > 
> > There is no way in spec to break the guest.
> > You can not fail to offer virtqueues.
> 
> You can always return 0 for the first queue.

I don't think guest drivers recover gracefully from this.
Do they?

> > Besides, there is no guarantee that virtqueue setup
> > happens after feature negotiation.
> 
> It is the only way that makes sense though (unless the guest would write
> 0 for its features).
>  Should we change that?

Not sure.  This was not always the case. Further
setup can fail with e.g ENOMEM and
driver could retry with a set of more conservative features.

I do think it would be nice to add a generic way for device to
notify guest about an internal failure.
This can only happen after DRIVER_OK status is written though,
and since existing drivers do not expect such failure, it might
be too late.

> >> The other is "the guest, though, would prefer not to do so".  It is
> >> different because the guest can proceed in a fallback mode even if the
> >> host doesn't offer it.
> > 
> > I think I get what your proposed SILENT means what I do not get
> > is the motivation. It looks like a premature optimization to me.
> 
> The motivation is to let the driver choose between two behaviors: the
> current one where ballooning is only done on request, and a more
> aggressive one.

Yes but why is being silent any good? Optimization?
Any data to show that it will help some workload?

...

> > OK so TELL says *when* to notify host, SILENT if set allows guest
> > to skip leak notifications? In this case TELL should just be ignored
> > when SILENT is set.
> 
> Yeah, that was my first idea.  However, there are existing drivers that
> ignore SILENT, so that would not be 100% exact.

Not sure I follow the logic.
They don't ack SILENT so that would be 100% exact.

-- 
MST

  reply	other threads:[~2012-09-10  6:02 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-06  7:46 [PATCH] virtio-balloon spec: provide a version of the "silent deflate" feature that works Paolo Bonzini
2012-09-06  7:46 ` Paolo Bonzini
2012-09-06  8:47 ` Michael S. Tsirkin
2012-09-06  8:47   ` Michael S. Tsirkin
2012-09-06  9:24   ` Paolo Bonzini
2012-09-06  9:24     ` Paolo Bonzini
2012-09-06  9:44     ` Michael S. Tsirkin
2012-09-06  9:44       ` Michael S. Tsirkin
2012-09-06  9:57       ` Paolo Bonzini
2012-09-06  9:57         ` Paolo Bonzini
2012-09-06 10:53         ` Michael S. Tsirkin
2012-09-06 10:53           ` Michael S. Tsirkin
2012-09-06 12:13           ` Paolo Bonzini
2012-09-06 12:13             ` Paolo Bonzini
2012-09-06 12:51             ` Michael S. Tsirkin
2012-09-06 12:51               ` Michael S. Tsirkin
2012-09-06 13:12               ` Paolo Bonzini
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  5:42                 ` Michael S. Tsirkin
2012-09-07  6:39                 ` Rusty Russell
2012-09-07  6:39                   ` Rusty Russell
2012-09-07  9:27                   ` Paolo Bonzini
2012-09-07  9:27                     ` Paolo Bonzini
2012-09-07 10:53                     ` Michael S. Tsirkin
2012-09-07 10:53                       ` Michael S. Tsirkin
2012-09-07 11:20                       ` Paolo Bonzini
2012-09-07 11:20                         ` Paolo Bonzini
2012-09-07 12:17                         ` Michael S. Tsirkin
2012-09-07 12:17                           ` Michael S. Tsirkin
2012-09-07 12:22                           ` Paolo Bonzini
2012-09-07 12:22                             ` Paolo Bonzini
2012-09-07 12:44                             ` Michael S. Tsirkin
2012-09-07 12:44                               ` Michael S. Tsirkin
2012-09-07 14:04                               ` Paolo Bonzini
2012-09-07 14:04                                 ` Paolo Bonzini
2012-09-07 14:25                                 ` Michael S. Tsirkin
2012-09-07 14:25                                   ` Michael S. Tsirkin
2012-09-07 14:44                                   ` Paolo Bonzini
2012-09-07 14:44                                     ` Paolo Bonzini
2012-09-08 22:22                                     ` Michael S. Tsirkin
2012-09-08 22:22                                       ` Michael S. Tsirkin
2012-09-10  5:50                                       ` Paolo Bonzini
2012-09-10  5:50                                       ` Paolo Bonzini
2012-09-10  6:03                                         ` Michael S. Tsirkin [this message]
2012-09-10  6:03                                           ` Michael S. Tsirkin
2012-09-10  6:38                                           ` Paolo Bonzini
2012-09-10  6:38                                             ` Paolo Bonzini
2012-09-10  6:47                                             ` Michael S. Tsirkin
2012-09-10  6:47                                               ` Michael S. Tsirkin
2012-09-10  6:55                                               ` Paolo Bonzini
2012-09-10  6:55                                                 ` Paolo Bonzini
2012-09-07 10:43                   ` Michael S. Tsirkin
2012-09-07 10:43                     ` Michael S. Tsirkin
2012-09-08  5:06                     ` Rusty Russell
2012-09-08  5:06                       ` Rusty Russell
2012-09-08 10:32                       ` Paolo Bonzini
2012-09-08 10:32                       ` Paolo Bonzini
2012-09-08 22:37                       ` Michael S. Tsirkin
2012-09-08 22:37                         ` Michael S. Tsirkin
2012-09-10  1:43                         ` Rusty Russell
2012-09-10  1:43                           ` Rusty Russell
2012-09-10  5:51                           ` Paolo Bonzini
2012-09-10  5:51                             ` Paolo Bonzini
2012-09-10  5:51                           ` Michael S. Tsirkin
2012-09-10  5:51                             ` Michael S. Tsirkin
2012-09-12  6:24                             ` Rusty Russell
2012-09-12  6:24                               ` Rusty Russell
2012-09-06 23:41 ` Michael S. Tsirkin
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=20120910060359.GB16819@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=virtualization@lists.linux-foundation.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.