Re: [PATCH v10 3/5] virtio_balloon: introduce migration primitivesto balloon pages

[Rafael Aquini]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/