[PATCH] mm: hugetlb: yield when prepping struct pages

[PATCH] mm: hugetlb: yield when prepping struct pages

[Cannon Matthews]

When booting with very large numbers of gigantic (i.e. 1G) pages, the
operations in the loop of gather_bootmem_prealloc, and specifically
prep_compound_gigantic_page, takes a very long time, and can cause a
softlockup if enough pages are requested at boot.

For example booting with 3844 1G pages requires prepping
(set_compound_head, init the count) over 1 billion 4K tail pages, which
takes considerable time. This should also apply to reserving the same
amount of memory as 2M pages, as the same number of struct pages
are affected in either case.

Add a cond_resched() to the outer loop in gather_bootmem_prealloc() to
prevent this lockup.

Tested: Booted with softlockup_panic=1 hugepagesz=1G hugepages=3844 and
no softlockup is reported, and the hugepages are reported as
successfully setup.

Signed-off-by: Cannon Matthews <cannonmatthews@google.com>
---
 mm/hugetlb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index a963f2034dfc..d38273c32d3b 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2169,6 +2169,7 @@ static void __init gather_bootmem_prealloc(void)
 		 */
 		if (hstate_is_gigantic(h))
 			adjust_managed_page_count(page, 1 << h->order);
+		cond_resched();
 	}
 }
 
-- 
2.18.0.rc2.346.g013aa6912e-goog

Re: [PATCH] mm: hugetlb: yield when prepping struct pages

[Mike Kravetz]

On 06/27/2018 02:44 PM, Cannon Matthews wrote:
> When booting with very large numbers of gigantic (i.e. 1G) pages, the
> operations in the loop of gather_bootmem_prealloc, and specifically
> prep_compound_gigantic_page, takes a very long time, and can cause a
> softlockup if enough pages are requested at boot.
> 
> For example booting with 3844 1G pages requires prepping

Wow!  I wish I had a system with that much memory to test. :)

> (set_compound_head, init the count) over 1 billion 4K tail pages, which
> takes considerable time. This should also apply to reserving the same
> amount of memory as 2M pages, as the same number of struct pages
> are affected in either case.

Actually, this change would not apply to 2M (on x86) pages.  The hugetlbfs
initialization code is a bit confusing, but alloc_bootmem_huge_page and
gather_bootmem_prealloc are only exercised in the case where huge page
order >= MAX_ORDER.

Allocation and initialization of 2M pages happens after the normal memory
allocators are setup via the routine hugetlb_hstate_alloc_pages.  And,
there is already a cond_resched in that loop today.

Note that 'else if' in the for loop of hugetlb_hstate_alloc_pages.  This
allows the same routine to be called for early gigantic page allocations
using the bootmem allocator, and later normal (2M) allocations using the
normal memory allocators.  To me, this is a source of confusion and is
something I plan to clean up in the future.

> Add a cond_resched() to the outer loop in gather_bootmem_prealloc() to
> prevent this lockup.
> 
> Tested: Booted with softlockup_panic=1 hugepagesz=1G hugepages=3844 and
> no softlockup is reported, and the hugepages are reported as
> successfully setup.
> 
> Signed-off-by: Cannon Matthews <cannonmatthews@google.com>

My only suggestion would be to remove the mention of 2M pages in the
commit message.  Thanks for adding this.

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
-- 
Mike Kravetz

> ---
>  mm/hugetlb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index a963f2034dfc..d38273c32d3b 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2169,6 +2169,7 @@ static void __init gather_bootmem_prealloc(void)
>  		 */
>  		if (hstate_is_gigantic(h))
>  			adjust_managed_page_count(page, 1 << h->order);
> +		cond_resched();
>  	}
>  }
>  
> 

Re: [PATCH] mm: hugetlb: yield when prepping struct pages

[Andrew Morton]

On Wed, 27 Jun 2018 16:27:24 -0700 Mike Kravetz <mike.kravetz@oracle.com> wrote:

> My only suggestion would be to remove the mention of 2M pages in the
> commit message.  Thanks for adding this.

I have removed that sentence.

> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>

