On Tue, May 12, 2015 at 11:43:39AM +0300, Dan Carpenter wrote:
> On Mon, May 11, 2015 at 11:54:44PM +0000, Naoya Horiguchi wrote:
> > @@ -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;
>
> Why not use UINT_MAX? It's more clear that it's not valid that way.
It's OK if code checker doesn't show "too much right shift" warning.
With UINT_MAX, inserting VM_BUG_ON(order == UINT_MAX) after for_each_hstate
loop might be safer (1 << UINT_MAX is clearly wrong.)
> Otherwise doing a complicated calculation it makes it seem like we will
> use the variable.
OK.
Is the below OK for you?
---
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 271e4432734c..804437505a84 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1188,7 +1188,7 @@ 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 *);
+ unsigned int order = UINT_MAX;
unsigned long pfn;
struct hstate *h;
@@ -1200,6 +1200,7 @@ void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
if (order > huge_page_order(h))
order = huge_page_order(h);
VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
+ VM_BUG_ON(order == UINT_MAX);
for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
dissolve_free_huge_page(pfn_to_page(pfn));
}
--
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