Re: [virtio-dev] Re: [PATCH] virtio-balloon: Disable free page hinting/reporting if page poison is disabled

[David Hildenbrand <david@redhat.com>]

>>> 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.

> 
> 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.

-- 
Thanks,

David / dhildenb