[RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages()

[RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages()

[Li Zhong]

It is possible for some platforms, such as powerpc to set HPAGE_SHIFT to
0 to indicate huge pages not supported. 

When this is the case, hugetlbfs could be disabled during boot time:
hugetlbfs: disabling because there are no supported hugepage sizes

Then in dissolve_free_huge_pages(), order is kept maximum (64 for
64bits), and the for loop below won't end:
for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)

The fix below returns directly if the order isn't set to a correct
value.

Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
 mm/hugetlb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2024bbd..a950817 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1093,6 +1093,10 @@ void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
 	for_each_hstate(h)
 		if (order > huge_page_order(h))
 			order = huge_page_order(h);
+
+	if (order == 8 * sizeof(void *))
+		return;
+
 	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
 	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@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages()

[Naoya Horiguchi]

Hi Zhong,

On Thu, Jul 24, 2014 at 05:36:25PM +0800, Li Zhong wrote:
> It is possible for some platforms, such as powerpc to set HPAGE_SHIFT to
> 0 to indicate huge pages not supported. 
> 
> When this is the case, hugetlbfs could be disabled during boot time:
> hugetlbfs: disabling because there are no supported hugepage sizes
> 
> Then in dissolve_free_huge_pages(), order is kept maximum (64 for
> 64bits), and the for loop below won't end:
> for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)

At first I wonder that why could dissolve_free_huge_pages() is called
if the platform doesn't support hugetlbfs. But I found that the function
is called by memory hotplug code without checking hugepage support.

So it looks to me straightforward and self-descriptive to check
hugepage_supported() just before calling dissolve_free_huge_pages().

Thanks,
Naoya Horiguchi

> The fix below returns directly if the order isn't set to a correct
> value.
> 
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> ---
>  mm/hugetlb.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 2024bbd..a950817 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1093,6 +1093,10 @@ void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
>  	for_each_hstate(h)
>  		if (order > huge_page_order(h))
>  			order = huge_page_order(h);
> +
> +	if (order == 8 * sizeof(void *))
> +		return;
> +
>  	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
>  	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@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages()

[Li Zhong]

On Thu, 2014-07-24 at 08:45 -0400, Naoya Horiguchi wrote:
> Hi Zhong,
> 
> On Thu, Jul 24, 2014 at 05:36:25PM +0800, Li Zhong wrote:
> > It is possible for some platforms, such as powerpc to set HPAGE_SHIFT to
> > 0 to indicate huge pages not supported. 
> > 
> > When this is the case, hugetlbfs could be disabled during boot time:
> > hugetlbfs: disabling because there are no supported hugepage sizes
> > 
> > Then in dissolve_free_huge_pages(), order is kept maximum (64 for
> > 64bits), and the for loop below won't end:
> > for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
> 
> At first I wonder that why could dissolve_free_huge_pages() is called
> if the platform doesn't support hugetlbfs. But I found that the function
> is called by memory hotplug code without checking hugepage support.
> 
> So it looks to me straightforward and self-descriptive to check
> hugepage_supported() just before calling dissolve_free_huge_pages().

Hi, Naoya,

Thank you for the review and suggestion.

I'll send a updated version. 

Thanks, Zhong

> 
> Thanks,
> Naoya Horiguchi
> 
> > The fix below returns directly if the order isn't set to a correct
> > value.
> > 
> > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> > ---
> >  mm/hugetlb.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 2024bbd..a950817 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1093,6 +1093,10 @@ void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
> >  	for_each_hstate(h)
> >  		if (order > huge_page_order(h))
> >  			order = huge_page_order(h);
> > +
> > +	if (order == 8 * sizeof(void *))
> > +		return;
> > +
> >  	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
> >  	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@kvack.org.  For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> > 
> 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH v2]mm: fix potential infinite loop in dissolve_free_huge_pages()

[Li Zhong]

It is possible for some platforms, such as powerpc to set HPAGE_SHIFT to
0 to indicate huge pages not supported. 

When this is the case, hugetlbfs could be disabled during boot time:
hugetlbfs: disabling because there are no supported hugepage sizes

Then in dissolve_free_huge_pages(), order is kept maximum (64 for
64bits), and the for loop below won't end:
for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)

As suggested by Naoya, below fix checks hugepages_supported() before
calling dissolve_free_huge_pages().

Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
 mm/memory_hotplug.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 469bbf5..f642701 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1695,7 +1695,8 @@ repeat:
 	 * dissolve free hugepages in the memory block before doing offlining
 	 * actually in order to make hugetlbfs's object counting consistent.
 	 */
-	dissolve_free_huge_pages(start_pfn, end_pfn);
+	if (hugepages_supported())
+		dissolve_free_huge_pages(start_pfn, end_pfn);
 	/* check again */
 	offlined_pages = check_pages_isolated(start_pfn, end_pfn);
 	if (offlined_pages < 0) {


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2]mm: fix potential infinite loop in dissolve_free_huge_pages()

[Naoya Horiguchi]

On Mon, Jul 28, 2014 at 10:20:43AM +0800, Li Zhong wrote:
> It is possible for some platforms, such as powerpc to set HPAGE_SHIFT to
> 0 to indicate huge pages not supported. 
> 
> When this is the case, hugetlbfs could be disabled during boot time:
> hugetlbfs: disabling because there are no supported hugepage sizes
> 
> Then in dissolve_free_huge_pages(), order is kept maximum (64 for
> 64bits), and the for loop below won't end:
> for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
> 
> As suggested by Naoya, below fix checks hugepages_supported() before
> calling dissolve_free_huge_pages().
> 
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>

Thanks!

Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

And I think that this patch can go into stable (3.12+) trees.

> ---
>  mm/memory_hotplug.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 469bbf5..f642701 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1695,7 +1695,8 @@ repeat:
>  	 * dissolve free hugepages in the memory block before doing offlining
>  	 * actually in order to make hugetlbfs's object counting consistent.
>  	 */
> -	dissolve_free_huge_pages(start_pfn, end_pfn);
> +	if (hugepages_supported())
> +		dissolve_free_huge_pages(start_pfn, end_pfn);
>  	/* check again */
>  	offlined_pages = check_pages_isolated(start_pfn, end_pfn);
>  	if (offlined_pages < 0) {
> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch] mm: fix potential infinite loop in dissolve_free_huge_pages() fix

[David Rientjes]

No legitimate reason to call dissolve_free_huge_pages() when 
!hugepages_supported().

Signed-off-by: David Rientjes <rientjes@google.com>
---
 To be folded into 
 mm-fix-potential-infinite-loop-in-dissolve_free_huge_pages.patch.

 mm/hugetlb.c        | 3 +++
 mm/memory_hotplug.c | 3 +--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1088,6 +1088,9 @@ void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
 	unsigned long pfn;
 	struct hstate *h;
 
+	if (!hugepages_supported())
+		return;
+
 	/* Set scan step to minimum hugepage size */
 	for_each_hstate(h)
 		if (order > huge_page_order(h))
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1726,8 +1726,7 @@ repeat:
 	 * dissolve free hugepages in the memory block before doing offlining
 	 * actually in order to make hugetlbfs's object counting consistent.
 	 */
-	if (hugepages_supported())
-		dissolve_free_huge_pages(start_pfn, end_pfn);
+	dissolve_free_huge_pages(start_pfn, end_pfn);
 	/* check again */
 	offlined_pages = check_pages_isolated(start_pfn, end_pfn);
 	if (offlined_pages < 0) {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>