[PATCH] x86/Hyper-V: Balloon up according to request page number

[PATCH] x86/Hyper-V: Balloon up according to request page number

[lantianyu1986]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Current code has assumption that balloon request memory size aligns
with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
balloon driver receives non-aligned balloon request, it produces warning
and balloon up more memory than requested in order to keep 2MB alignment.
Remove the warning and balloon up memory according to actual requested
memory size.

Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory block")
Cc: stable@vger.kernel.org
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 drivers/hv/hv_balloon.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 7f3e7ab22d5d..38ad0e44e927 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
 	if (num_pages < alloc_unit)
 		return 0;
 
-	for (i = 0; (i * alloc_unit) < num_pages; i++) {
+	for (i = 0; i < num_pages / alloc_unit; i++) {
 		if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
 			HV_HYP_PAGE_SIZE)
 			return i * alloc_unit;
@@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
 
 	}
 
-	return num_pages;
+	return i * alloc_unit;
 }
 
 static void balloon_up(union dm_msg_info *msg_info)
@@ -1737,9 +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
 	long avail_pages;
 	unsigned long floor;
 
-	/* The host balloons pages in 2M granularity. */
-	WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
-
 	/*
 	 * We will attempt 2M allocations. However, if we fail to
 	 * allocate 2M chunks, we will go back to PAGE_SIZE allocations.
-- 
2.14.5

Re: [PATCH] x86/Hyper-V: Balloon up according to request page number

[Vitaly Kuznetsov]

lantianyu1986@gmail.com writes:

> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> Current code has assumption that balloon request memory size aligns
> with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
> balloon driver receives non-aligned balloon request, it produces warning
> and balloon up more memory than requested in order to keep 2MB alignment.
> Remove the warning and balloon up memory according to actual requested
> memory size.
>
> Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory block")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
>  drivers/hv/hv_balloon.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 7f3e7ab22d5d..38ad0e44e927 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
>  	if (num_pages < alloc_unit)
>  		return 0;
>  
> -	for (i = 0; (i * alloc_unit) < num_pages; i++) {
> +	for (i = 0; i < num_pages / alloc_unit; i++) {
>  		if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
>  			HV_HYP_PAGE_SIZE)
>  			return i * alloc_unit;
> @@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
>  
>  	}
>  
> -	return num_pages;
> +	return i * alloc_unit;
>  }
>  
>  static void balloon_up(union dm_msg_info *msg_info)
> @@ -1737,9 +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
>  	long avail_pages;
>  	unsigned long floor;
>  
> -	/* The host balloons pages in 2M granularity. */
> -	WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
> -
>  	/*
>  	 * We will attempt 2M allocations. However, if we fail to
>  	 * allocate 2M chunks, we will go back to PAGE_SIZE allocations.

This looks correct but I've noticed we also have 

	/* Refuse to balloon below the floor, keep the 2M granularity. */
	if (avail_pages < num_pages || avail_pages - num_pages < floor) {
		pr_warn("Balloon request will be partially fulfilled. %s\n",
			avail_pages < num_pages ? "Not enough memory." :
			"Balloon floor reached.");

		num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
		num_pages -= num_pages % PAGES_IN_2M;
	}

in balloon_up(). If 2M granularity is not guaranteed in the first place
we can't keep it.

Also, when alloc_balloon_pages() is called with 2M alloc_unit and the
region is not 2M aligned, it will return someething < num_pages, the
next condition, however, only checks for 0:

                if (alloc_unit != 1 && num_ballooned == 0) {
                        alloc_unit = 1;
                        continue;
                }

we will proceed to sending a response to server and try doing next
iteration by calling alloc_balloon_pages() with 2M alloc_unit again,
this will finally return 0 and we will switch to 4k. I think we can
optimize this to:

                if (alloc_unit != 1 && num_ballooned != num_pages) {
			alloc_unit = 1;
		        continue;
		}

-- 
Vitaly

RE: [EXTERNAL] Re: [PATCH] x86/Hyper-V: Balloon up according to request page number

