>> >> That leaves us with either >> >> 1. "Pages hinted via VIRTIO_BALLOON_F_FREE_PAGE_HINT might get replaced >> by zero page. However, as soon as the page is written by the guest (even >> before the hinting request was processed by the host), the modified page >> will stay - whereby the unwritten parts might either be from the old, or >> from the zero page." - a QEMU BUG. > > How is this a bug? This is the behavior you would see with the current > balloon driver. When you put a page into a balloon it has the option > to either madvise it away, defer it, or just skip it because he > balloon is disabled. Is the "zero page" a typo? If it was > uninitialized data that would be a bug, but I don't see how a zero > page or the old data would be a bug. Sorry, I meant if this was the original design idea of hinting, then we would have a QEMU BUG as of now, as we might get get uninitialized data. Makes sense? [...] > >>> >>>> The current QEMU implementation would be to simply migrate all hinted >>>> pages. In the future we could optimize. Not sure if it's worth the trouble. >>> >>> So the trick for me is how best to go about sorting this all out. >>> There are two ways I see of doing it. >>> >>> The first approach would be to just assume that hinting should be >>> disassociated from poisoning. If I go that route that would more >>> closely match up with what happened in QEMU. The downside is that it >>> locks in the unmodified/uninitialized behavior and would require pages >>> to be rewritten when they come out of the balloon. However this is the >>> behavior we have now so it would only require specification >>> documentation changes. >> >> Right now, for simplicity, I prefer this and define >> VIRTIO_BALLOON_F_PAGE_POISON only for explicit deflation (balloon >> deflation via the deflate queue) and implicit deflation >> (VIRTIO_BALLOON_F_REPORTING). > > I don't recall us talking about the explicit deflation case before. The interesting part is that drivers/virtio/virtio_balloon.c mentions: "Let the hypervisor know that we are expecting a specific value to be written back in balloon pages.". But I just realized that you introduced this comment, not the original VIRTIO_BALLOON_F_PAGE_POISON commit. Should this have been "in reported pages when implicitly deflating them by reusing them." or sth. like that? > What is the expectation there? I assume we are saying either > poison_val or unmodified? If so I would think the inflate case makes > much more sense as that is where the madvise is called that will > discard the data. If so it would be pretty easy to just add a check > for the poison value to the same spot we check > qemu_balloon_is_inhibited. Okay, we have basically no idea what was the intention of VIRTIO_BALLOON_F_PAGE_POISON with basic deflation/inflation as well. So I think we can define what suits us. On the deflate path, we could always simply fill with poison_val. But there are nasty corner cases (esp. no VIRTIO_BALLOON_F_MUST_TELL_HOST). What would be your suggestion? Also don't care about VIRTIO_BALLOON_F_PAGE_POISON on ordinary inflation/deflation? At this point, I think this makes sense. -- Thanks, David / dhildenb _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
