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