Re: [PATCH v8 1/5] mm: introduce a common interface for balloon pagesmobility

From: Rik van Riel
Date: Thu Aug 23 2012 - 12:03:05 EST


On 08/23/2012 11:54 AM, Michael S. Tsirkin wrote:
On Thu, Aug 23, 2012 at 12:21:29PM -0300, Rafael Aquini wrote:
On Thu, Aug 23, 2012 at 04:53:29PM +0300, Michael S. Tsirkin wrote:
On Thu, Aug 23, 2012 at 10:06:07AM -0300, Rafael Aquini wrote:
On Thu, Aug 23, 2012 at 03:34:32PM +0300, Michael S. Tsirkin wrote:
So, nothing has changed here.

Yes, your patch does change things:
leak_balloon now might return without freeing any pages.
In that case we will not be making any progress, and just
spin, pinning CPU.

That's a transitory condition, that migh happen if leak_balloon() takes place
when compaction, or migration are under their way and it might only affects the
module unload case.

Regular operation seems even more broken: host might ask
you to leak memory but because it is under compaction
you might leak nothing. No?


And that is exactely what it wants to do. If there is (temporarily) nothing to leak,
then not leaking is the only sane thing to do.

It's an internal issue between balloon and mm. User does not care.

Having balloon pages being migrated
does not break the leak at all, despite it can last a little longer.


Not "longer" - apparently forever unless user resend the leak command.
It's wrong - it should
1. not tell host if nothing was done
2. after migration finished leak and tell host

Agreed. If the balloon is told to leak N pages, and could
not do so because those pages were locked, the balloon driver
needs to retry (maybe waiting on a page lock?) and not signal
completion until after the job has been completed.

Having the balloon driver wait on the page lock should be
fine, because compaction does not hold the page lock for
long.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/