* [RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages()
@ 2014-07-24 9:36 Li Zhong
2014-07-24 12:45 ` Naoya Horiguchi
0 siblings, 1 reply; 6+ messages in thread
From: Li Zhong @ 2014-07-24 9:36 UTC (permalink / raw)
To: linux-mm; +Cc: n-horiguchi, Andrew Morton
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>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages()
2014-07-24 9:36 [RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages() Li Zhong
@ 2014-07-24 12:45 ` Naoya Horiguchi
2014-07-28 1:28 ` Li Zhong
2014-07-28 2:20 ` [PATCH v2]mm: " Li Zhong
0 siblings, 2 replies; 6+ messages in thread
From: Naoya Horiguchi @ 2014-07-24 12:45 UTC (permalink / raw)
To: Li Zhong; +Cc: linux-mm, Andrew Morton
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages()
2014-07-24 12:45 ` Naoya Horiguchi
@ 2014-07-28 1:28 ` Li Zhong
2014-07-28 2:20 ` [PATCH v2]mm: " Li Zhong
1 sibling, 0 replies; 6+ messages in thread
From: Li Zhong @ 2014-07-28 1:28 UTC (permalink / raw)
To: Naoya Horiguchi; +Cc: linux-mm, Andrew Morton
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2]mm: fix potential infinite loop in dissolve_free_huge_pages()
2014-07-24 12:45 ` Naoya Horiguchi
2014-07-28 1:28 ` Li Zhong
@ 2014-07-28 2:20 ` Li Zhong
2014-07-28 14:33 ` Naoya Horiguchi
2014-07-30 1:09 ` [patch] mm: fix potential infinite loop in dissolve_free_huge_pages() fix David Rientjes
1 sibling, 2 replies; 6+ messages in thread
From: Li Zhong @ 2014-07-28 2:20 UTC (permalink / raw)
To: Naoya Horiguchi; +Cc: linux-mm, Andrew Morton, Nadia Yvette Chambers
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>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2]mm: fix potential infinite loop in dissolve_free_huge_pages()
2014-07-28 2:20 ` [PATCH v2]mm: " Li Zhong
@ 2014-07-28 14:33 ` Naoya Horiguchi
2014-07-30 1:09 ` [patch] mm: fix potential infinite loop in dissolve_free_huge_pages() fix David Rientjes
1 sibling, 0 replies; 6+ messages in thread
From: Naoya Horiguchi @ 2014-07-28 14:33 UTC (permalink / raw)
To: Li Zhong; +Cc: linux-mm, Andrew Morton, Nadia Yvette Chambers
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch] mm: fix potential infinite loop in dissolve_free_huge_pages() fix
2014-07-28 2:20 ` [PATCH v2]mm: " Li Zhong
2014-07-28 14:33 ` Naoya Horiguchi
@ 2014-07-30 1:09 ` David Rientjes
1 sibling, 0 replies; 6+ messages in thread
From: David Rientjes @ 2014-07-30 1:09 UTC (permalink / raw)
To: Li Zhong, Andrew Morton; +Cc: Naoya Horiguchi, linux-mm, Nadia Yvette Chambers
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-30 1:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-24 9:36 [RFC PATCH]mm: fix potential infinite loop in dissolve_free_huge_pages() Li Zhong
2014-07-24 12:45 ` Naoya Horiguchi
2014-07-28 1:28 ` Li Zhong
2014-07-28 2:20 ` [PATCH v2]mm: " Li Zhong
2014-07-28 14:33 ` Naoya Horiguchi
2014-07-30 1:09 ` [patch] mm: fix potential infinite loop in dissolve_free_huge_pages() fix David Rientjes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).