linux-kernel - Re: [PATCH v8 1/5] mm: introduce a common interface for balloon pages mobility

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 23 Aug 2012 09:13:39 -0300
From:	Rafael Aquini <aquini@...hat.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Peter Zijlstra <peterz@...radead.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	Rusty Russell <rusty@...tcorp.com.au>,
	Rik van Riel <riel@...hat.com>, Mel Gorman <mel@....ul.ie>,
	Andi Kleen <andi@...stfloor.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Minchan Kim <minchan@...nel.org>
Subject: Re: [PATCH v8 1/5] mm: introduce a common interface for balloon
 pages mobility

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().
So, nothing has changed here.

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/