From: Paolo Bonzini <pbonzini@redhat.com>
To: "Michael S. Tsirkin" <mst@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 08:38:09 +0200 [thread overview]
Message-ID: <504D8AD1.6050006@redhat.com> (raw)
In-Reply-To: <20120910060359.GB16819@redhat.com>
Il 10/09/2012 08:03, Michael S. Tsirkin ha scritto:
> 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?
No, that's the point ("break the guest" is really "break the driver").
>>> 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?
>
> 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.
Agreed.
>>>> 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?
Idle guests can move cache pages to the balloon. You can overcommit
more aggressively, because the host can madvise away a lot more memory.
>>> 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.
Hmm, then I'm not sure I follow yours. We agreed that delaying
notifications or skipping them is really the same thing, right?
I think we're just stuck in a linguistic problem, with "must not" being
wrong and "does not have to" being too verbose. Calling it
VIRTIO_BALLOON_F_SILENT_DEFLATE was a workaround for this, but perhaps
it adds more confusion.
Paolo
WARNING: multiple messages have this Message-ID (diff)
From: Paolo Bonzini <pbonzini@redhat.com>
To: "Michael S. Tsirkin" <mst@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 08:38:09 +0200 [thread overview]
Message-ID: <504D8AD1.6050006@redhat.com> (raw)
In-Reply-To: <20120910060359.GB16819@redhat.com>
Il 10/09/2012 08:03, Michael S. Tsirkin ha scritto:
> 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?
No, that's the point ("break the guest" is really "break the driver").
>>> 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?
>
> 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.
Agreed.
>>>> 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?
Idle guests can move cache pages to the balloon. You can overcommit
more aggressively, because the host can madvise away a lot more memory.
>>> 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.
Hmm, then I'm not sure I follow yours. We agreed that delaying
notifications or skipping them is really the same thing, right?
I think we're just stuck in a linguistic problem, with "must not" being
wrong and "does not have to" being too verbose. Calling it
VIRTIO_BALLOON_F_SILENT_DEFLATE was a workaround for this, but perhaps
it adds more confusion.
Paolo
next prev parent reply other threads:[~2012-09-10 6:38 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 6:03 ` Michael S. Tsirkin
2012-09-10 6:03 ` Michael S. Tsirkin
2012-09-10 6:38 ` Paolo Bonzini [this message]
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-10 5:50 ` 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=504D8AD1.6050006@redhat.com \
--to=pbonzini@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=mst@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.