OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison


On 20.05.20 18:25, Alexander Duyck wrote:
> On Wed, May 20, 2020 at 2:28 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 20.05.20 04:02, Alexander Duyck wrote:
>>> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>>>
>>> Page poison provides a way for the guest to notify the host of the content
>>> expected to be found in pages when they are added back to the guest after
>>> being discarded. The feature currently doesn't apply to the existing
>>> balloon features, however it will apply to an upcoming feature, free page
>>> reporting. Add documentation for the page poison feature describing the
>>> basic functionality and requirements.
>>>
>>
>> I would rephrase this, starting what it does *without* free page
>> reporting (which is not "provides a way for the guest to notify ..."),
>> and then eventually how this feature will also be used in the future as
>> well with free page reporting.
> 
> Below is a rewrite on this description. I'm thinking that we can
> probably call out the advantage to free page reporting in a different
> way. Basically with the page poison feature we know a few things about
> the behavior and I have called them out in the new patch description:
> 
> Page poison provides a way for the guest to notify the host that it is
> initializing or poisoning freed pages with some specific poison value. As a
> result of this we can infer a couple traits about the guest:
> 
> 1. Free pages will contain a specific pattern within the guest.
> 2. Modifying free pages from this value may cause an error in the guest.
> 3. Pages will be immediately written to by the driver when deflated.
> 
> There are currently no existing features that make use of this data. In the
> upcoming feature free page reporting we will need to make use of this to
> identify if we can evict pages from the guest without causing data
> corruption.
> 
> Add documentation for the page poison feature describing the basic
> functionality and requirements.
> 
> [...]
> 
>>> diff --git a/content.tex b/content.tex
>>> index 816b6c1b052e..89e9948b7399 100644
>>> --- a/content.tex
>>> +++ b/content.tex
>>> @@ -5026,6 +5026,9 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
>>>      page hinting. A virtqueue for providing hints as to what memory is
>>>      currently free is present. Configuration field \field{free_page_hint_cmd_id}
>>>      is valid.
>>> +\item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] The device has to be notified if
>>> +    the driver is expecting balloon pages to contain a certain value when
>>> +    returned. Configuration field poison_val is valid.
>>
>> That's not what it does in the context of this feature only, no?
>>
>> "A hint to the device, that the driver might immediately write
>> \field{poison_val} to pages after deflating them. Configuration field
>> \field{poison_val} is valid."
> 
> I'll probably just use this wording with a few slight tweaks. Thinking
> about it though I will get rid of "might" and replace it with "will".

I think that's always guaranteed by Linux as of now, so "will" makes sense.

[...]

>>> +
>>> +If the guest is not initializing or poisoning freed pages it should reject
>>
>> Sometimes you use "write to pages after deflating", here you use "freed
>> pages"
> 
> So when I am referencing "freed pages" I am talking about all free
> memory, while when I refer to "pages after deflating" I am talking
> about pages coming out of the balloon.
> 
> My thought is that there maybe be additional uses for "poison_val" be
> to feed it into some future use other than just the balloon portion of
> the deflation. Basically what this is telling us is that we could look
> for a pattern of pages containing nothing but poison_val if we wanted
> to do some sort of same page merging, or maybe define something to
> optimize migration by defining a poison page similar to a zero page
> that could be used to reduce migration overhead in the future.
> 
>>> +the VIRTIO_BALLOON_F_PAGE_POISON feature.
>>> +
>>> +If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the guest
>>> +will place the expected poison value into the \field{poison_val}
>>
>> again, "expected" is misleading in the context of this patch only.
> 
> I will rewrite this statement at follows:
>   If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
>   will place the initialization and/or poison value into the \field{poison_val}
>   configuration field data.
> 
> I think I might strengthen things a bit as well. In the driver
> normative section I think I might add the following:
>   The driver MUST initialize and/or poison the deflated pages with
>   \field{poison_val} when they are reused by the driver.
> 

Maybe simplify that whole "initialize and/or poison " handling across
this patch to "initialize with \field{poison_val}" - if the
initialization is used for poisoning or initialization doesn't matter
from spec POV.

In general looks good to me, I'll have another look at the full result.

Still wondering what to do with free page hinting ... in the meantime
I'll have a look at free page reporting :)

-- 
Thanks,

David / dhildenb



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]