On 04.02.20 21:33, Michael S. Tsirkin wrote:
> On Tue, Feb 04, 2020 at 05:56:22PM +0100, David Hildenbrand wrote:
>> [...]
>>
>>>>
>>>> Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be
>>>> that we do have VIRTIO_BALLOON_F_MUST_TELL_HOST. I assume this means
>>>> (-ENOCLUE) that we have to wait until the hypervisor notifies us via the STOP? Or
>>>> for which event do we have to wait? Because there is no way to *tell host* here
>>>> that we want to reuse a page. The hypervisor will *tell us* when we can reuse pages.
>>>> For the shrinker it is simple: Don't use the shrinker with
>>>> VIRTIO_BALLOON_F_MUST_TELL_HOST :) . But to fix Issue 1, we *would* have to wait
>>>> until we get a STOP signal. That is not really possible because it might
>>>> take an infinite amount of time.
>>>>
>>>> Michael, any clue on which event we have to wait with
>>>> VIRTIO_BALLOON_F_MUST_TELL_HOST? IMHO, I don't think
>>>> VIRTIO_BALLOON_F_MUST_TELL_HOST applies to VIRTIO_BALLOON_F_FREE_PAGE_HINT and
>>>> we'd better document that. It introduces complexity with no clear benefit.
>>>
>>> I meant that we must wait for host to see the hint. Signalled via using
>>> the buffer. But maybe that's too far in the meaning from
>>> VIRTIO_BALLOON_F_MUST_TELL_HOST and we need a separate new flag for
>>
>> Yes, that's what I think.
>>
>>> that. Then current code won't be broken (yay!) but we need to
>>> document another flag that's pretty similar.
>>
>> I mean, do we need a flag at all as long as there is no user?
>> Introducing a flag and documenting it if nobody uses it does not sound
>> like a work I will enjoy :)
>
> It's not the user. It's the non-orthogonality that I find inelegant.
>
> Let me try to formulate the issue, forgive me for thinking aloud
> (and I Cc'd virtio-dev since we are talking spec things here):
>
> The annoying thing is that with Alex's VIRTIO_BALLOON_F_REPORTING
> host does depend on guest not touching memory before host uses it.
> So functionally VIRTIO_BALLOON_F_FREE_PAGE_HINT and
> VIRTIO_BALLOON_F_REPORTING really are supposed to do
> exectly the same thing, with the differences being
> - VIRTIO_BALLOON_F_FREE_PAGE_HINT comes upon host's request.
> VIRTIO_BALLOON_F_REPORTING is initiated by guest.
> - VIRTIO_BALLOON_F_FREE_PAGE_HINT does not always wait for
> host to use the hint before touching the page.
> Well it almost always does, but there's an exception in the
> shrinker which tries to stop reporting as quickly as possible
> in the case of a slow host.
> VIRTIO_BALLOON_F_REPORTING always does.
> This means host can blow the page away when it sees the hint.
>
> Now the point is that with VIRTIO_BALLOON_F_REPORTING
> I think you really must wait for host to use the hint.
> But with VIRTIO_BALLOON_F_FREE_PAGE_HINT it depends
> on how host uses it. Something to think about,
> I'm not sure what is the best thing to do here.
I think VIRTIO_BALLOON_F_FREE_PAGE_HINT is really the special case and
shall be left alone (not messed with VIRTIO_BALLOON_F_MUST_TELL_HOST).
Initiated by the host, complicated protocol and semantics, guest can
reuse pages any time it wants ("hint").
VIRTIO_BALLOON_F_REPORTING is *basically* ordinary inflation on
stereoids (be able to report a size for each page and multiple pages in
one go) BUT, we can currently *never* have
VIRTIO_BALLOON_F_MUST_TELL_HOST semantics - there is no deflation.
We could rename VIRTIO_BALLOON_F_REPORTING to something like
VIRTIO_BALLOON_F_SIZE and make it obey to
VIRTIO_BALLOON_F_MUST_TELL_HOST (meaning, there would have to be a
deflate queue as well!) - but it contradicts to the real needs.
VIRTIO_BALLOON_F_REPORTING comnbined with
VIRTIO_BALLOON_F_MUST_TELL_HOST would not be usable by Linux for free
page reporting.
Well, as QEMU never sets VIRTIO_BALLOON_F_MUST_TELL_HOST we would be
fine. Alexander would have to add an inflate+deflate queue and make his
feature depend on !VIRTIO_BALLOON_F_MUST_TELL_HOST.
Is that the consistency you're looking for? Alexander, thoughts?
--
Thanks,
David / dhildenb