Thanks again.

Re: [PATCH] mm: hugetlb: yield when prepping struct pages

[Michal Hocko]

On Wed 27-06-18 14:44:47, Cannon Matthews wrote:
> When booting with very large numbers of gigantic (i.e. 1G) pages, the
> operations in the loop of gather_bootmem_prealloc, and specifically
> prep_compound_gigantic_page, takes a very long time, and can cause a
> softlockup if enough pages are requested at boot.
> 
> For example booting with 3844 1G pages requires prepping
> (set_compound_head, init the count) over 1 billion 4K tail pages, which
> takes considerable time. This should also apply to reserving the same
> amount of memory as 2M pages, as the same number of struct pages
> are affected in either case.
> 
> Add a cond_resched() to the outer loop in gather_bootmem_prealloc() to
> prevent this lockup.
> 
> Tested: Booted with softlockup_panic=1 hugepagesz=1G hugepages=3844 and
> no softlockup is reported, and the hugepages are reported as
> successfully setup.
> 
> Signed-off-by: Cannon Matthews <cannonmatthews@google.com>

Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!

> ---
>  mm/hugetlb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index a963f2034dfc..d38273c32d3b 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2169,6 +2169,7 @@ static void __init gather_bootmem_prealloc(void)
>  		 */
>  		if (hstate_is_gigantic(h))
>  			adjust_managed_page_count(page, 1 << h->order);
> +		cond_resched();
>  	}
>  }
>  
> -- 
> 2.18.0.rc2.346.g013aa6912e-goog

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: hugetlb: yield when prepping struct pages

[Cannon Matthews]

Thanks for the quick turnaround.

Good to know about the how the 2M code path differs, I have been
trying to trace through some of this and it's easy to get lost between
which applies to which size.

Thanks!
On Thu, Jun 28, 2018 at 12:03 PM Michal Hocko <mhocko@kernel.org> wrote:
>
> On Wed 27-06-18 14:44:47, Cannon Matthews wrote:
> > When booting with very large numbers of gigantic (i.e. 1G) pages, the
> > operations in the loop of gather_bootmem_prealloc, and specifically
> > prep_compound_gigantic_page, takes a very long time, and can cause a
> > softlockup if enough pages are requested at boot.
> >
> > For example booting with 3844 1G pages requires prepping
> > (set_compound_head, init the count) over 1 billion 4K tail pages, which
> > takes considerable time. This should also apply to reserving the same
> > amount of memory as 2M pages, as the same number of struct pages
> > are affected in either case.
> >
> > Add a cond_resched() to the outer loop in gather_bootmem_prealloc() to
> > prevent this lockup.
> >
> > Tested: Booted with softlockup_panic=1 hugepagesz=1G hugepages=3844 and
> > no softlockup is reported, and the hugepages are reported as
> > successfully setup.
> >
> > Signed-off-by: Cannon Matthews <cannonmatthews@google.com>
>
> Acked-by: Michal Hocko <mhocko@suse.com>
>
> Thanks!
>
> > ---
> >  mm/hugetlb.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index a963f2034dfc..d38273c32d3b 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -2169,6 +2169,7 @@ static void __init gather_bootmem_prealloc(void)
> >                */
> >               if (hstate_is_gigantic(h))
> >                       adjust_managed_page_count(page, 1 << h->order);
> > +             cond_resched();
> >       }
> >  }
> >
> > --
> > 2.18.0.rc2.346.g013aa6912e-goog
>
> --
> Michal Hocko
> SUSE Labs

Re: [PATCH] mm: hugetlb: yield when prepping struct pages

[Michal Hocko]

On Thu 28-06-18 15:16:46, Cannon Matthews wrote:
> Thanks for the quick turnaround.
> 
> Good to know about the how the 2M code path differs, I have been
> trying to trace through some of this and it's easy to get lost between
> which applies to which size.

Yeah, GB hugetlb pages implementation has been hacked into the existing
hugetlb code in a quite ugly way. We have done some cleanups since then
but there is still a lot of room for improvements.
-- 
Michal Hocko
SUSE Labs