[Tianyu Lan]

Hi Vitaly:
	Thanks for your review. I fix comments in V2. 

> From: linux-hyperv-owner@vger.kernel.org <linux-hyperv-
> owner@vger.kernel.org> On Behalf Of Vitaly Kuznetsov
> Sent: Tuesday, January 14, 2020 5:31 PM
> To: lantianyu1986@gmail.com
> Cc: linux-kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-
> mm@kvack.org; stable@vger.kernel.org; tglx@linutronix.de;
> mingo@redhat.com; bp@alien8.de; hpa@zytor.com; x86@kernel.org;
> dave.hansen@linux.intel.com; luto@kernel.org; peterz@infradead.org; KY
> Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> Stephen Hemminger <sthemmin@microsoft.com>; sashal@kernel.org;
> akpm@linux-foundation.org; Michael Kelley <mikelley@microsoft.com>;
> Dexuan Cui <decui@microsoft.com>
> Subject: [EXTERNAL] Re: [PATCH] x86/Hyper-V: Balloon up according to request
> page number
> 
> lantianyu1986@gmail.com writes:
> 
> > From: Tianyu Lan <Tianyu.Lan@microsoft.com>
> >
> > Current code has assumption that balloon request memory size aligns
> > with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
> > balloon driver receives non-aligned balloon request, it produces
> > warning and balloon up more memory than requested in order to keep 2MB
> alignment.
> > Remove the warning and balloon up memory according to actual requested
> > memory size.
> >
> > Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error
> > of 2MB memory block")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > ---
> >  drivers/hv/hv_balloon.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index
> > 7f3e7ab22d5d..38ad0e44e927 100644
> > --- a/drivers/hv/hv_balloon.c
> > +++ b/drivers/hv/hv_balloon.c
> > @@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
> >  	if (num_pages < alloc_unit)
> >  		return 0;
> >
> > -	for (i = 0; (i * alloc_unit) < num_pages; i++) {
> > +	for (i = 0; i < num_pages / alloc_unit; i++) {
> >  		if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
> >  			HV_HYP_PAGE_SIZE)
> >  			return i * alloc_unit;
> > @@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct
> > hv_dynmem_device *dm,
> >
> >  	}
> >
> > -	return num_pages;
> > +	return i * alloc_unit;
> >  }
> >
> >  static void balloon_up(union dm_msg_info *msg_info) @@ -1737,9
> > +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
> >  	long avail_pages;
> >  	unsigned long floor;
> >
> > -	/* The host balloons pages in 2M granularity. */
> > -	WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
> > -
> >  	/*
> >  	 * We will attempt 2M allocations. However, if we fail to
> >  	 * allocate 2M chunks, we will go back to PAGE_SIZE allocations.
> 
> This looks correct but I've noticed we also have
> 
> 	/* Refuse to balloon below the floor, keep the 2M granularity. */
> 	if (avail_pages < num_pages || avail_pages - num_pages < floor) {
> 		pr_warn("Balloon request will be partially fulfilled. %s\n",
> 			avail_pages < num_pages ? "Not enough memory." :
> 			"Balloon floor reached.");
> 
> 		num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
> 		num_pages -= num_pages % PAGES_IN_2M;
> 	}
> 
> in balloon_up(). If 2M granularity is not guaranteed in the first place we can't
> keep it.
> 
> Also, when alloc_balloon_pages() is called with 2M alloc_unit and the region is
> not 2M aligned, it will return someething < num_pages, the next condition,
> however, only checks for 0:
> 
>                 if (alloc_unit != 1 && num_ballooned == 0) {
>                         alloc_unit = 1;
>                         continue;
>                 }
> 
> we will proceed to sending a response to server and try doing next iteration by
> calling alloc_balloon_pages() with 2M alloc_unit again, this will finally return 0
> and we will switch to 4k. I think we can optimize this to:
> 
>                 if (alloc_unit != 1 && num_ballooned != num_pages) {
> 			alloc_unit = 1;
> 		        continue;
> 		}
> 
> --
> Vitaly