Re: [PATCH] xen-blkback: use balloon pages for persistent grants

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Feb 14, 2013 at 11:12:09AM +0100, Roger Pau Monne wrote:
> With current persistent grants implementation we are not freeing the
> persistent grants after we disconnect the device. Since grant map

Can you explain this in more details please? Isn't gnttab_set_unmap_op
in free_persistent_gnts doing the right job of putting in the right
mfn in? And then we could free the page?


> operations change the mfn of the allocated page, and we can no longer
> pass it to __free_page without setting the mfn to a sane value, use
> balloon grant pages instead, as the gntdev device does.


Wow. I did not realize that we leaving such a huge memory leak behind!
But I guess that was never an issue as we would recycle those persistent
grants to other domains.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: xen-devel@lists.xen.org
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  drivers/block/xen-blkback/blkback.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index c46824f..e6c2f6a 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -46,6 +46,7 @@
>  #include <xen/xen.h>
>  #include <asm/xen/hypervisor.h>
>  #include <asm/xen/hypercall.h>
> +#include <xen/balloon.h>
>  #include "common.h"
>  
>  /*
> @@ -239,6 +240,7 @@ static void free_persistent_gnts(struct rb_root *root, unsigned int num)
>  			ret = gnttab_unmap_refs(unmap, NULL, pages,
>  				segs_to_unmap);
>  			BUG_ON(ret);
> +			free_xenballooned_pages(segs_to_unmap, pages);
>  			segs_to_unmap = 0;
>  		}
>  
> @@ -527,8 +529,8 @@ static int xen_blkbk_map(struct blkif_request *req,
>  				GFP_KERNEL);
>  			if (!persistent_gnt)
>  				return -ENOMEM;
> -			persistent_gnt->page = alloc_page(GFP_KERNEL);
> -			if (!persistent_gnt->page) {
> +			if (alloc_xenballooned_pages(1, &persistent_gnt->page,
> +			    false)) {
>  				kfree(persistent_gnt);
>  				return -ENOMEM;
>  			}
> -- 
> 1.7.7.5 (Apple Git-26)
>