On Thu, Aug 23, 2012 at 09:13:39AM -0300, Rafael Aquini wrote: > On Thu, Aug 23, 2012 at 01:01:07PM +0300, Michael S. Tsirkin wrote: > > > So, when remove_common() calls leak_balloon() looping on > > > vb->num_pages, that won't become a tight loop. > > > The scheme was apparently working before this series, and it will remain > > > working > > > after it. > > > > It seems that before we would always leak all requested memory > > in one go. I can't tell why we have a while loop there at all. > > Rusty, could you clarify please? > > > > It seems that your claim isn't right. leak_balloon() cannot do it all at once, > as for each round it only releases 256 pages, at most; and the 'one go' would > require a couple of loop rounds at remove_common().
You are right in this respect. > 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. > > > > Just as before, same thing here. If you leaked less than required, > > > balloon() > > > will keep calling leak_balloon() until the balloon target is reached. This > > > scheme was working before, and it will keep working after this patch. > > > > > > > IIUC we never hit this path before. > > > So, how does balloon() works then? > It gets a request to leak a given number of pages and executes it, then tells host that it is done. It never needs to spin busy-waiting on a CPU for this. > > > > How about we signal config_change > > > > event when pages are back to pages_list? > > > > > > I really don't know what to tell you here, but, to me, it seems like an > > > overcomplication that isn't directly entangled with this patch purposes. > > > Besides, you cannot expect compation / migration happening and racing > > > against > > > leak_balloon() all the time to make them signal events to the later, so > > > we might > > > just be creating a wait-forever condition for leak_balloon(), IMHO. > > > > So use wait_event or similar, check for existance of isolated pages. > > > > The thing here is expecting compaction as being an external event to signal > actions to the balloon driver won't work as you desire. Also, as far as the > balloon driver is concerned, it's only a matter of time to accomplish a total, > or partial, balloon leak, even when we have some pages isolated from balloon's > page list. > > IMHO, you're attempting to complicate a simple thing that is already working > well. As said before, there are no guarantees you'll have isolated pages > by the time you're leaking the balloon, so you might leave it waiting forever > on something that will not happen. And if there are isolated pages while > balloon > is leaking, they'll have their chance to get back to the list before the > device > finishes its leaking job. Well busy wait pinning CPU is ugly. Instead we should block thread and wake it up when done. I don't mind how we fix it specifically. -- MST _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
