Re: [Qemu-devel] [RFC 1/5] virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4437 bytes --]

On Mon, Oct 15, 2018 at 06:43:06AM -0400, Michael S. Tsirkin wrote:
> On Mon, Oct 15, 2018 at 08:54:27AM +0200, David Hildenbrand wrote:
> > On 12/10/2018 20:05, Michael S. Tsirkin wrote:
> > > On Fri, Oct 12, 2018 at 02:24:27PM +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.
> > >>
> > >> 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 it actually needs it.  And, over the general timescale that
> > >> memory ballooning operates, it seems unlikely that one extra fault for the
> > >> guest will be a vast performance issue.
> > > 
> > > Thinking about it, it might be for RT guests.
> > > 
> > > So I suspect if you want to drop MADV_WILLNEED you need a flag
> > > telling qemu that's not the usecase.
> > > 
> > 
> > As far as I know RT guests
> > 
> > 1. mlock all memory
> > 2. use huge pages
> > 
> > "MADV_DONTNEED cannot be applied to locked pages, Huge TLB pages, or
> > VM_PFNMAP pages."
> > 
> > As DONTNEED never succeeded, WILLNEED will do nothing. (e.g. postcopy
> > temporarily has to unlock all memory in order to make DONTNEED work)
> 
> 
> Hmm you are right.
> Document this in commit log?
> 
> 
> > (And "Expect access in the near future." does not sound like guarantees
> > to me either way)
> 
> Should we be concerned about impact on performance though?
> 
> Thinking aloud it might have an impact.  But there is also a benefit.
> Let's document known effects so people know what to look for
> if they see issues?

Ok, my new text in the commit message is:

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.                           

Does that seem like it covers what it should?

> 	WILLNEED aggressively faults all memory in, removing it
> 	will cause just as much memory as needed to be allocated on access,
> 	slowing down the CPU at that point but conserving host memory.
> 
> 	With this patch device assignment (e.g. with VFIO) hotplug will take longer
> 	as it needs to allocate all this memory on hotplug.
> 	While this happens all VM is paused right now.

I'm not sure what you mean by this.  This patch is very specific to
the virtio-balloon deflate path.  "True" memory hotplug (e.g. via ACPI
or PAPR dynamic reconfiguration) is unaffected.  And if VFIO is in
play, all guest memory will generally be locked, just like with
realtime.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]