On Tue, Mar 05, 2019 at 07:14:09PM -0500, Michael S. Tsirkin wrote: > On Wed, Mar 06, 2019 at 10:35:12AM +1100, David Gibson wrote: > > On Tue, Mar 05, 2019 at 09:41:34AM -0500, Michael S. Tsirkin wrote: > > > On Tue, Mar 05, 2019 at 04:03:00PM +1100, David Gibson wrote: > > > > On Mon, Mar 04, 2019 at 09:29:24PM -0500, Michael S. Tsirkin wrote: > > > > > On Tue, Mar 05, 2019 at 11:52:08AM +1100, David Gibson wrote: > > > > > > On Thu, Feb 28, 2019 at 08:36:58AM -0500, Michael S. Tsirkin wrote: > > > > > > > On Thu, Feb 14, 2019 at 03:39:12PM +1100, David Gibson wrote: > > > > > > > > When the balloon is inflated, we discard memory place in it > > > > > > > > using madvise() > > > > > > > > with MADV_DONTNEED. And when we deflate it we use > > > > > > > > MADV_WILLNEED, which > > > > > > > > sounds like it makes sense but is actually unnecessary. > > > > > > > > > > > > > > > > The misleadingly named MADV_DONTNEED just discards the memory > > > > > > > > in question, > > > > > > > > it doesn't set any persistent state on it in-kernel; all that's > > > > > > > > necessary > > > > > > > > to bring the memory back is to touch it. MADV_WILLNEED in > > > > > > > > contrast > > > > > > > > specifically says that the memory will be used soon and faults > > > > > > > > it in. > > > > > > > > > > > > > > > > This patch simplify's the balloon operation by dropping the > > > > > > > > madvise() > > > > > > > > on deflate. This might have an impact on performance - it will > > > > > > > > move a > > > > > > > > delay at deflate time until that memory is actually touched, > > > > > > > > which > > > > > > > > might be more latency sensitive. However: > > > > > > > > > > > > > > > > * Memory that's being given back to the guest by deflating the > > > > > > > > balloon *might* be used soon, but it equally could just sit > > > > > > > > around > > > > > > > > in the guest's pools until needed (or even be faulted out > > > > > > > > again if > > > > > > > > the host is under memory pressure). > > > > > > > > > > > > > > > > * Usually, the timescale over which you'll be adjusting the > > > > > > > > balloon > > > > > > > > is long enough that a few extra faults after deflation > > > > > > > > aren't > > > > > > > > going to make a difference. > > > > > > > > > > > > > > > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > > > > > > > Reviewed-by: David Hildenbrand <da...@redhat.com> > > > > > > > > Reviewed-by: Michael S. Tsirkin <m...@redhat.com> > > > > > > > > > > > > > > I'm having second thoughts about this. It might affect > > > > > > > performance but > > > > > > > probably won't but we have no idea. Might cause latency jitter > > > > > > > after > > > > > > > deflate where it previously didn't happen. This kind of patch > > > > > > > should > > > > > > > really be accompanied by benchmarking results, not philosophy. > > > > > > > > > > > > I guess I see your point, much as it's annoying to spend time > > > > > > benchmarking a device that's basically broken by design. > > > > > > > > > > Because of 4K page thing? > > > > > > > > For one thing. I believe David H has bunch of other reasons. > > > > > > > > > It's an annoying bug for sure. There were > > > > > patches to add a feature bit to just switch to plan s/g format, but > > > > > they > > > > > were abandoned. You are welcome to revive them though. > > > > > Additionally or alternatively, we can easily add a field specifying > > > > > page size. > > > > > > > > We could, but I'm pretty disinclined to work on this when virtio-mem > > > > is a better solution in nearly every way. > > > > > > Then one way would be to just let balloon be. Make it behave same as > > > always and don't make changes to it :) > > > > I'd love to, but it is in real world use, so I think we do need to fix > > serious bugs in it - at least if they can be fixed on one side, > > without needing to roll out both qemu and guest changes (which adding > > page size negotiation would require). > > > Absolutely I'm just saying don't add optimizations in that case :)
I don't plan to.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
