[...]>> >> 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 forYes, 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 :) We can simply document "VIRTIO_BALLOON_F_MUST_TELL_HOST does not apply to FREE_PAGE_HINTING" and "with FREE_PAGE_HINTING, the guest can reuse pages any time, without waiting for a response/ack from the hypervisor". Thoughts? -- Thanks, David / dhildenb
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.> We can simply document "VIRTIO_BALLOON_F_MUST_TELL_HOST does not apply > to FREE_PAGE_HINTING" and "with FREE_PAGE_HINTING, the guest can reuse > pages any time, without waiting for a response/ack from the hypervisor". > > Thoughts? > > -- > Thanks, > > David / dhildenb
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