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

From: Tianyu Lan <redacted>

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

lantianyu1986@gmail.com writes:

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

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 [off-list ref]; Haiyang Zhang [off-list ref];
Stephen Hemminger [off-list ref]; sashal@kernel.org;
akpm@linux-foundation.org; Michael Kelley [off-list ref];
Dexuan Cui [off-list ref]
Subject: [EXTERNAL] Re: [PATCH] x86/Hyper-V: Balloon up according to request
page number

lantianyu1986@gmail.com writes:

quoted

From: Tianyu Lan <redacted>

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.

quoted

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 <redacted>
---
 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,

quoted

 	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