Re: [PATCH] virtio-balloon: Change balloon page size to 1MB

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/20/2012 11:34 AM, Prasad Joshi wrote:
> Signed-off-by: Prasad Joshi<prasadjoshi.linux@gmail.com>

This breaks compatibility with existing guests and more importantly, doesn't 
make any sense.  There's no commit message explaining why 1MB is a better number 
than 4K, 2MB which I could at least understand as an alternative to 4K.

Regards,

Anthony Liguori

> ---
>   drivers/virtio/virtio_balloon.c |   18 +++++++++---------
>   include/linux/virtio_balloon.h  |    4 +++-
>   2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 95aeedf..e352448 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -64,9 +64,8 @@ static u32 page_to_balloon_pfn(struct page *page)
>   {
>   	unsigned long pfn = page_to_pfn(page);
>
> -	BUILD_BUG_ON(PAGE_SHIFT<  VIRTIO_BALLOON_PFN_SHIFT);
>   	/* Convert pfn from Linux page size to balloon page size. */
> -	return pfn>>  (PAGE_SHIFT - VIRTIO_BALLOON_PFN_SHIFT);
> +	return pfn>>  VIRTIO_BALLOON_PAGE_ORDER;
>   }
>
>   static void balloon_ack(struct virtqueue *vq)
> @@ -102,8 +101,9 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
>   	num = min(num, ARRAY_SIZE(vb->pfns));
>
>   	for (vb->num_pfns = 0; vb->num_pfns<  num; vb->num_pfns++) {
> -		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY |
> -					__GFP_NOMEMALLOC | __GFP_NOWARN);
> +		struct page *page = alloc_pages(GFP_HIGHUSER | __GFP_NORETRY |
> +				__GFP_NOMEMALLOC | __GFP_NOWARN,
> +				VIRTIO_BALLOON_PAGE_ORDER);
>   		if (!page) {
>   			if (printk_ratelimit())
>   				dev_printk(KERN_INFO,&vb->vdev->dev,
> @@ -114,7 +114,7 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
>   			break;
>   		}
>   		vb->pfns[vb->num_pfns] = page_to_balloon_pfn(page);
> -		totalram_pages--;
> +		totalram_pages -= VIRTIO_BALLOON_PAGES;
>   		vb->num_pages++;
>   		list_add(&page->lru,&vb->pages);
>   	}
> @@ -131,8 +131,8 @@ static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
>   	unsigned int i;
>
>   	for (i = 0; i<  num; i++) {
> -		__free_page(pfn_to_page(pfns[i]));
> -		totalram_pages++;
> +		__free_pages(pfn_to_page(pfns[i]), VIRTIO_BALLOON_PAGE_ORDER);
> +		totalram_pages += VIRTIO_BALLOON_PAGES;
>   	}
>   }
>
> @@ -146,7 +146,7 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
>   	for (vb->num_pfns = 0; vb->num_pfns<  num; vb->num_pfns++) {
>   		page = list_first_entry(&vb->pages, struct page, lru);
>   		list_del(&page->lru);
> -		vb->pfns[vb->num_pfns] = page_to_balloon_pfn(page);
> +		vb->pfns[vb->num_pfns] = page_to_pfn(page);
>   		vb->num_pages--;
>   	}
>
> @@ -400,7 +400,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
>   	/* We don't have these pages in the balloon anymore! */
>   	list_for_each_entry_safe(page, page2,&vb->pages, lru) {
>   		list_del(&page->lru);
> -		totalram_pages++;
> +		totalram_pages += VIRTIO_BALLOON_PAGES;
>   	}
>   	return init_vqs(vdev->priv);
>   }
> diff --git a/include/linux/virtio_balloon.h b/include/linux/virtio_balloon.h
> index 652dc8b..86840fe 100644
> --- a/include/linux/virtio_balloon.h
> +++ b/include/linux/virtio_balloon.h
> @@ -33,7 +33,9 @@
>   #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
>
>   /* Size of a PFN in the balloon interface. */
> -#define VIRTIO_BALLOON_PFN_SHIFT 12
> +#define VIRTIO_BALLOON_PFN_SHIFT  20
> +#define VIRTIO_BALLOON_PAGE_ORDER (VIRTIO_BALLOON_PFN_SHIFT - PAGE_SHIFT)
> +#define VIRTIO_BALLOON_PAGES      (1U<<  VIRTIO_BALLOON_PAGE_ORDER)
>
>   struct virtio_balloon_config
>   {