On Wed, Apr 22, 2020 at 3:24 AM David Hildenbrand <[email protected]> wrote: > > >>> 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. > > > > That is kind of what I was thinking. The problem is that once again > > the current implementation works when page poisoning is enabled. Us > > disabling that wouldn't make much sense. > > > > The whole thing with the reporting is that we are essentially just > > ballooning in place. What we may do at some point in the future would > > be to add an additional feature bit to do that for the standard > > balloon/hinting case. Then when that is set, and we know the contents > > won't match we can then just skip the madvise or hinting calls. That > > way it becomes an opt-in which is what the poison was supposed to be, > > but wasn't because the QEMU side was never implemented. > > Yeah, introducing this later makes sense. > > So VIRTIO_BALLOON_F_PAGE_POISON really means: > - poison_val in the config is unlocked > - when active, the guest is using page poisoning/init on free with > poison_val ("for you information") > - it only changes the semantic of free page reporting, nothing else. > (when reusing reported pages in the guest, they will either have the > old content, or will be filled with poison_val.) > > Makes sense? That should be easy to document.
Yep, makes sense. In theory the old content or being filled with poison_val should be the same thing. > > > > In the meantime I still have to make more changes to my QEMU patch > > set. The way the config_size logic is implemented is somewhat of a > > pain when you factor in the way the host_features and poison were > > handled. > > Okay, I'll wait for updated QEMU patches. I got to the root cause of the issues I was seeing. The config size being dependent on the page poison feature was somewhat problematic as it affects where I can place the setting of the bit since I have to have it done before we call virtio_init. I should be submitting the patches this afternoon. I am just going through and making sure I have my bases covered and testing for any corner cases. _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
