Messages in this thread | | | Date | Thu, 23 Aug 2012 12:03:15 -0400 | From | Rik van Riel <> | Subject | Re: [PATCH v8 1/5] mm: introduce a common interface for balloon pages mobility |
| |
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.
| |