Re: [PATCH v16.1 6/9] virtio-balloon: Add support for providing free page reports to host

From: David Hildenbrand <david@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Alexander Duyck <alexander.duyck@gmail.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	willy@infradead.org, mhocko@kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org, mgorman@techsingularity.net,
	vbabka@suse.cz, yang.zhang.wz@gmail.com, nitesh@redhat.com,
	konrad.wilk@oracle.com, pagupta@redhat.com, riel@surriel.com,
	lcapitulino@redhat.com, dave.hansen@intel.com,
	wei.w.wang@intel.com, aarcange@redhat.com, pbonzini@redhat.com,
	dan.j.williams@intel.com, alexander.h.duyck@linux.intel.com,
	osalvador@suse.de
Subject: Re: [PATCH v16.1 6/9] virtio-balloon: Add support for providing free page reports to host
Date: Tue, 11 Feb 2020 15:31:18 +0100	[thread overview]
Message-ID: <d6b481fb-6c72-455d-f8e4-600a8677c7a8@redhat.com> (raw)
In-Reply-To: <20200211090052-mutt-send-email-mst@kernel.org>

On 11.02.20 15:07, Michael S. Tsirkin wrote:
> On Tue, Feb 11, 2020 at 01:19:31PM +0100, David Hildenbrand wrote:
>>>>
>>>> Did you see the discussion regarding unifying handling of
>>>> inflate/deflate/free_page_hinting_free_page_reporting, requested by
>>>> Michael? I think free page reporting is special and shall be left alone.
>>>
>>> Not sure what do you mean by "left alone here". Could you clarify?
>>
>> Don't try to unify handling like I proposed below, because it's
>> semantics are special.
>>
>>>
>>>> VIRTIO_BALLOON_F_REPORTING is nothing but a more advanced inflate, right
>>>> (sg, inflate based on size - not "virtio pages")?
>>>
>>>
>>> Not exactly - it's also initiated by guest as opposed to host, and
>>> not guided by the ballon size request set by the host.
>>
>> True, but AFAIKS you could use existing INFLATE/DEFLATE in a similar
>> way. There is no way for the hypervisor to nack a request. The balloon
>> size is not glued to inflate/deflate requests. The guests manually
>> updates it.
> 
> Hmm how isn't it? num_pages is the only way to inflate/deflate.

Usually, guests are nice and respond to num_pages changes in an
appropriate way, except:
- Triggering deflate: Unload the driver. Suspend/hibernate. OOM.
  (+ Reboot, although that's special)
- Triggering inflate + deflate: Simple balloon compaction / page
  migration.

But that's not what I meant.

"actual" is updated by the guest, not by the host. So the "actual
balloon size" is set by the guest. It's not glued to inflation/deflation
requests. "num_pages" is the host request.

AFAIKs, the guest could inflate/deflate (esp. temporarily) and
communicate via "actual" the actual balloon size as he sees it.

> Spec also says:
> The device is driven either by the receipt of a configuration change notification, or by changing guest memory
> needs, such as performing memory compaction or responding to out of memory conditions.
> 
> so ignoring compaction/oom (later is under-specified, not a good example
> to follow) yes inflate/deflate are tied to host specified configuration
Yes, "num_pages" is the host request. But I'd say the statement (esp.
"the device is driven by") in the spec is rather weak. It does not
explicitly state when inflation/deflation is allowed IMHO.

-- 
Thanks,

David / dhildenb