On Mon, May 11, 2015 at 02:17:48PM +0300, Dan Carpenter wrote:
> Hello Naoya Horiguchi,
>
> The patch c8721bbbdd36: "mm: memory-hotplug: enable memory hotplug to
> handle hugepage" from Sep 11, 2013, leads to the following static
> checker warning:
>
> mm/hugetlb.c:1203 dissolve_free_huge_pages()
> warn: potential right shift more than type allows '9,18,64'
>
> mm/hugetlb.c
> 1189 void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
> 1190 {
> 1191 unsigned int order = 8 * sizeof(void *);
> ^^^^^^^^^^^^^^^^^^
> Let's say order is 64.
Hi Dan, thank you for reporting.
order is supposed to be capped by running each hstates and finding the
minimum hugepage order as done in below code, and I intended that this
initialization gives potential maximum. I guess that keeping this to 64
doesn't solve the above warning, so we use 8 * sizeof(void *) - 1 or 63 ?
I don't test on 32-bit system, so not sure that this code can be used
by 32-bit system, but considering such case, keeping sizeof(void *)
seems better.
>
> 1192 unsigned long pfn;
> 1193 struct hstate *h;
> 1194
> 1195 if (!hugepages_supported())
> 1196 return;
> 1197
> 1198 /* Set scan step to minimum hugepage size */
> 1199 for_each_hstate(h)
> 1200 if (order > huge_page_order(h))
> 1201 order = huge_page_order(h);
> 1202 VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
> 1203 for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
> ^^^^^^^^^^
> 1 << 64 is undefined but let's say it's zero because that's normal for
> GCC. This is an endless loop.
That never happens if hstates is properly initialized, but we had better
avoid the potential risk.
How about the following patch?
Thanks,
Naoya Horiguchi
---
From: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
Date: Tue, 12 May 2015 08:17:10 +0900
Subject: [PATCH] mm/hugetlb: decrement initial value of order in
dissolve_free_huge_pages
Currently the initial value of order in dissolve_free_huge_page is 64 or 32,
which leads to the following warning in static checker:
mm/hugetlb.c:1203 dissolve_free_huge_pages()
warn: potential right shift more than type allows '9,18,64'
This is a potential risk of infinite loop, because 1 << order (== 0) is used
in for-loop like this:
for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
...
So this patch simply avoids the risk by decrementing the initial value.
Fixes: c8721bbbdd36 ("mm: memory-hotplug: enable memory hotplug to handle hugepage")
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
---
mm/hugetlb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c41b2a0ee273..74abfb44e4d0 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1086,7 +1086,8 @@ static void dissolve_free_huge_page(struct page *page)
*/
void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
{
- unsigned int order = 8 * sizeof(void *);
+ /* Initialized to "high enough" value which is capped later */
+ unsigned int order = 8 * sizeof(void *) - 1;
unsigned long pfn;
struct hstate *h;
--
2.1.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href