Re: [Xen-devel] freemem-slack and large memory environments

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 2 Mar 2015, Mike Latimer wrote:
> On Monday, March 02, 2015 04:15:41 PM Stefano Stabellini wrote:
> > On Mon, 2 Mar 2015, Ian Campbell wrote:
> > > ? "Continue as long as progress is being made" is exactly what
> > > 2563bca1154 "libxl: Wait for ballooning if free memory is increasing"
> > > was trying to implement, so it certainly was the idea behind the current
> > > implementation (it may well not have managed to implement the idea it
> > > was trying to).
> > 
> > I don't think so: pre-2563bca1154 it wasn't and 2563bca1154 doesn't
> > implement it properly.  I think we should revert it.
> 
> I agree. The intent was there, but we are not aware of any known problem 
> cases 
> that the changes will effect. Reverting seems like the right thing to do.
> 
> > I agree with the suggestion "continue as long as progress is being
> > made". I disagree that "continue as long as progress is being made" is
> > the current logic of freemem.
> > 
> > I don't have an opinion on whether that logic should be implemented in
> > freemem or libxl_wait_for_memory_target. The code below implements it in
> > libxl_wait_for_memory_target.
> 
> Seems like it should wait in libxl_wait_for_memory_target, as you have 
> implemented.
> 
> > > > The following changes to freemem implements the aforementioned logic by
> > > > improving libxl_wait_for_memory_target to keep going as long as dom0 is
> > > > able to make progress and removing the useless free_memkb >
> > > > free_memkb_prev check.
> 
> I just finished testing your changes and they work correctly in my 
> environment. 
> An interesting thing to note is that within the 1 second delay in the loop, 
> my 
> test machine was ballooning down about 512M. This resulted in the loop being 
> executed 127 times in order to balloon down the required 64G. (I'm not sure 
> if 
> there is any reason to worry about that though.)
>
> One small question on freemem...  The loop essentially looks like:
> 
>   libxl_get_free_memory:  Get free memory
>   If free_memkb >= need_memkb, return
>   libxl_set_memory_target:  Balloon dom0 to free memory for domU
>   libxl_wait_for_free_memory:  Wait for free memory for domU (max 10 seconds)
>   libxl_wait_for_memory_target:  Wait for dom0 to finish ballooning
>   Decrement retry and try again
> 
> Shouldn't libxl_wait_for_memory_target be before libxl_wait_for_free_memory?

The code works exactly as you described. The idea behind it is that
libxl_wait_for_free_memory should be enough, but if it isn't we should
figure out if we should try again of it was was because of an error by
checking if dom0 managed to balloon down (libxl_wait_for_memory_target).

With my changes to libxl_wait_for_memory_target, it doesn't make too
much sense to have both libxl_wait_for_free_memory and
libxl_wait_for_memory_target. We should probably remove the call to
libxl_wait_for_free_memory entirely.

Also it could make sense to increase the granularity of the wait time to
at least 2 sec (1G of memory being freed in your stats) to avoid waiting
cycles uselessly. I'll submit a series for it.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel