linux-kernel - Re: [PATCH v10 3/5] virtio_balloon: introduce migration primitives to balloon pages

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:	Tue, 18 Sep 2012 11:07:12 -0300
From:	Rafael Aquini <aquini@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	Rusty Russell <rusty@...tcorp.com.au>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Rik van Riel <riel@...hat.com>, Mel Gorman <mel@....ul.ie>,
	Andi Kleen <andi@...stfloor.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Minchan Kim <minchan@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH v10 3/5] virtio_balloon: introduce migration primitives
 to balloon pages

On Mon, Sep 17, 2012 at 03:15:52PM -0700, Andrew Morton wrote:
> > +	/* Number of balloon pages isolated from 'pages' list for compaction */
> > +	unsigned int num_isolated_pages;
> 
> Is it utterly inconceivable that this counter could exceed 4G, ever?
> 
> >  	/* Number of balloon pages we've told the Host we're not using. */
> >  	unsigned int num_pages;

I've just followed the same unit the driver writers had used to keep track of
how many pages are 'enlisted' to a given balloon device (num_pages). As
compaction can not isolate more pages than what a balloon device possess, yes,
num_isolated_pages won't get bigger than 4G pages.



> > +	mutex_lock(&vb->balloon_lock);
> >  	for (vb->num_pfns = 0; vb->num_pfns < num;
> >  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> > -		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY |
> > -					__GFP_NOMEMALLOC | __GFP_NOWARN);
> > +		struct page *page = alloc_page(vb_gfp_mask | __GFP_NORETRY |
> > +					       __GFP_NOWARN | __GFP_NOMEMALLOC);
> 
> That looks like an allocation which could easily fail.
>

That's not a big problem. If we fail that allocation and miss the desired 
balloon 'inflation' target at this round, the driver will take care of it
later, as it keeps chasing its targets.


 
> >  		if (!page) {
> >  			if (printk_ratelimit())
> >  				dev_printk(KERN_INFO, &vb->vdev->dev,
> 
> Strangely, we suppressed the core page allocator's warning and
> substituted this less useful one.
> 
> Also, it would be nice if someone could get that printk_ratelimit() out
> of there, for reasons described at the printk_ratelimit() definition
> site.
> 

Despite I agree 100% with you here, (IMHO) that was a change out of the scope 
for this patchseries original purposes and so I didn't propose it.

OTOH, I don't mind in introducing the aforementioned surgery by this patch, 
if the balloon driver folks are OK with it.


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